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 https://github.com/astral-sh/ruff/issues/9162.
This commit is contained in:
Charlie Marsh 2023-12-17 07:53:58 -05:00 committed by GitHub
parent 93d8c56d41
commit c944d23053
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 71 additions and 42 deletions

View file

@ -87,3 +87,6 @@ def f():
x: DataFrame[
int
] = 1
def func() -> DataFrame[[DataFrame[_P, _R]], DataFrame[_P, _R]]:
...

View file

@ -235,3 +235,17 @@ pub(crate) fn quote_annotation(
expr.range(),
))
}
/// Filter out any [`Edit`]s that are completely contained by any other [`Edit`].
pub(crate) fn filter_contained(edits: Vec<Edit>) -> Vec<Edit> {
let mut filtered: Vec<Edit> = Vec::with_capacity(edits.len());
for edit in edits {
if filtered
.iter()
.all(|filtered_edit| !filtered_edit.range().contains_range(edit.range()))
{
filtered.push(edit);
}
}
filtered
}

View file

@ -1,7 +1,6 @@
use std::borrow::Cow;
use anyhow::Result;
use itertools::Itertools;
use rustc_hash::FxHashMap;
use ruff_diagnostics::{Diagnostic, Fix, FixAvailability, Violation};
@ -13,7 +12,7 @@ use crate::checkers::ast::Checker;
use crate::codes::Rule;
use crate::fix;
use crate::importer::ImportedMembers;
use crate::rules::flake8_type_checking::helpers::quote_annotation;
use crate::rules::flake8_type_checking::helpers::{filter_contained, quote_annotation};
use crate::rules::flake8_type_checking::imports::ImportBinding;
/// ## What it does
@ -263,27 +262,29 @@ pub(crate) fn runtime_import_in_type_checking_block(
/// Generate a [`Fix`] to quote runtime usages for imports in a type-checking block.
fn quote_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) -> Result<Fix> {
let quote_reference_edits = imports
.iter()
.flat_map(|ImportBinding { binding, .. }| {
binding.references.iter().filter_map(|reference_id| {
let reference = checker.semantic().reference(*reference_id);
if reference.context().is_runtime() {
Some(quote_annotation(
reference.expression_id()?,
checker.semantic(),
checker.locator(),
checker.stylist(),
checker.generator(),
))
} else {
None
}
let quote_reference_edits = filter_contained(
imports
.iter()
.flat_map(|ImportBinding { binding, .. }| {
binding.references.iter().filter_map(|reference_id| {
let reference = checker.semantic().reference(*reference_id);
if reference.context().is_runtime() {
Some(quote_annotation(
reference.expression_id()?,
checker.semantic(),
checker.locator(),
checker.stylist(),
checker.generator(),
))
} else {
None
}
})
})
})
.collect::<Result<Vec<_>>>()?;
.collect::<Result<Vec<_>>>()?,
);
let mut rest = quote_reference_edits.into_iter().unique();
let mut rest = quote_reference_edits.into_iter();
let head = rest.next().expect("Expected at least one reference");
Ok(Fix::unsafe_edits(head, rest).isolate(Checker::isolation(
checker.semantic().parent_statement_id(node_id),

View file

@ -1,7 +1,6 @@
use std::borrow::Cow;
use anyhow::Result;
use itertools::Itertools;
use rustc_hash::FxHashMap;
use ruff_diagnostics::{Diagnostic, DiagnosticKind, Fix, FixAvailability, Violation};
@ -13,7 +12,9 @@ use crate::checkers::ast::Checker;
use crate::codes::Rule;
use crate::fix;
use crate::importer::ImportedMembers;
use crate::rules::flake8_type_checking::helpers::{is_typing_reference, quote_annotation};
use crate::rules::flake8_type_checking::helpers::{
filter_contained, is_typing_reference, quote_annotation,
};
use crate::rules::flake8_type_checking::imports::ImportBinding;
use crate::rules::isort::{categorize, ImportSection, ImportType};
@ -483,32 +484,34 @@ fn fix_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) ->
)?;
// Step 3) Quote any runtime usages of the referenced symbol.
let quote_reference_edits = imports
.iter()
.flat_map(|ImportBinding { binding, .. }| {
binding.references.iter().filter_map(|reference_id| {
let reference = checker.semantic().reference(*reference_id);
if reference.context().is_runtime() {
Some(quote_annotation(
reference.expression_id()?,
checker.semantic(),
checker.locator(),
checker.stylist(),
checker.generator(),
))
} else {
None
}
let quote_reference_edits = filter_contained(
imports
.iter()
.flat_map(|ImportBinding { binding, .. }| {
binding.references.iter().filter_map(|reference_id| {
let reference = checker.semantic().reference(*reference_id);
if reference.context().is_runtime() {
Some(quote_annotation(
reference.expression_id()?,
checker.semantic(),
checker.locator(),
checker.stylist(),
checker.generator(),
))
} else {
None
}
})
})
})
.collect::<Result<Vec<_>>>()?;
.collect::<Result<Vec<_>>>()?,
);
Ok(Fix::unsafe_edits(
remove_import_edit,
add_import_edit
.into_edits()
.into_iter()
.chain(quote_reference_edits.into_iter().unique()),
.chain(quote_reference_edits),
)
.isolate(Checker::isolation(
checker.semantic().parent_statement_id(node_id),

View file

@ -292,6 +292,10 @@ quote.py:78:24: TCH002 [*] Move third-party import `pandas.DataFrame` into a typ
88 |- int
89 |- ] = 1
89 |+ x: "DataFrame[int]" = 1
90 90 |
91 |- def func() -> DataFrame[[DataFrame[_P, _R]], DataFrame[_P, _R]]:
91 |+ def func() -> "DataFrame[[DataFrame[_P, _R]], DataFrame[_P, _R]]":
92 92 | ...
quote.py:78:35: TCH002 [*] Move third-party import `pandas.Series` into a type-checking block
|
@ -329,5 +333,9 @@ quote.py:78:35: TCH002 [*] Move third-party import `pandas.Series` into a type-c
88 |- int
89 |- ] = 1
89 |+ x: "DataFrame[int]" = 1
90 90 |
91 |- def func() -> DataFrame[[DataFrame[_P, _R]], DataFrame[_P, _R]]:
91 |+ def func() -> "DataFrame[[DataFrame[_P, _R]], DataFrame[_P, _R]]":
92 92 | ...