Skip to content

Commit 228cff5

Browse files
authored
refactor(semantic,linter): assert that Program is always the first node (#12123)
Follow on to #12087 > We might be able to improve perf of AstNodes::ancestor_ids a little by using a custom iterator, instead of std::iter::successors. > Also, node_id == NodeId::DUMMY would be a cheaper check than parent_id == node_id. The NodeId of Program in practice is always NodeId::DUMMY (0), but we don't have a static guarantee of that at present. But we could get that guarantee by adding assert!(self.parent_ids.is_empty()) to the top of AstNodes::add_program_node (or maybe just debug_assert!). > @overlookmotel in #12087 (comment) My changes: - added constant NodeId::ROOT (same value as NodeId::DUMMY, but leads to more intuitive code) - added asserts in `add_program_node` that nodes are empty and node kind that's being added is Program - ancestor_kinds and ancestors now build upon ancestor_ids, making std::iter::successors implementation obsolete - make AstNodeParentIter simpler by iterating over node ids and by making use of previous assertions of `add_program_node` - remove root, root_node and root_node_mut functions and replace their use with either new program function or NodeId::ROOT Results in general much less code, especially less boilerplate code in the linter around getting the program node. Also might hopefully yield some small benchmark improvements. Edit: Only very minor if not zero improvements on the linter benchmarks :( I also noticed while working on this that the ancestor iterator methods where documented as "The first node produced by this iterator is the first parent of the node pointed to by node_id". That statement was incorrect because all the iterators didn't yield the parent of node_id, but node_id itself as first item. I updated the documentation of these methods to correctly reflect their implementation.
1 parent a46708f commit 228cff5

File tree

14 files changed

+71
-138
lines changed

14 files changed

+71
-138
lines changed

crates/oxc_linter/src/rules/eslint/no_eval.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,11 +214,8 @@ impl Rule for NoEval {
214214
return;
215215
}
216216

217-
let root = ctx.nodes().get_node(ctx.nodes().root().unwrap());
218-
let program = root.kind().as_program().unwrap();
219-
220217
let is_valid = if scope_flags.is_top() {
221-
program.source_type.is_script()
218+
ctx.nodes().program().unwrap().source_type.is_script()
222219
} else {
223220
let node = ctx.nodes().get_node(ctx.scoping().get_node_id(scope_id));
224221
ast_util::is_default_this_binding(ctx, node, true)

crates/oxc_linter/src/rules/eslint/no_unreachable.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use oxc_cfg::{
88
};
99
use oxc_diagnostics::OxcDiagnostic;
1010
use oxc_macros::declare_oxc_lint;
11+
use oxc_semantic::NodeId;
1112
use oxc_span::{GetSpan, Span};
1213

1314
use crate::{context::LintContext, rule::Rule};
@@ -54,7 +55,7 @@ declare_oxc_lint!(
5455
impl Rule for NoUnreachable {
5556
fn run_once(&self, ctx: &LintContext) {
5657
let nodes = ctx.nodes();
57-
let Some(root) = nodes.root_node() else { return };
58+
let root = nodes.get_node(NodeId::ROOT);
5859
let cfg = ctx.cfg();
5960
let graph = cfg.graph();
6061

crates/oxc_linter/src/rules/eslint/sort_imports.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,7 @@ use std::{
66

77
use cow_utils::CowUtils;
88
use itertools::Itertools;
9-
use oxc_ast::{
10-
AstKind,
11-
ast::{ImportDeclaration, ImportDeclarationSpecifier, Statement},
12-
};
9+
use oxc_ast::ast::{ImportDeclaration, ImportDeclarationSpecifier, Statement};
1310
use oxc_diagnostics::OxcDiagnostic;
1411
use oxc_macros::declare_oxc_lint;
1512
use oxc_span::Span;
@@ -137,10 +134,7 @@ impl Rule for SortImports {
137134
}
138135

139136
fn run_once(&self, ctx: &LintContext) {
140-
let Some(root) = ctx.nodes().root_node() else {
141-
return;
142-
};
143-
let AstKind::Program(program) = root.kind() else { unreachable!() };
137+
let program = ctx.nodes().program().unwrap();
144138

145139
let mut import_declarations = vec![];
146140

crates/oxc_linter/src/rules/import/exports_last.rs

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
use itertools::Itertools;
2-
use oxc_ast::{
3-
AstKind,
4-
ast::{ModuleDeclaration, Statement},
5-
};
2+
use oxc_ast::ast::{ModuleDeclaration, Statement};
63
use oxc_diagnostics::OxcDiagnostic;
74
use oxc_macros::declare_oxc_lint;
85
use oxc_span::{GetSpan, Span};
@@ -54,19 +51,15 @@ declare_oxc_lint!(
5451
impl Rule for ExportsLast {
5552
fn run_once(&self, ctx: &LintContext<'_>) {
5653
// find last non export declaration index
57-
let Some(root) = ctx.nodes().root_node() else {
58-
return;
59-
};
60-
if let AstKind::Program(program) = root.kind() {
61-
let body = &program.body;
62-
let find_res =
63-
body.iter().rev().find_position(|statement| !is_exports_declaration(statement));
64-
if let Some((index, _)) = find_res {
65-
let end = body.len() - index;
66-
for statement in &body[0..end] {
67-
if is_exports_declaration(statement) {
68-
ctx.diagnostic(exports_last_diagnostic(statement.span()));
69-
}
54+
let program = ctx.nodes().program().unwrap();
55+
let body = &program.body;
56+
let find_res =
57+
body.iter().rev().find_position(|statement| !is_exports_declaration(statement));
58+
if let Some((index, _)) = find_res {
59+
let end = body.len() - index;
60+
for statement in &body[0..end] {
61+
if is_exports_declaration(statement) {
62+
ctx.diagnostic(exports_last_diagnostic(statement.span()));
7063
}
7164
}
7265
}

crates/oxc_linter/src/rules/import/first.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
use std::convert::From;
22

3-
use oxc_ast::{
4-
AstKind,
5-
ast::{Statement, TSModuleReference},
6-
};
3+
use oxc_ast::ast::{Statement, TSModuleReference};
74
use oxc_diagnostics::OxcDiagnostic;
85
use oxc_macros::declare_oxc_lint;
96
use oxc_span::Span;
@@ -113,10 +110,7 @@ impl Rule for First {
113110
let mut non_import_count = 0;
114111
let mut any_relative = false;
115112

116-
let Some(root) = ctx.nodes().root_node() else {
117-
return;
118-
};
119-
let AstKind::Program(program) = root.kind() else { unreachable!() };
113+
let program = ctx.nodes().program().unwrap();
120114

121115
for statement in &program.body {
122116
match statement {

crates/oxc_linter/src/rules/jest/no_duplicate_hooks.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,18 +104,15 @@ declare_oxc_lint!(
104104

105105
impl Rule for NoDuplicateHooks {
106106
fn run_once(&self, ctx: &LintContext) {
107-
let Some(root_node) = ctx.nodes().root_node() else {
108-
return;
109-
};
110107
let mut hook_contexts: FxHashMap<NodeId, Vec<FxHashMap<String, i32>>> =
111108
FxHashMap::default();
112-
hook_contexts.insert(root_node.id(), Vec::new());
109+
hook_contexts.insert(NodeId::ROOT, Vec::new());
113110

114111
let mut possibles_jest_nodes = collect_possible_jest_call_node(ctx);
115112
possibles_jest_nodes.sort_by_key(|n| n.node.id());
116113

117114
for possible_jest_node in possibles_jest_nodes {
118-
Self::run(&possible_jest_node, root_node.id(), &mut hook_contexts, ctx);
115+
Self::run(&possible_jest_node, NodeId::ROOT, &mut hook_contexts, ctx);
119116
}
120117
}
121118
}

crates/oxc_linter/src/rules/nextjs/no_async_client_component.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,7 @@ declare_oxc_lint!(
9090

9191
impl Rule for NoAsyncClientComponent {
9292
fn run_once(&self, ctx: &LintContext) {
93-
let Some(root) = ctx.nodes().root_node() else {
94-
return;
95-
};
96-
let AstKind::Program(program) = root.kind() else { unreachable!() };
93+
let program = ctx.nodes().program().unwrap();
9794

9895
if program.directives.iter().any(|directive| directive.directive.as_str() == "use client") {
9996
for node in &program.body {

crates/oxc_linter/src/rules/typescript/consistent_type_imports.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -576,8 +576,7 @@ fn get_type_only_named_import<'a>(
576576
ctx: &LintContext<'a>,
577577
source: &str,
578578
) -> Option<&'a ImportDeclaration<'a>> {
579-
let root = ctx.nodes().root_node()?;
580-
let program = root.kind().as_program()?;
579+
let program = ctx.nodes().program().unwrap();
581580

582581
for stmt in &program.body {
583582
let Statement::ImportDeclaration(import_decl) = stmt else {

crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
use oxc_ast::{
2-
AstKind,
3-
ast::{Statement, TSModuleReference},
4-
};
1+
use oxc_ast::ast::{Statement, TSModuleReference};
52
use oxc_diagnostics::OxcDiagnostic;
63
use oxc_macros::declare_oxc_lint;
74
use oxc_span::{GetSpan, Span};
@@ -109,10 +106,7 @@ impl Rule for TripleSlashReference {
109106
}
110107

111108
fn run_once(&self, ctx: &LintContext) {
112-
let Some(root) = ctx.nodes().root_node() else {
113-
return;
114-
};
115-
let AstKind::Program(program) = root.kind() else { unreachable!() };
109+
let program = ctx.nodes().program().unwrap();
116110

117111
// We don't need to iterate over all comments since Triple-slash directives are only valid at the top of their containing file.
118112
// We are trying to get the first statement start potioin, falling back to the program end if statement does not exist

crates/oxc_linter/src/rules/unicorn/no_empty_file.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use oxc_ast::AstKind;
21
use oxc_diagnostics::OxcDiagnostic;
32
use oxc_macros::declare_oxc_lint;
43
use oxc_span::Span;
@@ -44,11 +43,7 @@ declare_oxc_lint!(
4443

4544
impl Rule for NoEmptyFile {
4645
fn run_once(&self, ctx: &LintContext) {
47-
let Some(root) = ctx.nodes().root_node() else {
48-
return;
49-
};
50-
51-
let AstKind::Program(program) = root.kind() else { unreachable!() };
46+
let program = ctx.nodes().program().unwrap();
5247
if program.body.iter().any(|node| !is_empty_stmt(node)) {
5348
return;
5449
}

0 commit comments

Comments
 (0)