Add parenthesized flag to ExprTuple and ExprGenerator (#9614)

astral-sh · Feb 26, 2024 · 77c5561 · 77c5561
1 parent ab4bd71
commit 77c5561
Show file tree

Hide file tree

Showing 65 changed files with 391 additions and 139 deletions.
diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs
@@ -134,6 +134,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             elts,
             ctx,
             range: _,
+            parenthesized: _,
         })
         | Expr::List(ast::ExprList {
             elts,
@@ -1451,6 +1452,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                 generators,
                 elt: _,
                 range: _,
+                parenthesized: _,
             },
         ) => {
             if checker.enabled(Rule::UnnecessaryListIndexLookup) {

diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs
@@ -1007,6 +1007,7 @@ where
                 elt,
                 generators,
                 range: _,
+                parenthesized: _,
             }) => {
                 self.visit_generators(generators);
                 self.visit_expr(elt);
@@ -1327,6 +1328,7 @@ where
                                 elts,
                                 ctx,
                                 range: _,
+                                parenthesized: _,
                             }) = slice.as_ref()
                             {
                                 let mut iter = elts.iter();

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/duplicate_exceptions.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/duplicate_exceptions.rs
@@ -109,6 +109,7 @@ fn type_pattern(elts: Vec<&Expr>) -> Expr {
         elts: elts.into_iter().cloned().collect(),
         ctx: ExprContext::Load,
         range: TextRange::default(),
+        parenthesized: true,
     }
     .into()
 }

diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/multiple_starts_ends_with.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/multiple_starts_ends_with.rs
@@ -173,6 +173,7 @@ pub(crate) fn multiple_starts_ends_with(checker: &mut Checker, expr: &Expr) {
                     .collect(),
                 ctx: ExprContext::Load,
                 range: TextRange::default(),
+                parenthesized: true,
             });
             let node1 = Expr::Name(ast::ExprName {
                 id: arg_name.into(),

diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs
@@ -72,6 +72,7 @@ pub(crate) fn unnecessary_literal_union<'a>(checker: &mut Checker, expr: &'a Exp
                     elts,
                     range: _,
                     ctx: _,
+                    parenthesized: _,
                 }) = slice.as_ref()
                 {
                     for expr in elts {
@@ -123,6 +124,7 @@ pub(crate) fn unnecessary_literal_union<'a>(checker: &mut Checker, expr: &'a Exp
                 elts: literal_exprs.into_iter().cloned().collect(),
                 range: TextRange::default(),
                 ctx: ExprContext::Load,
+                parenthesized: true,
             })),
             range: TextRange::default(),
             ctx: ExprContext::Load,
@@ -148,6 +150,7 @@ pub(crate) fn unnecessary_literal_union<'a>(checker: &mut Checker, expr: &'a Exp
                             elts,
                             range: TextRange::default(),
                             ctx: ExprContext::Load,
+                            parenthesized: true,
                         })),
                         range: TextRange::default(),
                         ctx: ExprContext::Load,

diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs
@@ -130,6 +130,7 @@ pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr)
                                 .collect(),
                             ctx: ExprContext::Load,
                             range: TextRange::default(),
+                            parenthesized: true,
                         })),
                         ctx: ExprContext::Load,
                         range: TextRange::default(),
@@ -151,6 +152,7 @@ pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr)
                             elts: exprs.into_iter().cloned().collect(),
                             ctx: ExprContext::Load,
                             range: TextRange::default(),
+                            parenthesized: true,
                         })),
                         ctx: ExprContext::Load,
                         range: TextRange::default(),

diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/parametrize.rs
@@ -337,6 +337,7 @@ fn check_names(checker: &mut Checker, decorator: &Decorator, expr: &Expr) {
                                 .collect(),
                             ctx: ExprContext::Load,
                             range: TextRange::default(),
+                            parenthesized: true,
                         });
                         diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
                             format!("({})", checker.generator().expr(&node)),
@@ -444,6 +445,7 @@ fn check_names(checker: &mut Checker, decorator: &Decorator, expr: &Expr) {
                             elts: elts.clone(),
                             ctx: ExprContext::Load,
                             range: TextRange::default(),
+                            parenthesized: true,
                         });
                         diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
                             format!("({})", checker.generator().expr(&node)),

diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/ast_bool_op.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/ast_bool_op.rs
@@ -428,6 +428,7 @@ pub(crate) fn duplicate_isinstance_call(checker: &mut Checker, expr: &Expr) {
                         .collect(),
                     ctx: ExprContext::Load,
                     range: TextRange::default(),
+                    parenthesized: true,
                 };
                 let node1 = ast::ExprName {
                     id: "isinstance".into(),
@@ -543,6 +544,7 @@ pub(crate) fn compare_with_tuple(checker: &mut Checker, expr: &Expr) {
             elts: comparators.into_iter().map(Clone::clone).collect(),
             ctx: ExprContext::Load,
             range: TextRange::default(),
+            parenthesized: true,
         };
         let node1 = ast::ExprName {
             id: id.into(),
@@ -718,7 +720,7 @@ fn get_short_circuit_edit(
         generator.expr(expr)
     };
     Edit::range_replacement(
-        if matches!(expr, Expr::Tuple(ast::ExprTuple { elts, ctx: _, range: _}) if !elts.is_empty())
+        if matches!(expr, Expr::Tuple(ast::ExprTuple { elts, ctx: _, range: _, parenthesized: _}) if !elts.is_empty())
         {
             format!("({content})")
         } else {

diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/reimplemented_builtin.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/reimplemented_builtin.rs
@@ -382,6 +382,7 @@ fn return_stmt(id: &str, test: &Expr, target: &Expr, iter: &Expr, generator: Gen
             range: TextRange::default(),
         }],
         range: TextRange::default(),
+        parenthesized: false,
     };
     let node1 = ast::ExprName {
         id: id.into(),

diff --git a/crates/ruff_linter/src/rules/pylint/rules/redefined_loop_name.rs b/crates/ruff_linter/src/rules/pylint/rules/redefined_loop_name.rs
@@ -305,6 +305,7 @@ fn assignment_targets_from_expr<'a>(
             ctx: ExprContext::Store,
             elts,
             range: _,
+            parenthesized: _,
         }) => Box::new(
             elts.iter()
                 .flat_map(|elt| assignment_targets_from_expr(elt, dummy_variable_rgx)),

diff --git a/crates/ruff_linter/src/rules/pylint/rules/repeated_equality_comparison.rs b/crates/ruff_linter/src/rules/pylint/rules/repeated_equality_comparison.rs
@@ -145,6 +145,7 @@ pub(crate) fn repeated_equality_comparison(checker: &mut Checker, bool_op: &ast:
                         elts: comparators.iter().copied().cloned().collect(),
                         range: TextRange::default(),
                         ctx: ExprContext::Load,
+                        parenthesized: true,
                     })]),
                     range: bool_op.range(),
                 })),

diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/os_error_alias.rs
@@ -127,6 +127,7 @@ fn tuple_diagnostic(checker: &mut Checker, tuple: &ast::ExprTuple, aliases: &[&E
                 elts: remaining,
                 ctx: ExprContext::Load,
                 range: TextRange::default(),
+                parenthesized: true,
             };
             format!("({})", checker.generator().expr(&node.into()))
         };

diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/timeout_error_alias.rs
@@ -141,6 +141,7 @@ fn tuple_diagnostic(checker: &mut Checker, tuple: &ast::ExprTuple, aliases: &[&E
                 elts: remaining,
                 ctx: ExprContext::Load,
                 range: TextRange::default(),
+                parenthesized: true,
             };
             format!("({})", checker.generator().expr(&node.into()))
         };

diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep695_type_alias.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/use_pep695_type_alias.rs
@@ -127,6 +127,7 @@ pub(crate) fn non_pep695_type_alias(checker: &mut Checker, stmt: &StmtAnnAssign)
                                     range: TextRange::default(),
                                     elts: constraints.into_iter().cloned().collect(),
                                     ctx: ast::ExprContext::Load,
+                                    parenthesized: true,
                                 })))
                             }
                             None => None,

diff --git a/crates/ruff_linter/src/rules/refurb/rules/repeated_append.rs b/crates/ruff_linter/src/rules/refurb/rules/repeated_append.rs
@@ -347,6 +347,7 @@ fn make_suggestion(group: &AppendGroup, generator: Generator) -> String {
         elts,
         ctx: ast::ExprContext::Load,
         range: TextRange::default(),
+        parenthesized: true,
     };
     // Make `var.extend`.
     // NOTE: receiver is the same for all appends and that's why we can take the first.

diff --git a/crates/ruff_linter/src/rules/ruff/rules/collection_literal_concatenation.rs b/crates/ruff_linter/src/rules/ruff/rules/collection_literal_concatenation.rs
@@ -162,6 +162,7 @@ fn concatenate_expressions(expr: &Expr) -> Option<(Expr, Type)> {
             elts: new_elts,
             ctx: ExprContext::Load,
             range: TextRange::default(),
+            parenthesized: true,
         }
         .into(),
     };

diff --git a/crates/ruff_linter/src/rules/ruff/rules/never_union.rs b/crates/ruff_linter/src/rules/ruff/rules/never_union.rs
@@ -116,6 +116,7 @@ pub(crate) fn never_union(checker: &mut Checker, expr: &Expr) {
                 elts,
                 ctx: _,
                 range: _,
+                parenthesized: _,
             }) = slice.as_ref()
             else {
                 return;
@@ -157,6 +158,7 @@ pub(crate) fn never_union(checker: &mut Checker, expr: &Expr) {
                                         elts: rest,
                                         ctx: ast::ExprContext::Load,
                                         range: TextRange::default(),
+                                        parenthesized: true,
                                     })),
                                     ctx: ast::ExprContext::Load,
                                     range: TextRange::default(),

diff --git a/crates/ruff_linter/src/rules/ruff/rules/sequence_sorting.rs b/crates/ruff_linter/src/rules/ruff/rules/sequence_sorting.rs
@@ -134,23 +134,23 @@ impl InferredMemberType {
 /// single-line tuple literals *can* be unparenthesized.
 /// We keep the original AST node around for the
 /// Tuple variant so that this can be queried later.
-#[derive(Debug)]
-pub(super) enum SequenceKind<'a> {
+#[derive(Copy, Clone, Debug)]
+pub(super) enum SequenceKind {
     List,
     Set,
-    Tuple(&'a ast::ExprTuple),
+    Tuple { parenthesized: bool },
 }
 
-impl SequenceKind<'_> {
+impl SequenceKind {
     // N.B. We only need the source code for the Tuple variant here,
     // but if you already have a `Locator` instance handy,
     // getting the source code is very cheap.
-    fn surrounding_brackets(&self, source: &str) -> (&'static str, &'static str) {
+    fn surrounding_brackets(self) -> (&'static str, &'static str) {
         match self {
             Self::List => ("[", "]"),
             Self::Set => ("{", "}"),
-            Self::Tuple(ast_node) => {
-                if ast_node.is_parenthesized(source) {
+            Self::Tuple { parenthesized } => {
+                if parenthesized {
                     ("(", ")")
                 } else {
                     ("", "")
@@ -159,19 +159,19 @@ impl SequenceKind<'_> {
         }
     }
 
-    const fn opening_token_for_multiline_definition(&self) -> TokenKind {
+    const fn opening_token_for_multiline_definition(self) -> TokenKind {
         match self {
             Self::List => TokenKind::Lsqb,
             Self::Set => TokenKind::Lbrace,
-            Self::Tuple(_) => TokenKind::Lpar,
+            Self::Tuple { .. } => TokenKind::Lpar,
         }
     }
 
-    const fn closing_token_for_multiline_definition(&self) -> TokenKind {
+    const fn closing_token_for_multiline_definition(self) -> TokenKind {
         match self {
             Self::List => TokenKind::Rsqb,
             Self::Set => TokenKind::Rbrace,
-            Self::Tuple(_) => TokenKind::Rpar,
+            Self::Tuple { .. } => TokenKind::Rpar,
         }
     }
 }
@@ -217,15 +217,15 @@ impl<'a> SequenceElements<'a> {
 /// that can be inserted into the
 /// source code as a `range_replacement` autofix.
 pub(super) fn sort_single_line_elements_sequence(
-    kind: &SequenceKind,
+    kind: SequenceKind,
     elts: &[ast::Expr],
     elements: &[&str],
     locator: &Locator,
     sorting_style: SortingStyle,
 ) -> String {
     let element_pairs = SequenceElements::new(elements, elts);
     let last_item_index = element_pairs.last_item_index();
-    let (opening_paren, closing_paren) = kind.surrounding_brackets(locator.contents());
+    let (opening_paren, closing_paren) = kind.surrounding_brackets();
     let mut result = String::from(opening_paren);
     // We grab the original source-code ranges using `locator.slice()`
     // rather than using the expression generator, as this approach allows
@@ -334,7 +334,7 @@ impl MultilineStringSequenceValue {
     /// Return `None` if the analysis fails for whatever reason.
     pub(super) fn from_source_range(
         range: TextRange,
-        kind: &SequenceKind,
+        kind: SequenceKind,
         locator: &Locator,
     ) -> Option<MultilineStringSequenceValue> {
         // Parse the multiline string sequence using the raw tokens.
@@ -486,7 +486,7 @@ impl Ranged for MultilineStringSequenceValue {
 /// in the original source code.
 fn collect_string_sequence_lines(
     range: TextRange,
-    kind: &SequenceKind,
+    kind: SequenceKind,
     locator: &Locator,
 ) -> Option<(Vec<StringSequenceLine>, bool)> {
     // These first two variables are used for keeping track of state

diff --git a/crates/ruff_linter/src/rules/ruff/rules/sort_dunder_all.rs b/crates/ruff_linter/src/rules/ruff/rules/sort_dunder_all.rs
@@ -152,9 +152,13 @@ fn sort_dunder_all(checker: &mut Checker, target: &ast::Expr, node: &ast::Expr)
 
     let (elts, range, kind) = match node {
         ast::Expr::List(ast::ExprList { elts, range, .. }) => (elts, *range, SequenceKind::List),
-        ast::Expr::Tuple(tuple_node @ ast::ExprTuple { elts, range, .. }) => {
-            (elts, *range, SequenceKind::Tuple(tuple_node))
-        }
+        ast::Expr::Tuple(tuple_node @ ast::ExprTuple { elts, range, .. }) => (
+            elts,
+            *range,
+            SequenceKind::Tuple {
+                parenthesized: tuple_node.parenthesized,
+            },
+        ),
         _ => return,
     };
 
@@ -166,7 +170,7 @@ fn sort_dunder_all(checker: &mut Checker, target: &ast::Expr, node: &ast::Expr)
     let mut diagnostic = Diagnostic::new(UnsortedDunderAll, range);
 
     if let SortClassification::UnsortedAndMaybeFixable { items } = elts_analysis {
-        if let Some(fix) = create_fix(range, elts, &items, &kind, checker) {
+        if let Some(fix) = create_fix(range, elts, &items, kind, checker) {
             diagnostic.set_fix(fix);
         }
     }
@@ -187,7 +191,7 @@ fn create_fix(
     range: TextRange,
     elts: &[ast::Expr],
     string_items: &[&str],
-    kind: &SequenceKind,
+    kind: SequenceKind,
     checker: &Checker,
 ) -> Option<Fix> {
     let locator = checker.locator();

diff --git a/crates/ruff_linter/src/rules/ruff/rules/sort_dunder_slots.rs b/crates/ruff_linter/src/rules/ruff/rules/sort_dunder_slots.rs
@@ -157,7 +157,9 @@ impl<'a> StringLiteralDisplay<'a> {
                 }
             }
             ast::Expr::Tuple(tuple_node @ ast::ExprTuple { elts, range, .. }) => {
-                let display_kind = DisplayKind::Sequence(SequenceKind::Tuple(tuple_node));
+                let display_kind = DisplayKind::Sequence(SequenceKind::Tuple {
+                    parenthesized: tuple_node.parenthesized,
+                });
                 Self {
                     elts: Cow::Borrowed(elts),
                     range: *range,
@@ -211,7 +213,7 @@ impl<'a> StringLiteralDisplay<'a> {
             (DisplayKind::Sequence(sequence_kind), true) => {
                 let analyzed_sequence = MultilineStringSequenceValue::from_source_range(
                     self.range(),
-                    sequence_kind,
+                    *sequence_kind,
                     locator,
                 )?;
                 assert_eq!(analyzed_sequence.len(), self.elts.len());
@@ -220,7 +222,7 @@ impl<'a> StringLiteralDisplay<'a> {
             // Sorting multiline dicts is unsupported
             (DisplayKind::Dict { .. }, true) => return None,
             (DisplayKind::Sequence(sequence_kind), false) => sort_single_line_elements_sequence(
-                sequence_kind,
+                *sequence_kind,
                 &self.elts,
                 items,
                 locator,
@@ -242,7 +244,7 @@ impl<'a> StringLiteralDisplay<'a> {
 /// Python provides for builtin containers.
 #[derive(Debug)]
 enum DisplayKind<'a> {
-    Sequence(SequenceKind<'a>),
+    Sequence(SequenceKind),
     Dict { values: &'a [ast::Expr] },
 }
 

diff --git a/crates/ruff_python_ast/src/comparable.rs b/crates/ruff_python_ast/src/comparable.rs
@@ -977,6 +977,7 @@ impl<'a> From<&'a ast::Expr> for ComparableExpr<'a> {
                 elt,
                 generators,
                 range: _,
+                parenthesized: _,
             }) => Self::GeneratorExp(ExprGeneratorExp {
                 elt: elt.into(),
                 generators: generators.iter().map(Into::into).collect(),
@@ -1072,6 +1073,7 @@ impl<'a> From<&'a ast::Expr> for ComparableExpr<'a> {
                 elts,
                 ctx: _,
                 range: _,
+                parenthesized: _,
             }) => Self::Tuple(ExprTuple {
                 elts: elts.iter().map(Into::into).collect(),
             }),