Changes to the way auto-deferral is indicated

This change adds a new extension hook `use-package-autoloads/<KEYWORD>` for
specifying exactly which autoloads a keyword should imply. This is the proper
way to indicate autoloads, rather than adding to the `:commands` entry as was
done before.

Further, autoloading now must occur in order to cause implied deferred
loading; if :bind is used with only lambda forms, for example, this will not
cause deferred loading without `:defer t`.

4 files changed, 181 insertions, 109 deletions

diff --git a/etc/USE-PACKAGE-NEWS b/etc/USE-PACKAGE-NEWS
index 76556be9b2e..c34376d3ffe 100644
--- a/etc/USE-PACKAGE-NEWS
+++ b/etc/USE-PACKAGE-NEWS
@@ -30,10 +30,6 @@
   parameter, but are now done as an extension to `rest`. Please see
   `use-package-handler/:bind` for a canonical example.
 
-- For extension authors, if you add a keyword to `use-package-keywords` whose
-  presence should indicate deferred loading, please also add it to
-  `use-package-deferring-keywords`.
-
 ### Other changes
 
 - Upgrade license to GPL 3.
@@ -154,6 +150,28 @@
   it is loaded, `helm-descbinds` itself is not loaded until the user presses
   `C-h b`.
 
+- For extension authors, if you add a keyword to `use-package-keywords` whose
+  presence should indicate deferred loading, please also add it to
+  `use-package-deferring-keywords`. Note that this is a bit of a sledgehammer,
+  in that the mere presence of these keywords implies deferred loading. For a
+  more subtle approach, see the new `use-package-autoloads/<KEYWORD>` support
+  mentioned in the next bullet.
+
+- For extension authors, if you wish deferred loading to possibly occur,
+  create functions named `use-package-autoloads/<KEYWORD>` for each keyword
+  that you define, returning an alist of the form `(SYMBOL . TYPE)` of symbols
+  to be autoloaded. `SYMBOL` should be an interactive function, and `TYPE` the
+  smybol `command`, but this functionality may be extended in future. These
+  autoloads are established if deferred loading is to happen.
+
+- If you specify a lambda form rather than a function symbol in any of the
+  constructs that *might* introduce autoloads: `:bind`, `:bind*`,
+  `:interpreter`, `:mode`, `:magic`, `:magic-fallback`, and `:hook`: then
+  deferred loading will no longer be implied, since there's nothing to
+  associate an autoload with that could later load the module. In these cases,
+  it will be as if you'd specified `:demand t`, in order to ensure the lambda
+  form is able to execute in the context of the loaded package.
+
 - For extension authors, there is a new customization variable
   `use-package-merge-key-alist` that specifies how values passed to multiple
   occurences of the same key should be merged into a single value, during
diff --git a/lisp/use-package/use-package-bind-key.el b/lisp/use-package/use-package-bind-key.el
index 4a3d421522b..6a8e3fbe8eb 100644
--- a/lisp/use-package/use-package-bind-key.el
+++ b/lisp/use-package/use-package-bind-key.el
@@ -114,20 +114,23 @@ deferred until the prefix key sequence is pressed."
 ;;;###autoload
 (defalias 'use-package-normalize/:bind* 'use-package-normalize-binder)
 
+;; jww (2017-12-07): This is too simplistic. It will fail to determine
+;; autoloads in this situation:
+;;   (use-package foo
+;;     :bind (:map foo-map (("C-a" . func))))
+;;;###autoload
+(defalias 'use-package-autoloads/:bind 'use-package-autoloads-mode)
+;;;###autoload
+(defalias 'use-package-autoloads/:bind* 'use-package-autoloads-mode)
+
 ;;;###autoload
 (defun use-package-handler/:bind
     (name keyword args rest state &optional bind-macro)
-  (let* ((result (use-package-normalize-commands args))
-         (nargs (car result))
-         (commands (cdr result)))
-    (use-package-concat
-     (use-package-process-keywords name
-       (use-package-sort-keywords
-        (use-package-plist-append rest :commands commands))
-       state)
-     `((ignore
-        (,(if bind-macro bind-macro 'bind-keys)
-         :package ,name ,@nargs))))))
+  (use-package-concat
+   (use-package-process-keywords name rest state)
+   `((ignore
+      (,(if bind-macro bind-macro 'bind-keys)
+       :package ,name ,@(use-package-normalize-commands args))))))
 
 (defun use-package-handler/:bind* (name keyword arg rest state)
   (use-package-handler/:bind name keyword arg rest state 'bind-keys*))
diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
index 175e023a5a4..001ffde1534 100644
--- a/lisp/use-package/use-package-core.el
+++ b/lisp/use-package/use-package-core.el
@@ -100,17 +100,15 @@ declaration is incorrect."
   :group 'use-package)
 
 (defcustom use-package-deferring-keywords
-  '(:bind
-    :bind*
-    :bind-keymap
+  '(:bind-keymap
     :bind-keymap*
-    :interpreter
-    :mode
-    :magic
-    :magic-fallback
-    :hook
     :commands)
-  "Unless `:demand' is used, keywords in this list imply deferred loading."
+  "Unless `:demand' is used, keywords in this list imply deferred loading.
+The reason keywords like `:hook' are not in this list is that
+they only imply deferred loading if they reference actual
+function symbols that can be autoloaded from the module; whereas
+the default keywords provided here always defer loading unless
+otherwise requested."
   :type '(repeat symbol)
   :group 'use-package)
 
@@ -160,7 +158,7 @@ See also `use-package-defaults', which uses this value."
                (and use-package-always-demand
                     (not (plist-member args :defer))
                     (not (plist-member args :demand))))))
-  "Alist of default values for `use-package' keywords.
+  "Default values for specified `use-package' keywords.
 Each entry in the alist is a list of three elements. The first
 element is the `use-package' keyword and the second is a form
 that can be evaluated to get the default value. The third element
@@ -497,8 +495,9 @@ extending any keys already present."
            (xs (use-package-split-list #'keywordp (cdr input)))
            (args (car xs))
            (tail (cdr xs))
-           (normalizer (intern (concat "use-package-normalize/"
-                                       (symbol-name keyword))))
+           (normalizer
+            (intern-soft (concat "use-package-normalize/"
+                                 (symbol-name keyword))))
            (arg (and (functionp normalizer)
                      (funcall normalizer name keyword args))))
       (if (memq keyword use-package-keywords)
@@ -562,6 +561,32 @@ extending any keys already present."
         (setq args (use-package-plist-maybe-put
                     args (nth 0 spec) (eval (nth 1 spec))))))
 
+    ;; Determine any autoloads implied by the keywords used.
+    (let ((iargs args)
+          commands)
+      (while iargs
+        (when (keywordp (car iargs))
+          (let ((autoloads
+                 (intern-soft (concat "use-package-autoloads/"
+                                      (symbol-name (car iargs))))))
+            (when (functionp autoloads)
+              (setq commands
+                    ;; jww (2017-12-07): Right now we just ignored the type of
+                    ;; the autoload being requested, and assume they are all
+                    ;; `command'.
+                    (append (mapcar
+                             #'car
+                             (funcall autoloads name-symbol (car iargs)
+                                      (cadr iargs)))
+                            commands)))))
+        (setq iargs (cddr iargs)))
+      (when commands
+        (setq args
+              ;; Like `use-package-plist-append', but removing duplicates.
+              (plist-put args :commands
+                         (delete-dups
+                          (append commands (plist-get args :commands)))))))
+
     ;; If byte-compiling, pre-load the package so all its symbols are in
     ;; scope. This is done by prepending statements to the :preface.
     (when (bound-and-true-p byte-compile-current-file)
@@ -593,10 +618,13 @@ extending any keys already present."
                                 use-package-deferring-keywords)))
       (setq args (append args '(:defer t))))
 
+    ;; The :load keyword overrides :no-require
     (when (and (plist-member args :load)
                (plist-member args :no-require))
       (setq args (use-package-plist-delete args :no-require)))
 
+    ;; If at this point no :load, :defer or :no-require has been seen, then
+    ;; :load the package itself.
     (when (and (not (plist-member args :load))
                (not (plist-member args :defer))
                (not (plist-member args :no-require)))
@@ -851,22 +879,12 @@ If RECURSED is non-nil, recurse into sublists."
         (t v)))
 
 (defun use-package-normalize-commands (args)
-  "Map over ARGS of the form ((_ . F) ...).
-Normalizing functional F's and returning a list of F's
-representing symbols (that may need to be autloaded)."
-  (let ((nargs (mapcar
-                #'(lambda (x)
-                    (if (consp x)
-                        (cons (car x)
-                              (use-package-normalize-function (cdr x)))
-                      x)) args)))
-    (cons nargs
-          (delete
-           nil (mapcar
-                #'(lambda (x)
-                    (and (consp x)
-                         (use-package-non-nil-symbolp (cdr x))
-                         (cdr x))) nargs)))))
+  "Map over ARGS of the form ((_ . F) ...), normalizing functional F's."
+  (mapcar #'(lambda (x)
+              (if (consp x)
+                  (cons (car x) (use-package-normalize-function (cdr x)))
+                x))
+          args))
 
 (defun use-package-normalize-mode (name keyword args)
   "Normalize arguments for keywords which add regexp/mode pairs to an alist."
@@ -876,6 +894,27 @@ representing symbols (that may need to be autloaded)."
                      #'use-package-recognize-function
                      name)))
 
+(defun use-package-autoloads-mode (name keyword args)
+  (mapcar
+   #'(lambda (x) (cons (cdr x) 'command))
+   (cl-remove-if-not #'(lambda (x)
+                         (and (consp x)
+                              (use-package-non-nil-symbolp (cdr x))))
+                     args)))
+
+(defun use-package-handle-mode (name alist args rest state)
+  "Handle keywords which add regexp/mode pairs to an alist."
+  (use-package-concat
+   (use-package-process-keywords name rest state)
+   `((ignore
+      ,@(mapcar
+         #'(lambda (thing)
+             `(add-to-list
+               ',alist
+               ',(cons (use-package-normalize-regex (car thing))
+                       (cdr thing))))
+         (use-package-normalize-commands args))))))
+
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;
 ;;; Statistics
@@ -1078,26 +1117,8 @@ meaning:
 
 ;;;; :interpreter
 
-(defun use-package-handle-mode (name alist args rest state)
-  "Handle keywords which add regexp/mode pairs to an alist."
-  (let* ((result (use-package-normalize-commands args))
-         (nargs (car result))
-         (commands (cdr result)))
-    (use-package-concat
-     (use-package-process-keywords name
-       (use-package-sort-keywords
-        (use-package-plist-append rest :commands commands))
-       state)
-     `((ignore
-        ,@(mapcar
-           #'(lambda (thing)
-               `(add-to-list
-                 ',alist
-                 ',(cons (use-package-normalize-regex (car thing))
-                         (cdr thing))))
-           nargs))))))
-
 (defalias 'use-package-normalize/:interpreter 'use-package-normalize-mode)
+(defalias 'use-package-autoloads/:interpreter 'use-package-autoloads-mode)
 
 (defun use-package-handler/:interpreter (name keyword arg rest state)
   (use-package-handle-mode name 'interpreter-mode-alist arg rest state))
@@ -1105,6 +1126,7 @@ meaning:
 ;;;; :mode
 
 (defalias 'use-package-normalize/:mode 'use-package-normalize-mode)
+(defalias 'use-package-autoloads/:mode 'use-package-autoloads-mode)
 
 (defun use-package-handler/:mode (name keyword arg rest state)
   (use-package-handle-mode name 'auto-mode-alist arg rest state))
@@ -1112,6 +1134,7 @@ meaning:
 ;;;; :magic
 
 (defalias 'use-package-normalize/:magic 'use-package-normalize-mode)
+(defalias 'use-package-autoloads/:magic 'use-package-autoloads-mode)
 
 (defun use-package-handler/:magic (name keyword arg rest state)
   (use-package-handle-mode name 'magic-mode-alist arg rest state))
@@ -1119,6 +1142,7 @@ meaning:
 ;;;; :magic-fallback
 
 (defalias 'use-package-normalize/:magic-fallback 'use-package-normalize-mode)
+(defalias 'use-package-autoloads/:magic-fallback 'use-package-autoloads-mode)
 
 (defun use-package-handler/:magic-fallback (name keyword arg rest state)
   (use-package-handle-mode name 'magic-fallback-mode-alist arg rest state))
@@ -1145,31 +1169,27 @@ meaning:
          #'use-package-recognize-function
          name label arg))))
 
+(defalias 'use-package-autoloads/:hook 'use-package-autoloads-mode)
+
 (defun use-package-handler/:hook (name keyword args rest state)
   "Generate use-package custom keyword code."
-  (let* ((result (use-package-normalize-commands args))
-         (nargs (car result))
-         (commands (cdr result)))
-    (use-package-concat
-     (use-package-process-keywords name
-       (use-package-sort-keywords
-        (use-package-plist-append rest :commands commands))
-       state)
-     `((ignore
-        ,@(cl-mapcan
-           #'(lambda (def)
-               (let ((syms (car def))
-                     (fun (cdr def)))
-                 (when fun
-                   (mapcar
-                    #'(lambda (sym)
-                        `(add-hook
-                          (quote ,(intern
-                                   (concat (symbol-name sym)
-                                           use-package-hook-name-suffix)))
-                          (function ,fun)))
-                    (if (use-package-non-nil-symbolp syms) (list syms) syms)))))
-           nargs))))))
+  (use-package-concat
+   (use-package-process-keywords name rest state)
+   `((ignore
+      ,@(cl-mapcan
+         #'(lambda (def)
+             (let ((syms (car def))
+                   (fun (cdr def)))
+               (when fun
+                 (mapcar
+                  #'(lambda (sym)
+                      `(add-hook
+                        (quote ,(intern
+                                 (concat (symbol-name sym)
+                                         use-package-hook-name-suffix)))
+                        (function ,fun)))
+                  (if (use-package-non-nil-symbolp syms) (list syms) syms)))))
+         (use-package-normalize-commands args))))))
 
 ;;;; :commands
 
diff --git a/test/lisp/use-package/use-package-tests.el b/test/lisp/use-package/use-package-tests.el
index ffff7ced326..08ac5293a99 100644
--- a/test/lisp/use-package/use-package-tests.el
+++ b/test/lisp/use-package/use-package-tests.el
@@ -1001,30 +1001,30 @@
 
 (ert-deftest use-package-test/:hook-1 ()
   (let ((byte-compile-current-file t))
-    (should
-     (equal
-      (expand-minimally
-       (use-package foo
-         :bind (("C-a" . key))
-         :hook (hook . fun)))
-      '(progn
-         (eval-and-compile
-           (eval-when-compile
-             (with-demoted-errors
-                 "Cannot load foo: %S" nil
-                 (load "foo" nil t))))
-         (unless (fboundp 'fun)
-           (autoload #'fun "foo" nil t))
-         (eval-when-compile
-           (declare-function fun "foo"))
-         (unless (fboundp 'key)
-           (autoload #'key "foo" nil t))
-         (eval-when-compile
-           (declare-function key "foo"))
-         (ignore
-          (add-hook 'hook-hook #'fun))
-         (ignore
-          (bind-keys :package foo ("C-a" . key))))))))
+    (match-expansion
+     (use-package foo
+       :bind (("C-a" . key))
+       :hook (hook . fun))
+     `(progn
+        (eval-and-compile
+          (eval-when-compile
+            (with-demoted-errors
+                "Cannot load foo: %S" nil
+                (load "foo" nil t))))
+        (unless
+            (fboundp 'key)
+          (autoload #'key "foo" nil t))
+        (eval-when-compile
+          (declare-function key "foo"))
+        (unless
+            (fboundp 'fun)
+          (autoload #'fun "foo" nil t))
+        (eval-when-compile
+          (declare-function fun "foo"))
+        (ignore
+         (add-hook 'hook-hook #'fun))
+        (ignore
+         (bind-keys :package foo ("C-a" . key)))))))
 
 (ert-deftest use-package-test/:hook-2 ()
   (match-expansion
@@ -1079,6 +1079,37 @@
              #'(lambda nil
                  (bind-key "" erefactor-map emacs-lisp-mode-map)))))))))
 
+(ert-deftest use-package-test/:hook-6 ()
+  (match-expansion
+   (use-package erefactor
+     :load-path "foo"
+     :after elisp-mode
+     :hook (emacs-lisp-mode . function))
+   `(progn
+      (eval-and-compile
+        (add-to-list 'load-path "/Users/johnw/.emacs.d/foo"))
+      (eval-after-load 'elisp-mode
+        '(progn
+           (unless (fboundp 'function)
+             (autoload #'function "erefactor" nil t))
+           (ignore
+            (add-hook 'emacs-lisp-mode-hook #'function)))))))
+
+(ert-deftest use-package-test/:hook-7 ()
+  (match-expansion
+   (use-package erefactor
+     :load-path "foo"
+     :after elisp-mode
+     :hook (emacs-lisp-mode . (lambda () (function))))
+   `(progn
+      (eval-and-compile
+        (add-to-list 'load-path "/Users/johnw/.emacs.d/foo"))
+      (eval-after-load 'elisp-mode
+        '(progn
+           (require 'erefactor nil nil)
+           (ignore
+            (add-hook 'emacs-lisp-mode-hook #'(lambda nil (function)))))))))
+
 (ert-deftest use-package-test-normalize/:custom ()
   (flet ((norm (&rest args)
                (apply #'use-package-normalize/:custom