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

warn about missing requires and imports #339

Closed
5 tasks done
borkdude opened this issue Jul 10, 2019 · 5 comments
Closed
5 tasks done

warn about missing requires and imports #339

borkdude opened this issue Jul 10, 2019 · 5 comments
Assignees
Labels
enhancement New feature or request
Projects

Comments

@borkdude
Copy link
Member

borkdude commented Jul 10, 2019

Warn about the following:

(ns foo)
(clojure.string/starts-with? ...)

;;=> missing require (or import) for clojure.string

(ns bar)
(foo/x ...)

;;=> unknown alias foo

although we cannot know if foo is supposed to be an alias, a single-segmented namespace, or Java classname.

We would have to exclude known things like goog in CLJS.
Clj-kondo already have a list of all the by default available classes like System.

The thing that should be very well researched is false positives.
There's already a function for detecting Java class names:
https://github.com/borkdude/clj-kondo/blob/2b4229ea652021e0e5ec6c73082fcc4c7e35d5f2/src/clj_kondo/impl/namespace.clj#L152

TODO:

  • exclude js/ and goog/ in CLJS
  • lint foo/bar usage, not only in call position
  • think more about project.clj defproject kind of macros. can fully qualified symbols occur in macros without proper requires / imports? in fact, user/defproject is the only macro that I need to include for now. Maybe special case it.

Note: the current workaround doesn't work in emacs, since the filename is not included

  • change linter name to something more neutral like 'unresolved-namespace' which is more factual. The message could be something like 'Unresolved namespaced. Did you forget a require?`.

Maybe later:

  • in comment sections, handle requires as if they are top-level
@borkdude borkdude added the enhancement New feature or request label Jul 10, 2019
@borkdude
Copy link
Member Author

borkdude commented Jul 11, 2019

From #clojure Slack:

borkdude [5:17 PM]
Hmm. In Clojure when you write:

bar/baz`

bar/baz could in theory be a public field in a class bar right? so it's hard to say this is an unresolved symbol by linting statically
bronsa [5:19 PM]
there's no ambiguity between Klass/field and ns/var
borkdude [5:19 PM]
so yes?
in CLJS-land there are also things like goog/isFunction
that are just "there" without requiring
bronsa [5:21 PM]
yes, w/o keeping a runtime mapping of the available classes it's not easy to distinguish the two
borkdude [5:21 PM]
currently clj-kondo doesn't report anything with a namespace as unresolved, but thinking about this more, I think it's the correct behavior because of the above
👍
(given that underreporting is better than overreporting)

@borkdude borkdude added the expired expired - might be closed due to too little activity label Jul 11, 2019
@borkdude
Copy link
Member Author

borkdude commented Nov 6, 2019

Related #515

@borkdude borkdude changed the title foo/bar is not reported as unresolved when foo is not required re-consider: foo/bar is not reported as unresolved when foo is not required Nov 6, 2019
@borkdude borkdude reopened this Nov 6, 2019
@borkdude borkdude added this to Needs triage in clj-kondo via automation Nov 6, 2019
@borkdude borkdude moved this from Needs triage to Needs hammock time in clj-kondo Nov 6, 2019
@borkdude borkdude changed the title re-consider: foo/bar is not reported as unresolved when foo is not required re-consider: warn about missing requires and imports Nov 6, 2019
@borkdude borkdude moved this from Needs hammock time to In progress in clj-kondo Nov 18, 2019
@borkdude borkdude removed the expired expired - might be closed due to too little activity label Nov 18, 2019
@borkdude borkdude moved this from In progress to High priority (next release) in clj-kondo Nov 23, 2019
@borkdude borkdude self-assigned this Nov 23, 2019
@borkdude borkdude moved this from High priority (next release) to In progress in clj-kondo Nov 28, 2019
@borkdude borkdude changed the title re-consider: warn about missing requires and imports warn about missing requires and imports Nov 28, 2019
@borkdude borkdude added this to To do in Global board via automation Dec 4, 2019
@borkdude borkdude moved this from To do to In progress in Global board Dec 4, 2019
borkdude added a commit that referenced this issue Dec 14, 2019
@borkdude borkdude moved this from In progress to Next release in clj-kondo Dec 14, 2019
@borkdude borkdude moved this from In progress to Done in Global board Dec 14, 2019
clj-kondo automation moved this from Next release to Done Dec 14, 2019
@markus-wa
Copy link

Hi. I was wondering what the rationale is for expecting a require for all used namespaces.

I'm fairly new to Clojure and I couldn't find this being suggested elsewhere. The code seems to work with or without require.

PS: Thanks for the awesome tool!

@borkdude
Copy link
Member Author

@markus-wa In general, one should not rely on other namespaces to load certain libraries for you. E.g.: it's an implementation detail in clojure.core whether some of the already required namespaces loads clojure.string. You should explicitly require it, if you want to use that namespace.

@markus-wa
Copy link

Ah ok, that makes sense, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
clj-kondo
  
Done
Development

No branches or pull requests

2 participants