Skip to content

Commit 864196b

Browse files
authored
Add Checker::context method, deduplicate Unicode checks (#19609)
Summary -- This PR adds a `Checker::context` method that returns the underlying `LintContext` to unify `Candidate::into_diagnostic` and `Candidate::report_diagnostic` in our ambiguous Unicode character checks. This avoids some duplication and also avoids collecting a `Vec` of `Candidate`s only to iterate over it later. Test Plan -- Existing tests
1 parent ae26fa0 commit 864196b

File tree

2 files changed

+29
-59
lines changed

2 files changed

+29
-59
lines changed

crates/ruff_linter/src/checkers/ast/mod.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,16 @@ impl<'a> Checker<'a> {
590590
member,
591591
})
592592
}
593+
594+
/// Return the [`LintContext`] for the current analysis.
595+
///
596+
/// Note that you should always prefer calling methods like `settings`, `report_diagnostic`, or
597+
/// `is_rule_enabled` directly on [`Checker`] when possible. This method exists only for the
598+
/// rare cases where rules or helper functions need to be accessed by both a `Checker` and a
599+
/// `LintContext` in different analysis phases.
600+
pub(crate) const fn context(&self) -> &'a LintContext<'a> {
601+
self.context
602+
}
593603
}
594604

595605
pub(crate) struct TypingImporter<'a, 'b> {

crates/ruff_linter/src/rules/ruff/rules/ambiguous_unicode_character.rs

Lines changed: 19 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use crate::checkers::ast::{Checker, LintContext};
1212
use crate::preview::is_unicode_to_unicode_confusables_enabled;
1313
use crate::rules::ruff::rules::Context;
1414
use crate::rules::ruff::rules::confusables::confusable;
15-
use crate::settings::LinterSettings;
1615

1716
/// ## What it does
1817
/// Checks for ambiguous Unicode characters in strings.
@@ -180,9 +179,7 @@ pub(crate) fn ambiguous_unicode_character_comment(
180179
range: TextRange,
181180
) {
182181
let text = locator.slice(range);
183-
for candidate in ambiguous_unicode_character(text, range, context.settings()) {
184-
candidate.into_diagnostic(Context::Comment, context);
185-
}
182+
ambiguous_unicode_character(text, range, Context::Comment, context);
186183
}
187184

188185
/// RUF001, RUF002
@@ -203,22 +200,19 @@ pub(crate) fn ambiguous_unicode_character_string(checker: &Checker, string_like:
203200
match part {
204201
ast::StringLikePart::String(string_literal) => {
205202
let text = checker.locator().slice(string_literal);
206-
for candidate in
207-
ambiguous_unicode_character(text, string_literal.range(), checker.settings())
208-
{
209-
candidate.report_diagnostic(checker, context);
210-
}
203+
ambiguous_unicode_character(
204+
text,
205+
string_literal.range(),
206+
context,
207+
checker.context(),
208+
);
211209
}
212210
ast::StringLikePart::Bytes(_) => {}
213211
ast::StringLikePart::FString(FString { elements, .. })
214212
| ast::StringLikePart::TString(TString { elements, .. }) => {
215213
for literal in elements.literals() {
216214
let text = checker.locator().slice(literal);
217-
for candidate in
218-
ambiguous_unicode_character(text, literal.range(), checker.settings())
219-
{
220-
candidate.report_diagnostic(checker, context);
221-
}
215+
ambiguous_unicode_character(text, literal.range(), context, checker.context());
222216
}
223217
}
224218
}
@@ -228,13 +222,12 @@ pub(crate) fn ambiguous_unicode_character_string(checker: &Checker, string_like:
228222
fn ambiguous_unicode_character(
229223
text: &str,
230224
range: TextRange,
231-
settings: &LinterSettings,
232-
) -> Vec<Candidate> {
233-
let mut candidates = Vec::new();
234-
225+
context: Context,
226+
lint_context: &LintContext,
227+
) {
235228
// Most of the time, we don't need to check for ambiguous unicode characters at all.
236229
if text.is_ascii() {
237-
return candidates;
230+
return;
238231
}
239232

240233
// Iterate over the "words" in the text.
@@ -246,7 +239,7 @@ fn ambiguous_unicode_character(
246239
if !word_candidates.is_empty() {
247240
if word_flags.is_candidate_word() {
248241
for candidate in word_candidates.drain(..) {
249-
candidates.push(candidate);
242+
candidate.into_diagnostic(context, lint_context);
250243
}
251244
}
252245
word_candidates.clear();
@@ -257,21 +250,23 @@ fn ambiguous_unicode_character(
257250
// case, it's always included as a diagnostic.
258251
if !current_char.is_ascii() {
259252
if let Some(representant) = confusable(current_char as u32).filter(|representant| {
260-
is_unicode_to_unicode_confusables_enabled(settings) || representant.is_ascii()
253+
is_unicode_to_unicode_confusables_enabled(lint_context.settings())
254+
|| representant.is_ascii()
261255
}) {
262256
let candidate = Candidate::new(
263257
TextSize::try_from(relative_offset).unwrap() + range.start(),
264258
current_char,
265259
representant,
266260
);
267-
candidates.push(candidate);
261+
candidate.into_diagnostic(context, lint_context);
268262
}
269263
}
270264
} else if current_char.is_ascii() {
271265
// The current word contains at least one ASCII character.
272266
word_flags |= WordFlags::ASCII;
273267
} else if let Some(representant) = confusable(current_char as u32).filter(|representant| {
274-
is_unicode_to_unicode_confusables_enabled(settings) || representant.is_ascii()
268+
is_unicode_to_unicode_confusables_enabled(lint_context.settings())
269+
|| representant.is_ascii()
275270
}) {
276271
// The current word contains an ambiguous unicode character.
277272
word_candidates.push(Candidate::new(
@@ -289,13 +284,11 @@ fn ambiguous_unicode_character(
289284
if !word_candidates.is_empty() {
290285
if word_flags.is_candidate_word() {
291286
for candidate in word_candidates.drain(..) {
292-
candidates.push(candidate);
287+
candidate.into_diagnostic(context, lint_context);
293288
}
294289
}
295290
word_candidates.clear();
296291
}
297-
298-
candidates
299292
}
300293

301294
bitflags! {
@@ -373,39 +366,6 @@ impl Candidate {
373366
};
374367
}
375368
}
376-
377-
fn report_diagnostic(self, checker: &Checker, context: Context) {
378-
if !checker
379-
.settings()
380-
.allowed_confusables
381-
.contains(&self.confusable)
382-
{
383-
let char_range = TextRange::at(self.offset, self.confusable.text_len());
384-
match context {
385-
Context::String => checker.report_diagnostic_if_enabled(
386-
AmbiguousUnicodeCharacterString {
387-
confusable: self.confusable,
388-
representant: self.representant,
389-
},
390-
char_range,
391-
),
392-
Context::Docstring => checker.report_diagnostic_if_enabled(
393-
AmbiguousUnicodeCharacterDocstring {
394-
confusable: self.confusable,
395-
representant: self.representant,
396-
},
397-
char_range,
398-
),
399-
Context::Comment => checker.report_diagnostic_if_enabled(
400-
AmbiguousUnicodeCharacterComment {
401-
confusable: self.confusable,
402-
representant: self.representant,
403-
},
404-
char_range,
405-
),
406-
};
407-
}
408-
}
409369
}
410370

411371
struct NamedUnicode(char);

0 commit comments

Comments
 (0)