New Lint: `wildcard imports` #5029

flip1995 · 2020-01-09T18:08:06Z

Fixes #1228

A few notes:

I put this lint in the pedantic group, even though in the issue restriction was mentioned.
Every fallout fix was automatically applied by cargo fix (This produced 3 unused_imports warnings) and are in commit 7e834c8. So reverting these changes wouldn't be a problem.

A few ideas:

A configuration to specify the amount of imported Items, where a * might be better.
~~A configuration to disable the lint for enums. Or just disable the lint for enums, since there is enum_glob_use~~ I moved enum_glob_use into this lint in 12937f0

A few quotes from the issue:

Is there a way to ask the compiler about the modules or symbols that the current file is using?

Yes there is. I found it, once I was nearly finished implementing it myself. See 321d64a

one hard optional feature would be to figure out what is currently used and add a suggestion to replace it with a full import list.

Yeah that was pretty hard, until I found the query for this. Queries are cool, but too hard to find.

FWIW VS Code and Intellij IDEA both offer imports deglobbing which replace * with required imports.

And now, Clippy can do this too! 🎉

Your thoughts on the notes/ideas?

changelog: Add new lint [wildcard imports]. Add suggestion to [enum_glob_use]

flip1995

Or maybe we could move the enum_glob_use lint into this one. It appears, that this lint catches use Enum::*s, that enum_glob_use doesn't catch. Also we could provide a suggestion for the enum_glob_use lint by moving it into this LintPass.

clippy_lints/src/assign_ops.rs

flip1995 · 2020-01-09T18:11:38Z

clippy_lints/src/consts.rs

@@ -153,8 +153,6 @@ impl Constant {

 /// Parses a `LitKind` to a `Constant`.
 pub fn lit_to_constant(lit: &LitKind, ty: Option<Ty<'_>>) -> Constant {
-    use syntax::ast::*;


This produced one unused_imports warning, since FloatTy and LitKind was imported twice after fixing it.

It might be worth adding this to the Known Problems section?

clippy_dev/src/stderr_length_check.rs

clippy_dev/src/lib.rs

bors · 2020-01-11T21:03:38Z

☔ The latest upstream changes (presumably #5040) made this pull request unmergeable. Please resolve the merge conflicts.

flip1995 · 2020-01-14T12:59:22Z

Or maybe we could move the enum_glob_use lint into this one.

Done in 12937f0

bors · 2020-01-15T21:48:57Z

☔ The latest upstream changes (presumably #5034) made this pull request unmergeable. Please resolve the merge conflicts.

llogiq · 2020-01-17T08:57:50Z

Is the import-use query overeager? The BufRead case may be because the respective method of Read and BufRead are both called read, so it may be that resolve is too limited to distinguish which is needed.

The FloatTy and Lit case looks like our filtering doesn't completely work, though. Or am I missing something?`

flip1995 · 2020-01-17T21:47:43Z

Is the import-use query overeager?

Yes it appears to be, but as long as it introduces only warnings, this should be ok.

The FloatTy and Lit case looks like our filtering doesn't completely work, though. Or am I missing something?`

FloatTy and LitKind was imported twice: once on the module level and once on the function level (with a *). This probably only happens by accident and should be fixed, by either only importing it at function level or at module level. But IMO the user has to decide where he wants this import. So signaling this by producing an unknown_imports warning seems fine to me.

bors · 2020-02-08T00:01:24Z

☔ The latest upstream changes (presumably #5140) made this pull request unmergeable. Please resolve the merge conflicts.

ghost · 2020-02-08T15:33:26Z

IMO, this lint shouldn't warn about prelude modules since their only purpose is to be glob imported. Or if it does, it should resolve the re-exports. Suggesting something like use std::io::prelude::Read; looks really wrong to me.

bors · 2020-02-10T08:39:44Z

☔ The latest upstream changes (presumably #5148) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2020-02-17T03:35:05Z

☔ The latest upstream changes (presumably #5182) made this pull request unmergeable. Please resolve the merge conflicts.

phansch

Just some nits, apart from that it looks good to me 💯

phansch · 2020-02-21T06:06:09Z

clippy_lints/src/consts.rs

@@ -153,8 +153,6 @@ impl Constant {

 /// Parses a `LitKind` to a `Constant`.
 pub fn lit_to_constant(lit: &LitKind, ty: Option<Ty<'_>>) -> Constant {
-    use syntax::ast::*;


It might be worth adding this to the Known Problems section?

clippy_lints/src/wildcard_imports.rs

bors · 2020-02-21T09:58:28Z

☔ The latest upstream changes (presumably #5202) made this pull request unmergeable. Please resolve the merge conflicts.

Queries are cool, but too hard to find.

phansch · 2020-02-21T11:32:07Z

@bors r+

bors · 2020-02-21T11:32:09Z

📌 Commit 4dd2252 has been approved by phansch

bors · 2020-02-21T11:32:17Z

⌛ Testing commit 4dd2252 with merge e342047...

bors · 2020-02-21T11:44:28Z

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: phansch
Pushing e342047 to master...

flip1995 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 9, 2020

flip1995 commented Jan 9, 2020

View reviewed changes

flip1995 force-pushed the wildcard_imports branch 4 times, most recently from b5ab922 to 719526f Compare January 14, 2020 12:58

flip1995 changed the title ~~[WIP] New Lint: wildcard imports~~ New Lint: wildcard imports Jan 14, 2020

flip1995 force-pushed the wildcard_imports branch from 719526f to 737c3cf Compare February 6, 2020 18:51

flip1995 force-pushed the wildcard_imports branch from 737c3cf to 3ada27d Compare February 9, 2020 12:04

flip1995 force-pushed the wildcard_imports branch 2 times, most recently from c4b3534 to 0e8f8bd Compare February 10, 2020 09:09

phansch self-requested a review February 20, 2020 13:25

phansch reviewed Feb 21, 2020

View reviewed changes

flip1995 force-pushed the wildcard_imports branch 2 times, most recently from c91a9fb to cc13da5 Compare February 21, 2020 09:30

flip1995 added 2 commits February 21, 2020 11:02

Add tests for wildcard_imports lint

8a572a5

Implement wildcard_imports lint

ba1d50c

flip1995 force-pushed the wildcard_imports branch from cc13da5 to 2483017 Compare February 21, 2020 10:02

flip1995 added 2 commits February 21, 2020 11:14

Run update_lints

4229dbc

Let's do it the easy way

3f5ed28

Queries are cool, but too hard to find.

flip1995 added 6 commits February 21, 2020 11:14

Move enum_glob_use lint into wildcard_imports pass

06a6189

Filter prelude glob imports

f4f781d

Add test for prelude import

b67764d

Fix fallout

8472ecd

Don't use use ty::TyKind::*

b562a51

Fix suggestion for weird formattings

4dd2252

flip1995 force-pushed the wildcard_imports branch from 2483017 to 4dd2252 Compare February 21, 2020 10:40

flip1995 requested a review from phansch February 21, 2020 10:51

bors merged commit e342047 into rust-lang:master Feb 21, 2020

This was referenced Feb 21, 2020

Added restriction lint: pattern-type-mismatch #4841

Merged

Don't trigger use_debug lint in Debug impl #5047

Merged

flip1995 deleted the wildcard_imports branch February 21, 2020 12:09

hudson-ayers mentioned this pull request Apr 20, 2020

Fix clippy lints + Run Clippy in Travis-CI tock/tock#1773

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Lint: `wildcard imports` #5029

New Lint: `wildcard imports` #5029

flip1995 commented Jan 9, 2020 •

edited

Loading

flip1995 left a comment

flip1995 Jan 9, 2020

phansch Feb 21, 2020

bors commented Jan 11, 2020

flip1995 commented Jan 14, 2020

bors commented Jan 15, 2020

llogiq commented Jan 17, 2020

flip1995 commented Jan 17, 2020

bors commented Feb 8, 2020

ghost commented Feb 8, 2020

bors commented Feb 10, 2020

bors commented Feb 17, 2020

phansch left a comment

phansch Feb 21, 2020

bors commented Feb 21, 2020

phansch commented Feb 21, 2020

bors commented Feb 21, 2020

bors commented Feb 21, 2020

bors commented Feb 21, 2020

New Lint: wildcard imports #5029

New Lint: wildcard imports #5029

Conversation

flip1995 commented Jan 9, 2020 • edited Loading

A few notes:

A few ideas:

A few quotes from the issue:

flip1995 left a comment

Choose a reason for hiding this comment

flip1995 Jan 9, 2020

Choose a reason for hiding this comment

phansch Feb 21, 2020

Choose a reason for hiding this comment

bors commented Jan 11, 2020

flip1995 commented Jan 14, 2020

bors commented Jan 15, 2020

llogiq commented Jan 17, 2020

flip1995 commented Jan 17, 2020

bors commented Feb 8, 2020

ghost commented Feb 8, 2020

bors commented Feb 10, 2020

bors commented Feb 17, 2020

phansch left a comment

Choose a reason for hiding this comment

phansch Feb 21, 2020

Choose a reason for hiding this comment

bors commented Feb 21, 2020

phansch commented Feb 21, 2020

bors commented Feb 21, 2020

bors commented Feb 21, 2020

bors commented Feb 21, 2020

New Lint: `wildcard imports` #5029

New Lint: `wildcard imports` #5029

flip1995 commented Jan 9, 2020 •

edited

Loading