Support reloading namespaces (#1035)

Fixes #1060

Author: chrisrink10

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
  * Added a compiler metadata flag for suppressing warnings when Var indirection is unavoidable (#1052)
  * Added the `--emit-generated-python` CLI argument to control whether generated Python code strings are stored by the runtime for each compiled namespace (#1045)
+ * Added the ability to reload namespaces using the `:reload` flag on `require` (#1060)
 
 ### Changed
  * The compiler will issue a warning when adding any alias that might conflict with any other alias (#1045)

pyproject.toml

@@ -32,6 +32,7 @@ include = ["README.md", "LICENSE"]
 [tool.poetry.dependencies]
 python = "^3.8"
 attrs = ">=22.2.0"
+graphlib-backport = { version = "^1.1.0", python = "<3.9" }
 immutables = ">=0.20,<1.0.0"
 prompt-toolkit = ">=3.0.0,<4.0.0"
 pyrsistent = ">=0.18.0,<1.0.0"

src/basilisp/core.lpy

@@ -4944,7 +4944,7 @@
   Basilisp will attempt to load a corresponding namespace starting with 'basilisp.'
   automatically, aliasing the imported namespace as the requested namespace name.
   This should allow for automatically importing Clojure's included core libraries such
-  as ``clojure.string``"
+  as ``clojure.string``."
   [req]
   (rewrite-clojure-libspec
    (cond
@@ -4955,26 +4955,49 @@
                     (ex-info "Invalid libspec for require"
                              {:value req})))))
 
+(def ^:private require-flags #{:reload :reload-all})
+
+(defn ^:private libspecs-and-flags
+  "Return a vector of ``[libspecs flags]``."
+  [req]
+  (let [groups (group-by #(if (contains? require-flags %)
+                            :flags
+                            :libspecs)
+                         req)]
+    [(:libspecs groups) (set (:flags groups))]))
+
 (defn ^:private require-lib
-  "Require the library described by ``libspec`` into the Namespace ``requiring-ns``\\."
-  [requiring-ns libspec]
-  (let [required-ns-sym  (:namespace libspec)]
-    ;; In order to enable direct linking of Vars as Python variables, required
-    ;; namespaces must be `require*`ed into the namespace. That's not possible
-    ;; to do without a macro, so we're using this hacky approach to eval the
-    ;; code directly (which will effectively add it to the root namespace module).
-    (eval (list 'require*
-                (if-let [ns-alias (:as libspec)]
-                  [required-ns-sym :as ns-alias]
-                  required-ns-sym))
-          requiring-ns)
-    ;; Add a special alias for the original namespace (e.g. `clojure.*`), if we
-    ;; rewrote the namespace on require.
-    (when-let [original-ns (:original-namespace libspec)]
-      (.add-alias requiring-ns (the-ns required-ns-sym) original-ns))
-    ;; If an `:as-alias` is requested, apply that as well.
-    (when-let [as-alias (:as-alias libspec)]
-      (.add-alias requiring-ns (the-ns required-ns-sym) as-alias))
+  "Require the library described by ``libspec`` into the Namespace ``requiring-ns``\\
+  subject to the provided ``flags``."
+  [requiring-ns libspec flags]
+  (let [required-ns-sym  (:namespace libspec)
+        existing-ns      (find-ns required-ns-sym)]
+    (cond
+      #?@(:lpy39+
+          [(and existing-ns (contains? flags :reload-all))
+           (.reload-all (the-ns required-ns-sym))])
+
+      (and existing-ns (contains? flags :reload))
+      (.reload (the-ns required-ns-sym))
+
+      :else
+      (do
+        ;; In order to enable direct linking of Vars as Python variables, required
+        ;; namespaces must be `require*`ed into the namespace. That's not possible
+        ;; to do without a macro, so we're using this hacky approach to eval the
+        ;; code directly (which will effectively add it to the root namespace module).
+        (eval (list 'require*
+                    (if-let [ns-alias (:as libspec)]
+                      [required-ns-sym :as ns-alias]
+                      required-ns-sym))
+              requiring-ns)
+        ;; Add a special alias for the original namespace (e.g. `clojure.*`), if we
+        ;; rewrote the namespace on require.
+        (when-let [original-ns (:original-namespace libspec)]
+          (.add-alias requiring-ns (the-ns required-ns-sym) original-ns))
+        ;; If an `:as-alias` is requested, apply that as well.
+        (when-let [as-alias (:as-alias libspec)]
+          (.add-alias requiring-ns (the-ns required-ns-sym) as-alias))))
     ;; Reset the namespace to the requiring namespace, since it was likely changed
     ;; during the require process
     (set! *ns* requiring-ns)))
@@ -5032,17 +5055,44 @@
   - ``:refer [& syms]`` which will refer syms in the local namespace directly
   - ``:refer :all`` which will refer all symbols from the namespace directly
 
+  Arguments may also be flags, which are optional. Flags are keywords. The following
+  flags are supported:
+
+  - ``:reload`` if provided, attempt to reload the namespace
+  - ``:reload-all`` if provided, attempt to reload all named namespaces and the
+    namespaces loaded by those namespaces as a directed graph
+
   Use of ``require`` directly is discouraged in favor of the ``:require`` directive in
-  the :lpy:fn:`ns` macro."
+  the :lpy:fn:`ns` macro.
+
+  .. warning::
+
+     Reloading namespaces has many of the same limitations described for
+     :external:py:func:`importlib.reload_module`. Below is a non-exhaustive set of
+     limitations of reloading Basilisp namespace:
+
+     - Vars will be re-``def``'ed based on the current definition of the underlying
+       file. If the file has not changed, then the namespace file will be reloaded
+       according to the current :ref:`namespace_caching` settings. If a Var is
+       removed from the source file, it will not be removed or updated by a reload.
+     - References to objects from previous versions of modules (particularly those
+       external to the namespace) are not rebound by reloading. In Basilisp code,
+       this problem can be limited by disabling :ref:`inlining` and
+       :ref:`direct_linking`.
+     - Updates to type or record definitions will not be reflected to previously
+       instantiated objects of those types."
   [& args]
-  (let [current-ns *ns*]
-    (doseq [libspec (map require-libspec args)]
+  (let [current-ns *ns*
+        groups     (libspecs-and-flags args)
+        libspecs   (first groups)
+        flags      (second groups)]
+    (doseq [libspec (map require-libspec libspecs)]
       (if (and (:as-alias libspec) (not (:as libspec)))
         (let [alias-target (or (find-ns (:namespace libspec))
                                (create-ns (:namespace libspec)))]
           (.add-alias current-ns alias-target (:as-alias libspec)))
         (do
-          (require-lib current-ns libspec)
+          (require-lib current-ns libspec flags)
 
           ;; Add refers
           (let [new-ns    (the-ns (:namespace libspec))
@@ -5075,12 +5125,18 @@
   ``:require`` directive of the :lpy:fn:`ns` macro or the ``:use`` directive of the
   ``ns`` macro."
   [ns-sym & filters]
-  (let [current-ns *ns*]
+  (let [current-ns *ns*
+        groups     (group-by #(if (contains? require-flags %)
+                                :flags
+                                :filters)
+                             filters)]
     ;; It is necessary to first require the Namespace directly, otherwise
     ;; when refer attempts to load the Namespace later, it will fail.
-    (->> (require-libspec ns-sym)
-         (require-lib current-ns))
-    (->> (apply vector ns-sym filters)
+    (require-lib current-ns
+                 (require-libspec ns-sym)
+                 (set (:flags groups)))
+    (->> (:filters groups)
+         (apply vector ns-sym)
          (require-libspec)
          (refer-lib current-ns))))
 
@@ -5113,12 +5169,16 @@
   Use of ``use`` directly is discouraged in favor of the ``:use`` directive in the
   :lpy:fn:`ns` macro."
   [& args]
-  (let [current-ns *ns*]
-    (doseq [libspec (map require-libspec args)]
+  (let [current-ns *ns*
+        groups     (libspecs-and-flags args)
+        libspecs   (first groups)
+        flags      (second groups)]
+    (doseq [libspec (map require-libspec libspecs)]
       ;; Remove the :refer key to avoid having require-lib
       ;; perform a full :refer, instead we'll use refer-lib
-      (->> (dissoc libspec :refer)
-           (require-lib current-ns))
+      (require-lib current-ns
+                   (dissoc libspec :refer)
+                   flags)
       (refer-lib current-ns libspec))
     nil))
 

src/basilisp/importer.py

@@ -19,6 +19,8 @@
     cast,
 )
 
+from typing_extensions import TypedDict
+
 from basilisp.lang import compiler as compiler
 from basilisp.lang import reader as reader
 from basilisp.lang import runtime as runtime
@@ -133,12 +135,17 @@ def _is_namespace_package(path: str) -> bool:
     return no_inits and has_basilisp_files
 
 
+class ImporterCacheEntry(TypedDict, total=False):
+    spec: ModuleSpec
+    module: BasilispModule
+
+
 class BasilispImporter(MetaPathFinder, SourceLoader):  # pylint: disable=abstract-method
     """Python import hook to allow directly loading Basilisp code within
     Python."""
 
     def __init__(self):
-        self._cache: MutableMapping[str, dict] = {}
+        self._cache: MutableMapping[str, ImporterCacheEntry] = {}
 
     def find_spec(
         self,
@@ -379,7 +386,11 @@ def exec_module(self, module: types.ModuleType) -> None:
         assert isinstance(module, BasilispModule)
 
         fullname = module.__name__
-        cached = self._cache[fullname]
+        if (cached := self._cache.get(fullname)) is None:
+            spec = module.__spec__
+            assert spec is not None, "Module must have a spec"
+            cached = {"spec": spec}
+            self._cache[spec.name] = cached
         cached["module"] = module
         spec = cached["spec"]
         filename = spec.loader_state["filename"]

src/basilisp/lang/runtime.py

@@ -71,6 +71,11 @@
 from basilisp.lang.util import OBJECT_DUNDER_METHODS, demunge, is_abstract, munge
 from basilisp.util import Maybe
 
+if sys.version_info >= (3, 9):
+    import graphlib
+else:
+    import graphlib  # type: ignore
+
 logger = logging.getLogger(__name__)
 
 # Public constants
@@ -670,6 +675,49 @@ def __repr__(self):
     def __hash__(self):
         return hash(self._name)
 
+    def _get_required_namespaces(self) -> vec.PersistentVector["Namespace"]:
+        """Return a vector of all required namespaces (loaded via `require`, `use`,
+        or `refer`).
+
+        This vector will include `basilisp.core` unless the namespace was created
+        manually without requiring it."""
+        ts: graphlib.TopologicalSorter = graphlib.TopologicalSorter()
+
+        def add_nodes(ns: Namespace) -> None:
+            # basilisp.core does actually create a cycle by requiring namespaces
+            # that require it, so we cannot add it to the topological sorter here,
+            # or it will throw a cycle error
+            if ns.name == CORE_NS:
+                return
+
+            for aliased_ns in ns.aliases.values():
+                ts.add(ns, aliased_ns)
+                add_nodes(aliased_ns)
+
+            for referred_var in ns.refers.values():
+                referred_ns = referred_var.ns
+                ts.add(ns, referred_ns)
+                add_nodes(referred_ns)
+
+        add_nodes(self)
+        return vec.vector(ts.static_order())
+
+    def reload_all(self) -> "Namespace":
+        """Reload all dependency namespaces as by `Namespace.reload()`."""
+        sorted_reload_order = self._get_required_namespaces()
+        logger.debug(f"Computed :reload-all order: {sorted_reload_order}")
+
+        for ns in sorted_reload_order:
+            ns.reload()
+
+        return self
+
+    def reload(self) -> "Namespace":
+        """Reload code in this namespace by reloading the underlying Python module."""
+        with self._lock:
+            importlib.reload(self.module)
+            return self
+
     def require(self, ns_name: str, *aliases: sym.Symbol) -> BasilispModule:
         """Require the Basilisp Namespace named by `ns_name` and add any aliases given
         to this Namespace.
@@ -686,6 +734,7 @@ def require(self, ns_name: str, *aliases: sym.Symbol) -> BasilispModule:
             ns_sym = sym.symbol(ns_name)
             ns = self.get(ns_sym)
             assert ns is not None, "Namespace must exist after being required"
+            self.add_alias(ns, ns_sym)
             if aliases:
                 self.add_alias(ns, *aliases)
             return ns_module