Fixed https://github.com/clojure/clojurescript/commit/9c39d871fd58a20c75b918564a10072a3200bd40

I spent a bit of time looking at this today. One thing that’s probably not completely obvious is that the point of parse-ns is to have a thing that can compute the dependencies of a namespace w/o analyzing the whole source file. This is required for builds - i.e. what are the files that need to be compiled. We start with main and use parse-ns to figure out the dependency graph quickly.

The problem is that the moment you introduce macros that can require, parse-ns cannot reliably do this job without loading macros. This also complicates self-hosted parity because macros will have to be loaded asynchronously.

There’s also the look-ahead problem which is not solved by the proposed patch - https://clojure.atlassian.net/browse/CLJS-3312

Given these issues and what this follow-up patch has to do I think this is getting pretty messy and we should probably just back out https://clojure.atlassian.net/browse/CLJS-3276 for now.

I thought this would be easy due to the work we did for cljs.user around loading multiple cljs.user files and supporting require usage. But now I see they are fundamentally different problems - the cljs.user case works because it’s a special syntax for writing an implicit ns form. But introducing macros brings a whole level of new complexity.

I’ve attached a patch but not sure it’s good enough.

• parse-ns does not generally analyze deps (unless explicitly asked to do so, but most of the time we don’t want that because it’s costly)

• parse-ns does expand macros following the ns form to support the use case of https://clojure.atlassian.net/browse/CLJS-3276 (injecting namespaces into the build)

• macros like clojure.test/test-ns-block call cljs.analyzer/find-ns, but if they are being expanded as part of parse-ns then not all namespace information is available yet in the compiler env, because we didn’t analyze dependencies.

The current patch makes sure that when parse-ns analyzes successive forms for a given namespace (ns, ns*, require, import, etc), that namespaces that are referenced by previous forms are added to the compiler env (at least by name), so a macro like test-ns-block will find that namespace (it will look like an empty namespace). It will get expanded again when the namespace is fully analyzed later on.

This fixes the case that @Mike Fikes illustrated above, where you (:require [foo.tests)) (run-tests 'foo.tests), but if foo.tests is required transitively then this will break, so this will probably still break some existing builds.

I’m starting to think that maybe https://clojure.atlassian.net/browse/CLJS-3276 should be opt-in? Maybe with metadata on the macro? I know that’s not particularly pretty, but it’s a fairly special use case anyway.

There might be a more elegant solution, but the current analyzer implementation is somewhat hard to follow. I’d have to do a better analysis of the different places where parse-ns is used. Currently it seems it’s called in a few different places to get information about a namespace before it’s fully analyzed, and then again as part of the actual analysis process. Potentially only that last call really needs these potentially-requiring macros to be expanded.