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

Crate name found in --extern gets suggested verbatim on unresolved import #113035

Closed
fmease opened this issue Jun 25, 2023 · 8 comments · Fixed by #116001
Closed

Crate name found in --extern gets suggested verbatim on unresolved import #113035

fmease opened this issue Jun 25, 2023 · 8 comments · Fixed by #116001
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@fmease
Copy link
Member

fmease commented Jun 25, 2023

Given the code in file user.rs as shown below:

use a_great_crate::item;

rustc outputs the following when invoked via rustc user.rs --crate-type=lib --edition=2021 --extern=a-great-crate=liba_great_crate.rlib:

error[E0432]: unresolved import `a_great_crate`
 --> user.rs:1:5
  |
1 | use a_great_crate::item;
  |     ^^^^^^^^^^^^^ use of undeclared crate or module `a_great_crate`
  |
help: there is a crate or module with a similar name
  |
1 | use a-great-crate::item;
  |     ~~~~~~~~~~~~~

error: aborting due to previous error

For more information about this error, try `rustc --explain E0432`.

While ideally, in this specific case it should either suggest nothing of if feasible suggest changing the --extern flag to --extern=a_great_crate=liba_great_crate.rlib (the latter is probably P-low). I did actually get confused by the suggestion when I originally faced it.

In fact, rustc suggests whatever garbage you put in the flag (e.g. on rustc … '--extern=a&-gre?t#crate=xxx'). In these cases it shouldn't consider the name at all (I realize though that this might be a case of garbage in, garbage out).

@rustbot label T-compiler A-diagnostics A-suggestion-diagnostics D-invalid-suggestion

@rustbot rustbot added A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 25, 2023
@est31
Copy link
Member

est31 commented Jun 26, 2023

Rustc could in fact issue an error if an invalid identifier is passed as extern name, because there is no way this identifier can be used by the program anyways (outside of the force option on --extern added by #109421 there is no use case). The only issue with that is that it's going to be a breaking change so needs to be phased in gradually. Alternatively, one could just issue a warning lint.

@fmease
Copy link
Member Author

fmease commented Jun 30, 2023

Sounds even better! For some reason that didn't come to my mind when opening this issue

@est31
Copy link
Member

est31 commented Jul 6, 2023

I've filed an MCP (zulip).

@est31
Copy link
Member

est31 commented Aug 10, 2023

MCP is accepted now. Now, one could file a pull request. It should be pretty simple.

@fmease
Copy link
Member Author

fmease commented Aug 18, 2023

@rustbot claim

@est31
Copy link
Member

est31 commented Aug 18, 2023

A low-level check would be to use rustc_lexer::is_ident. Of course, if you have any better ideas, write me.

@fmease
Copy link
Member Author

fmease commented Sep 10, 2023

I'm sorry for not getting back to you. There's validate_crate_name which I plan on using if possible. Hoping to send a PR soon-ish, couldn't be as active in the project in the last few weeks as I wanted.

@est31
Copy link
Member

est31 commented Sep 10, 2023

@fmease no pressure! I'm in a similar situation in fact. Thanks for the heads up. Yeah, validate_crate_name looks like a good candidate, definitely worth to explore it. I think in addition the is_ident check would still help, as validate_crate_name does not reject names starting with numbers. But we can discuss details when you send the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants