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

Add a future-compatibility warning on allowed feature name characters. #8814

Merged
merged 1 commit into from
Oct 28, 2020

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Oct 27, 2020

This adds a restriction on the valid syntax of a feature name. An warning is issued if a feature does not match the new validation, with the intent that it will be an error in the future.

The new restriction is:

The changes around passing in config to Summary can mostly be reverted when this is changed to an error.

I'm a little concerned that we don't have a mechanism to silence the warning. Should we add one?

@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 27, 2020
@alexcrichton
Copy link
Member

@bors: r+

Looks good to me, thanks! We historically haven't ever had a way to silence Cargo warnings, they're all intended to be immediately actionable and not ones you'd want to silence in theory. If this becomes an issue though we can always turn it back off and figure out a way to deal with warnings.

@bors
Copy link
Contributor

bors commented Oct 28, 2020

📌 Commit b731190 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 28, 2020
@bors
Copy link
Contributor

bors commented Oct 28, 2020

⌛ Testing commit b731190 with merge 358ee54...

@bors
Copy link
Contributor

bors commented Oct 28, 2020

☀️ Test successful - checks-actions
Approved by: alexcrichton
Pushing 358ee54 to master...

@bors bors merged commit 358ee54 into rust-lang:master Oct 28, 2020
@ehuss ehuss added this to the 1.49.0 milestone Feb 6, 2022
bors added a commit that referenced this pull request Jun 20, 2023
Convert valid feature name warning to an error.

This converts the feature name validation check from a warning to an error. This was added in #8814 in Rust 1.49 released in 2020-12-31 (about 2.5 years ago) with a warning that it will become a hard error in the future.

These extended characters aren't allowed on crates.io, so this should only impact users of other registries, or people who don't publish to a registry. The warning message requested anyone impacted by it to let us know. We got one report, and added support for . as result. Since there weren't any other reports, I think it should be pretty safe to move forward.

The diff here is a little large because it removes the pass-through of `config` since it isn't needed anymore.
Additionally, the tests were restructured since testing every edge case in an integration test has a lot of overhead. Instead, there is now a unit test which runs much faster, with the integration test just verifying that it fires and checks the two forms of error messages.

Closes #8813
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants