Skip to content

Commit

Permalink
Simplify quote annotator logic for list expression (#14425)
Browse files Browse the repository at this point in the history
## 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`):
```
       | <Callable> '[' <Concatenate> '[' (type_expression ',')+
                    (name | '...') ']' ',' type_expression ']'
             (where name must be a valid in-scope ParamSpec)
       | <Callable> '[' '[' maybe_unpacked (',' maybe_unpacked)*
                    ']' ',' type_expression ']'
```

## Test Plan

`cargo insta test`
  • Loading branch information
dhruvmanila authored Nov 18, 2024
1 parent cd2ae5a commit 38a385f
Showing 1 changed file with 9 additions and 11 deletions.
20 changes: 9 additions & 11 deletions crates/ruff_linter/src/rules/flake8_type_checking/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, ..
Expand Down

0 comments on commit 38a385f

Please sign in to comment.