New linter to warn about mismatch between :arglists and actual fn args

[jafingerhut]

:arglists is sometimes modified by library developers in an attempt to document variations on argument lists when there are many possibilities. Nicola has pointed out that the Clojure documentation for def [1] says that :arglists will be "a list of vector(s) of argument forms, as were supplied to defn" and the clojure Compiler uses :arglists for type-hints handling.

http://clojure.org/special_forms#Special%20Forms--%28def%20symbol%20init?%29

The warning message should have a brief description, perhaps with a link to a section of the Eastwood docs explaining the situation, and what is recommended instead. Bonus points if reply changes 'doc' macro check for a different keyword like :doc-arglists before :arglists (or shows both if they both exist)

[jafingerhut]

Discussion of technical feasibility of this idea, and counterargument why maybe it should not be done this way, here: https://groups.google.com/forum/#!topic/clojure/5mfnoVWYjRs

[jafingerhut]

Clojure macros that explicitly set :arglists to something noticeably different than the actual argument lists: defn, defmacro, defmulti, ns. There are many other occurrences of :arglists in core.clj, but they all set the :arglists to exactly what it would be if defn were available and had been used. I think the main reason there are so many :arglists occurrences in core.clj is because most of them are before the definition of defn itself.

Grep'ing for "arglists" in Clojure's Java source code finds several occurrences in Compiler.java and RT.java. Investigate those to see how it is used by the compiler.

[jafingerhut]

First version of :bad-arglists linter commited here: c2b8cb5

I will wait to close this issue until a few more enhancements are made to it.

[gfredericks]

Would this catch shenanigans such as: https://github.com/clojure/java.jdbc/blob/master/src/main/clojure/clojure/java/jdbc.clj#L979-981?

My project is currently failing the :wrong-arity linter because of calling insert! with a totally acceptable five arguments.

[jafingerhut]

I am still not quite decided what to do about this idea. It looks like you were also involved in the clojure-dev discussion about my idea for adding a :doc-arglists key that would be used by doc when it exists, and encouraging Clojure developers to override that key instead of :arglists when they want to make human-readable arglists for documentation purposes, but not for automatic parsing by Clojure tools.

https://mail.google.com/mail/u/0/#search/label%3Aclojure-dev+doc-arglists/143a42591d46b8e6

If something like that occurs, then something like this linter idea in Eastwood would be a way to encourage people to switch over to 'the new way'.

If nothing like that occurs, then this linter still might be useful, but it might merely tell one about what can go wrong with the Eastwood :wrong-arity linters and why, rather than actually be a sign of a bug in your code.

[gfredericks]

Yeah, it also brings up the question of suppressing warnings (via metadata?). I mentioned that in IRC and @Bronsa pointed out plausible uses for suppressing warnings on forms that can't take metadata, such as keywords (for keyword-misspelling linters). I assume that's been thought about a lot more than I personally have though.

[gfredericks]

Keyword misspellings could be dealt with via a more global registry of intentional keywords.

[jafingerhut]

Yes, there is a separate issue for suppressing warnings on individual forms here: #21 with some discussion.

My current thinking there is that using metadata will likely be good enough for most purposes where one would like to suppress lint warnings, even though it can not be applied to arbitrary Clojure subexpressions like keywords, numbers, strings, etc. If cases arise where someone wants even more focused warnings suppression than that, we'll have to think about it more, or do something like what you suggest for keywords warnings.

[gfredericks]

Thanks @jafingerhut

[lvh]

FYI, I'm hitting the macro issue mentioned above when using defspec from clojure test.check. I get linter warnings like:

{:linter :bad-arglists,
:msg "Function on var mem-store-roundtrip defined taking # args [0 1 :or-more] but :arglists metadata has # args [0]",
:file "icecap/store/mem_test.clj",
:line 8,
:column 10}

I must admit I don't quite understand this bug, because when I inspect the metadata, I see:

icecap.store.mem-test> (meta #'mem-store-roundtrip)
{:ns #<Namespace icecap.store.mem-test>,
 :name mem-store-roundtrip,
 :file "icecap/store/mem_test.clj",
 :column 1,
 :line 8,
 :arglists
 ([]
  [times__20047__auto__
   &
   {:as quick-check-opts__20050__auto__,
    :keys [seed__20048__auto__ max-size__20049__auto__]}]),
 :test #<mem_test$fn__24298 icecap.store.mem_test$fn__24298@66eee890>,
 :clojure.test.check.clojure-test/defspec true}

... so it appears the :arglists does specify multiple arities?

[muhuk]

I'm having the exact same issue with @lvh.

[jafingerhut]

I have cloned the icecap project mentioned above: https://github.com/lvh/icecap

I have this commit id: cadaeb01f81ff4b886d5d44db3609a32b1c7229c

I have run 'lein eastwood' with Eastwood versions 0.1.4 through 0.2.2 in the root directory of this project, and do not see any :bad-arglists warnings at all.

Can you reproduce the warning using 'lein eastwood', perhaps with some extra command line options?

I tried running 'lein lint' as in the log linked here: https://travis-ci.org/lvh/icecap/jobs/36042714#L1426 but I got the following error message:
Could not find artifact jonase:kibit:jar:0.0.9-SNAPSHOT
This could be due to a typo in :dependencies or network issues.
If you are behind a proxy, try setting the 'http_proxy' environment variable.

Is there another command line can you reproduce it with?

[muhuk]

@jafingerhut in my case I don't have icecap as a dep. No special command, just lein eastwood.

If you can't reproduce I can try creating a minimal project and pust it to GH.

[jafingerhut]

I see a similar bad-arglists warning when I run Eastwood version 0.2.1 on the automat project, which uses defspec from test.check.

I made some changes to the bad-arglists linter in Eastwood version 0.2.2, very recently released, which eliminate that warning. Can you try out Eastwood 0.2.2 to see if it helps in your case, too?

[muhuk]

I've just checked with 0.2.2 and that warning is gone. Awesome! Thanks.

[lvh]

Same here, thanks!

[jafingerhut]

bad-arglists linter seems to be in a reasonable state as of Eastwood 0.2.2 release. All issues raised in the comments for this issue are either taken care of, have a separate issue raised for them, except for the :doc-arglists idea, which is a suggested change in Clojure itself that seems unlikely to occur.