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

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

Conversation

HTGAzureX1212
Copy link
Contributor

@HTGAzureX1212 HTGAzureX1212 commented Jan 23, 2024

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
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.

Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -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

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
…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.)
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 23, 2024
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
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 23, 2024
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
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
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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants