venantius / yagni

A Leiningen plugin for finding dead code
Eclipse Public License 1.0
219 stars 10 forks source link

Support clojurescript projects #26

Open viebel opened 9 years ago

viebel commented 9 years ago

I have tried to run lein yagni on a very small cljs project: https://github.com/viebel/cljs-explore And nothing happened. The cljs files are not parsed.

venantius commented 9 years ago

I'm pushing this to the 0.2.0 release.

The first blocker is a clojure.tools.namespace problem - Yagni uses the project's namespace tracker to figure out which namespaces exist based on which files can be found in the source-paths, but http://dev.clojure.org/jira/browse/TNS-5?page=com.atlassian.jira.plugin.system.issuetabpanels:changehistory-tabpanel#issue-tabs indicates that it only targets .clj files, ignoring other filetypes in the .clj* universe.

There may be additional work to do after that, I don't know, but that's the first place to start.

viebel commented 9 years ago

I have submitted a patch in Dec 14 regarding cljs support in clojure.tools.namespace. It has been rejected because "until a final decision has been made regarding Feature Expressions, which may include alternative file extensions."

Now that clojure supports reader conditionals, the patch might be approved

venantius commented 9 years ago

Hah, that's funny.

In any case, I'm fine building a direct workaround into Yagni in the meanwhile. I don't see a particularly compelling reason to delay implementing this until tools.namespace catches up to reader conditionals.

Only thing to note is that I personally do have something of a development schedule here. If you felt like taking it upon yourself to put together a PR I'd accept it, but otherwise I'm going to prioritize better JVM interop and tagged literals for the 0.1.x releases and delay implementation of this for 0.2.0

viebel commented 9 years ago

hmm.. I gave it a try but it's not easy: how could I make clojure.tools.namespace.track/load parse cljs files?

Meanwhile, I have renamed my cljs files to clj in order to make them parsed by yagni. But it didn't work at all. Running lein yagni is causing a compilation of the project. The compilation failed with:

Unable to resolve symbol: clj->js in this context, compiling:(cljs_explore/foo.clj:4:14).

So I removed the cljs specific code from my code. And then it works.

Also, lein yagni seems to trigger a running of the project. Do you know why?

venantius commented 9 years ago

The best way would probably be to refactor some reasonable section of that codebase out and just place it directly in Yagni; an alternative would be to use with-redefs or equivalent to just redefine the behavior of a single function.

The compilation error you're experiencing there is not being caused by Yagni, it's being caused by the fact that renaming the .cljs file to .clj means that you don't have access to clj->js anymore.

Yagni doesn't trigger a running of the project, but require-ing a namespace that has import side effects (as your sample project does) causes such side effects to evaluate.

viebel commented 9 years ago
  1. I'll give it a try later
  2. You mean that when we'll include cljs namespaces, this error will disappear?
  3. Be aware that cljs project almost always have side-effects. There is no main function called from outside. The code executes itself. So there will almost always be a namespace with side effects
venantius commented 9 years ago
  1. I'm doing a lot of work on the 0.1.2 branch right now, which is mostly addressing some of the problems Yagni currently has with JVM interoperability, and the more time I've spent there the more I suspect I'm likely to see a similar rabbit warren of compiler issues when it comes to ClojureScript. In particular, as I don't have as much experience with ClojureScript, I'm worried that resolving JS interop references may prove to be tricky, but that will just have to be a bridge that we cross when we come to it.
  2. Yes, it should. The error is a Clojure compiler error - nothing to do with Yagni. If you were to try to load that namespace into a Clojure REPL with the .clj extension you should see the same error.
  3. Hmm. There's some subtlety here; cljs projects targeting the Node runtime will have a main method, and others could take advantage of the sort of code layout in the Modern ClojureScript tutorial here, but it is likely the sort of thing we'd want to be explicit about in the README.
(defn init []
  (if (and js/document
           (.-getElementById js/document))
    (let [theForm (.getElementById js/document "shoppingForm")]
      (set! (.-onsubmit theForm) calculate))))

(set! (.-onload js/window) init)
viebel commented 9 years ago

look at this thread about cljs support in tools.namespace

venantius commented 9 years ago

Good catch, I'll comment in there.

viebel commented 9 years ago

any update about supporting cljs support

venantius commented 9 years ago

None, but you'll be happy to know that I'm now actively developing in ClojureScript on a daily basis, so I'll probably take a serious look at doing a port in the near future.

viebel commented 9 years ago

Great! Happy cljs coding :)

On Fri, Oct 2, 2015 at 3:27 AM, Ursa americanus kermodei < notifications@github.com> wrote:

None, but you'll be happy to know that I'm now actively developing in ClojureScript on a daily basis, so I'll probably take a serious look at doing a port in the near future.

— Reply to this email directly or view it on GitHub https://github.com/venantius/yagni/issues/26#issuecomment-144883689.

"Are we what we become or do we become what we are?" - Prof. Beno Gross

arichiardi commented 8 years ago

This would be a great addition given that eastwood does not support ClojureScript. Any update/things to do?

venantius commented 8 years ago

At the moment I'm afraid my attentions are focused elsewhere. I'd happily take a pull request if someone felt up to the challenge.

vemv commented 7 years ago

Still no luck?

Cheers - Victor

venantius commented 7 years ago

I'm afraid Yagni is basically unmaintained at the moment. My day job right now doesn't involve me writing Clojure and while I use some of my Clojure tooling with frequency (e.g. Ultra, my vim plugins) I'm not using Yagni at all. If I were going to return to this project I would probably consider a comprehensive rewrite.

venantius commented 7 years ago

That having been said, I'm more than open to a PR.

vemv commented 7 years ago

Thanks for taking the time to answer - much appreciated!

st4cks1defl0w commented 4 years ago

If anyone is interested, I wrote a complementary tool for cljs https://github.com/stacksideflow/cljs-yagni (implementation-wise this tool is very different though, based on dynamic analysis and source-maps), tested on some biggish codebases.