From 14fddbe6c3e9a2859e18e0687897f533efd9c7c8 Mon Sep 17 00:00:00 2001
From: Kodi Arfer <kodi@arfer.net>
Date: Thu, 3 Nov 2016 00:35:58 -0700
Subject: [PATCH] Give `require` the same features as `import` (#1142)

Give `require` the same features as `import`

You can now do (require foo), (require [foo [a b c]]), (require [foo [*]]), and (require [foo :as bar]). The first and last forms get you macros named foo.a, foo.b, etc. or bar.a, bar.b, etc., respectively. The second form only gets the macros in the list.

Implements #1118 and perhaps partly addresses #277.

N.B. The new meaning of (require foo) will cause all existing code that uses macros to break. Simply replace these forms with (require [foo [*]]) to get your code working again.

There's a bit of a hack involved in the forms (require foo) or (require [foo :as bar]). When you call (foo.a ...) or (bar.a ...), Hy doesn't actually look inside modules. Instead, these (require ...) forms give the macros names that have periods in them, which happens to work fine with the way Hy finds and interprets macro calls.

* Make `require` syntax stricter and add tests

* Update documentation for `require`

* Documentation wording improvements

* Allow :as in `require` name lists
---
 docs/contrib/anaphoric.rst                  |  2 +-
 docs/contrib/flow.rst                       |  4 +-
 docs/contrib/loop.rst                       |  2 +-
 docs/contrib/multi.rst                      |  2 +-
 docs/language/api.rst                       | 78 +++++++++++++++++++--
 docs/tutorial.rst                           | 13 ++--
 hy/cmdline.py                               |  4 +-
 hy/compiler.py                              | 43 ++++++++++--
 hy/contrib/curry.hy                         |  3 +-
 hy/contrib/loop.hy                          |  6 +-
 hy/contrib/meth.hy                          | 12 ++--
 hy/macros.py                                | 36 +++++++---
 tests/compilers/test_ast.py                 | 14 ++++
 tests/native_tests/contrib/alias.hy         |  2 +-
 tests/native_tests/contrib/anaphoric.hy     |  2 +-
 tests/native_tests/contrib/botsbuildbots.hy |  3 +-
 tests/native_tests/contrib/curry.hy         |  2 +-
 tests/native_tests/contrib/loop.hy          |  2 +-
 tests/native_tests/contrib/meth.hy          |  2 +-
 tests/native_tests/contrib/multi.hy         |  2 +-
 tests/native_tests/language.hy              | 33 +++++++--
 tests/resources/tlib.py                     |  9 ++-
 22 files changed, 222 insertions(+), 54 deletions(-)

diff --git a/docs/contrib/anaphoric.rst b/docs/contrib/anaphoric.rst
index 51ece00..40341da 100644
--- a/docs/contrib/anaphoric.rst
+++ b/docs/contrib/anaphoric.rst
@@ -15,7 +15,7 @@ concise and easy to read.
 
 To use these macros you need to require the hy.contrib.anaphoric module like so:
 
-``(require hy.contrib.anaphoric)``
+``(require [hy.contrib.anaphoric [*]])``
 
 .. _ap-if:
 
diff --git a/docs/contrib/flow.rst b/docs/contrib/flow.rst
index fa15f73..e99e00e 100644
--- a/docs/contrib/flow.rst
+++ b/docs/contrib/flow.rst
@@ -25,7 +25,7 @@ Example:
 
 .. code-block:: hy
 
-    (require hy.contrib.flow)
+    (require [hy.contrib.flow [case]])
 
     (defn temp-commenter [temp]
         (case temp
@@ -48,7 +48,7 @@ Example:
 
 .. code-block:: hy
 
-    (require hy.contrib.flow)
+    (require [hy.contrib.flow [switch]])
 
     (defn temp-commenter [temp]
         (switch temp
diff --git a/docs/contrib/loop.rst b/docs/contrib/loop.rst
index aaef239..e5a3766 100644
--- a/docs/contrib/loop.rst
+++ b/docs/contrib/loop.rst
@@ -45,7 +45,7 @@ Example:
 
 .. code-block:: hy
 
-    (require hy.contrib.loop)
+    (require [hy.contrib.loop [loop]])
 
     (defn factorial [n]
       (loop [[i n] [acc 1]]
diff --git a/docs/contrib/multi.rst b/docs/contrib/multi.rst
index aa094cd..5740557 100644
--- a/docs/contrib/multi.rst
+++ b/docs/contrib/multi.rst
@@ -9,7 +9,7 @@ args and/or kwargs. Inspired by Clojure's take on ``defn``.
 
 .. code-block:: clj
 
-    => (require hy.contrib.multi)
+    => (require [hy.contrib.multi [defmulti]])
     =>   (defmulti fun
     ...     ([a] "a")
     ...     ([a b] "a b")
diff --git a/docs/language/api.rst b/docs/language/api.rst
index fa09c82..379c6d4 100644
--- a/docs/language/api.rst
+++ b/docs/language/api.rst
@@ -1210,16 +1210,84 @@ alternatively be written using the apostrophe (``'``) symbol.
 require
 -------
 
-``require`` is used to import macros from a given module. It takes at least one
-parameter specifying the module which macros should be imported. Multiple
-modules can be imported with a single ``require``.
+``require`` is used to import macros from one or more given modules. It allows
+parameters in all the same formats as ``import``. The ``require`` form itself
+produces no code in the final program: its effect is purely at compile-time, for
+the benefit of macro expansion. Specifically, ``require`` imports each named
+module and then makes each requested macro available in the current module.
 
-The following example will import macros from ``module-1`` and ``module-2``:
+The following are all equivalent ways to call a macro named ``foo`` in the module ``mymodule``:
 
 .. code-block:: clj
 
-    (require module-1 module-2)
+    (require mymodule)
+    (mymodule.foo 1)
 
+    (require [mymodule :as M])
+    (M.foo 1)
+
+    (require [mymodule [foo]])
+    (foo 1)
+
+    (require [mymodule [*]])
+    (foo 1)
+
+    (require [mymodule [foo :as bar]])
+    (bar 1)
+
+Macros that call macros
+~~~~~~~~~~~~~~~~~~~~~~~
+
+One aspect of ``require`` that may be surprising is what happens when one
+macro's expansion calls another macro. Suppose ``mymodule.hy`` looks like this:
+
+.. code-block:: clj
+
+    (defmacro repexpr [n expr]
+      ; Evaluate the expression n times
+      ; and collect the results in a list.
+      `(list (map (fn [_] ~expr) (range ~n))))
+
+    (defmacro foo [n]
+      `(repexpr ~n (input "Gimme some input: ")))
+
+And then, in your main program, you write:
+
+.. code-block:: clj
+
+    (require [mymodule [foo]])
+
+    (print (mymodule.foo 3))
+
+Running this raises ``NameError: name 'repexpr' is not defined``, even though
+writing ``(print (foo 3))`` in ``mymodule`` works fine. The trouble is that your
+main program doesn't have the macro ``repexpr`` available, since it wasn't
+imported (and imported under exactly that name, as opposed to a qualified name).
+You could do ``(require [mymodule [*]])`` or ``(require [mymodule [foo
+repexpr]])``, but a less error-prone approach is to change the definition of
+``foo`` to require whatever sub-macros it needs:
+
+.. code-block:: clj
+
+    (defmacro foo [n]
+      `(do
+        (require mymodule)
+        (mymodule.repexpr ~n (raw-input "Gimme some input: "))))
+
+It's wise to use ``(require mymodule)`` here rather than ``(require [mymodule
+[repexpr]])`` to avoid accidentally shadowing a function named ``repexpr`` in
+the main program.
+
+Qualified macro names
+~~~~~~~~~~~~~~~~~~~~~
+
+Note that in the current implementation, there's a trick in qualified macro
+names, like ``mymodule.foo`` and ``M.foo`` in the above example. These names
+aren't actually attributes of module objects; they're just identifiers with
+periods in them. In fact, ``mymodule`` and ``M`` aren't defined by these
+``require`` forms, even at compile-time. None of this will hurt you unless try
+to do introspection of the current module's set of defined macros, which isn't
+really supported anyway.
 
 rest / cdr
 ----------
diff --git a/docs/tutorial.rst b/docs/tutorial.rst
index 834332d..5c3225c 100644
--- a/docs/tutorial.rst
+++ b/docs/tutorial.rst
@@ -568,15 +568,18 @@ Macros are useful when one wishes to extend Hy or write their own
 language on top of that. Many features of Hy are macros, like ``when``,
 ``cond`` and ``->``.
 
-To use macros defined in a different module, it is not enough to
-``import`` the module, because importing happens at run-time, while we
-would need macros at compile-time. Instead of importing the module
-with macros, ``require`` must be used:
+What if you want to use a macro that's defined in a different
+module? The special form ``import`` won't help, because it merely
+translates to a Python ``import`` statement that's executed at
+run-time, and macros are expanded at compile-time, that is,
+during the translate from Hy to Python. Instead, use ``require``,
+which imports the module and makes macros available at
+compile-time. ``require`` uses the same syntax as ``import``.
 
 .. code-block:: clj
 
    => (require tutorial.macros)
-   => (rev (1 2 3 +))
+   => (tutorial.macros.rev (1 2 3 +))
    6
 
 Hy <-> Python interop
diff --git a/hy/cmdline.py b/hy/cmdline.py
index 35f75c6..c24a2f8 100644
--- a/hy/cmdline.py
+++ b/hy/cmdline.py
@@ -173,8 +173,8 @@ def ideas_macro():
 
 """)])
 
-require("hy.cmdline", "__console__")
-require("hy.cmdline", "__main__")
+require("hy.cmdline", "__console__", all_macros=True)
+require("hy.cmdline", "__main__", all_macros=True)
 
 SIMPLE_TRACEBACKS = True
 
diff --git a/hy/compiler.py b/hy/compiler.py
index b6c6d27..aa993b7 100644
--- a/hy/compiler.py
+++ b/hy/compiler.py
@@ -1737,10 +1737,45 @@ class HyASTCompiler(object):
         "unimport" it after we've completed `thing' so that we don't pollute
         other envs.
         """
-        expression.pop(0)
-        for entry in expression:
-            __import__(entry)  # Import it fo' them macros.
-            require(entry, self.module_name)
+        for entry in expression[1:]:
+            if isinstance(entry, HySymbol):
+                # e.g., (require foo)
+                __import__(entry)
+                require(entry, self.module_name, all_macros=True,
+                        prefix=entry)
+            elif isinstance(entry, HyList) and len(entry) == 2:
+                # e.g., (require [foo [bar baz :as MyBaz bing]])
+                # or (require [foo [*]])
+                module, names = entry
+                if not isinstance(names, HyList):
+                    raise HyTypeError(names,
+                                      "(require) name lists should be HyLists")
+                __import__(module)
+                if '*' in names:
+                    if len(names) != 1:
+                        raise HyTypeError(names, "* in a (require) name list "
+                                                 "must be on its own")
+                    require(module, self.module_name, all_macros=True)
+                else:
+                    assignments = {}
+                    while names:
+                        if len(names) > 1 and names[1] == HyKeyword(":as"):
+                            k, _, v = names[:3]
+                            del names[:3]
+                            assignments[k] = v
+                        else:
+                            symbol = names.pop(0)
+                            assignments[symbol] = symbol
+                    require(module, self.module_name, assignments=assignments)
+            elif (isinstance(entry, HyList) and len(entry) == 3
+                    and entry[1] == HyKeyword(":as")):
+                # e.g., (require [foo :as bar])
+                module, _, prefix = entry
+                __import__(module)
+                require(module, self.module_name, all_macros=True,
+                        prefix=prefix)
+            else:
+                raise HyTypeError(entry, "unrecognized (require) syntax")
         return Result()
 
     @builds("and")
diff --git a/hy/contrib/curry.hy b/hy/contrib/curry.hy
index ae3e4da..0bf5bd7 100644
--- a/hy/contrib/curry.hy
+++ b/hy/contrib/curry.hy
@@ -17,4 +17,5 @@
 
 
 (defmacro defnc [name args &rest body]
-  `(def ~name (fnc [~@args] ~@body)))
+  `(do (require hy.contrib.curry)
+       (def ~name (hy.contrib.curry.fnc [~@args] ~@body))))
diff --git a/hy/contrib/loop.hy b/hy/contrib/loop.hy
index a9b98f6..d114140 100644
--- a/hy/contrib/loop.hy
+++ b/hy/contrib/loop.hy
@@ -70,7 +70,8 @@
 (defmacro defnr [name lambda-list &rest body]
   (if (not (= (type name) HySymbol))
     (macro-error name "defnr takes a name as first argument"))
-  `(setv ~name (fnr ~lambda-list ~@body)))
+  `(do (require hy.contrib.loop)
+       (setv ~name (hy.contrib.loop.fnr ~lambda-list ~@body))))
 
 
 (defmacro/g! loop [bindings &rest body]
@@ -87,5 +88,6 @@
   ;; and erroring if not is a giant TODO.
   (let [fnargs (map (fn [x] (first x)) bindings)
         initargs (map second bindings)]
-    `(do (defnr ~g!recur-fn [~@fnargs] ~@body)
+    `(do (require hy.contrib.loop)
+         (hy.contrib.loop.defnr ~g!recur-fn [~@fnargs] ~@body)
          (~g!recur-fn ~@initargs))))
diff --git a/hy/contrib/meth.hy b/hy/contrib/meth.hy
index c52e03c..6774830 100644
--- a/hy/contrib/meth.hy
+++ b/hy/contrib/meth.hy
@@ -9,19 +9,23 @@
        (defn ~name ~params
          (do ~@code)))))
 
+(defn rwm [name path method params code]
+  `(do (require hy.contrib.meth)
+       (hy.contrib.meth.route-with-methods ~name ~path ~method ~params ~@code)))
+
 ;; Some macro examples
 (defmacro route [name path params &rest code]
   "Get request"
-  `(route-with-methods ~name ~path ["GET"] ~params ~@code))
+  (rwm name path ["GET"] params code))
 
 (defmacro post-route [name path params &rest code]
   "Post request"
-  `(route-with-methods ~name ~path ["POST"] ~params ~@code))
+  (rwm name path ["POST"] params code))
 
 (defmacro put-route [name path params &rest code]
   "Put request"
-  `(route-with-methods ~name ~path ["PUT"] ~params ~@code))
+  (rwm name path ["PUT"] params code))
 
 (defmacro delete-route [name path params &rest code]
   "Delete request"
-  `(route-with-methods ~name ~path ["DELETE"] ~params ~@code))
+  (rwm name path ["DELETE"] params code))
diff --git a/hy/macros.py b/hy/macros.py
index 154e258..e59f760 100644
--- a/hy/macros.py
+++ b/hy/macros.py
@@ -84,22 +84,36 @@ def reader(name):
     return _
 
 
-def require(source_module, target_module):
-    """Load the macros from `source_module` in the namespace of
-    `target_module`.
+def require(source_module, target_module,
+            all_macros=False, assignments={}, prefix=""):
+    """Load macros from `source_module` in the namespace of
+    `target_module`. `assignments` maps old names to new names, but is
+    ignored if `all_macros` is true. If `prefix` is nonempty, it is
+    prepended to the name of each imported macro. (This means you get
+    macros named things like "mymacromodule.mymacro", which looks like
+    an attribute of a module, although it's actually just a symbol
+    with a period in its name.)
 
     This function is called from the `require` special form in the compiler.
 
     """
-    macros = _hy_macros[source_module]
-    refs = _hy_macros[target_module]
-    for name, macro in macros.items():
-        refs[name] = macro
 
-    readers = _hy_reader[source_module]
-    reader_refs = _hy_reader[target_module]
-    for name, reader in readers.items():
-        reader_refs[name] = reader
+    seen_names = set()
+    if prefix:
+        prefix += "."
+
+    for d in _hy_macros, _hy_reader:
+        for name, macro in d[source_module].items():
+            seen_names.add(name)
+            if all_macros:
+                d[target_module][prefix + name] = macro
+            elif name in assignments:
+                d[target_module][prefix + assignments[name]] = macro
+
+    if not all_macros:
+        unseen = frozenset(assignments.keys()).difference(seen_names)
+        if unseen:
+            raise ImportError("cannot require names: " + repr(list(unseen)))
 
 
 def load_macros(module_name):
diff --git a/tests/compilers/test_ast.py b/tests/compilers/test_ast.py
index 31c90bd..67e645e 100644
--- a/tests/compilers/test_ast.py
+++ b/tests/compilers/test_ast.py
@@ -249,6 +249,20 @@ def test_ast_good_import_from():
     can_compile("(import [x [y]])")
 
 
+def test_ast_require():
+    "Make sure AST respects (require) syntax"
+    can_compile("(require tests.resources.tlib)")
+    can_compile("(require [tests.resources.tlib [qplah parald]])")
+    can_compile("(require [tests.resources.tlib [*]])")
+    can_compile("(require [tests.resources.tlib :as foobar])")
+    can_compile("(require [tests.resources.tlib [qplah :as quiz]])")
+    can_compile("(require [tests.resources.tlib [qplah :as quiz parald]])")
+    cant_compile("(require [tests.resources.tlib])")
+    cant_compile("(require [tests.resources.tlib [* qplah]])")
+    cant_compile("(require [tests.resources.tlib [qplah *]])")
+    cant_compile("(require [tests.resources.tlib [* *]])")
+
+
 def test_ast_good_get():
     "Make sure AST can compile valid get"
     can_compile("(get x y)")
diff --git a/tests/native_tests/contrib/alias.hy b/tests/native_tests/contrib/alias.hy
index 7f3e66c..a88cb93 100644
--- a/tests/native_tests/contrib/alias.hy
+++ b/tests/native_tests/contrib/alias.hy
@@ -1,4 +1,4 @@
-(require hy.contrib.alias)
+(require [hy.contrib.alias [defn-alias]])
 
 (defn test-defn-alias []
   (defn-alias [tda-main tda-a1 tda-a2] [] :bazinga)
diff --git a/tests/native_tests/contrib/anaphoric.hy b/tests/native_tests/contrib/anaphoric.hy
index 69caac2..60f2998 100644
--- a/tests/native_tests/contrib/anaphoric.hy
+++ b/tests/native_tests/contrib/anaphoric.hy
@@ -19,7 +19,7 @@
 ;; DEALINGS IN THE SOFTWARE.
 
 (import [hy.errors [HyMacroExpansionError]])
-(require hy.contrib.anaphoric)
+(require [hy.contrib.anaphoric [*]])
 
 ;;;; some simple helpers
 
diff --git a/tests/native_tests/contrib/botsbuildbots.hy b/tests/native_tests/contrib/botsbuildbots.hy
index 7fc0d7b..1355119 100644
--- a/tests/native_tests/contrib/botsbuildbots.hy
+++ b/tests/native_tests/contrib/botsbuildbots.hy
@@ -1,5 +1,4 @@
-(import [hy.contrib.botsbuildbots [*]])
-(require hy.contrib.botsbuildbots)
+(require [hy.contrib.botsbuildbots [Botsbuildbots]])
 
 (defn test-botsbuildbots []
   (assert (> (len (first (Botsbuildbots))) 50)))
diff --git a/tests/native_tests/contrib/curry.hy b/tests/native_tests/contrib/curry.hy
index 5d73ca7..7db504f 100644
--- a/tests/native_tests/contrib/curry.hy
+++ b/tests/native_tests/contrib/curry.hy
@@ -1,4 +1,4 @@
-(require hy.contrib.curry)
+(require [hy.contrib.curry [defnc]])
 
 
 (defnc s [x y z] ((x z) (y z)))  ; λxyz.xz(yz)
diff --git a/tests/native_tests/contrib/loop.hy b/tests/native_tests/contrib/loop.hy
index 175111a..5b67c5c 100644
--- a/tests/native_tests/contrib/loop.hy
+++ b/tests/native_tests/contrib/loop.hy
@@ -1,4 +1,4 @@
-(require hy.contrib.loop)
+(require [hy.contrib.loop [loop]])
 (import sys)
 
 (defn tco-sum [x y]
diff --git a/tests/native_tests/contrib/meth.hy b/tests/native_tests/contrib/meth.hy
index e1d42da..7fe30f9 100644
--- a/tests/native_tests/contrib/meth.hy
+++ b/tests/native_tests/contrib/meth.hy
@@ -1,4 +1,4 @@
-(require hy.contrib.meth)
+(require [hy.contrib.meth [route post-route put-route delete-route]])
 
 (defclass FakeMeth []
   "Mocking decorator class"
diff --git a/tests/native_tests/contrib/multi.hy b/tests/native_tests/contrib/multi.hy
index 5ce9932..51162b0 100644
--- a/tests/native_tests/contrib/multi.hy
+++ b/tests/native_tests/contrib/multi.hy
@@ -18,7 +18,7 @@
 ;; FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 ;; DEALINGS IN THE SOFTWARE.
 
-(require hy.contrib.multi)
+(require [hy.contrib.multi [defmulti]])
 
 
 (defn test-basic-multi []
diff --git a/tests/native_tests/language.hy b/tests/native_tests/language.hy
index 54b3e61..b9385db 100644
--- a/tests/native_tests/language.hy
+++ b/tests/native_tests/language.hy
@@ -1101,11 +1101,34 @@
 
 (defn test-require []
   "NATIVE: test requiring macros from python code"
-  (try
-    (assert (= "this won't happen" (qplah 1 2 3 4)))
-  (except [NameError]))
+  (try (qplah 1 2 3 4)
+       (except [NameError] True)
+       (else (assert False)))
+  (try (parald 1 2 3 4)
+       (except [NameError] True)
+       (else (assert False)))
+  (require [tests.resources.tlib [qplah]])
+  (assert (= (qplah 1 2 3) [8 1 2 3]))
+  (try (parald 1 2 3 4)
+       (except [NameError] True)
+       (else (assert False)))
   (require tests.resources.tlib)
-  (assert (= [1 2 3] (qplah 1 2 3))))
+  (assert (= (tests.resources.tlib.parald 1 2 3) [9 1 2 3]))
+  (try (parald 1 2 3 4)
+       (except [NameError] True)
+       (else (assert False)))
+  (require [tests.resources.tlib :as T])
+  (assert (= (T.parald 1 2 3) [9 1 2 3]))
+  (try (parald 1 2 3 4)
+       (except [NameError] True)
+       (else (assert False)))
+  (require [tests.resources.tlib [parald :as p]])
+  (assert (= (p 1 2 3) [9 1 2 3]))
+  (try (parald 1 2 3 4)
+       (except [NameError] True)
+       (else (assert False)))
+  (require [tests.resources.tlib [*]])
+  (assert (= (parald 1 2 3) [9 1 2 3])))
 
 
 (defn test-require-native []
@@ -1125,7 +1148,7 @@
                   (assert (= x [3 2 1]))
                   "success")
               (except [NameError] "failure"))))
-  (require tests.native_tests.native_macros)
+  (require [tests.native_tests.native_macros [rev]])
   (assert (= "success"
              (try
               (do (setv x [])
diff --git a/tests/resources/tlib.py b/tests/resources/tlib.py
index 7ce0ee5..bc214a0 100644
--- a/tests/resources/tlib.py
+++ b/tests/resources/tlib.py
@@ -1,7 +1,12 @@
 from hy.macros import macro
-from hy import HyList
+from hy import HyList, HyInteger
 
 
 @macro("qplah")
 def tmac(*tree):
-    return HyList(tree)
+    return HyList((HyInteger(8), ) + tree)
+
+
+@macro("parald")
+def tmac2(*tree):
+    return HyList((HyInteger(9), ) + tree)