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

Jump to bottom

Split Diagnostics for Uncommon Codepoints: Add List to Display Characters Involved #120259

Merged
merged 3 commits into from
Jan 24, 2024
Merged

Split Diagnostics for Uncommon Codepoints: Add List to Display Characters Involved #120259

merged 3 commits into from
Jan 24, 2024

Conversation

HTGAzureX1212
Copy link
Contributor

@HTGAzureX1212 HTGAzureX1212 commented Jan 23, 2024 •
edited
Loading

This Pull Request adds a list of the uncommon codepoints involved in the uncommon_codepoints lint, as outlined as a first step in #120228.

Example rendered diagnostic:

error: identifier contains an uncommon Unicode codepoint: 'µ'
  --> $DIR/lint-uncommon-codepoints.rs:3:7
   |
LL | const µ: f64 = 0.000001;
   |       ^
   |
note: the lint level is defined here
  --> $DIR/lint-uncommon-codepoints.rs:1:9
   |
LL | #![deny(uncommon_codepoints)]
   |         ^^^^^^^^^^^^^^^^^^^

(Retrying #120258.)

@rustbot
Copy link
Collaborator

rustbot commented Jan 23, 2024

r? @cjgillot

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 23, 2024
@HTGAzureX1212 HTGAzureX1212 mentioned this pull request Jan 23, 2024
Closed
fee1-dead
fee1-dead reviewed Jan 23, 2024
View reviewed changes
Copy link
Member

@fee1-dead fee1-dead left a comment

Choose a reason for hiding this comment

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

It would probably make sense to make this diagnostic reflect how many codepoints there are, by indicating whether there is a single codepoint or multiple. Added suggestions on how that could be achieved.

compiler/rustc_lint/src/lints.rs Outdated Show resolved Hide resolved
compiler/rustc_lint/messages.ftl Outdated Show resolved Hide resolved
@HTGAzureX1212
Copy link
Contributor Author

It would probably make sense to make this diagnostic reflect how many codepoints there are, by indicating whether there is a single codepoint or multiple. Added suggestions on how that could be achieved.

Agreed. I should have done that, implemented in the new commits.

Manishearth
Manishearth approved these changes Jan 23, 2024
View reviewed changes
Copy link
Member

@Manishearth Manishearth left a comment •
edited
Loading

Choose a reason for hiding this comment

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

lgtm

compiler/rustc_lint/messages.ftl
@@ -240,7 +240,10 @@ lint_hidden_unicode_codepoints = unicode codepoint changing visible direction of

lint_identifier_non_ascii_char = identifier contains non-ASCII characters

lint_identifier_uncommon_codepoints = identifier contains uncommon Unicode codepoints
lint_identifier_uncommon_codepoints = identifier contains {$codepoints_len ->
[one] an uncommon Unicode codepoint
Copy link
Member

Choose a reason for hiding this comment

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

praise: well done with pluralization

compiler/rustc_lint/src/lints.rs
pub struct IdentifierUncommonCodepoints;
pub struct IdentifierUncommonCodepoints {
pub codepoints: Vec<char>,
pub codepoints_len: usize,
Copy link
Member

Choose a reason for hiding this comment

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

cc @davidtwco would be nice to autogenerate _len parameters for vector fields

@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jan 23, 2024

📌 Commit da1d0c4 has been approved by Manishearth

It is now in the queue for this repository.

@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 Jan 23, 2024
fmease added a commit to fmease/rust that referenced this pull request Jan 23, 2024
@fmease
Rollup merge of rust-lang#120259 - HTGAzureX1212:HTGAzureX1212/split-…
ed207ae 
…diagnostics-uncommon-codepoints, r=Manishearth

Split Diagnostics for Uncommon Codepoints: Add List to Display Characters Involved

This Pull Request adds a list of the uncommon codepoints involved in the `uncommon_codepoints` lint, as outlined as a first step in rust-lang#120228.

Example rendered diagnostic:
```
error: identifier contains an uncommon Unicode codepoint: 'µ'
  --> $DIR/lint-uncommon-codepoints.rs:3:7
   |
LL | const µ: f64 = 0.000001;
   |       ^
   |
note: the lint level is defined here
  --> $DIR/lint-uncommon-codepoints.rs:1:9
   |
LL | #![deny(uncommon_codepoints)]
   |         ^^^^^^^^^^^^^^^^^^^
```

(Retrying rust-lang#120258.)
@fmease fmease mentioned this pull request Jan 23, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 23, 2024
@bors
Auto merge of rust-lang#120279 - fmease:rollup-4nhar13, r=fmease
6de065d 
Rollup of 12 pull requests

Successful merges:

 - rust-lang#112806 (Small code improvements in `collect_intra_doc_links.rs`)
 - rust-lang#119460 (coverage: Never emit improperly-ordered coverage regions)
 - rust-lang#119766 (Split tait and impl trait in assoc items logic)
 - rust-lang#120062 (llvm: change data layout bug to an error and make it trigger more)
 - rust-lang#120099 (linker: Refactor library linking methods in `trait Linker`)
 - rust-lang#120139 (Do not normalize closure signature when building `FnOnce` shim)
 - rust-lang#120160 (Manually implement derived `NonZero` traits.)
 - rust-lang#120171 (Fix assume and assert in jump threading)
 - rust-lang#120183 (Add `#[coverage(off)]` to closures introduced by `#[test]` and `#[bench]`)
 - rust-lang#120195 (add several resolution test cases)
 - rust-lang#120259 (Split Diagnostics for Uncommon Codepoints: Add List to Display Characters Involved)
 - rust-lang#120261 (Provide structured suggestion to use trait objects in some cases of `if` arm type divergence)

r? `@ghost`
`@rustbot` modify labels: rollup
@fmease fmease mentioned this pull request Jan 23, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 23, 2024
@bors
Auto merge of rust-lang#120283 - fmease:rollup-rk0f6r5, r=fmease
0b77301 
Rollup of 9 pull requests

Successful merges:

 - rust-lang#112806 (Small code improvements in `collect_intra_doc_links.rs`)
 - rust-lang#119766 (Split tait and impl trait in assoc items logic)
 - rust-lang#120139 (Do not normalize closure signature when building `FnOnce` shim)
 - rust-lang#120160 (Manually implement derived `NonZero` traits.)
 - rust-lang#120171 (Fix assume and assert in jump threading)
 - rust-lang#120183 (Add `#[coverage(off)]` to closures introduced by `#[test]` and `#[bench]`)
 - rust-lang#120195 (add several resolution test cases)
 - rust-lang#120259 (Split Diagnostics for Uncommon Codepoints: Add List to Display Characters Involved)
 - rust-lang#120261 (Provide structured suggestion to use trait objects in some cases of `if` arm type divergence)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4d9b983 into rust-lang:master Jan 24, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 24, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 24, 2024
@rust-timer
Unrolled build for rust-lang#120259
555acf0 
Rollup merge of rust-lang#120259 - HTGAzureX1212:HTGAzureX1212/split-diagnostics-uncommon-codepoints, r=Manishearth

Split Diagnostics for Uncommon Codepoints: Add List to Display Characters Involved

This Pull Request adds a list of the uncommon codepoints involved in the `uncommon_codepoints` lint, as outlined as a first step in rust-lang#120228.

Example rendered diagnostic:
```
error: identifier contains an uncommon Unicode codepoint: 'µ'
  --> $DIR/lint-uncommon-codepoints.rs:3:7
   |
LL | const µ: f64 = 0.000001;
   |       ^
   |
note: the lint level is defined here
  --> $DIR/lint-uncommon-codepoints.rs:1:9
   |
LL | #![deny(uncommon_codepoints)]
   |         ^^^^^^^^^^^^^^^^^^^
```

(Retrying rust-lang#120258.)
@HTGAzureX1212 HTGAzureX1212 deleted the HTGAzureX1212/split-diagnostics-uncommon-codepoints branch January 24, 2024 06:04
@HTGAzureX1212 HTGAzureX1212 mentioned this pull request Feb 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fee1-dead fee1-dead fee1-dead left review comments

@Manishearth Manishearth Manishearth approved these changes

Assignees

@cjgillot cjgillot

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.77.0
Development

Successfully merging this pull request may close these issues.
6 participants
@HTGAzureX1212 @rustbot @Manishearth @bors @fee1-dead @cjgillot