diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote3.py b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote3.py index f2c624886056e..31721427950af 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote3.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_type_checking/quote3.py @@ -53,3 +53,18 @@ def f(): def test_annotated_non_typing_reference(user: Annotated[str, Depends(get_foo)]): pass + +def f(): + from typing import Literal + from third_party import Type + + def test_string_contains_opposite_quote_do_not_fix(self, type1: Type[Literal["'"]], type2: Type[Literal["\'"]]): + pass + + +def f(): + from typing import Literal + from third_party import Type + + def test_quote_contains_backslash(self, type1: Type[Literal["\n"]], type2: Type[Literal["\""]]): + pass diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs index c8f2c9b657a9e..66e4400715cbe 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs @@ -1,11 +1,11 @@ -use anyhow::Result; -use ast::visitor::source_order; -use ruff_python_ast::visitor::source_order::SourceOrderVisitor; use std::cmp::Reverse; +use anyhow::Result; + use ruff_diagnostics::Edit; use ruff_python_ast::helpers::{map_callable, map_subscript}; use ruff_python_ast::name::QualifiedName; +use ruff_python_ast::visitor::source_order::{SourceOrderVisitor, TraversalSignal}; use ruff_python_ast::{self as ast, Decorator, Expr}; use ruff_python_codegen::{Generator, Stylist}; use ruff_python_semantic::{ @@ -221,8 +221,8 @@ pub(crate) fn is_singledispatch_implementation( /// This requires more than just wrapping the reference itself in quotes. For example: /// - When quoting `Series` in `Series[pd.Timestamp]`, we want `"Series[pd.Timestamp]"`. /// - When quoting `kubernetes` in `kubernetes.SecurityContext`, we want `"kubernetes.SecurityContext"`. -/// - When quoting `Series` in `Series["pd.Timestamp"]`, we want `"Series[pd.Timestamp]"`. (This is currently unsupported.) -/// - When quoting `Series` in `Series[Literal["pd.Timestamp"]]`, we want `"Series[Literal['pd.Timestamp']]"`. (This is currently unsupported.) +/// - When quoting `Series` in `Series["pd.Timestamp"]`, we want `"Series[pd.Timestamp]"`. +/// - When quoting `Series` in `Series[Literal["pd.Timestamp"]]`, we want `"Series[Literal['pd.Timestamp']]"`. /// /// In general, when expanding a component of a call chain, we want to quote the entire call chain. pub(crate) fn quote_annotation( @@ -272,7 +272,7 @@ pub(crate) fn quote_annotation( let quote = stylist.quote(); let mut quote_annotator = QuoteAnnotator::new(semantic, stylist); quote_annotator.visit_expr(expr); - let annotation = quote_annotator.into_annotation(); + let annotation = quote_annotator.into_annotation()?; Ok(Edit::range_replacement( format!("{quote}{annotation}{quote}"), @@ -313,6 +313,7 @@ pub(crate) struct QuoteAnnotator<'a> { semantic: &'a SemanticModel<'a>, state: Vec, annotation: String, + cannot_fix: bool, } impl<'a> QuoteAnnotator<'a> { @@ -322,15 +323,30 @@ impl<'a> QuoteAnnotator<'a> { semantic, state: Vec::new(), annotation: String::new(), + cannot_fix: false, } } - fn into_annotation(self) -> String { - self.annotation + fn into_annotation(self) -> Result { + if self.cannot_fix { + Err(anyhow::anyhow!( + "Cannot quote annotation because it already contains opposite quote or escape character" + )) + } else { + Ok(self.annotation) + } } } -impl<'a> source_order::SourceOrderVisitor<'a> for QuoteAnnotator<'a> { +impl<'a> SourceOrderVisitor<'a> for QuoteAnnotator<'a> { + fn enter_node(&mut self, _node: ast::AnyNodeRef<'a>) -> TraversalSignal { + if self.cannot_fix { + TraversalSignal::Skip + } else { + TraversalSignal::Traverse + } + } + fn visit_expr(&mut self, expr: &'a Expr) { let generator = Generator::from(self.stylist); @@ -388,10 +404,13 @@ impl<'a> source_order::SourceOrderVisitor<'a> for QuoteAnnotator<'a> { let source = match self.state.last().copied() { Some(QuoteAnnotatorState::Literal | QuoteAnnotatorState::AnnotatedRest) => { let mut source = generator.expr(expr); - source = source.replace( - self.stylist.quote().as_char(), - &self.stylist.quote().opposite().as_char().to_string(), - ); + let opposite_quote = &self.stylist.quote().opposite().as_char().to_string(); + // If the quotes we are going to insert in this source already exists set the auto quote outcome + // to failed. Because this means we are inserting quotes that are in the string and they collect. + if source.contains(opposite_quote) || source.contains('\\') { + self.cannot_fix = true; + } + source = source.replace(self.stylist.quote().as_char(), opposite_quote); source } None diff --git a/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 b/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 index ff6f2e6437542..41ed6558a21ac 100644 --- a/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 +++ b/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 @@ -1,5 +1,6 @@ --- source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +snapshot_kind: text --- quote.py:2:24: TCH002 [*] Move third-party import `pandas.DataFrame` into a type-checking block | diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote2.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote2.py.snap index dc87bebfccda0..2554aae580bc9 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote2.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote2.py.snap @@ -1,5 +1,6 @@ --- source: crates/ruff_linter/src/rules/flake8_type_checking/mod.rs +snapshot_kind: text --- quote2.py:2:44: TCH002 [*] Move third-party import `django.contrib.auth.models.AbstractBaseUser` into a type-checking block | diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote3.py.snap b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote3.py.snap index 01853a363783d..3a33fc66a88ea 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote3.py.snap +++ b/crates/ruff_linter/src/rules/flake8_type_checking/snapshots/ruff_linter__rules__flake8_type_checking__tests__quote_typing-only-third-party-import_quote3.py.snap @@ -150,4 +150,26 @@ quote3.py:40:37: TCH002 [*] Move third-party import `django.contrib.auth.models` 45 |+ def test_attribute_typing_literal(arg: 'models.AbstractBaseUser[Literal["admin"]]'): 43 46 | pass 44 47 | -45 48 | +45 48 | + +quote3.py:59:29: TCH002 Move third-party import `third_party.Type` into a type-checking block + | +57 | def f(): +58 | from typing import Literal +59 | from third_party import Type + | ^^^^ TCH002 +60 | +61 | def test_string_contains_opposite_quote_do_not_fix(self, type1: Type[Literal["'"]], type2: Type[Literal["\'"]]): + | + = help: Move into type-checking block + +quote3.py:67:29: TCH002 Move third-party import `third_party.Type` into a type-checking block + | +65 | def f(): +66 | from typing import Literal +67 | from third_party import Type + | ^^^^ TCH002 +68 | +69 | def test_quote_contains_backslash(self, type1: Type[Literal["\n"]], type2: Type[Literal["\""]]): + | + = help: Move into type-checking block