Allow use super::*; glob imports

[MrAwesome]

changelog: Allow super::* glob imports

fixes #5554
fixes #5569

A first pass at #5554 - this allows all use super::* to pass, which may or may not be desirable. The original issue was around allowing test modules to import their entire parent modules - I'm happy to modify this to do that instead, may just need some guidance on how to implement that (I played around a bit with #[cfg(test)] but from what I can gather, clippy itself isn't in test mode when running, even if the code in question is being checked for the test target).

[flip1995]

A good heuristic would be to check if the name of module we're currently in, contains the string "test". You can do this by setting a flag self.in_test_module = true in a check_item function and reset it in check_item_post. Documentation: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/struct.BuiltinCombinedLateLintPass.html#method.check_item_post

[flip1995]

You cannot check for the cfg(test) attribute, since cfg/cfg_attrs don't exist anymore in the HIR.

[dylni]

Since wildcard_imports is starting to warn in less cases, should it have a configuration variable to always warn? That would also help make the documentation clearer about it not warning in all cases by default.

[flip1995]

We could add this, yes

[MrAwesome]

Updated to check for "test" in the parent module name. I ended up doing this inline for simplicity's sake, happy to set a field on WildcardImports if that seems better.

[flip1995]

I ended up doing this inline for simplicity's sake, happy to set a field on WildcardImports if that seems better.

That was my first idea when I reviewed this PR too. But that doesn't work, if the super import isn't directly in the main module. And allowing use super::* in test functions seems reasonable too. Playground for test case.

[flip1995]

Better than a bool would be to add a counter how many test modules deep the lint check is, so it doesn't start linting in test modules again, just because a submodule is also called *test*.

[MrAwesome]

Updated to check for "test" in parent module/function names.

@dylni I also went ahead and added the flag for #5569 since I'm already warmed up on the code.

[flip1995]

Thanks for also implementing #5569!

[MrAwesome]

Okay, updated to remove that field and check the itemkind. I checked for either fn or mod, should I do just mod?

Also - are there docs I should update as well?

[dylni]

@MrAwesome Thanks! It's great to see it will be available immediately with this PR.

[flip1995]

Yes please exclude Fn here, since test is a pretty common string in a method name, like test_for_xyz, IMO.

[MrAwesome]

Updated to use Mod only, reflected that in the tests, and updated the docs to reflect all the new functionality.

[flip1995]

Please rebase on top of master to get rid of the merge commits.

[MrAwesome]

This look correct? Most of my experience is with Mercurial so I want to make extra sure I don't cause some weird repo state.

[flip1995]

Perfect, thanks!

@bors r+

[bors]

📌 Commit b69200b has been approved by flip1995

[bors]

⌛ Testing commit b69200b with merge 43a1777...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 43a1777 to master...