From 38a385fb6f8af98c7a2407c643cb241d823ea200 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Mon, 18 Nov 2024 12:33:19 +0530 Subject: [PATCH] Simplify quote annotator logic for list expression (#14425) ## Summary Follow-up to #14371, this PR simplifies the visitor logic for list expressions to remove the state management. We just need to make sure that we visit the nested expressions using the `QuoteAnnotator` and not the `Generator`. This is similar to what's being done for binary expressions. As per the [grammar](https://typing.readthedocs.io/en/latest/spec/annotations.html#grammar-token-expression-grammar-annotation_expression), list expressions can be present which can contain other type expressions (`Callable`): ``` | '[' '[' (type_expression ',')+ (name | '...') ']' ',' type_expression ']' (where name must be a valid in-scope ParamSpec) | '[' '[' maybe_unpacked (',' maybe_unpacked)* ']' ',' type_expression ']' ``` ## Test Plan `cargo insta test` --- .../src/rules/flake8_type_checking/helpers.rs | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) 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 6793becbd1..7da64c8bf7 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs @@ -392,23 +392,21 @@ impl<'a> SourceOrderVisitor<'a> for QuoteAnnotator<'a> { } self.state.pop(); } + // For the following expressions, we just need to make sure to visit the nested + // expressions using the quote annotator and not use the generator. This is so that any + // subscript elements nested within them are identified and quoted correctly. Expr::List(ast::ExprList { elts, .. }) => { - let Some((first, remaining)) = elts.split_first() else { - return; - }; + let mut first = true; self.annotation.push('['); - self.visit_expr(first); - if let Some(last) = self.state.last_mut() { - if *last == QuoteAnnotatorState::AnnotatedFirst { - *last = QuoteAnnotatorState::AnnotatedRest; + for expr in elts { + if first { + first = false; + } else { + self.annotation.push_str(", "); } - } - for expr in remaining { - self.annotation.push_str(", "); self.visit_expr(expr); } self.annotation.push(']'); - self.state.pop(); } Expr::BinOp(ast::ExprBinOp { left, op, right, ..