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

Linter error on -> and ->> without forms. #36

Closed
iafilatov opened this issue Dec 11, 2017 · 6 comments
Closed

Linter error on -> and ->> without forms. #36

iafilatov opened this issue Dec 11, 2017 · 6 comments

Comments

@iafilatov
Copy link

-> and ->> with one arg could have it's use cases. For ex. mapping "to a structure":

(map #(-> [%]) [1 2 3]) => ([1] [2] [3])
(map #(-> {:foo %}) [1 2 3]) => ({:foo 1} {:foo 2} {:foo 3})
(map #(-> `(~%)) [1 2 3]) => ((1) (2) (3))

which cause "No forms in ->". This has to be written like

(map #(vec [%]) [1 2 3])          ;; i put vec in your ver so you get vec...
(map #(assoc {} :foo %) [1 2 3])  ;; almost like m = {}; for i in ...
(map #(list %) [1 2 3])           ;; ok, this probably is a tie

I assume the linter wants to guard against mistakes like

(let [val ...]
  (-> val)
  (do-something-with-val))  ;; should be in ->

but I can't think of any examples where this could go unnoticed and not cause problems immediately.

@candid82
Copy link
Owner

Interesting, I did not realize -> can be used this way. I do think this is somewhat uncommon and can easily be rewritten with fn, which is only marginally longer but more clear IMO: (map (fn [x] [x]) [1 2 3]). Of course someone who is used to this trick with -> may find fn version less clear, it's all subjective.
Most of the things that Joker catches would be caught otherwise once sent to and ran in REPL. I still find it very useful to be able to catch silly mistakes as early as possible and consider this the core value of Joker's linter mode. So I'd hesitate removing "no forms" check for threading macros. Perhaps it can be made optional. I've initially wanted to keep Joker's config file as small and simple as possible, but looks like more configurable linting rules are necessary in order to satisfy different people's needs...

@iafilatov
Copy link
Author

Sure, but it becomes worse with more args:

(map #(-> {:foo %2 :bar %1 :baz %4 :qux %3}) coll)

vs.

(map (fn [a b c d] {:foo b :bar a :baz d :qux c}) coll)

Anyway, this is subjective and not really the point. The point is that in this case linter appears to limit the functionality of the language and the "limitations vs. mistakes caught" ratio is not in its favor (unlike, say, checking for things like (is (= (should-return-false)) false) in tests, e.g. one arg forms of =, >, < ...).

@rgdelato
Copy link

rgdelato commented Dec 13, 2017

I also didn't know that anyone used threading macros this way, though I've occasionally used do to achieve a similar effect. The symbols do and -> even have the same number of characters.
(map #(do [%]) [1 2 3])
(map #(do {:foo %2 :bar %1 :baz %4 :qux %3}) [1] [2] [3] [4])

@snoe
Copy link

snoe commented Dec 13, 2017

Most of the things that Joker catches would be caught otherwise once sent to and ran in REPL.

I've hit bugs where the threaded form updates some stateful thing and the error only crops up at some later point, so I appreciate the linting.

(->> things)
(mapv update-in-db!)

The :rules configuration seems like a natural place to turn off these types of rules.

@candid82
Copy link
Owner

This is addressed by 5b5375c: you can now turn off linting for this case by setting no-forms-threading rule to false (it's true by default):

{:rules {:no-forms-threading false}}

Thanks @snoe, @rgdelato and @iafilatov for your input.

@iafilatov
Copy link
Author

Cool, thanks!

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