nrepl / weasel

ClojureScript browser REPL using WebSockets
The Unlicense
324 stars 35 forks source link

Support for multi src path analysis #28

Open mfikes opened 9 years ago

mfikes commented 9 years ago

When I piggyback into the Weasel REPL, many "false-positive" code analysis warnings are emitted. These are mostly of the form "Referred var some.namespace/var-name does not exist," but other similar warnings are emitted. Nothing breaks for me, and the REPL functions.

This occurs when (cljs.repl/analyze-source (:src this)) is called from within weasel.repl.websocket/websocket-setup-env.

I've traced the root cause down to an inability of the ClojureScript compiler to locate the source of dependent namespaces to analyze. To fix this appears to require two changes:

  1. The :root in the ClojureScript compiler environment needs to be set to the source root. This root needs to follow the JVM classpath convention. In other words it needs to be precisely at the level containing the namespace directories. Using the weasel project structure itself as an example, this would be src/cljs/.
  2. For projects that have multiple roots (:source-paths per lein-cljsbuild) a separate analysis needs to be done for each distinct root. Thus, Weasel would need to accomodate a :src-paths option in lieu of its current :src option.

With a :src-paths option, if I replace (cljs.repl/analyze-source (:src this)) with the following form

(let [compiler-env (::env/compiler this)
      previous-root (:root @compiler-env)]
  (doseq [src-path (:src-paths this)]
    (swap! compiler-env assoc :root (File. src-path))
    (env/with-compiler-env compiler-env
      (cljs.repl/analyze-source src-path)))
  (swap! compiler-env assoc :root previous-root))

then piggybacking into Weasel works without any "false-positive" warnings, and if you inspect the ClojureScript compiler's behavior, you can see that it succeeds in loading dependent namespaces and analyzing them.

mfikes commented 9 years ago

One problem with the approach outlined above is that it analyzes source roots in sequence, building up compiler state. This is fine if the source roots are analyzed in an order consistent with their dependency relation, but it doesn't work if the source roots are analyzed in an arbitrary order.

Somehow, the ordering of :source-paths for lein cljsbuild is inconsequential; it copes with any ordering.

mfikes commented 9 years ago

When building with lein cljsbuild, source is located for dependent namespace analysis not by checking for it relative to the :root key in the ClojureScript compiler environment, but by instead relying on io/resource, with the source being on the classpath.

mfikes commented 9 years ago

I can no longer reproduce this issue. I did the due diligence of attempting to revert back to the versions of the releases that existed at the time I wrote this issue in order to reproduce it, with no success. So, I'll close this issue now.

mfikes commented 9 years ago

This actually still occurs; not sure why I failed to reproduce it earlier.

In fact, it appears with the latest ClojureScript compiler (0.0-2655) what was previously a warning becomes an error.

mfikes commented 9 years ago

I've created a reduction in the form of a minimal repository illustrating the problem: https://github.com/mfikes/weasel-src-paths

mfikes commented 9 years ago

I found that, by adding the additional source path to the project-level :source-paths setting (in addition to the cljsbuild build-specific setting), things work. I've added this to the previously mentioned sample repo. But... this wouldn't solve the general problem when you have different sets of source paths per cljsbuild build key.

tomjakubowski commented 9 years ago

I think there's still an issue, although it takes a somewhat different form. Weasel doesn't do its own analysis anymore but I still get missing var warnings in that example project if I run (do (in-ns 'weasel-src-paths.core) (foo)) at a Weasel REPL. I also can't use the referred var (hello) from that namespace at all:

WARNING: Use of undeclared Var weasel-src-paths.core/hello at line 1 <cljs repl>
#<TypeError: Cannot read property 'call' of undefined>
TypeError: Cannot read property 'call' of undefined
    at eval (eval at <anonymous> (file:///home/tom/tmp/weasel-src-paths/target/cljsbuild-main.js:44527:260), <anonymous>:1:96)
    at eval (eval at <anonymous> (file:///home/tom/tmp/weasel-src-paths/target/cljsbuild-main.js:44527:260), <anonymous>:13:4)
    at file:///home/tom/tmp/weasel-src-paths/target/cljsbuild-main.js:44527:255
    at file:///home/tom/tmp/weasel-src-paths/target/cljsbuild-main.js:44537:4
    at G__11673__2 (file:///home/tom/tmp/weasel-src-paths/target/cljsbuild-main.js:31217:116)
    at G__11673 [as call] (file:///home/tom/tmp/weasel-src-paths/target/cljsbuild-main.js:31923:28)
    at null.<anonymous> (file:///home/tom/tmp/weasel-src-paths/target/cljsbuild-main.js:44650:80)
    at goog.events.EventTarget.fireListeners (file:///home/tom/tmp/weasel-src-paths/target/cljsbuild-main.js:38353:23)
    at Function.goog.events.EventTarget.dispatchEventInternal_ (file:///home/tom/tmp/weasel-src-paths/target/cljsbuild-main.js:38398:26)
    at goog.events.EventTarget.dispatchEvent (file:///home/tom/tmp/weasel-src-paths/target/cljsbuild-main.js:38312:34)
tomjakubowski commented 9 years ago

I suspect these errors are related to #48

MalloZup commented 5 years ago

autogenerated with https://github.com/MalloZup/doghub: issue inactive since 450 days. Please update the issue or close it