Implement mixed script confusable lint. #72770
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Manishearth
reviewed
Jun 4, 2020
Manishearth
approved these changes
Jun 4, 2020
|
|
||
| cx.struct_span_lint(MIXED_SCRIPT_CONFUSABLES, sp, |lint| { | ||
| let message = format!( | ||
| "Unicode augmented script group `{:?}` usage in this crate consists solely of mixed script confusables, including {:?} and maybe other characters.", |
There was a problem hiding this comment.
"The usage of Script Group {:?} in this crate consists solely of mixed script confusables, including {:?}"
There was a problem hiding this comment.
Done in principle. Didn't find out a proper way to generate a long error message including line-breaks, so i moved the second sub-sentence to a note. Please review again to see if this is ok.
|
☔ The latest upstream changes (presumably #73511) made this pull request unmergeable. Please resolve the merge conflicts. |
crlf0710
force-pushed
the
mixed_script_confusable
branch
from
d9b51ed to
748fdfe
Compare
This comment has been minimized.
This comment has been minimized.
crlf0710
force-pushed
the
mixed_script_confusable
branch
from
748fdfe to
4add62a
Compare
Manishearth
reviewed
Jun 23, 2020
| #![deny(mixed_script_confusables)] | ||
|
|
||
| struct ΑctuallyNotLatin; | ||
| //~^ ERROR The usage of Script Group `AugmentedScriptSet {Grek}` in this crate consists solely of |
There was a problem hiding this comment.
Debug impls should not be used here, we should instead have something that will display Greek and Greek + Foo + Bar without the AugmentedScriptSet {} bit. ScriptExtension already has a Debug impl that could be used for this. I can instead give it a Display impl that's better here.
There was a problem hiding this comment.
Manishearth
approved these changes
Jun 23, 2020
|
Sure, i can add more tests after switching Debug to Display, and try to make it test out more logic branching. And do you have anything specially want to cover within the tests? |
|
Mostly just make sure all cases are covered. E.g. having an ident |
crlf0710
force-pushed
the
mixed_script_confusable
branch
from
4add62a to
bec58fb
Compare
Manishearth
approved these changes
Jun 25, 2020
src/test/ui/lint/rfc-2457-non-ascii-idents/lint-mixed-script-confusables.stderr
Show resolved
Hide resolved
crlf0710
force-pushed
the
mixed_script_confusable
branch
from
bec58fb to
25e864e
Compare
|
@bors r+ |
|
📌 Commit 25e864e has been approved by |
bors
added
S-waiting-on-bors
Manishearth
added a commit
to Manishearth/rust
that referenced
this pull request
Jun 26, 2020
…=Manishearth Implement mixed script confusable lint. This implements the mixed script confusable lint defined in RFC 2457. This is blocked on rust-lang#72069 and unicode-rs/unicode-security#13, and will need a Cargo.toml version bump after those are resolved. The lint message warning is sub-optimal for now. We'll need a mechanism to properly output `AugmentScriptSet` to screen, this is to be added in `unicode-security` crate. r? @Manishearth
Manishearth
added a commit
to Manishearth/rust
that referenced
this pull request
Jun 26, 2020
…=Manishearth Implement mixed script confusable lint. This implements the mixed script confusable lint defined in RFC 2457. This is blocked on rust-lang#72069 and unicode-rs/unicode-security#13, and will need a Cargo.toml version bump after those are resolved. The lint message warning is sub-optimal for now. We'll need a mechanism to properly output `AugmentScriptSet` to screen, this is to be added in `unicode-security` crate. r? @Manishearth
This was referenced Jun 26, 2020
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 26, 2020
…arth Rollup of 14 pull requests Successful merges: - rust-lang#72617 (Add a fast path for `std::thread::panicking`.) - rust-lang#72738 (Self contained linking option) - rust-lang#72770 (Implement mixed script confusable lint.) - rust-lang#73418 (Add unstable `core::mem::variant_count` intrinsic) - rust-lang#73460 (Emit line info for generator variants) - rust-lang#73534 (Provide suggestions for some moved value errors) - rust-lang#73538 (make commented examples use valid syntax, and be more consistent ) - rust-lang#73581 (Create 0766 error code) - rust-lang#73619 (Document the mod keyword) - rust-lang#73621 (Document the mut keyword) - rust-lang#73648 (Document the return keyword) - rust-lang#73673 (Fix ptr doc warnings.) - rust-lang#73674 (Tweak binop errors) - rust-lang#73687 (Clean up E0701 explanation) Failed merges: - rust-lang#73708 (Explain move errors that occur due to method calls involving `self` (take two)) r? @ghost
23 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements the mixed script confusable lint defined in RFC 2457.
This is blocked on #72069 and unicode-rs/unicode-security#13, and will need a Cargo.toml version bump after those are resolved.
The lint message warning is sub-optimal for now. We'll need a mechanism to properly output
AugmentScriptSetto screen, this is to be added inunicode-securitycrate.r? @Manishearth