TCH fixes remove quotation marks from Literal strings

[dscorbett]

The fixes for TCH001, TCH002, and TCH003 sometimes delete quotation marks from Literals when lint.flake8-type-checking.quote-annotations = true. This can make a program fail to type-check. I am not sure exactly what makes it delete quotation marks but it seems to only happen to literals within nested brackets.

$ ruff --version
ruff 0.7.4

$ cat tch003.py
from collections.abc import Sequence
from typing import Callable, Literal
def f(x: Sequence[Callable[[Literal["1"]], bool]]) -> bool:
    return False
def g(x: Literal["1"]) -> bool:
    return False
print(f([g]))

$ mypy tch003.py
Success: no issues found in 1 source file

$ ruff check --isolated --config 'lint.flake8-type-checking.quote-annotations = true' --select TCH003 tch003.py --unsafe-fixes --fix
Found 1 error (1 fixed, 0 remaining).

$ cat tch003.py
from typing import Callable, Literal, TYPE_CHECKING

if TYPE_CHECKING:
    from collections.abc import Sequence
def f(x: "Sequence[Callable[[Literal[1]], bool]]") -> bool:
    return False
def g(x: Literal["1"]) -> bool:
    return False
print(f([g]))

$ mypy tch003.py
tch003.py:9: error: List item 0 has incompatible type "Callable[[Literal['1']], bool]"; expected "Callable[[Literal[1]], bool]"  [list-item]
Found 1 error in 1 file (checked 1 source file)

With "x" instead of "1" in that example, "x" is rewritten to x and the final mypy error is:

$ mypy tch003.py
tch003.py:5: error: Name "x" is not defined  [name-defined]
Found 1 error in 1 file (checked 1 source file)

[dylwil3]

Thanks so much!

Maybe this logic needs some inspection (or the QuoteAnnotator's Visitor implementation)?

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

Lines 219 to 281 in c847cad

	/// Wrap a type annotation in quotes.
	///
	/// 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]"`.
	/// - 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(
	node_id: NodeId,
	semantic: &SemanticModel,
	stylist: &Stylist,
	) -> Result<Edit> {
	let expr = semantic.expression(node_id).expect("Expression not found");
	if let Some(parent_id) = semantic.parent_expression_id(node_id) {
	match semantic.expression(parent_id) {
	Some(Expr::Subscript(parent)) => {
	if expr == parent.value.as_ref() {
	// If we're quoting the value of a subscript, we need to quote the entire
	// expression. For example, when quoting `DataFrame` in `DataFrame[int]`, we
	// should generate `"DataFrame[int]"`.
	return quote_annotation(parent_id, semantic, stylist);
	}
	}
	Some(Expr::Attribute(parent)) => {
	if expr == parent.value.as_ref() {
	// If we're quoting the value of an attribute, we need to quote the entire
	// expression. For example, when quoting `DataFrame` in `pd.DataFrame`, we
	// should generate `"pd.DataFrame"`.
	return quote_annotation(parent_id, semantic, stylist);
	}
	}
	Some(Expr::Call(parent)) => {
	if expr == parent.func.as_ref() {
	// If we're quoting the function of a call, we need to quote the entire
	// expression. For example, when quoting `DataFrame` in `DataFrame()`, we
	// should generate `"DataFrame()"`.
	return quote_annotation(parent_id, semantic, stylist);
	}
	}
	Some(Expr::BinOp(parent)) => {
	if parent.op.is_bit_or() {
	// If we're quoting the left or right side of a binary operation, we need to
	// quote the entire expression. For example, when quoting `DataFrame` in
	// `DataFrame | Series`, we should generate `"DataFrame | Series"`.
	return quote_annotation(parent_id, semantic, stylist);
	}
	}
	_ => {}
	}
	}
	let quote = stylist.quote();
	let mut quote_annotator = QuoteAnnotator::new(semantic, stylist);
	quote_annotator.visit_expr(expr);
	let annotation = quote_annotator.into_annotation()?;
	Ok(Edit::range_replacement(
	format!("{quote}{annotation}{quote}"),
	expr.range(),
	))
	}

Also appears that the nesting in the type annotation has some effect. For example, this fixes correctly:

from collections.abc import Sequence
from typing import Callable, Literal


def f(x: Sequence[Literal["1"]]) -> bool:
    return False

becomes:

from typing import Callable, Literal, TYPE_CHECKING

if TYPE_CHECKING:
    from collections.abc import Sequence


def f(x: "Sequence[Literal['1']]") -> bool:
    return False

Playground link

[dylwil3]

Looks like the issue was that lists were not handled (i.e. the first argument to Callable, which is a list) as the quoted annotation builder was recursing through the expressions. Gonna check a few more edge cases to see if some other things weren't handled.

But if I miss any I'm counting on you to find a slick example 😂 you're so good at this!