Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

no ns form found #133

Closed
benedekfazekas opened this issue Dec 21, 2015 · 16 comments
Closed

no ns form found #133

benedekfazekas opened this issue Dec 21, 2015 · 16 comments

Comments

@benedekfazekas
Copy link
Member

reported by @whysoserious

Hi, I spotted a weird bug while using clj-refactor (I hope that it's a good place to let you know about that)
In some situations when I invoke cljr- command I get errors like this:

Caused by: java.lang.IllegalStateException: No ns form at /Users/jan/Dev/taxamo-webhooks/test/clj/io/aviso/webhooks/resources/event_delivery_test.clj
 at refactor_nrepl.core$read_ns_form.invoke (core.clj:206)
    refactor_nrepl.ns.ns_parser$get_libspecs_from_file.invoke (ns_parser.clj:140)
    refactor_nrepl.ns.libspecs$put_cached_libspec.invoke (libspecs.clj:33)
    refactor_nrepl.ns.libspecs$get_libspec_from_file_with_caching.invoke (libspecs.clj:41)
    clojure.core$partial$fn__4527.invoke (core.clj:2493)
    clojure.core$juxt$fn__4510.invoke (core.clj:2463)
    clojure.core$map$fn__4553.invoke (core.clj:2624)
    clojure.lang.LazySeq.sval (LazySeq.java:40)
    clojure.lang.LazySeq.seq (LazySeq.java:49)
    clojure.lang.RT.seq (RT.java:507)
    clojure.core/seq (core.clj:137)
    clojure.core$map$fn__4553.invoke (core.clj:2616)
    clojure.lang.LazySeq.sval (LazySeq.java:40)
    clojure.lang.LazySeq.seq (LazySeq.java:49)
    clojure.lang.Cons.next (Cons.java:39)
    clojure.lang.RT.next (RT.java:674)
    clojure.core/next (core.clj:64)
    clojure.core$concat$cat__4217$fn__4218.invoke (core.clj:707)
    clojure.lang.LazySeq.sval (LazySeq.java:40)
    clojure.lang.LazySeq.seq (LazySeq.java:49)
    clojure.lang.ChunkedCons.chunkedNext (ChunkedCons.java:59)
    clojure.core$chunk_next.invoke (core.clj:673)
    clojure.core$reduce1.invoke (core.clj:908)
    clojure.core$reduce1.invoke (core.clj:900)
    clojure.core$merge_with.doInvoke (core.clj:2936)
    clojure.lang.RestFn.applyTo (RestFn.java:139)
    clojure.core$apply.invoke (core.clj:632)
    refactor_nrepl.ns.libspecs$sym_by_file_AMPERSAND_fullname.invoke (libspecs.clj:67)
    refactor_nrepl.ns.libspecs$referred_syms_by_file_AMPERSAND_fullname.invoke (libspecs.clj:78)
    refactor_nrepl.find.find_symbol$find_symbol_in_file.invoke (find_symbol.clj:115)
    clojure.core$partial$fn__4529.invoke (core.clj:2500)
    clojure.core$map$fn__4553.invoke (core.clj:2624)
    clojure.lang.LazySeq.sval (LazySeq.java:40)
    clojure.lang.LazySeq.seq (LazySeq.java:49)
    clojure.lang.RT.seq (RT.java:507)
    clojure.core/seq (core.clj:137)
    clojure.core$apply.invoke (core.clj:630)
    clojure.core$mapcat.doInvoke (core.clj:2660)
    clojure.lang.RestFn.invoke (RestFn.java:423)
    refactor_nrepl.find.find_symbol$find_global_symbol.invoke (find_symbol.clj:140)
    refactor_nrepl.find.find_symbol$find_symbol$fn__36469.invoke (find_symbol.clj:255)
    clojure.core$binding_conveyor_fn$fn__4444.invoke (core.clj:1916)
    clojure.lang.AFn.call (AFn.java:18)
    java.util.concurrent.FutureTask.run (FutureTask.java:266)
    java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1142)
    java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:617)
    java.lang.Thread.run (Thread.java:745)

Message at the top is a lie
This ns form does exist
I need to restart cider to make it work again
and at some random moment I start getting these errors again

@expez
Copy link
Member

expez commented Dec 22, 2015

@benedekfazekas I'm not sure what to do about this. If there's a bug here, it's in tools.namespace. tools.namespace attempts to read an ns form and returns nil when none is found, at which point we throw the exception above because we cannot continue.

I've never never seen this myself and without being able to reproduce it it's going to be real hard to fix.

@whysoserious
Copy link
Contributor

Christmas is coming. No promises but I'll try to find some time and debug this issue.

@benedekfazekas
Copy link
Member Author

@whysoserious some more info, something reproducible would be nice.

@expez no worries. just wanted to record this so we don't forget to follow up (incl. @whysoserious :) )

@whysoserious
Copy link
Contributor

Found it! File at the top of the stacktrace /Users/jan/Dev/taxamo-webhooks/test/clj/io/aviso/webhooks/resources/event_delivery_test.clj had all lines commented out. There was no ns form thus this exception was thrown.

However, my question is, how should we handle such situation? Imho we should silently ignore faulty .clj file and proceed as if it did not exist.

@expez
Copy link
Member

expez commented Dec 23, 2015

However, my question is, how should we handle such situation? Imho we should silently ignore faulty .clj file and proceed as if it did not exist.

Frankly, I think this is fine. There's no reason to have faux clj files on the classpath like this, and the error message was also spot on about how to fix the problem.

I'm always the one taking the more conservative stance, so @benedekfazekas might disagree with me here. I just think this is such an uncommen edge-case that it isn't worth making changes which might cause us to do the wrong thing when instead we should've aborted.

@whysoserious
Copy link
Contributor

I'd make it user-friendly. Empty or commented out .clj files do not break Clojure projects. I can compile such project, create an uberjar and run it. Thus, as a user, I'd expect that refactor-nrepl would work as well.

@magnars
Copy link
Contributor

magnars commented Feb 9, 2016

I'm having the same problem. I suggest two potential fixes:

  • have (setq cljr-ignore-analyzer-errors t) ignore these errors too.
  • add an option that allows the user to specify files or patterns to ignore.

@benedekfazekas
Copy link
Member Author

took a stab on this one (ignoring errors when cljr-ignore-analyzer-errors) but did not really work out unfortunately.

@magnars
Copy link
Contributor

magnars commented Feb 23, 2016

Okay, then my stance is with @whysoserious:

I'd make it user-friendly. Empty or commented out .clj files do not break Clojure projects. I can compile such project, create an uberjar and run it. Thus, as a user, I'd expect that refactor-nrepl would work as well.

@benedekfazekas
Copy link
Member Author

good. then the time I spent on this is not wasted ;) however no fix yet

@benedekfazekas
Copy link
Member Author

After thinking about it I took a more direct way to fix it: I don't see any harm if we are not overly strict with the projects we work with in terms of the presence of the ns declaration. The worst thing that can happen I guess that we can not really do anything with the empty/no ns macro .clj files but I guess no users expects much tooling to work there.

To prove my point I added an empty clj file to our testproject and with minimal changes all the tests pass. As I am aware this is a debated issue I pushed the fix on a branch. Pls have your say ;)

@magnars
Copy link
Contributor

magnars commented Mar 8, 2016

I'm all for this change, since it mirrors how Clojure itself handles
ns-free clj files.
tir. 8. mar. 2016 kl. 16.24 skrev Benedek Fazekas <[email protected]

:

After thinking about it I took a more direct way to fix it: I don't see
any harm if we are not overly strict with the projects we work with in
terms of the presence of the ns declaration. The worst thing that can
happen I guess that we can not really do anything with the empty/no ns
macro .clj files but I guess no users expects much tooling to work there.

To prove my point I added an empty clj file to our testproject and with
minimal changes all the tests pass. As I am aware this is a debated issue I
pushed the fix on a branch. Pls have your say ;)


Reply to this email directly or view it on GitHub
#133 (comment)
.

@benedekfazekas
Copy link
Member Author

snapshot is published on clojars. let me know guys if that fixes your problem.

@magnars
Copy link
Contributor

magnars commented Mar 14, 2016

Great, thanks! I'll check it out.

@magnars
Copy link
Contributor

magnars commented Mar 14, 2016

Wohoo, it's working! <3 Thank you.

@benedekfazekas
Copy link
Member Author

greatto! (was not aware you can do reactions on github now...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants