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

Change "A non-empty glob must import something with the glob's visibility" to be a lint? #62334

Closed
fpoli opened this issue Jul 3, 2019 · 4 comments · Fixed by #65539
Closed
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-resolve Area: Name resolution E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@fpoli
Copy link
Contributor

fpoli commented Jul 3, 2019

Consider the following three examples: A and C are accepted and B has a compilation error. However, the message reported in B looks more like a lint than a compilation error.

A consequence of this error is that adding a non-public function to a module (e.g. the bar in B) may break code that imports from that module. This causes surprises when refactoring.

Shouldn't "A non-empty glob must import something with the glob's visibility" be a lint?

The original discussion is here: https://users.rust-lang.org/t/a-non-empty-glob-must-import-something-with-the-globs-visibility-uhhh-okay-but-why

A

pub use self::a::*;
mod a {
    fn foo() {}
}

B

// ERROR: A non-empty glob must import something with the glob's visibility
pub use self::b::*;
mod b {
    fn foo() {}
    pub(super) fn bar() {}
}

C

// okay: there's a pub item
pub use self::c::*;
mod c {
    fn foo() {}
    pub(super) fn bar() {}
    pub fn baz() {}
}
@petrochenkov
Copy link
Contributor

petrochenkov commented Jul 3, 2019

The error was introduced in #35894 as a part of import modularization, and discussed in one of the related issues, but I can't find where exactly.

The error was introduced by analogy with errors for single imports (and just to be conservative):

mod m {
    fn f() {}
}

use m::f; // Doesn't import anything and therefore reports an error.
use m::g; // Doesn't import anything and therefore reports an error.
use m::*; // Doesn't import anything and therefore reports an error.

The error is not technically necessary, and we should be able to report it as a lint for glob imports while keeping it an error for single imports.

@jonas-schievink jonas-schievink added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-resolve Area: Name resolution T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jul 28, 2019
@fpoli
Copy link
Contributor Author

fpoli commented Aug 28, 2019

Would this be a "good first issue" or is it complex to fix?

@petrochenkov
Copy link
Contributor

No, not complex.

  • Find where "non-empty glob must import something" is reported.
  • Replace span_err with buffer_lint(UNUSED_IMPORTS, ...).

@petrochenkov petrochenkov added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Oct 17, 2019
@traxys
Copy link
Contributor

traxys commented Oct 18, 2019

I want to do this issue but the problem I have is with a test (ui/imports/reexports) that checks for this error, I don't know the compiler structure so I don't know when compiling this file the lint will not be reported because of the other errors or not. Else I changed the span_err to buffer_lint

Centril added a commit to Centril/rust that referenced this issue Oct 29, 2019
resolve: Turn the "non-empty glob must import something" error into a lint

This fixes rust-lang#62334 by changing the error to a lint warning the glob. I changed the test but I'm very unsure of what I did as I do not know how to correctly check for the warning
@bors bors closed this as completed in 0d755ff Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-resolve Area: Name resolution E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants