Skip to content

Commit c944d23

Browse files
Avoid nested quotations in auto-quoting fix (#9168)
## Summary Given `Callable[[Callable[_P, _R]], Callable[_P, _R]]` from the originating issue, when quoting `Callable`, we quoted the inner `[Callable[_P, _R]]`, and then created a separate edit for the outer `Callable`. Since there's an extra level of nesting in the subscript, the edit for `[Callable[_P, _R]]` correctly did _not_ expand to the entire expression. However, in this case, we should discard the inner edit, since the expression is getting quoted by the outer edit anyway. Closes #9162.
1 parent 93d8c56 commit c944d23

File tree

5 files changed

+71
-42
lines changed

5 files changed

+71
-42
lines changed

crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote.py

+3
Original file line numberDiff line numberDiff line change
@@ -87,3 +87,6 @@ class C:
8787
x: DataFrame[
8888
int
8989
] = 1
90+
91+
def func() -> DataFrame[[DataFrame[_P, _R]], DataFrame[_P, _R]]:
92+
...

crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs

+14
Original file line numberDiff line numberDiff line change
@@ -235,3 +235,17 @@ pub(crate) fn quote_annotation(
235235
expr.range(),
236236
))
237237
}
238+
239+
/// Filter out any [`Edit`]s that are completely contained by any other [`Edit`].
240+
pub(crate) fn filter_contained(edits: Vec<Edit>) -> Vec<Edit> {
241+
let mut filtered: Vec<Edit> = Vec::with_capacity(edits.len());
242+
for edit in edits {
243+
if filtered
244+
.iter()
245+
.all(|filtered_edit| !filtered_edit.range().contains_range(edit.range()))
246+
{
247+
filtered.push(edit);
248+
}
249+
}
250+
filtered
251+
}

crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs

+22-21
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use std::borrow::Cow;
22

33
use anyhow::Result;
4-
use itertools::Itertools;
54
use rustc_hash::FxHashMap;
65

76
use ruff_diagnostics::{Diagnostic, Fix, FixAvailability, Violation};
@@ -13,7 +12,7 @@ use crate::checkers::ast::Checker;
1312
use crate::codes::Rule;
1413
use crate::fix;
1514
use crate::importer::ImportedMembers;
16-
use crate::rules::flake8_type_checking::helpers::quote_annotation;
15+
use crate::rules::flake8_type_checking::helpers::{filter_contained, quote_annotation};
1716
use crate::rules::flake8_type_checking::imports::ImportBinding;
1817

1918
/// ## What it does
@@ -263,27 +262,29 @@ pub(crate) fn runtime_import_in_type_checking_block(
263262

264263
/// Generate a [`Fix`] to quote runtime usages for imports in a type-checking block.
265264
fn quote_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) -> Result<Fix> {
266-
let quote_reference_edits = imports
267-
.iter()
268-
.flat_map(|ImportBinding { binding, .. }| {
269-
binding.references.iter().filter_map(|reference_id| {
270-
let reference = checker.semantic().reference(*reference_id);
271-
if reference.context().is_runtime() {
272-
Some(quote_annotation(
273-
reference.expression_id()?,
274-
checker.semantic(),
275-
checker.locator(),
276-
checker.stylist(),
277-
checker.generator(),
278-
))
279-
} else {
280-
None
281-
}
265+
let quote_reference_edits = filter_contained(
266+
imports
267+
.iter()
268+
.flat_map(|ImportBinding { binding, .. }| {
269+
binding.references.iter().filter_map(|reference_id| {
270+
let reference = checker.semantic().reference(*reference_id);
271+
if reference.context().is_runtime() {
272+
Some(quote_annotation(
273+
reference.expression_id()?,
274+
checker.semantic(),
275+
checker.locator(),
276+
checker.stylist(),
277+
checker.generator(),
278+
))
279+
} else {
280+
None
281+
}
282+
})
282283
})
283-
})
284-
.collect::<Result<Vec<_>>>()?;
284+
.collect::<Result<Vec<_>>>()?,
285+
);
285286

286-
let mut rest = quote_reference_edits.into_iter().unique();
287+
let mut rest = quote_reference_edits.into_iter();
287288
let head = rest.next().expect("Expected at least one reference");
288289
Ok(Fix::unsafe_edits(head, rest).isolate(Checker::isolation(
289290
checker.semantic().parent_statement_id(node_id),

crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs

+24-21
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use std::borrow::Cow;
22

33
use anyhow::Result;
4-
use itertools::Itertools;
54
use rustc_hash::FxHashMap;
65

76
use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix, FixAvailability, Violation};
@@ -13,7 +12,9 @@ use crate::checkers::ast::Checker;
1312
use crate::codes::Rule;
1413
use crate::fix;
1514
use crate::importer::ImportedMembers;
16-
use crate::rules::flake8_type_checking::helpers::{is_typing_reference, quote_annotation};
15+
use crate::rules::flake8_type_checking::helpers::{
16+
filter_contained, is_typing_reference, quote_annotation,
17+
};
1718
use crate::rules::flake8_type_checking::imports::ImportBinding;
1819
use crate::rules::isort::{categorize, ImportSection, ImportType};
1920

@@ -483,32 +484,34 @@ fn fix_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) ->
483484
)?;
484485

485486
// Step 3) Quote any runtime usages of the referenced symbol.
486-
let quote_reference_edits = imports
487-
.iter()
488-
.flat_map(|ImportBinding { binding, .. }| {
489-
binding.references.iter().filter_map(|reference_id| {
490-
let reference = checker.semantic().reference(*reference_id);
491-
if reference.context().is_runtime() {
492-
Some(quote_annotation(
493-
reference.expression_id()?,
494-
checker.semantic(),
495-
checker.locator(),
496-
checker.stylist(),
497-
checker.generator(),
498-
))
499-
} else {
500-
None
501-
}
487+
let quote_reference_edits = filter_contained(
488+
imports
489+
.iter()
490+
.flat_map(|ImportBinding { binding, .. }| {
491+
binding.references.iter().filter_map(|reference_id| {
492+
let reference = checker.semantic().reference(*reference_id);
493+
if reference.context().is_runtime() {
494+
Some(quote_annotation(
495+
reference.expression_id()?,
496+
checker.semantic(),
497+
checker.locator(),
498+
checker.stylist(),
499+
checker.generator(),
500+
))
501+
} else {
502+
None
503+
}
504+
})
502505
})
503-
})
504-
.collect::<Result<Vec<_>>>()?;
506+
.collect::<Result<Vec<_>>>()?,
507+
);
505508

506509
Ok(Fix::unsafe_edits(
507510
remove_import_edit,
508511
add_import_edit
509512
.into_edits()
510513
.into_iter()
511-
.chain(quote_reference_edits.into_iter().unique()),
514+
.chain(quote_reference_edits),
512515
)
513516
.isolate(Checker::isolation(
514517
checker.semantic().parent_statement_id(node_id),

crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote.py.snap

+8
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,10 @@ quote.py:78:24: TCH002 [*] Move third-party import `pandas.DataFrame` into a typ
292292
88 |- int
293293
89 |- ] = 1
294294
89 |+ x: "DataFrame[int]" = 1
295+
90 90 |
296+
91 |- def func() -> DataFrame[[DataFrame[_P, _R]], DataFrame[_P, _R]]:
297+
91 |+ def func() -> "DataFrame[[DataFrame[_P, _R]], DataFrame[_P, _R]]":
298+
92 92 | ...
295299

296300
quote.py:78:35: TCH002 [*] Move third-party import `pandas.Series` into a type-checking block
297301
|
@@ -329,5 +333,9 @@ quote.py:78:35: TCH002 [*] Move third-party import `pandas.Series` into a type-c
329333
88 |- int
330334
89 |- ] = 1
331335
89 |+ x: "DataFrame[int]" = 1
336+
90 90 |
337+
91 |- def func() -> DataFrame[[DataFrame[_P, _R]], DataFrame[_P, _R]]:
338+
91 |+ def func() -> "DataFrame[[DataFrame[_P, _R]], DataFrame[_P, _R]]":
339+
92 92 | ...
332340

333341

0 commit comments

Comments
 (0)