diff --git a/crates/ruff/resources/test/fixtures/ruff/confusables.py b/crates/ruff/resources/test/fixtures/ruff/confusables.py index b642cebd6f..3ae350887f 100644 --- a/crates/ruff/resources/test/fixtures/ruff/confusables.py +++ b/crates/ruff/resources/test/fixtures/ruff/confusables.py @@ -8,7 +8,24 @@ def f(): ... -def g(): +def f(): """Here's a docstring with a greek rho: ρ""" # And here's a comment with a greek alpha: ∗ ... + + +x = "𝐁ad string" +x = "−" + +# This should be ignored, since it contains an unambiguous unicode character, and no +# ASCII. +x = "Русский" + +# The first word should be ignored, while the second should be included, since it +# contains ASCII. +x = "βα Bαd" + +# The two characters should be flagged here. The first character is a "word" +# consisting of a single ambiguous character, while the second character is a "word +# boundary" (whitespace) that it itself ambiguous. +x = "Р усский" diff --git a/crates/ruff/src/rules/ruff/rules/ambiguous_unicode_character.rs b/crates/ruff/src/rules/ruff/rules/ambiguous_unicode_character.rs index c82fafc0be..23e7584bed 100644 --- a/crates/ruff/src/rules/ruff/rules/ambiguous_unicode_character.rs +++ b/crates/ruff/src/rules/ruff/rules/ambiguous_unicode_character.rs @@ -1,3 +1,4 @@ +use bitflags::bitflags; use ruff_text_size::{TextLen, TextRange, TextSize}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, DiagnosticKind, Edit, Fix}; @@ -23,8 +24,7 @@ impl AlwaysAutofixableViolation for AmbiguousUnicodeCharacterString { representant, } = self; format!( - "String contains ambiguous unicode character `{confusable}` (did you mean \ - `{representant}`?)" + "String contains ambiguous unicode character `{confusable}` (did you mean `{representant}`?)" ) } @@ -51,8 +51,7 @@ impl AlwaysAutofixableViolation for AmbiguousUnicodeCharacterDocstring { representant, } = self; format!( - "Docstring contains ambiguous unicode character `{confusable}` (did you mean \ - `{representant}`?)" + "Docstring contains ambiguous unicode character `{confusable}` (did you mean `{representant}`?)" ) } @@ -79,8 +78,7 @@ impl AlwaysAutofixableViolation for AmbiguousUnicodeCharacterComment { representant, } = self; format!( - "Comment contains ambiguous unicode character `{confusable}` (did you mean \ - `{representant}`?)" + "Comment contains ambiguous unicode character `{confusable}` (did you mean `{representant}`?)" ) } @@ -103,50 +101,151 @@ pub(crate) fn ambiguous_unicode_character( let text = locator.slice(range); - for (relative_offset, current_char) in text.char_indices() { - if !current_char.is_ascii() { - // Search for confusing characters. - if let Some(representant) = CONFUSABLES.get(&(current_char as u32)).copied() { - if !settings.allowed_confusables.contains(¤t_char) { - let char_range = TextRange::at( - TextSize::try_from(relative_offset).unwrap() + range.start(), - current_char.text_len(), - ); + // Most of the time, we don't need to check for ambiguous unicode characters at all. + if text.is_ascii() { + return diagnostics; + } - let mut diagnostic = Diagnostic::new::( - match context { - Context::String => AmbiguousUnicodeCharacterString { - confusable: current_char, - representant: representant as char, - } - .into(), - Context::Docstring => AmbiguousUnicodeCharacterDocstring { - confusable: current_char, - representant: representant as char, - } - .into(), - Context::Comment => AmbiguousUnicodeCharacterComment { - confusable: current_char, - representant: representant as char, - } - .into(), - }, - char_range, - ); - if settings.rules.enabled(diagnostic.kind.rule()) { - if settings.rules.should_fix(diagnostic.kind.rule()) { - #[allow(deprecated)] - diagnostic.set_fix(Fix::unspecified(Edit::range_replacement( - (representant as char).to_string(), - char_range, - ))); + // Iterate over the "words" in the text. + let mut word_flags = WordFlags::empty(); + let mut word_candidates: Vec = vec![]; + for (relative_offset, current_char) in text.char_indices() { + // Word boundary. + if !current_char.is_alphanumeric() { + if !word_candidates.is_empty() { + if word_flags.is_candidate_word() { + for candidate in word_candidates.drain(..) { + if let Some(diagnostic) = candidate.into_diagnostic(context, settings) { + diagnostics.push(diagnostic); } + } + } + word_candidates.clear(); + } + word_flags = WordFlags::empty(); + + // Check if the boundary character is itself an ambiguous unicode character, in which + // case, it's always included as a diagnostic. + if !current_char.is_ascii() { + if let Some(representant) = CONFUSABLES.get(&(current_char as u32)).copied() { + let candidate = Candidate::new( + TextSize::try_from(relative_offset).unwrap() + range.start(), + current_char, + representant as char, + ); + if let Some(diagnostic) = candidate.into_diagnostic(context, settings) { diagnostics.push(diagnostic); } } } + } else if current_char.is_ascii() { + // The current word contains at least one ASCII character. + word_flags |= WordFlags::ASCII; + } else if let Some(representant) = CONFUSABLES.get(&(current_char as u32)).copied() { + // The current word contains an ambiguous unicode character. + word_candidates.push(Candidate::new( + TextSize::try_from(relative_offset).unwrap() + range.start(), + current_char, + representant as char, + )); + } else { + // The current word contains at least one unambiguous unicode character. + word_flags |= WordFlags::UNAMBIGUOUS_UNICODE; } } + // End of the text. + if !word_candidates.is_empty() { + if word_flags.is_candidate_word() { + for candidate in word_candidates.drain(..) { + if let Some(diagnostic) = candidate.into_diagnostic(context, settings) { + diagnostics.push(diagnostic); + } + } + } + word_candidates.clear(); + } + diagnostics } + +bitflags! { + #[derive(Default, Debug, Copy, Clone, PartialEq, Eq)] + pub struct WordFlags: u8 { + /// The word contains at least one ASCII character (like `B`). + const ASCII = 0b0000_0001; + /// The word contains at least one unambiguous unicode character (like `β`). + const UNAMBIGUOUS_UNICODE = 0b0000_0010; + } +} + +impl WordFlags { + /// Return `true` if the flags indicate that the word is a candidate for flagging + /// ambiguous unicode characters. + /// + /// We follow VS Code's logic for determining whether ambiguous unicode characters within a + /// given word should be flagged, i.e., we flag a word if it contains at least one ASCII + /// character, or is purely unicode but _only_ consists of ambiguous characters. + /// + /// See: [VS Code](https://github.com/microsoft/vscode/issues/143720#issuecomment-1048757234) + const fn is_candidate_word(self) -> bool { + self.contains(WordFlags::ASCII) || !self.contains(WordFlags::UNAMBIGUOUS_UNICODE) + } +} + +/// An ambiguous unicode character in the text. +struct Candidate { + /// The offset of the candidate in the text. + offset: TextSize, + /// The ambiguous unicode character. + confusable: char, + /// The character with which the ambiguous unicode character is confusable. + representant: char, +} + +impl Candidate { + fn new(offset: TextSize, confusable: char, representant: char) -> Self { + Self { + offset, + confusable, + representant, + } + } + + fn into_diagnostic(self, context: Context, settings: &Settings) -> Option { + if !settings.allowed_confusables.contains(&self.confusable) { + let char_range = TextRange::at(self.offset, self.confusable.text_len()); + let mut diagnostic = Diagnostic::new::( + match context { + Context::String => AmbiguousUnicodeCharacterString { + confusable: self.confusable, + representant: self.representant, + } + .into(), + Context::Docstring => AmbiguousUnicodeCharacterDocstring { + confusable: self.confusable, + representant: self.representant, + } + .into(), + Context::Comment => AmbiguousUnicodeCharacterComment { + confusable: self.confusable, + representant: self.representant, + } + .into(), + }, + char_range, + ); + if settings.rules.enabled(diagnostic.kind.rule()) { + if settings.rules.should_fix(diagnostic.kind.rule()) { + #[allow(deprecated)] + diagnostic.set_fix(Fix::unspecified(Edit::range_replacement( + self.representant.to_string(), + char_range, + ))); + } + return Some(diagnostic); + } + } + None + } +} diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__confusables.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__confusables.snap index 75d127a751..157fae36d3 100644 --- a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__confusables.snap +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__confusables.snap @@ -56,4 +56,75 @@ confusables.py:7:62: RUF003 [*] Comment contains ambiguous unicode character ` 9 9 | 10 10 | +confusables.py:17:6: RUF001 [*] String contains ambiguous unicode character `𝐁` (did you mean `B`?) + | +17 | x = "𝐁ad string" + | ^ RUF001 +18 | x = "−" + | + = help: Replace `𝐁` with `B` + +ℹ Suggested fix +14 14 | ... +15 15 | +16 16 | +17 |-x = "𝐁ad string" + 17 |+x = "Bad string" +18 18 | x = "−" +19 19 | +20 20 | # This should be ignored, since it contains an unambiguous unicode character, and no + +confusables.py:26:10: RUF001 [*] String contains ambiguous unicode character `α` (did you mean `a`?) + | +26 | # The first word should be ignored, while the second should be included, since it +27 | # contains ASCII. +28 | x = "βα Bαd" + | ^ RUF001 +29 | +30 | # The two characters should be flagged here. The first character is a "word" + | + = help: Replace `α` with `a` + +ℹ Suggested fix +23 23 | +24 24 | # The first word should be ignored, while the second should be included, since it +25 25 | # contains ASCII. +26 |-x = "βα Bαd" + 26 |+x = "βα Bad" +27 27 | +28 28 | # The two characters should be flagged here. The first character is a "word" +29 29 | # consisting of a single ambiguous character, while the second character is a "word + +confusables.py:31:6: RUF001 [*] String contains ambiguous unicode character `Р` (did you mean `P`?) + | +31 | # consisting of a single ambiguous character, while the second character is a "word +32 | # boundary" (whitespace) that it itself ambiguous. +33 | x = "Р усский" + | ^ RUF001 + | + = help: Replace `Р` with `P` + +ℹ Suggested fix +28 28 | # The two characters should be flagged here. The first character is a "word" +29 29 | # consisting of a single ambiguous character, while the second character is a "word +30 30 | # boundary" (whitespace) that it itself ambiguous. +31 |-x = "Р усский" + 31 |+x = "P усский" + +confusables.py:31:7: RUF001 [*] String contains ambiguous unicode character ` ` (did you mean ` `?) + | +31 | # consisting of a single ambiguous character, while the second character is a "word +32 | # boundary" (whitespace) that it itself ambiguous. +33 | x = "Р усский" + | ^ RUF001 + | + = help: Replace ` ` with ` ` + +ℹ Suggested fix +28 28 | # The two characters should be flagged here. The first character is a "word" +29 29 | # consisting of a single ambiguous character, while the second character is a "word +30 30 | # boundary" (whitespace) that it itself ambiguous. +31 |-x = "Р усский" + 31 |+x = "Р усский" +