Description
Proposal
In rust-lang/rust#113035 , @fmease has pointed out that rustc accepts any --extern crate name, even ones that wouldn't be valid identifiers:
touch b.rs; rustc b.rs --crate-type=lib
touch a.rs; rustc a.rs --edition=2021 --crate-type=lib --extern hi=libb.rlib
rustc a.rs --edition=2021 --crate-type=lib --extern "hello//i;d"=libb.rlibAs pointed out in #113035, these invalid --extern crate names are then used to provide (wrong) "did you mean" suggestions.
error[E0432]: unresolved import `hello__i_d`
--> a.rs:1:5
|
1 | use hello__i_d::hi;
| ^^^^^^^^^^ use of undeclared crate or module `hello__i_d`
|
help: there is a crate or module with a similar name
|
1 | use hello//i;d::hi;
| ~~~~~~~~~~
I propose requiring the extern crate names to be valid identifiers, and error if they are not. This prevents user confusion and wrong expectations.
So e.g. hello//i;d would be disallowed, just as hello-i-d would be, but hello_i_d would work (cargo already converts - to _, this should not affect crates with - in their name; cargo's ). hällo_world would still work as it is a valid identifier.
It would technically be a breaking change, but I don't think people specify such invalid --extern names outside of accidents. I guess a crater run before merging it would be a good idea. I don't think an entire forwards compat lint is needed.
The central reason why I am suggesting this is that it would be impossible to use such invalidly named externs. There is only one situation I can think of where you sometimes would opt for an invalid identifier, which is use via the force param, added by rust-lang/rust#109421. I said sometimes, because often you just want add the force param for crates that have important symbols. For those cases (extern with force arg plus invalid name), I propose suggesting the _ name in the error, as it is already present in the language via use foo::Trait as _; and let _ = <some expr that is must_use>;, and otherwise, suggest removing the --extern argument.
Similar to #610 in that it also proposes to do more CLI arg validation. There is also precedent in that #![crate_name = "hello-//world"] is not accepted by the compiler either.
To expand on the cargo point earlier, cargo's - to _ conversion also applies to crate renaming:
[dependencies]
"hi-world" = { package = "anyhow", version = "1.0" }for example gets passed as --extern hi_world=/path/to/libanyhow-deadbeef.rlib. If you try to put // or :, cargo will already error. There might, or might not, be a small subset of invalid identifiers that cargo doesn't catch, but I doubt this shouldn't be a problem I think. I think it's better to have an error message when you try to include it via cargo than to have it when you try to use it.
Mentors or Reviewers
An implementation should be pretty easy. I mainly do this MCP for the approval part of it. I can help to mentor this.
Process
The main points of the Major Change Process are as follows:
- File an issue describing the proposal.
- A compiler team member or contributor who is knowledgeable in the area can second by writing
@rustbot second.- Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a
-C flag, then full team check-off is required. - Compiler team members can initiate a check-off via
@rfcbot fcp mergeon either the MCP or the PR.
- Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a
- Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.
You can read more about Major Change Proposals on forge.
Comments
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.