From 5ba765cec2e653226ffe7bb8692c06934b900c76 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Fri, 26 Sep 2025 04:10:30 +0000 Subject: [PATCH] refactor(semantic): move AstNode::flags to struct of arrays in AstNodes (#14136) part of #13646 --- crates/oxc_linter/src/ast_util.rs | 2 +- .../src/rules/eslint/require_yield.rs | 2 +- .../src/rules/jsdoc/implements_on_classes.rs | 2 +- .../oxc_linter/src/rules/jsdoc/no_defaults.rs | 2 +- .../src/rules/jsdoc/require_param.rs | 2 +- .../rules/jsdoc/require_param_description.rs | 2 +- .../src/rules/jsdoc/require_param_name.rs | 2 +- .../src/rules/jsdoc/require_param_type.rs | 2 +- .../src/rules/jsdoc/require_returns.rs | 2 +- .../jsdoc/require_returns_description.rs | 2 +- .../src/rules/jsdoc/require_returns_type.rs | 2 +- .../src/rules/jsdoc/require_yields.rs | 4 +- crates/oxc_linter/src/utils/jsdoc.rs | 2 +- crates/oxc_semantic/src/builder.rs | 3 +- crates/oxc_semantic/src/checker/mod.rs | 5 +- crates/oxc_semantic/src/jsdoc/finder.rs | 18 +++++-- crates/oxc_semantic/src/node.rs | 54 +++++++++---------- 17 files changed, 57 insertions(+), 51 deletions(-) diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index 690240749bcb2..45524a7cd34e9 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -608,7 +608,7 @@ pub fn is_callee<'a>(node: &AstNode<'a>, semantic: &Semantic<'a>) -> bool { fn has_jsdoc_this_tag<'a>(semantic: &Semantic<'a>, node: &AstNode<'a>) -> bool { let Some(jsdocs) = get_function_nearest_jsdoc_node(node, semantic) - .and_then(|node| semantic.jsdoc().get_all_by_node(node)) + .and_then(|node| semantic.jsdoc().get_all_by_node(semantic.nodes(), node)) else { return false; }; diff --git a/crates/oxc_linter/src/rules/eslint/require_yield.rs b/crates/oxc_linter/src/rules/eslint/require_yield.rs index eebf88a51b77e..2d99f06eda895 100644 --- a/crates/oxc_linter/src/rules/eslint/require_yield.rs +++ b/crates/oxc_linter/src/rules/eslint/require_yield.rs @@ -37,7 +37,7 @@ declare_oxc_lint!( impl Rule for RequireYield { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { if let AstKind::Function(func) = node.kind() - && !node.flags().has_yield() + && !ctx.nodes().flags(node.id()).has_yield() && func.generator && func.body.as_ref().is_some_and(|body| !body.statements.is_empty()) { diff --git a/crates/oxc_linter/src/rules/jsdoc/implements_on_classes.rs b/crates/oxc_linter/src/rules/jsdoc/implements_on_classes.rs index 52e55ba3b1246..faf155118d6aa 100644 --- a/crates/oxc_linter/src/rules/jsdoc/implements_on_classes.rs +++ b/crates/oxc_linter/src/rules/jsdoc/implements_on_classes.rs @@ -87,7 +87,7 @@ impl Rule for ImplementsOnClasses { } let Some(jsdocs) = get_function_nearest_jsdoc_node(node, ctx) - .and_then(|node| ctx.jsdoc().get_all_by_node(node)) + .and_then(|node| ctx.jsdoc().get_all_by_node(ctx.nodes(), node)) else { return; }; diff --git a/crates/oxc_linter/src/rules/jsdoc/no_defaults.rs b/crates/oxc_linter/src/rules/jsdoc/no_defaults.rs index b92e47dd70d78..95fadde96a44c 100644 --- a/crates/oxc_linter/src/rules/jsdoc/no_defaults.rs +++ b/crates/oxc_linter/src/rules/jsdoc/no_defaults.rs @@ -71,7 +71,7 @@ impl Rule for NoDefaults { } let Some(jsdocs) = get_function_nearest_jsdoc_node(node, ctx) - .and_then(|node| ctx.jsdoc().get_all_by_node(node)) + .and_then(|node| ctx.jsdoc().get_all_by_node(ctx.nodes(), node)) else { return; }; diff --git a/crates/oxc_linter/src/rules/jsdoc/require_param.rs b/crates/oxc_linter/src/rules/jsdoc/require_param.rs index c7d0af1cda9b0..9666758cf3a8f 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_param.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_param.rs @@ -127,7 +127,7 @@ impl Rule for RequireParam { return; }; // If no JSDoc is found, skip - let Some(jsdocs) = ctx.jsdoc().get_all_by_node(func_def_node) else { + let Some(jsdocs) = ctx.jsdoc().get_all_by_node(ctx.nodes(), func_def_node) else { return; }; diff --git a/crates/oxc_linter/src/rules/jsdoc/require_param_description.rs b/crates/oxc_linter/src/rules/jsdoc/require_param_description.rs index bf078e6e0f99e..72a5cf4fe7617 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_param_description.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_param_description.rs @@ -61,7 +61,7 @@ impl Rule for RequireParamDescription { // If no JSDoc is found, skip let Some(jsdocs) = get_function_nearest_jsdoc_node(node, ctx) - .and_then(|node| ctx.jsdoc().get_all_by_node(node)) + .and_then(|node| ctx.jsdoc().get_all_by_node(ctx.nodes(), node)) else { return; }; diff --git a/crates/oxc_linter/src/rules/jsdoc/require_param_name.rs b/crates/oxc_linter/src/rules/jsdoc/require_param_name.rs index 4fc6b3d7f6a78..1ccfa7760e28d 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_param_name.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_param_name.rs @@ -54,7 +54,7 @@ impl Rule for RequireParamName { // If no JSDoc is found, skip let Some(jsdocs) = get_function_nearest_jsdoc_node(node, ctx) - .and_then(|node| ctx.jsdoc().get_all_by_node(node)) + .and_then(|node| ctx.jsdoc().get_all_by_node(ctx.nodes(), node)) else { return; }; diff --git a/crates/oxc_linter/src/rules/jsdoc/require_param_type.rs b/crates/oxc_linter/src/rules/jsdoc/require_param_type.rs index fd8c335e1a499..26455a7e8802d 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_param_type.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_param_type.rs @@ -61,7 +61,7 @@ impl Rule for RequireParamType { // If no JSDoc is found, skip let Some(jsdocs) = get_function_nearest_jsdoc_node(node, ctx) - .and_then(|node| ctx.jsdoc().get_all_by_node(node)) + .and_then(|node| ctx.jsdoc().get_all_by_node(ctx.nodes(), node)) else { return; }; diff --git a/crates/oxc_linter/src/rules/jsdoc/require_returns.rs b/crates/oxc_linter/src/rules/jsdoc/require_returns.rs index 43a93b9d4c589..75ace33157b39 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_returns.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_returns.rs @@ -179,7 +179,7 @@ impl Rule for RequireReturns { continue; }; // If no JSDoc is found, skip - let Some(jsdocs) = ctx.jsdoc().get_all_by_node(func_def_node) else { + let Some(jsdocs) = ctx.jsdoc().get_all_by_node(ctx.nodes(), func_def_node) else { continue; }; diff --git a/crates/oxc_linter/src/rules/jsdoc/require_returns_description.rs b/crates/oxc_linter/src/rules/jsdoc/require_returns_description.rs index f0b25f35d1d52..b402d4fc750e3 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_returns_description.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_returns_description.rs @@ -55,7 +55,7 @@ impl Rule for RequireReturnsDescription { // If no JSDoc is found, skip let Some(jsdocs) = get_function_nearest_jsdoc_node(node, ctx) - .and_then(|node| ctx.jsdoc().get_all_by_node(node)) + .and_then(|node| ctx.jsdoc().get_all_by_node(ctx.nodes(), node)) else { return; }; diff --git a/crates/oxc_linter/src/rules/jsdoc/require_returns_type.rs b/crates/oxc_linter/src/rules/jsdoc/require_returns_type.rs index 5095170c12186..7c6fc24a077e9 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_returns_type.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_returns_type.rs @@ -54,7 +54,7 @@ impl Rule for RequireReturnsType { // If no JSDoc is found, skip let Some(jsdocs) = get_function_nearest_jsdoc_node(node, ctx) - .and_then(|node| ctx.jsdoc().get_all_by_node(node)) + .and_then(|node| ctx.jsdoc().get_all_by_node(ctx.nodes(), node)) else { return; }; diff --git a/crates/oxc_linter/src/rules/jsdoc/require_yields.rs b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs index 09f57a3c413be..06905ec98c906 100644 --- a/crates/oxc_linter/src/rules/jsdoc/require_yields.rs +++ b/crates/oxc_linter/src/rules/jsdoc/require_yields.rs @@ -124,7 +124,7 @@ impl Rule for RequireYields { { // If no JSDoc is found, skip let Some(jsdocs) = get_function_nearest_jsdoc_node(node, ctx) - .and_then(|node| ctx.jsdoc().get_all_by_node(node)) + .and_then(|node| ctx.jsdoc().get_all_by_node(ctx.nodes(), node)) else { return; }; @@ -216,7 +216,7 @@ impl Rule for RequireYields { // If no JSDoc is found, skip let Some(jsdocs) = get_function_nearest_jsdoc_node(generator_func_node, ctx) - .and_then(|node| ctx.jsdoc().get_all_by_node(node)) + .and_then(|node| ctx.jsdoc().get_all_by_node(ctx.nodes(), node)) else { return; }; diff --git a/crates/oxc_linter/src/utils/jsdoc.rs b/crates/oxc_linter/src/utils/jsdoc.rs index 8db197dc21650..8b1a0d2251f00 100644 --- a/crates/oxc_linter/src/utils/jsdoc.rs +++ b/crates/oxc_linter/src/utils/jsdoc.rs @@ -61,7 +61,7 @@ pub fn get_function_nearest_jsdoc_node<'a, 'b>( ) -> Option<&'b AstNode<'a>> { let mut current_node = node; // Whether the node has attached JSDoc or not is determined by `JSDocBuilder` - while semantic.jsdoc().get_all_by_node(current_node).is_none() { + while semantic.jsdoc().get_all_by_node(semantic.nodes(), current_node).is_none() { // Tie-breaker, otherwise every loop will end at `Program` node! // Maybe more checks should be added match current_node.kind() { diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index de418078aa1b3..e61e5b8a9d658 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -2150,8 +2150,7 @@ impl<'a> SemanticBuilder<'a> { AstKind::YieldExpression(_) => { // If not in a function, `current_function_node_id` is `NodeId` of `Program`. // But it shouldn't be possible for `yield` to be at top level - that's a parse error. - *self.nodes.get_node_mut(self.current_function_node_id).flags_mut() |= - NodeFlags::HasYield; + *self.nodes.flags_mut(self.current_function_node_id) |= NodeFlags::HasYield; } AstKind::CallExpression(call_expr) => { if !call_expr.optional && call_expr.callee.is_specific_id("eval") { diff --git a/crates/oxc_semantic/src/checker/mod.rs b/crates/oxc_semantic/src/checker/mod.rs index 576ee24480de4..e875f877259e3 100644 --- a/crates/oxc_semantic/src/checker/mod.rs +++ b/crates/oxc_semantic/src/checker/mod.rs @@ -136,8 +136,9 @@ pub fn check_unresolved_exports(ctx: &SemanticBuilder<'_>) { for reference_ids in ctx.unresolved_references.root().values() { for reference_id in reference_ids { let reference = ctx.scoping.get_reference(*reference_id); - let node = ctx.nodes.get_node(reference.node_id()); - if node.flags().has_export_specifier() + let node_id = reference.node_id(); + let node = ctx.nodes.get_node(node_id); + if ctx.nodes.flags(node_id).has_export_specifier() && let AstKind::IdentifierReference(ident) = node.kind() { ctx.errors.borrow_mut().push(undefined_export(&ident.name, ident.span)); diff --git a/crates/oxc_semantic/src/jsdoc/finder.rs b/crates/oxc_semantic/src/jsdoc/finder.rs index 5fc3bee4b3dfd..21798380e2858 100644 --- a/crates/oxc_semantic/src/jsdoc/finder.rs +++ b/crates/oxc_semantic/src/jsdoc/finder.rs @@ -2,7 +2,7 @@ use rustc_hash::FxHashMap; use oxc_span::{GetSpan, Span}; -use crate::AstNode; +use crate::{AstNode, AstNodes}; use super::parser::JSDoc; @@ -18,16 +18,24 @@ impl<'a> JSDocFinder<'a> { Self { attached, not_attached } } - pub fn get_one_by_node<'b>(&'b self, node: &AstNode<'a>) -> Option> { - let jsdocs = self.get_all_by_node(node)?; + pub fn get_one_by_node<'b>( + &'b self, + nodes: &AstNodes<'a>, + node: &AstNode<'a>, + ) -> Option> { + let jsdocs = self.get_all_by_node(nodes, node)?; // If flagged, at least 1 JSDoc is attached // If multiple JSDocs are attached, return the last = nearest jsdocs.last().cloned() } - pub fn get_all_by_node<'b>(&'b self, node: &AstNode<'a>) -> Option>> { - if !node.flags().has_jsdoc() { + pub fn get_all_by_node<'b>( + &'b self, + nodes: &AstNodes<'a>, + node: &AstNode<'a>, + ) -> Option>> { + if !nodes.flags(node.id()).has_jsdoc() { return None; } diff --git a/crates/oxc_semantic/src/node.rs b/crates/oxc_semantic/src/node.rs index d09086acedf43..cd6af68cdd531 100644 --- a/crates/oxc_semantic/src/node.rs +++ b/crates/oxc_semantic/src/node.rs @@ -26,8 +26,6 @@ pub struct AstNode<'a> { /// Associated `BasicBlockId` in CFG (initialized by control_flow) #[cfg(feature = "cfg")] cfg_id: BlockNodeId, - - flags: NodeFlags, } impl<'a> AstNode<'a> { @@ -37,21 +35,14 @@ impl<'a> AstNode<'a> { kind: AstKind<'a>, scope_id: ScopeId, cfg_id: BlockNodeId, - flags: NodeFlags, id: NodeId, ) -> Self { - Self { id, kind, scope_id, cfg_id, flags } + Self { id, kind, scope_id, cfg_id } } #[cfg(not(feature = "cfg"))] - pub(crate) fn new( - kind: AstKind<'a>, - scope_id: ScopeId, - _cfg_id: (), - flags: NodeFlags, - id: NodeId, - ) -> Self { - Self { id, kind, scope_id, flags } + pub(crate) fn new(kind: AstKind<'a>, scope_id: ScopeId, _cfg_id: (), id: NodeId) -> Self { + Self { id, kind, scope_id } } /// This node's unique identifier. @@ -84,18 +75,6 @@ impl<'a> AstNode<'a> { pub fn scope_id(&self) -> ScopeId { self.scope_id } - - /// Flags providing additional information about the node. - #[inline] - pub fn flags(&self) -> NodeFlags { - self.flags - } - - /// Get a mutable reference to this node's flags. - #[inline] - pub fn flags_mut(&mut self) -> &mut NodeFlags { - &mut self.flags - } } impl GetSpan for AstNode<'_> { @@ -118,6 +97,8 @@ pub struct AstNodes<'a> { nodes: IndexVec>, /// `node` -> `parent` parent_ids: IndexVec, + /// `node` -> `flags` + flags: IndexVec, /// Stores a set of bits of a fixed size, where each bit represents a single [`AstKind`]. If the bit is set (1), /// then the AST contains at least one node of that kind. If the bit is not set (0), then the AST does not contain /// any nodes of that kind. @@ -204,6 +185,18 @@ impl<'a> AstNodes<'a> { &mut self.nodes[node_id] } + /// Get flags for a node. + #[inline] + pub fn flags(&self, node_id: NodeId) -> NodeFlags { + self.flags[node_id] + } + + /// Get a mutable reference to a node's flags. + #[inline] + pub fn flags_mut(&mut self, node_id: NodeId) -> &mut NodeFlags { + &mut self.flags[node_id] + } + /// Get the [`Program`] that's also the root of the AST. #[inline] pub fn program(&self) -> &'a Program<'a> { @@ -232,8 +225,9 @@ impl<'a> AstNodes<'a> { flags: NodeFlags, ) -> NodeId { let node_id = self.parent_ids.push(parent_node_id); - let node = AstNode::new(kind, scope_id, cfg_id, flags, node_id); + let node = AstNode::new(kind, scope_id, cfg_id, node_id); self.nodes.push(node); + self.flags.push(flags); self.node_kinds_set.set(kind.ty()); node_id } @@ -249,8 +243,9 @@ impl<'a> AstNodes<'a> { flags: NodeFlags, ) -> NodeId { let node_id = self.parent_ids.push(parent_node_id); - let node = AstNode::new(kind, scope_id, (), flags, node_id); + let node = AstNode::new(kind, scope_id, (), node_id); self.nodes.push(node); + self.flags.push(flags); self.node_kinds_set.set(kind.ty()); node_id } @@ -274,7 +269,8 @@ impl<'a> AstNodes<'a> { "Program node must be of kind `AstKind::Program`" ); self.parent_ids.push(NodeId::ROOT); - self.nodes.push(AstNode::new(kind, scope_id, cfg_id, flags, NodeId::ROOT)); + self.nodes.push(AstNode::new(kind, scope_id, cfg_id, NodeId::ROOT)); + self.flags.push(flags); self.node_kinds_set.set(AstType::Program); NodeId::ROOT } @@ -293,7 +289,8 @@ impl<'a> AstNodes<'a> { "Program node must be of kind `AstKind::Program`" ); self.parent_ids.push(NodeId::ROOT); - self.nodes.push(AstNode::new(kind, scope_id, (), flags, NodeId::ROOT)); + self.nodes.push(AstNode::new(kind, scope_id, (), NodeId::ROOT)); + self.flags.push(flags); self.node_kinds_set.set(AstType::Program); NodeId::ROOT } @@ -302,6 +299,7 @@ impl<'a> AstNodes<'a> { pub fn reserve(&mut self, additional: usize) { self.nodes.reserve(additional); self.parent_ids.reserve(additional); + self.flags.reserve(additional); } /// Checks if the AST contains any nodes of the given types.