refactor(semantic): add ancestor_kinds iterator function (#7217)

for convenience, I've added a new function called `ancestor_kinds` which loops overall `ancestors` and gets their `AstKind`. this is a common pattern in a couple of places. I also did some somewhat related refactoring to remove places where we were manually calling `AstNode::kind` instead of using `ancestor_kinds` or calling `parent_kind`.
oxc-project · Nov 9, 2024 · c5485ae · c5485ae
1 parent abf1602
commit c5485ae
Show file tree

Hide file tree

Showing 14 changed files with 37 additions and 34 deletions.
diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs
@@ -259,10 +259,10 @@ pub fn nth_outermost_paren_parent<'a, 'b>(
 pub fn iter_outer_expressions<'a, 's>(
     semantic: &'s Semantic<'a>,
     node_id: NodeId,
-) -> impl Iterator<Item = &'s AstNode<'a>> + 's {
-    semantic.nodes().ancestors(node_id).skip(1).filter(|parent| {
+) -> impl Iterator<Item = AstKind<'a>> + 's {
+    semantic.nodes().ancestor_kinds(node_id).skip(1).filter(|parent| {
         !matches!(
-            parent.kind(),
+            parent,
             AstKind::ParenthesizedExpression(_)
                 | AstKind::TSAsExpression(_)
                 | AstKind::TSSatisfiesExpression(_)

diff --git a/crates/oxc_linter/src/rules/eslint/func_names.rs b/crates/oxc_linter/src/rules/eslint/func_names.rs
@@ -479,7 +479,7 @@ impl Rule for FuncNames {
 }
 
 fn guess_function_name<'a>(ctx: &LintContext<'a>, parent_id: NodeId) -> Option<Cow<'a, str>> {
-    for parent_kind in ctx.nodes().ancestors(parent_id).map(AstNode::kind) {
+    for parent_kind in ctx.nodes().ancestor_kinds(parent_id) {
         match parent_kind {
             AstKind::ParenthesizedExpression(_)
             | AstKind::TSAsExpression(_)

diff --git a/crates/oxc_linter/src/rules/eslint/no_empty_function.rs b/crates/oxc_linter/src/rules/eslint/no_empty_function.rs
@@ -76,7 +76,7 @@ fn get_function_name_and_kind<'a>(
     node: &AstNode<'a>,
     ctx: &LintContext<'a>,
 ) -> (&'static str, Option<Cow<'a, str>>) {
-    for parent in ctx.nodes().ancestors(node.id()).skip(1).map(AstNode::kind) {
+    for parent in ctx.nodes().ancestor_kinds(node.id()).skip(1) {
         match parent {
             AstKind::Function(f) => {
                 if let Some(name) = f.name() {

diff --git a/crates/oxc_linter/src/rules/eslint/no_new_func.rs b/crates/oxc_linter/src/rules/eslint/no_new_func.rs
@@ -62,16 +62,15 @@ impl Rule for NoNewFunc {
                 Some((obj_id, call_expr.span))
             }
             AstKind::MemberExpression(mem_expr) => {
-                let parent: Option<&AstNode<'a>> =
-                    ctx.nodes().ancestors(node.id()).skip(1).find(|node| {
+                let parent: Option<AstKind<'a>> =
+                    ctx.nodes().ancestor_kinds(node.id()).skip(1).find(|node| {
                         !matches!(
-                            node.kind(),
+                            node,
                             AstKind::ChainExpression(_) | AstKind::ParenthesizedExpression(_)
                         )
                     });
 
-                let Some(AstKind::CallExpression(parent_call_expr)) = parent.map(AstNode::kind)
-                else {
+                let Some(AstKind::CallExpression(parent_call_expr)) = parent else {
                     return;
                 };
 

diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/allowed.rs
@@ -1,7 +1,7 @@
 //! This module checks if an unused variable is allowed. Note that this does not
 //! consider variables ignored by name pattern, but by where they are declared.
 use oxc_ast::{ast::*, AstKind};
-use oxc_semantic::{AstNode, NodeId, Semantic};
+use oxc_semantic::{NodeId, Semantic};
 use oxc_span::GetSpan;
 
 use super::{options::ArgsOption, NoUnusedVars, Symbol};
@@ -255,7 +255,7 @@ impl NoUnusedVars {
         param: &FormalParameter<'a>,
         params_id: NodeId,
     ) -> bool {
-        let mut parents_iter = semantic.nodes().ancestors(params_id).skip(1).map(AstNode::kind);
+        let mut parents_iter = semantic.nodes().ancestor_kinds(params_id).skip(1);
 
         // in function declarations, the parent immediately before the
         // FormalParameters is a TSDeclareBlock

diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_vars.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_vars.rs
@@ -2,7 +2,7 @@ use oxc_ast::{
     ast::{Expression, VariableDeclarator},
     AstKind,
 };
-use oxc_semantic::{AstNode, NodeId};
+use oxc_semantic::NodeId;
 use oxc_span::CompactStr;
 
 use super::{count_whitespace_or_commas, BindingInfo, NoUnusedVars, Symbol};
@@ -36,7 +36,7 @@ impl NoUnusedVars {
             return fixer.noop();
         }
 
-        let Some(parent) = symbol.nodes().parent_node(decl_id).map(AstNode::kind) else {
+        let Some(parent) = symbol.nodes().parent_kind(decl_id) else {
             #[cfg(debug_assertions)]
             panic!("VariableDeclarator nodes should always have a parent node");
             #[cfg(not(debug_assertions))]

diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/symbol.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/symbol.rs
@@ -124,8 +124,7 @@ impl<'s, 'a> Symbol<'s, 'a> {
     {
         let parents_iter = self
             .nodes()
-            .ancestors(node_id)
-            .map(AstNode::kind)
+            .ancestor_kinds(node_id)
             // no skip
             .filter(|kind| Self::is_relevant_kind(*kind));
 

diff --git a/crates/oxc_linter/src/rules/jsx_a11y/alt_text.rs b/crates/oxc_linter/src/rules/jsx_a11y/alt_text.rs
@@ -187,9 +187,7 @@ impl Rule for AltText {
         // <object>
         if let Some(custom_tags) = &self.object {
             if name == "object" || custom_tags.iter().any(|i| i == name) {
-                let maybe_parent =
-                    ctx.nodes().parent_node(node.id()).map(oxc_semantic::AstNode::kind);
-                if let Some(AstKind::JSXElement(parent)) = maybe_parent {
+                if let Some(AstKind::JSXElement(parent)) = ctx.nodes().parent_kind(node.id()) {
                     object_rule(jsx_el, parent, ctx);
                     return;
                 }

diff --git a/crates/oxc_linter/src/rules/jsx_a11y/heading_has_content.rs b/crates/oxc_linter/src/rules/jsx_a11y/heading_has_content.rs
@@ -102,8 +102,7 @@ impl Rule for HeadingHasContent {
             return;
         }
 
-        let maybe_parent = ctx.nodes().parent_node(node.id()).map(oxc_semantic::AstNode::kind);
-        if let Some(AstKind::JSXElement(parent)) = maybe_parent {
+        if let Some(AstKind::JSXElement(parent)) = ctx.nodes().parent_kind(node.id()) {
             if object_has_accessible_child(ctx, parent) {
                 return;
             }

diff --git a/crates/oxc_linter/src/rules/node/no_exports_assign.rs b/crates/oxc_linter/src/rules/node/no_exports_assign.rs
@@ -92,8 +92,8 @@ impl Rule for NoExportsAssign {
             };
         }
 
-        let parent = ctx.nodes().parent_node(node.id());
-        if let Some(AstKind::AssignmentExpression(assign_expr)) = parent.map(AstNode::kind) {
+        if let Some(AstKind::AssignmentExpression(assign_expr)) = ctx.nodes().parent_kind(node.id())
+        {
             if is_module_exports(assign_expr.left.as_member_expression(), ctx) {
                 return;
             }

diff --git a/crates/oxc_linter/src/rules/unicorn/explicit_length_check.rs b/crates/oxc_linter/src/rules/unicorn/explicit_length_check.rs
@@ -269,9 +269,7 @@ impl Rule for ExplicitLengthCheck {
                     self.report(ctx, ancestor, is_negative, static_member_expr, true);
                     return;
                 }
-                let parent = ctx.nodes().parent_node(node.id());
-                let kind = parent.map(AstNode::kind);
-                match kind {
+                match ctx.nodes().parent_kind(node.id()) {
                     Some(AstKind::LogicalExpression(LogicalExpression {
                         operator, right, ..
                     })) if *operator == LogicalOperator::And

diff --git a/crates/oxc_linter/src/rules/unicorn/no_null.rs b/crates/oxc_linter/src/rules/unicorn/no_null.rs
@@ -180,14 +180,14 @@ impl Rule for NoNull {
         };
 
         let mut parents = iter_outer_expressions(ctx, node.id());
-        let Some(parent_kind) = parents.next().map(AstNode::kind) else {
+        let Some(parent_kind) = parents.next() else {
             ctx.diagnostic_with_fix(no_null_diagnostic(null_literal.span), |fixer| {
                 fix_null(fixer, null_literal)
             });
             return;
         };
 
-        let grandparent_kind = parents.next().map(AstNode::kind);
+        let grandparent_kind = parents.next();
         match (parent_kind, grandparent_kind) {
             (AstKind::Argument(_), Some(AstKind::CallExpression(call_expr)))
                 if match_call_expression_pass_case(null_literal, call_expr) =>

diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_dom_node_text_content.rs b/crates/oxc_linter/src/rules/unicorn/prefer_dom_node_text_content.rs
@@ -61,8 +61,7 @@ impl Rule for PreferDomNodeTextContent {
                     return;
                 }
 
-                let mut ancestor_kinds =
-                    ctx.nodes().ancestors(node.id()).skip(1).map(AstNode::kind);
+                let mut ancestor_kinds = ctx.nodes().ancestor_kinds(node.id()).skip(1);
                 let (Some(parent_node_kind), Some(grand_parent_node_kind)) =
                     (ancestor_kinds.next(), ancestor_kinds.next())
                 else {
@@ -87,8 +86,7 @@ impl Rule for PreferDomNodeTextContent {
                     return;
                 }
 
-                let mut ancestor_kinds =
-                    ctx.nodes().ancestors(node.id()).skip(1).map(AstNode::kind);
+                let mut ancestor_kinds = ctx.nodes().ancestor_kinds(node.id()).skip(1);
                 let (Some(parent_node_kind), Some(grand_parent_node_kind)) =
                     (ancestor_kinds.next(), ancestor_kinds.next())
                 else {

diff --git a/crates/oxc_semantic/src/node.rs b/crates/oxc_semantic/src/node.rs
@@ -127,7 +127,7 @@ impl<'a> AstNodes<'a> {
         self.nodes.is_empty()
     }
 
-    /// Walk up the AST, iterating over each parent node.
+    /// Walk up the AST, iterating over each parent [`AstNode`].
     ///
     /// The first node produced by this iterator is the first parent of the node
     /// pointed to by `node_id`. The last node will usually be a `Program`.
@@ -136,6 +136,18 @@ impl<'a> AstNodes<'a> {
         AstNodeParentIter { current_node_id: Some(node_id), nodes: self }
     }
 
+    /// Walk up the AST, iterating over each parent [`AstKind`].
+    ///
+    /// The first node produced by this iterator is the first parent of the node
+    /// pointed to by `node_id`. The last node will is a [`AstKind::Program`].
+    #[inline]
+    pub fn ancestor_kinds(
+        &self,
+        node_id: NodeId,
+    ) -> impl Iterator<Item = AstKind<'a>> + Clone + '_ {
+        self.ancestors(node_id).map(AstNode::kind)
+    }
+
     /// Access the underlying struct from [`oxc_ast`].
     ///
     /// ## Example
@@ -214,7 +226,7 @@ impl<'a> AstNodes<'a> {
         self.root().map(|id| self.get_node_mut(id))
     }
 
-    /// Walk up the AST, iterating over each parent node.
+    /// Walk up the AST, iterating over each parent [`NodeId`].
     ///
     /// The first node produced by this iterator is the first parent of the node
     /// pointed to by `node_id`. The last node will always be a [`Program`].