unresolved symbol

[borkdude]

Report unresolved vars

@mynomoto in the #clj-kondo slack channel:

I would be happy with what joker does, do not support use nor :refer-all.

• register var using declare and def
• ignore methods like .getMessage, anything starting with a dot probably
• special symbols like catch and finally (only in try)
• only report first occurrence of unresolved symbol
• defprotocol: take into account doctrings
• defrecord introduces map->...
• CLJS vars are not recognized: -hash, *print-err-fn*, js*, *eval-fn*, -write, -flush
• Add tests for extracting vars like the above
• local ns config
• multiple namespaces with different configs in 1 file
• defmethod false positive: unresolved symbol #174 (comment)
• if-some false positive: unresolved symbol #174 (comment)
• config for excluding false positives
• resolve all public classes from java.lang (we can leverage our ExtractJava stuff for this, might be good to check against joker's list here
• resolve java imports (:import (clojure.lang ExceptionInfo))
• some redundant lets seem to be unreported (spec/alpha.clj line 976)
• nested map destructuring: (let [{{:keys [:a]} :stats} {:stats {:a 1}}] a)
• fully qualified java classes: clojure.lang.BigInt
• areduce
• this-as
• memfn
• goog-define

[borkdude]

@mynomoto Since you were the first to ask for this feature, would you be willing to give an early version of this a try?

A binary can be downloaded by going to CircleCI https://circleci.com/gh/borkdude/clj-kondo/tree/unresolved-symbols, select the newest buildlinux job, go to artifacts and download the zip.

Or try out the unresolved-symbols branch from Clojure.

I'm inclined to set the default level for this linter to :off since it may result in too many false positives for some people. I'm curious to here your feedback.

[mynomoto]

I'm trying to get the binary but cannot find it on https://circleci.com/gh/borkdude/clj-kondo/3529 (the latest buildlinux). There is a step Uploading artifacts but could not find a section or another thing called artifacts on that page.

[borkdude]

@mynomoto Aw, it seems the assets view isn't public on CircleCI. Here's the link to the latest:
https://3529-176829714-gh.circle-artifacts.com/0/release/clj-kondo-2019.06.08-alpha-SNAPSHOT-linux-amd64.zip
It seems if you update the build number (the first 4 digits in the link), that works.

[mynomoto]

Some false positives:

• Destructuring and using a function from a map
• defrecord (fields are used elsewhere from this)
• macros that define variables (expected)

I'm not sure about how to proceed on the last one because it happens a lot and may require lots of configuration. Joker solves it by allowing to define equivalent macros that joker understand.

[borkdude]

@mynomoto Could you give a code example the first one? E.g.:

(let [{:keys [:foo]} {:foo (fn [])}]
  (foo))

seems to work?

[mynomoto]

The one that I found is a destructuring on defmethod arguments.

[borkdude]

@mynomoto:
Can you try again with: https://3531-176829714-gh.circle-artifacts.com/0/release/clj-kondo-2019.06.08-alpha-SNAPSHOT-linux-amd64.zip
I believe the first two issues to be resolved.

For the third option, there is already the option to lint a macro as if it is a built-in macro. If you could provide a code example of such a macro, I could look if that's possible and else figure out something else, possibly looking at how joker solves it.

[mynomoto]

We have many macros, most of def-something type, which is simple to deal with in the way you are describing. But we have a macro to generate standalone functions for all protocol defined functions (to spec the functions), which is too specific for clj-kondo to be concerned with. And we use a library https://github.com/nubank/matcher-combinators that introduces a symbol match? via side-effects.

[borkdude]

@mynomoto In the latter case, were you able to solve it with joker and if so, how? Could you give a code snippet example?

[borkdude]

@mynomoto I'm considering this new feature: a namespace local config. This only works regarding unresolved symbols for now:

That should make it easier to exclude unresolved symbols from being warned about on a per-namespace basis?

binary: https://3538-176829714-gh.circle-artifacts.com/0/release/clj-kondo-2019.06.08-alpha-SNAPSHOT-linux-amd64.zip

[mynomoto]

@mynomoto In the latter case, were you able to solve it with joker and if so, how? Could you give a code snippet example?

I'm not sure about what joker is doing because I have no configuration for that and it doesn't complain. It may treat macros differently.

That should make it easier to exclude unresolved symbols from being warned about on a per-namespace basis?

I probably wouldn't use that because naming is uniform on the application I'm working so global would work better for this use case. I also don't like to have to change things in two places, I think that is more error prone than I'm confortable with.

[borkdude]

@mynomoto so can you give me an example of a code fragment where joker doesn't complain (without config) and clj-kondo does?

[mynomoto]

Sure:

(ns cljkondo
  (:require
    [clojure.test :refer [deftest testing is]]
    matcher-combinators.test))

(deftest cljkondo
  (testing "tef-checkings"
    (is
      (match?
        {:foo "000000000000000070"
         :bar "000004"}
        {:foo "000000000000000070"
         :bar "000004"}))))

clj-kondo complains about match? and joker doesn't on the above example.

[borkdude]

@mynomoto Thank you for being so helpful with your feedback, I really appreciate it.

This is because joker doesn't report unresolved symbols in *known-macros* and deftest is added to that list by default: https://github.com/candid82/joker/blob/0a944e31b88f0d663977f22eb4553572060dda11/core/data/linter_joker.joke#L2

I guess clj-kondo can take the same approach: don't report unresolved symbols in a configurable list of macros. Would that be better than making a list of excluded symbols?

[mynomoto]

Actually joker is being too broad in this case. It can be configured to whitelist symbols inside macros, which would be more appropriate in this case. In this case, I'm not sure about the mechanism because putting match? inside deftest but outside is causes joker to complain.
The most useful thing would be make clj-kondo aware of macro effects because other linters could work inside the macro. Joker achieves that by expading macros but I know that is not something that clj-kondo is supposed to do. I wonder if it is possible to declare what the macro does without expanding it in a simple/easy way. I had to rewrite some macros for joker and would like to avoid that in the future if possible but that power it possible to support every macro.

[borkdude]

@mynomoto We already have a :skip-args config for some functions/macros that causes the arity linter to be silent inside certain calls. We could extend that to unresolved symbols:

{:linters {:unresolved-symbol {:skip-args [clojure.test/is]}}}

That's effectively the same as joker's :known-macros [clojure.test/is] but only for this type of linter.

Also we could let people specify symbols that are introduced by a macro, something like:

{:linters {:unresolved-symbol {:creates-vars {riemann.streams/where [service event]}}}}

[mynomoto]

I think this may be expected but there is a perf hit on this branch. Before it just returned and now is taking 18 seconds for my project.

[mynomoto]

@mynomoto We already have a :skip-args config for some functions/macros that causes the arity linter to be silent inside certain calls. We could extend that to unresolved symbols:
Also we could let people specify symbols that are introduced by a macro, something like:

I think that is a good place to start, 👍

[borkdude]

@mynomoto

I think this may be expected but there is a perf hit on this branch. Before it just returned and now is taking 18 seconds for my project.

Wow. Not acceptable. That's not what I'm seeing on CircleCI though (https://3563-176829714-gh.circle-artifacts.com/0/release/performance.txt):

linting took 7476ms, errors: 3785, warnings: 1106

==== Lint a single file (emulate in-editor usage)
linting took 34ms, errors: 3, warnings: 0

real 0m0.038s

These times are comparable to the master branch.

Are you using a config file? Can you post it here, so I might be able to reproduce? Is your linted code public, so I can test?

This is the latest linux binary:
https://3563-176829714-gh.circle-artifacts.com/0/release/clj-kondo-2019.06.08-alpha-SNAPSHOT-linux-amd64.zip

[mynomoto]

Other problem is macros that define arbitrary vars, in this case hugsql that creates vars based on statements defined on .sql files. I'm just throwing this here for awareness, not expecting a solution at this point.

[borkdude]

Other problem is macros that define arbitrary vars

This can usually be solved by using (declare introduced-var1 introduced-var2).

[mynomoto]

This can usually be solved by using (declare introduced-var1 introduced-var2).

Nice, thanks for that!

[mynomoto]

Are you using a config file? Can you post it here, so I might be able to reproduce? Is your linted code public, so I can test?

The code is not public, and to be clear for any file the result is instantaneous, linting the whole project for cache purposes is the command that is taking 18 seconds. The whole project has 796 clojure files with 107233 lines of code excluding blanks and comments.

The following is the config file for the project (minus other excluded namespaces)

{:lint-as {cats.core/mlet                        clojure.core/let
           clojure.java.jdbc/with-db-transaction clojure.core/let}
 :linters {:unused-namespace {:exclude [matcher-combinators.test
                                        cats.builtin
                                        ".*\\.spec$"]}
           :unresolved-symbol {:exclude [match?
                                         thrown?
                                         thrown-with-msg?]}}}

[mynomoto]

False positive: using a namespace to define a defmethod is warning that the namespace is unused

(ns cljkondo
  (:require
    [integrant.core :as ig]))

(defmethod ig/pre-init-spec :my/key [_] ::args)

False positive on tag literals

#inst "2019-05-24"

I think both of those are regressions, they were not reported before.

[mynomoto]

False positive on if-some

(if-some [foo true]
  foo
  false)

[borkdude]

@mynomoto Regarding the performance issue, can you try a couple of older versions of clj-kondo and see how they were doing? You can get them at: https://github.com/borkdude/clj-kondo/releases

[mynomoto]

@borkdude call me crazy, cannot reproduce. Lastest 6 versions take about the same time. Sorry about the noise.

[borkdude]

@mynomoto No problem at all, it's good to keep an eye on performance. What I can imagine that happened is that you only linted src before, but later on linted the entire classpath (for filling the cache) or that you maybe added more dependencies.

[borkdude]

@mynomoto The reported false positives should be fixed in https://3579-176829714-gh.circle-artifacts.com/0/release/clj-kondo-2019.06.08-alpha-SNAPSHOT-linux-amd64.zip

[mynomoto]

More false positives

(ns cljkondo
  (:import
    (clojure.lang ExceptionInfo)))

(defn ex-info?
  [x]
  (instance? ExceptionInfo x)) ; error: unresolved symbol ExceptionInfo

(defmacro foo
  [s1 s2]
  `(def ~s1
     ~@(let [v (resolve s2)] ; error: unresolved symbol v
         (when (.hasRoot v)
           (list (.getRawRoot v))))))

Object ; error: unresolved symbol Object
BigDecimal ; error: unresolved symbol BigDecimal

(defprotocol IFoo
  "Foo"
  (-foo [this]))

(identity IFoo) ; error: unresolved symbol IFoo

[borkdude]

@mynomoto Some false positives from the above code have been fixed in https://3597-176829714-gh.circle-artifacts.com/0/release/clj-kondo-2019.06.08-alpha-SNAPSHOT-linux-amd64.zip

Also it supports some form of config which is documented here: https://github.com/borkdude/clj-kondo/blob/unresolved-symbols/doc/config.md#exclude-an-unresolved-symbol-from-being-reported
Please let me know if this config works for you.

One remaining issue is resolving all public classes from the java.lang package. I've already got some code related to this, so that shouldn't be too difficult. The other remaining issue is resolving names from :import which shouldn't be too hard either. I'll report back when those issues are fixed.

[borkdude]

@mynomoto This version supports java imports:

https://3613-176829714-gh.circle-artifacts.com/0/release/clj-kondo-2019.06.08-alpha-SNAPSHOT-linux-amd64.zip

TODO: add a list of java.lang classes that are imported by default.

[borkdude]

@mynomoto This version completes the TODO of the previous comment: all default imported classes are now recognized (like Class, String, etc.).

https://3631-176829714-gh.circle-artifacts.com/0/release/clj-kondo-2019.06.08-alpha-SNAPSHOT-linux-amd64.zip

Also the docs descript all options how to prevent false positives: https://github.com/borkdude/clj-kondo/blob/unresolved-symbols/doc/config.md#exclude-unresolved-symbols-from-being-reported

For now I turned down the level to :info so people can gradually adapt to this new linter.
I hope this is getting ready for a release. Feel free to test once more.

[borkdude]

Excluded a few false positives.

Latest linux: https://3671-176829714-gh.circle-artifacts.com/0/release/clj-kondo-2019.06.08-alpha-SNAPSHOT-linux-amd64.zip
Latest mac: https://3670-176829714-gh.circle-artifacts.com/0/release/clj-kondo-2019.06.08-alpha-SNAPSHOT-macos-amd64.zip

[mynomoto]

False positive

(ns cljkondo)

(defrecord Foo
  [bar baz])

(defn foo?
  [v]
  (instance? Foo v)) ; info: unresolved symbol Foo

[mynomoto]

Has something changed on how the command works? Ale seems not able use clj-kondo anymore.

[borkdude]

@mynomoto I've lowered the level of this linter from :error to :info now. Does ale not show this level? In emacs/flycheck I see:

If ale doesn't support it, we could make the level to :warning as the default. You can also do this in the config: {:linters {:unresolved-symbol {:level :warning}}} right now.

[mynomoto]

That was it, thanks!

[borkdude]

@mynomoto defrecord false positive fixed in:

https://3673-176829714-gh.circle-artifacts.com/0/release/clj-kondo-2019.06.08-alpha-SNAPSHOT-linux-amd64.zip

[borkdude]

Fixed with 3dce371
I expect more false positives, so the level is set to :info instead of :error for now.

[mynomoto]

One more false positive:

(definterface Foo (foo []))

(identity Foo) ; warning: unresolved symbol Foo

[mynomoto]

Another one:

(ns cljkondo
  (:require
    [schema.core :as sc]))

(sc/defn bar :- #(last %) ; warning: unresolved symbol %
  [x]
  x)

[mynomoto]

And my largest project is fully linted again! Thanks for all this work!

[borkdude]

@mynomoto Thank you. The last two false positives should be fixed with:

https://3769-176829714-gh.circle-artifacts.com/0/release/clj-kondo-2019.06.08-alpha-SNAPSHOT-linux-amd64.zip