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.

Sign up for GitHub

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

Hint use of use crate:: on invalid mod #71288

Closed
wants to merge 1 commit into from
Closed

Hint use of use crate:: on invalid mod #71288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/librustc_expand/module.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,14 @@ pub fn default_submod_path<'a>(
mod_name,
);
err.help(&format!(
"to create the module `{}`, create file \"{}\"",
"to create the module `{}` here, create file \"{}\"",
mod_name,
default_path.display(),
));
err.help(&format!(
"if there is `mod {}` elsewhere in the crate already, import it with `use crate::` instead",
mod_name,
));
Comment on lines +287 to +290
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do the following instead

err.span_suggestion_verbose(
    span,
    "if the module exists elsewhere in the crate already, you can import instead",
    format!("use crate::{};", mod_name),
    Applicability::MaybeIncorrect,
)

That being said, it'd be nice to actually check that the module exists in the crate and provide the suggestion with higher certainty. Do not have the time to check what the code would have to be to verify that crate::mod_name exists, but @petrochenkov might know of the top of their head. Otherwise, would you mind exploring how to do that, @kornelski? We want to minimize the cases where we provide incorrect suggestions, as those can be worse than terse ones. If we can make that check, then we can provide the help to create the file exclusively if the mod doesn't exist alredy.

Copy link
Contributor Author

@kornelski kornelski May 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to dig in further, but I'd prefer to land this change in the meantime.
The message starts with a disclaimer, it intentionally only suggests use instead of showing a potentially-incorrect path. So I think risk of confusion is low, and mentioning use is an important improvement.

This check currently happens very early in the parsing, so a naive mod existence checks at this stage would be unreliable and sensitive to order of definitions. A proper fix may be quite involved, e.g. create a "broken mod" AST node for a later pass to pick it up and scan modules after they're all parsed. That might take longer to implement, so the current PR is IMHO good to have in the meantime.

Err(err)
}
(true, true) => {
Expand Down
3 changes: 2 additions & 1 deletion src/test/ui/error-codes/E0583.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ error[E0583]: file not found for module `module_that_doesnt_exist`
LL | mod module_that_doesnt_exist;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: to create the module `module_that_doesnt_exist`, create file "$DIR/module_that_doesnt_exist.rs"
= help: to create the module `module_that_doesnt_exist` here, create file "$DIR/module_that_doesnt_exist.rs"
= help: if there is `mod module_that_doesnt_exist` elsewhere in the crate already, import it with `use crate::` instead

error: aborting due to previous error

Expand Down
3 changes: 2 additions & 1 deletion src/test/ui/invalid-module-declaration/invalid-module-declaration.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ error[E0583]: file not found for module `baz`
LL | pub mod baz;
| ^^^^^^^^^^^^
|
= help: to create the module `baz`, create file "$DIR/auxiliary/foo/bar/baz.rs"
= help: to create the module `baz` here, create file "$DIR/auxiliary/foo/bar/baz.rs"
= help: if there is `mod baz` elsewhere in the crate already, import it with `use crate::` instead

error: aborting due to previous error

Expand Down
3 changes: 2 additions & 1 deletion src/test/ui/missing_non_modrs_mod/missing_non_modrs_mod.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ error[E0583]: file not found for module `missing`
LL | mod missing;
| ^^^^^^^^^^^^
|
= help: to create the module `missing`, create file "$DIR/foo/missing.rs"
= help: to create the module `missing` here, create file "$DIR/foo/missing.rs"
= help: if there is `mod missing` elsewhere in the crate already, import it with `use crate::` instead

error: aborting due to previous error

Expand Down
3 changes: 2 additions & 1 deletion src/test/ui/missing_non_modrs_mod/missing_non_modrs_mod_inline.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ error[E0583]: file not found for module `missing`
LL | mod missing;
| ^^^^^^^^^^^^
|
= help: to create the module `missing`, create file "$DIR/foo_inline/inline/missing.rs"
= help: to create the module `missing` here, create file "$DIR/foo_inline/inline/missing.rs"
= help: if there is `mod missing` elsewhere in the crate already, import it with `use crate::` instead

error: aborting due to previous error

Expand Down
3 changes: 2 additions & 1 deletion src/test/ui/parser/mod_file_not_exist.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// ignore-windows

mod not_a_real_file; //~ ERROR file not found for module `not_a_real_file`
//~^ HELP to create the module `not_a_real_file`, create file
//~^ HELP to create the module `not_a_real_file` here, create file
//~| HELP if there is `mod not_a_real_file` elsewhere in the crate already

fn main() {
assert_eq!(mod_file_aux::bar(), 10);
Expand Down
5 changes: 3 additions & 2 deletions src/test/ui/parser/mod_file_not_exist.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ error[E0583]: file not found for module `not_a_real_file`
LL | mod not_a_real_file;
| ^^^^^^^^^^^^^^^^^^^^
|
= help: to create the module `not_a_real_file`, create file "$DIR/not_a_real_file.rs"
= help: to create the module `not_a_real_file` here, create file "$DIR/not_a_real_file.rs"
= help: if there is `mod not_a_real_file` elsewhere in the crate already, import it with `use crate::` instead

error[E0433]: failed to resolve: use of undeclared type or module `mod_file_aux`
--> $DIR/mod_file_not_exist.rs:7:16
--> $DIR/mod_file_not_exist.rs:8:16
|
LL | assert_eq!(mod_file_aux::bar(), 10);
| ^^^^^^^^^^^^ use of undeclared type or module `mod_file_aux`
Expand Down
3 changes: 2 additions & 1 deletion src/test/ui/parser/mod_file_not_exist_windows.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// only-windows

mod not_a_real_file; //~ ERROR file not found for module `not_a_real_file`
//~^ HELP to create the module `not_a_real_file`, create file
//~^ HELP to create the module `not_a_real_file` here, create file
//~^ HELP if there is `mod not_a_real_file` elsewhere in the crate already

fn main() {
assert_eq!(mod_file_aux::bar(), 10);
Expand Down
3 changes: 2 additions & 1 deletion src/test/ui/parser/mod_file_not_exist_windows.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ error[E0583]: file not found for module `not_a_real_file`
LL | mod not_a_real_file;
| ^^^^^^^^^^^^^^^^^^^^
|
= help: to create the module `not_a_real_file`, create file "$DIR/not_a_real_file.rs"
= help: to create the module `not_a_real_file` here, create file "$DIR/not_a_real_file.rs"
= help: if there is `mod not_a_real_file` elsewhere in the crate already, import it with `use crate::` instead

error[E0433]: failed to resolve: use of undeclared type or module `mod_file_aux`
--> $DIR/mod_file_not_exist_windows.rs:7:16
Expand Down