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

False positive "Namespace is never used" #210

Closed
emlyn opened this issue Nov 8, 2016 · 7 comments · Fixed by #353
Closed

False positive "Namespace is never used" #210

emlyn opened this issue Nov 8, 2016 · 7 comments · Fixed by #353

Comments

@emlyn
Copy link
Contributor

emlyn commented Nov 8, 2016

When importing a class defined in another namespace with defrecord, it is necessary to first require the namespace so that the class is compiled before it can be imported (see https://clojuredocs.org/clojure.core/defrecord#example-542692d2c026201cdc326f87), but eastwood warns that the required namespace is never used.

@emlyn
Copy link
Contributor Author

emlyn commented Jan 6, 2017

Actually a workaround for this is to set :load-ns true on the defrecord so that the require is not needed.

@dijonkitchen
Copy link
Contributor

Any updates or should it be closed?

@jafingerhut
Copy link
Collaborator

jafingerhut commented Sep 28, 2018

There were some improvements made to Eastwood on its reporting of unused namespaces, but I do not recall whether those improvements fix this case, or not.

Here is a commit with the last of those changes to Eastwood that I am aware of. Those changes have been part of Eastwood since release version 0.2.5, so if someone could retest the problem case with Eastwood 0.2.5 or later and report here, that would be good to know. 9c2a32f

Note: I would not be surprised if the problem case reported here is not improved by the changes I mentioned.

@mrkam2
Copy link
Contributor

mrkam2 commented Jan 20, 2021

As of the "0.3.13" version, this is still an issue.

@mrkam2 mrkam2 mentioned this issue Jan 20, 2021
@mrkam2
Copy link
Contributor

mrkam2 commented Jan 20, 2021

I added a draft PR with a testcase: #351.

@mrkam2
Copy link
Contributor

mrkam2 commented Mar 4, 2021

Are there any plans to address this issue?

@vemv
Copy link
Collaborator

vemv commented Mar 4, 2021

(I'm merely an occasional contributor)

Looks like implementing a solution would entail creating a function akin to protocols-used, but for classes (namely those coming from defrecord + deftype) instead:

used-protocols (protocols-used asts)

Here is a sample classes-used implementation that worked for me just now, against your test case:

(defn classes-used [asts]
  (->> asts
       (mapcat ast/nodes)
       (keep :o-tag) ;; other "interesting" keys that can include a used class include: `:tag`, `:val`, `:result`
       set))

I don't have enough tools.analyzer expertise to tell which of the keys I have observed to be "interesting" (:o-tag, :tag, :val, :result) would be the most suitable for properly fixing this linter.

I'd encourage you to open a PR with a fix, spending a little more time in research than I did. Currently I lack the time/capability of opening proper PRs against Eastwood.

Cheers - V

vemv pushed a commit to reducecombine/eastwood that referenced this issue Mar 17, 2021
vemv pushed a commit to reducecombine/eastwood that referenced this issue Mar 27, 2021
vemv pushed a commit to reducecombine/eastwood that referenced this issue Mar 27, 2021
vemv pushed a commit to reducecombine/eastwood that referenced this issue Mar 27, 2021
slipset pushed a commit that referenced this issue Mar 27, 2021
Co-authored-by: Alexander Kouznetsov <alexander@liftoff.io>
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

Successfully merging a pull request may close this issue.

5 participants