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

Option to accept specific unqualified symbols? #50

Closed
blak3mill3r opened this issue Mar 10, 2018 · 11 comments
Closed

Option to accept specific unqualified symbols? #50

blak3mill3r opened this issue Mar 10, 2018 · 11 comments

Comments

@blak3mill3r
Copy link
Contributor

I understand that joker doesn't try to resolve symbols from external namespaces, and it does show an error if I use a symbol that was referred with a :refer :all in the require form.

I do not use :refer :all much, but there are a few cases (like com.rpl.specter) where I make an exception, and I would be fine with configuring joker to know about those few exceptions.

Would it be easy to add an option I could put in the .joker config file :known-unqualified-syms [ALL collect collect-one MAP-KEYS ... ] and then not get false positives from joker when using those symbols unqualified? (or, is there another way to silence these warnings that I missed?)

If someone can point me in the right direction, I might be able to do it myself. I just discovered joker today (and it's awesome! 🚀 ).

@candid82
Copy link
Owner

candid82 commented Mar 10, 2018

This should be relatively easy to implement, but I am not sure this is a good idea. I don't find having to refer each individual var in require too big of a price for the benefits symbol resolution linting provides. If you do (ns xxx (:require [com.rpl.specter :refer [collect collect-one...]])) you essentially get your :known-unqualified-syms option, only on a per-file basis. This might be slightly more tedious compared to global option if you have lots of files that use com.rpl.specter, but it gives you fine-grained control and prevents potential false negatives. I also find it useful in general to be able to tell where each symbol in the file comes from just by looking at ns declaration.
If enough people ask for it, I might add the global option, but right now I am not fully convinced it's necessary.

@blak3mill3r
Copy link
Contributor Author

blak3mill3r commented Mar 10, 2018

Yeah, I hear you, and I agree, and tend to avoid :refer :all and encourage everyone else to do so.

This might be slightly more tedious compared to global option if you have lots of files that use com.rpl.specter

That's the thing... I have tons of those. I use specter all over the place, I have 100% drunk its kool-aid and want its many symbols referred everywhere, just like clojure.core. Being explicit in every single ns form for something so ubiquitous in our code would be quite burdensome, so we make an exception and :refer :all it.

Without this option, I have to tolerate the false-positive linter errors (ugh), or not use it :( , or change hundreds of ns forms.

@candid82
Copy link
Owner

OK, this is not my cup of tee, but it does look like a legitimate use case. I've implemented a feature that should help with this and other similar issues and make the linter more customizable. Joker will now execute the following files (if they exist) before linting your file: .jokerd/linter.cljc (for both Clojure and ClojureScript), .jokerd/linter.clj (Clojure only), .jokerd/linter.cljs (ClojureScript only). The rules for locating .jokerd directory are the same as for locating .joker file.

To make Joker aware of Specter definitions, you can put something like this in .jokerd/linter.clj file:

(defmacro transform [apath transform-fn structure])
... <other Specter declarations>

You don't need (or want) to provide the bodies, just the names and args lists.
This should work better than a simple list of "known" symbols as this allows Joker to distinguish macros vs function vs everything else and warn on invalid arities.
To get rid of "unused namespace" warning, you can add com.rpl.specter to :ignored-unused-namespaces in your .joker file:

{:ignored-unused-namespaces [com.rpl.specter]}

It'd be great if you could try this out and let me know how it works in practice.

@blak3mill3r
Copy link
Contributor Author

Great! I would love to try it. I wasn't able to get it to compile with go 1.8.3. Did you do a new compiled release?

@blak3mill3r
Copy link
Contributor Author

tj/node-prune#13 looks like maybe I need 1.9.x ... I will try that and report back.

@candid82
Copy link
Owner

I build with 1.9 locally, have not tried previous versions. I can build a binary for you if it's easier. Don't want to do a release just yet.

@blak3mill3r
Copy link
Contributor Author

blak3mill3r commented Mar 13, 2018

It works with 1.10

@candid82
Copy link
Owner

Cool!
One correction: if you put (defmacro transform ...) (with an empty body) in the linter file, Joker will execute it while linting your file, essentially removing (and therefore skipping linting of) everything inside transform call. If this is not desirable, try declaring macros as functions:

(defn transform [apath transform-fn structure])

See this test for full example.

@blak3mill3r
Copy link
Contributor Author

Aha, I noticed that it wasn't linting the symbols passed to transform when I followed your first instructions.

This is fantastic, it's much better than the simple thing I had in mind!

The only thing that did not work for me just now is putting this in ~/.joker

{:ignored-unused-namespaces [com.rpl.specter]}

It works as expected if I put that in a parent directory of the file being linted.

@blak3mill3r
Copy link
Contributor Author

Ah, perhaps this is because I have a .joker file in one of the parent directories, so it uses the first one found (it does not merge them)...

@candid82
Copy link
Owner

Correct, it uses the first one found and doesn't merge configs.

blak3mill3r added a commit to blak3mill3r/joker that referenced this issue Jun 8, 2019
mention `(in-ns 'joker.core)` and link to candid82#50
blak3mill3r added a commit to blak3mill3r/joker that referenced this issue Jun 8, 2019
mention `(in-ns 'joker.core)` and link to candid82#50
blak3mill3r added a commit to blak3mill3r/joker that referenced this issue Jun 8, 2019
mention `(in-ns 'joker.core)` and link to candid82#50
blak3mill3r added a commit to blak3mill3r/joker that referenced this issue Jun 8, 2019
mention `(in-ns 'joker.core)` and link to candid82#50
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

2 participants