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

Enhance macro_use_imports lint with a auto applicable suggstion #5275

Closed
flip1995 opened this issue Mar 5, 2020 · 2 comments · Fixed by #5279
Closed

Enhance macro_use_imports lint with a auto applicable suggstion #5275

flip1995 opened this issue Mar 5, 2020 · 2 comments · Fixed by #5279
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages E-hard Call for participation: This a hard problem and requires more experience or effort to work on L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@flip1995
Copy link
Member

flip1995 commented Mar 5, 2020

The macro_use_imports lint was implemented in #5230, but is currently missing a auto applicable suggestion. For this suggestion, every macro used in the module would have been detected and from this set of macros, a suggestion has to be build.

@flip1995 flip1995 added E-hard Call for participation: This a hard problem and requires more experience or effort to work on C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages L-suggestion Lint: Improving, adding or fixing lint suggestions labels Mar 5, 2020
@DevinR528
Copy link
Contributor

I have made a rough start on this here. I had a few questions.
What do we do with re exported paths such as println from std::prelude the macro is from std::macros which is private? this isn't a great example since not many would explicitly import std::prelude but I'd imagine it holds true for other crates

If the import is only the crate name (no more ::mod::mod) because macros float to the top level is it safe to assume crate::macro` is always valid?

I'm sure there are other things I'm not doing correctly, should I move this to a PR?

@flip1995
Copy link
Member Author

flip1995 commented Mar 6, 2020

I must admit, that I don't really know, how I would handle this.

I think that crate::macro can still be a reexport from a private module of crate. It will definitely need extensive testing, including macros from sub crates, similar to

// run-rustfix
// aux-build:wildcard_imports_helper.rs
#![warn(clippy::wildcard_imports)]
#![allow(unused)]
#![warn(unused_imports)]
extern crate wildcard_imports_helper;

What I would suggest is to first write a huge test base, with every case you can think of, and then work from there. If you have an idea for a test case, but don't know how to implement it in the Clippy test suite, you can open a PR, and we can work it out together.

bors added a commit that referenced this issue Jun 8, 2020
Macro use sugg

changelog: Add auto applicable suggstion to macro_use_imports

fixes #5275

<s>Where exactly is the `wildcard_imports_helper` I haven't been able to import anything ex.
`use lazy_static;` or something like for that I get version/compiler conflicts?</s>
Found it.

Should we also check for `#[macro_use] extern crate`, although this is still depended on for stuff like `rustc_private`?
bors added a commit that referenced this issue Jun 9, 2020
Macro use sugg

changelog: Add auto applicable suggstion to macro_use_imports

fixes #5275

<s>Where exactly is the `wildcard_imports_helper` I haven't been able to import anything ex.
`use lazy_static;` or something like for that I get version/compiler conflicts?</s>
Found it.

Should we also check for `#[macro_use] extern crate`, although this is still depended on for stuff like `rustc_private`?
@bors bors closed this as completed in f065d4b Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages E-hard Call for participation: This a hard problem and requires more experience or effort to work on L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants