Implement iter(), len() and is_empty() for all display-literal AST nodes (#12807)

Author: AlexWaygood

crates/ruff_linter/src/checkers/ast/mod.rs

@@ -1290,8 +1290,8 @@ impl<'a> Visitor<'a> for Checker<'a> {
                             let Keyword { arg, value, .. } = keyword;
                             match (arg.as_ref(), value) {
                                 // Ex) NamedTuple("a", **{"a": int})
-                                (None, Expr::Dict(ast::ExprDict { items, .. })) => {
-                                    for ast::DictItem { key, value } in items {
+                                (None, Expr::Dict(dict)) => {
+                                    for ast::DictItem { key, value } in dict {
                                         if let Some(key) = key.as_ref() {
                                             self.visit_non_type_definition(key);
                                             self.visit_type_definition(value);

crates/ruff_linter/src/fix/edits.rs

@@ -151,24 +151,23 @@ pub(crate) fn add_to_dunder_all<'a>(
     stylist: &Stylist,
 ) -> Vec<Edit> {
     let (insertion_point, export_prefix_length) = match expr {
-        Expr::List(ExprList { elts, range, .. }) => (
-            elts.last()
-                .map_or(range.end() - "]".text_len(), Ranged::end),
+        Expr::List(ExprList { elts, .. }) => (
+            elts.last().map_or(expr.end() - "]".text_len(), Ranged::end),
             elts.len(),
         ),
         Expr::Tuple(tup) if tup.parenthesized => (
             tup.elts
                 .last()
                 .map_or(tup.end() - ")".text_len(), Ranged::end),
-            tup.elts.len(),
+            tup.len(),
         ),
         Expr::Tuple(tup) if !tup.parenthesized => (
             tup.elts
                 .last()
                 .expect("unparenthesized empty tuple is not possible")
                 .range()
                 .end(),
-            tup.elts.len(),
+            tup.len(),
         ),
         _ => {
             // we don't know how to insert into this expression

crates/ruff_linter/src/rules/fastapi/rules/fastapi_redundant_response_model.rs

@@ -122,17 +122,15 @@ fn is_identical_types(
     return_value: &Expr,
     semantic: &SemanticModel,
 ) -> bool {
-    if let (Some(response_mode_name_expr), Some(return_value_name_expr)) = (
-        response_model_arg.as_name_expr(),
-        return_value.as_name_expr(),
-    ) {
+    if let (Expr::Name(response_mode_name_expr), Expr::Name(return_value_name_expr)) =
+        (response_model_arg, return_value)
+    {
         return semantic.resolve_name(response_mode_name_expr)
             == semantic.resolve_name(return_value_name_expr);
     }
-    if let (Some(response_mode_subscript), Some(return_value_subscript)) = (
-        response_model_arg.as_subscript_expr(),
-        return_value.as_subscript_expr(),
-    ) {
+    if let (Expr::Subscript(response_mode_subscript), Expr::Subscript(return_value_subscript)) =
+        (response_model_arg, return_value)
+    {
         return is_identical_types(
             &response_mode_subscript.value,
             &return_value_subscript.value,
@@ -143,15 +141,13 @@ fn is_identical_types(
             semantic,
         );
     }
-    if let (Some(response_mode_tuple), Some(return_value_tuple)) = (
-        response_model_arg.as_tuple_expr(),
-        return_value.as_tuple_expr(),
-    ) {
-        return response_mode_tuple.elts.len() == return_value_tuple.elts.len()
+    if let (Expr::Tuple(response_mode_tuple), Expr::Tuple(return_value_tuple)) =
+        (response_model_arg, return_value)
+    {
+        return response_mode_tuple.len() == return_value_tuple.len()
             && response_mode_tuple
-                .elts
                 .iter()
-                .zip(return_value_tuple.elts.iter())
+                .zip(return_value_tuple)
                 .all(|(x, y)| is_identical_types(x, y, semantic));
     }
     false

crates/ruff_linter/src/rules/flake8_bandit/rules/django_extra.rs

@@ -1,6 +1,6 @@
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, violation};
-use ruff_python_ast::{self as ast, Expr, ExprAttribute, ExprDict, ExprList};
+use ruff_python_ast::{self as ast, Expr, ExprAttribute};
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
@@ -65,8 +65,8 @@ fn is_call_insecure(call: &ast::ExprCall) -> bool {
         if let Some(argument) = call.arguments.find_argument(argument_name, position) {
             match argument_name {
                 "select" => match argument {
-                    Expr::Dict(ExprDict { items, .. }) => {
-                        if items.iter().any(|ast::DictItem { key, value }| {
+                    Expr::Dict(dict) => {
+                        if dict.iter().any(|ast::DictItem { key, value }| {
                             key.as_ref()
                                 .is_some_and(|key| !key.is_string_literal_expr())
                                 || !value.is_string_literal_expr()
@@ -77,8 +77,8 @@ fn is_call_insecure(call: &ast::ExprCall) -> bool {
                     _ => return true,
                 },
                 "where" | "tables" => match argument {
-                    Expr::List(ExprList { elts, .. }) => {
-                        if !elts.iter().all(Expr::is_string_literal_expr) {
+                    Expr::List(list) => {
+                        if !list.iter().all(Expr::is_string_literal_expr) {
                             return true;
                         }
                     }

crates/ruff_linter/src/rules/flake8_bandit/rules/shell_injection.rs

@@ -530,11 +530,11 @@ fn is_partial_path(expr: &Expr) -> bool {
 /// subprocess.Popen(["/usr/local/bin/rsync", "*", "some_where:"], shell=True)
 /// ```
 fn is_wildcard_command(expr: &Expr) -> bool {
-    if let Expr::List(ast::ExprList { elts, .. }) = expr {
+    if let Expr::List(list) = expr {
         let mut has_star = false;
         let mut has_command = false;
-        for elt in elts {
-            if let Some(text) = string_literal(elt) {
+        for item in list {
+            if let Some(text) = string_literal(item) {
                 has_star |= text.contains('*');
                 has_command |= text.contains("chown")
                     || text.contains("chmod")

crates/ruff_linter/src/rules/flake8_bugbear/rules/duplicate_value.rs

@@ -49,16 +49,16 @@ impl Violation for DuplicateValue {
 /// B033
 pub(crate) fn duplicate_value(checker: &mut Checker, set: &ast::ExprSet) {
     let mut seen_values: FxHashSet<ComparableExpr> = FxHashSet::default();
-    for (index, elt) in set.elts.iter().enumerate() {
-        if elt.is_literal_expr() {
-            let comparable_value: ComparableExpr = elt.into();
+    for (index, value) in set.iter().enumerate() {
+        if value.is_literal_expr() {
+            let comparable_value = ComparableExpr::from(value);
 
             if !seen_values.insert(comparable_value) {
                 let mut diagnostic = Diagnostic::new(
                     DuplicateValue {
-                        value: checker.generator().expr(elt),
+                        value: checker.generator().expr(value),
                     },
-                    elt.range(),
+                    value.range(),
                 );
 
                 diagnostic.try_set_fix(|| {
@@ -73,7 +73,7 @@ pub(crate) fn duplicate_value(checker: &mut Checker, set: &ast::ExprSet) {
 
 /// Remove the member at the given index from the [`ast::ExprSet`].
 fn remove_member(set: &ast::ExprSet, index: usize, source: &str) -> Result<Edit> {
-    if index < set.elts.len() - 1 {
+    if index < set.len() - 1 {
         // Case 1: the expression is _not_ the last node, so delete from the start of the
         // expression to the end of the subsequent comma.
         // Ex) Delete `"a"` in `{"a", "b", "c"}`.

crates/ruff_linter/src/rules/flake8_bugbear/rules/reuse_of_groupby_generator.rs

@@ -315,15 +315,15 @@ pub(crate) fn reuse_of_groupby_generator(
     let Expr::Call(ast::ExprCall { func, .. }) = &iter else {
         return;
     };
-    let Expr::Tuple(ast::ExprTuple { elts, .. }) = target else {
+    let Expr::Tuple(tuple) = target else {
         // Ignore any `groupby()` invocation that isn't unpacked
         return;
     };
-    if elts.len() != 2 {
+    if tuple.len() != 2 {
         return;
     }
     // We have an invocation of groupby which is a simple unpacking
-    let Expr::Name(ast::ExprName { id: group_name, .. }) = &elts[1] else {
+    let Expr::Name(ast::ExprName { id: group_name, .. }) = &tuple.elts[1] else {
         return;
     };
     // Check if the function call is `itertools.groupby`

crates/ruff_linter/src/rules/flake8_bugbear/rules/static_key_dict_comprehension.rs

@@ -72,7 +72,7 @@ pub(crate) fn static_key_dict_comprehension(checker: &mut Checker, dict_comp: &a
 /// comprehension.
 fn is_constant(key: &Expr, names: &FxHashMap<&str, &ast::ExprName>) -> bool {
     match key {
-        Expr::Tuple(ast::ExprTuple { elts, .. }) => elts.iter().all(|elt| is_constant(elt, names)),
+        Expr::Tuple(tuple) => tuple.iter().all(|elem| is_constant(elem, names)),
         Expr::Name(ast::ExprName { id, .. }) => !names.contains_key(id.as_str()),
         Expr::Attribute(ast::ExprAttribute { value, .. }) => is_constant(value, names),
         Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => {

crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_generator_dict.rs

@@ -61,13 +61,13 @@ pub(crate) fn unnecessary_generator_dict(
     let Expr::Generator(ast::ExprGenerator { elt, .. }) = argument else {
         return;
     };
-    let Expr::Tuple(ast::ExprTuple { elts, .. }) = elt.as_ref() else {
+    let Expr::Tuple(tuple) = &**elt else {
         return;
     };
-    if elts.len() != 2 {
+    if tuple.len() != 2 {
         return;
     }
-    if elts.iter().any(Expr::is_starred_expr) {
+    if tuple.iter().any(Expr::is_starred_expr) {
         return;
     }
     let mut diagnostic = Diagnostic::new(UnnecessaryGeneratorDict, expr.range());

crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_list_comprehension_dict.rs

@@ -62,10 +62,10 @@ pub(crate) fn unnecessary_list_comprehension_dict(
     let Expr::ListComp(ast::ExprListComp { elt, .. }) = argument else {
         return;
     };
-    let Expr::Tuple(ast::ExprTuple { elts, .. }) = elt.as_ref() else {
+    let Expr::Tuple(tuple) = &**elt else {
         return;
     };
-    if elts.len() != 2 {
+    if tuple.len() != 2 {
         return;
     }
     let mut diagnostic = Diagnostic::new(UnnecessaryListComprehensionDict, expr.range());

crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_literal_dict.rs

@@ -74,7 +74,7 @@ pub(crate) fn unnecessary_literal_dict(
     // Accept `dict((1, 2), ...))` `dict([(1, 2), ...])`.
     if !elts
         .iter()
-        .all(|elt| matches!(&elt, Expr::Tuple(ast::ExprTuple { elts, .. }) if elts.len() == 2))
+        .all(|elt| matches!(&elt, Expr::Tuple(tuple) if tuple.len() == 2))
     {
         return;
     }

crates/ruff_linter/src/rules/flake8_pie/rules/multiple_starts_ends_with.rs

@@ -163,8 +163,8 @@ pub(crate) fn multiple_starts_ends_with(checker: &mut Checker, expr: &Expr) {
                 elts: words
                     .iter()
                     .flat_map(|value| {
-                        if let Expr::Tuple(ast::ExprTuple { elts, .. }) = value {
-                            Left(elts.iter())
+                        if let Expr::Tuple(tuple) = value {
+                            Left(tuple.iter())
                         } else {
                             Right(iter::once(*value))
                         }

crates/ruff_linter/src/rules/flake8_pie/rules/reimplemented_container_builtin.rs

@@ -1,4 +1,4 @@
-use ruff_python_ast::{self as ast, Expr, ExprLambda};
+use ruff_python_ast::{Expr, ExprLambda};
 
 use ruff_diagnostics::{Diagnostic, Edit, Fix};
 use ruff_diagnostics::{FixAvailability, Violation};
@@ -70,8 +70,8 @@ pub(crate) fn reimplemented_container_builtin(checker: &mut Checker, expr: &Expr
     }
 
     let container = match &**body {
-        Expr::List(ast::ExprList { elts, .. }) if elts.is_empty() => Container::List,
-        Expr::Dict(ast::ExprDict { items, .. }) if items.is_empty() => Container::Dict,
+        Expr::List(list) if list.is_empty() => Container::List,
+        Expr::Dict(dict) if dict.is_empty() => Container::Dict,
         _ => return,
     };
     let mut diagnostic = Diagnostic::new(ReimplementedContainerBuiltin { container }, expr.range());

crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs

@@ -87,13 +87,13 @@ pub(crate) fn unnecessary_dict_kwargs(checker: &mut Checker, call: &ast::ExprCal
             .iter_keys()
             .filter_map(|key| key.and_then(as_kwarg))
             .collect();
-        if kwargs.len() != dict.items.len() {
+        if kwargs.len() != dict.len() {
             continue;
         }
 
         let mut diagnostic = Diagnostic::new(UnnecessaryDictKwargs, keyword.range());
 
-        if dict.items.is_empty() {
+        if dict.is_empty() {
             diagnostic.try_set_fix(|| {
                 remove_argument(
                     keyword,

crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_spread.rs

@@ -49,7 +49,7 @@ impl Violation for UnnecessarySpread {
 pub(crate) fn unnecessary_spread(checker: &mut Checker, dict: &ast::ExprDict) {
     // The first "end" is the start of the dictionary, immediately following the open bracket.
     let mut prev_end = dict.start() + TextSize::from(1);
-    for ast::DictItem { key, value } in &dict.items {
+    for ast::DictItem { key, value } in dict {
         if key.is_none() {
             // We only care about when the key is None which indicates a spread `**`
             // inside a dict.

crates/ruff_linter/src/rules/flake8_pyi/rules/bad_generator_return_type.rs

@@ -162,12 +162,11 @@ pub(crate) fn bad_generator_return_type(
     // - if not, don't emit the diagnostic
     let yield_type_info = match returns {
         ast::Expr::Subscript(ast::ExprSubscript { slice, .. }) => match slice.as_ref() {
-            ast::Expr::Tuple(slice_tuple @ ast::ExprTuple { .. }) => {
+            ast::Expr::Tuple(slice_tuple) => {
                 if !slice_tuple
-                    .elts
                     .iter()
                     .skip(1)
-                    .all(|elt| is_any_or_none(elt, semantic))
+                    .all(|element| is_any_or_none(element, semantic))
                 {
                     return;
                 }

crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_literal_union.rs

@@ -67,8 +67,8 @@ pub(crate) fn redundant_literal_union<'a>(checker: &mut Checker, union: &'a Expr
     let mut func = |expr: &'a Expr, _parent: &'a Expr| {
         if let Expr::Subscript(ast::ExprSubscript { value, slice, .. }) = expr {
             if checker.semantic().match_typing_expr(value, "Literal") {
-                if let Expr::Tuple(ast::ExprTuple { elts, .. }) = slice.as_ref() {
-                    typing_literal_exprs.extend(elts.iter());
+                if let Expr::Tuple(tuple) = &**slice {
+                    typing_literal_exprs.extend(tuple);
                 } else {
                     typing_literal_exprs.push(slice);
                 }

crates/ruff_linter/src/rules/flake8_pyi/rules/simple_defaults.rs

@@ -298,10 +298,10 @@ fn is_valid_default_value_with_annotation(
                     .iter()
                     .all(|e| is_valid_default_value_with_annotation(e, false, locator, semantic));
         }
-        Expr::Dict(ast::ExprDict { items, range: _ }) => {
+        Expr::Dict(dict) => {
             return allow_container
-                && items.len() <= 10
-                && items.iter().all(|ast::DictItem { key, value }| {
+                && dict.len() <= 10
+                && dict.iter().all(|ast::DictItem { key, value }| {
                     key.as_ref().is_some_and(|key| {
                         is_valid_default_value_with_annotation(key, false, locator, semantic)
                     }) && is_valid_default_value_with_annotation(value, false, locator, semantic)

crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs

@@ -70,19 +70,15 @@ pub(crate) fn unnecessary_literal_union<'a>(checker: &mut Checker, expr: &'a Exp
                     literal_subscript = Some(value.as_ref());
                 }
 
+                let slice = &**slice;
+
                 // flatten already-unioned literals to later union again
-                if let Expr::Tuple(ast::ExprTuple {
-                    elts,
-                    range: _,
-                    ctx: _,
-                    parenthesized: _,
-                }) = slice.as_ref()
-                {
-                    for expr in elts {
-                        literal_exprs.push(expr);
+                if let Expr::Tuple(tuple) = slice {
+                    for item in tuple {
+                        literal_exprs.push(item);
                     }
                 } else {
-                    literal_exprs.push(slice.as_ref());
+                    literal_exprs.push(slice);
                 }
             }
         } else {

crates/ruff_linter/src/rules/flake8_pyi/rules/unrecognized_version_info.rs

@@ -181,7 +181,7 @@ fn version_check(
     }
 
     // Tuple comparison, e.g., `sys.version_info == (3, 4)`.
-    let Expr::Tuple(ast::ExprTuple { elts, .. }) = comparator else {
+    let Expr::Tuple(tuple) = comparator else {
         if checker.enabled(Rule::UnrecognizedVersionInfoCheck) {
             checker
                 .diagnostics
@@ -190,15 +190,15 @@ fn version_check(
         return;
     };
 
-    if !elts.iter().all(is_int_constant) {
+    if !tuple.iter().all(is_int_constant) {
         // All tuple elements must be integers, e.g., `sys.version_info == (3, 4)` instead of
         // `sys.version_info == (3.0, 4)`.
         if checker.enabled(Rule::UnrecognizedVersionInfoCheck) {
             checker
                 .diagnostics
                 .push(Diagnostic::new(UnrecognizedVersionInfoCheck, test.range()));
         }
-    } else if elts.len() > 2 {
+    } else if tuple.len() > 2 {
         // Must compare against major and minor version only, e.g., `sys.version_info == (3, 4)`
         // instead of `sys.version_info == (3, 4, 0)`.
         if checker.enabled(Rule::PatchVersionComparison) {
@@ -216,7 +216,7 @@ fn version_check(
                 _ => return,
             };
 
-            if elts.len() != expected_length {
+            if tuple.len() != expected_length {
                 checker.diagnostics.push(Diagnostic::new(
                     WrongTupleLengthVersionComparison { expected_length },
                     test.range(),