Skip to content

Commit c7889c3

Browse files
authored
perf(semantic,linter): simplify implementation and uses of ancestors iterators (#12164)
This simplifies the implementation and uses of `ancestor_ids`, `ancestor_kinds` and `ancestors` iterators by changing its first returned item from the provided node to the first parent of this node. This makes the implementation of those iterators simpler and makes code in most lint rules shorter by mainly removing the often called `.skip(1)`. All tests are green, but as always I probably broke something untested because of the scale and impact of this change. I'm not quite happy with me having to resort to `iter::once` twice in `no_unused_vars` lint rule, but I couldn't come up with a better solution. But having much less code, simpler implementation of the iterators and hopefully a performance gain in the linter benchmarks is definitely worth the change. Continuation of #12123 and #12087 Closes #12151
1 parent facd3cd commit c7889c3

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+125
-176
lines changed

crates/oxc_linter/src/ast_util.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,6 @@ pub fn outermost_paren_parent<'a, 'b>(
241241
semantic
242242
.nodes()
243243
.ancestors(node.id())
244-
.skip(1)
245244
.find(|parent| !matches!(parent.kind(), AstKind::ParenthesizedExpression(_)))
246245
}
247246

@@ -253,7 +252,6 @@ pub fn nth_outermost_paren_parent<'a, 'b>(
253252
semantic
254253
.nodes()
255254
.ancestors(node.id())
256-
.skip(1)
257255
.filter(|parent| !matches!(parent.kind(), AstKind::ParenthesizedExpression(_)))
258256
.nth(n)
259257
}
@@ -264,7 +262,7 @@ pub fn iter_outer_expressions<'a, 's>(
264262
semantic: &'s Semantic<'a>,
265263
node_id: NodeId,
266264
) -> impl Iterator<Item = AstKind<'a>> + 's {
267-
semantic.nodes().ancestor_kinds(node_id).skip(1).filter(|parent| {
265+
semantic.nodes().ancestor_kinds(node_id).filter(|parent| {
268266
!matches!(
269267
parent,
270268
AstKind::ParenthesizedExpression(_)
@@ -661,7 +659,7 @@ pub fn is_default_this_binding<'a>(
661659
current_node = parent;
662660
}
663661
AstKind::ReturnStatement(_) => {
664-
let upper_func = semantic.nodes().ancestors(parent.id()).skip(1).find(|node| {
662+
let upper_func = semantic.nodes().ancestors(parent.id()).find(|node| {
665663
matches!(
666664
node.kind(),
667665
AstKind::Function(_) | AstKind::ArrowFunctionExpression(_)

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ impl Rule for FuncNames {
272272

273273
// a function which is calling itself inside is always valid
274274
let ast_span =
275-
ctx.nodes().ancestors(node.id()).skip(1).find_map(|p| match p.kind() {
275+
ctx.nodes().ancestors(node.id()).find_map(|p| match p.kind() {
276276
AstKind::Function(func) => {
277277
let func_name = func.name()?;
278278

@@ -314,7 +314,7 @@ impl Rule for FuncNames {
314314
ctx.diagnostic_with_fix(
315315
unnamed_diagnostic(&func_name_complete, report_span),
316316
|fixer| {
317-
guess_function_name(ctx, parent_node.id()).map_or_else(
317+
guess_function_name(ctx, node.id()).map_or_else(
318318
|| fixer.noop(),
319319
|name| {
320320
// if this name shadows a variable in the outer scope **and** that name is referenced
@@ -346,8 +346,8 @@ impl Rule for FuncNames {
346346
}
347347
}
348348

349-
fn guess_function_name<'a>(ctx: &LintContext<'a>, parent_id: NodeId) -> Option<Cow<'a, str>> {
350-
for parent_kind in ctx.nodes().ancestor_kinds(parent_id) {
349+
fn guess_function_name<'a>(ctx: &LintContext<'a>, node_id: NodeId) -> Option<Cow<'a, str>> {
350+
for parent_kind in ctx.nodes().ancestor_kinds(node_id) {
351351
match parent_kind {
352352
AstKind::ParenthesizedExpression(_)
353353
| AstKind::TSAsExpression(_)

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,6 @@ impl Rule for FuncStyle {
297297
let arrow_func_ancestor = semantic
298298
.nodes()
299299
.ancestors(node.id())
300-
.skip(1)
301300
.find(|v| matches!(v.kind(), AstKind::FunctionBody(_)))
302301
.map(|el| semantic.nodes().parent_node(el.id()));
303302
if let Some(ret) = arrow_func_ancestor {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ impl Rule for InitDeclarations {
135135
return;
136136
}
137137
let decl_ancestor =
138-
ctx.nodes().ancestor_kinds(node.id()).skip(1).find(|el| {
138+
ctx.nodes().ancestor_kinds(node.id()).find(|el| {
139139
matches!(el, AstKind::TSModuleDeclaration(ts_module_decl) if ts_module_decl.declare)
140140
});
141141
if decl_ancestor.is_some() {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ declare_oxc_lint!(
115115
impl Rule for MaxDepth {
116116
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
117117
if should_count(node, ctx.nodes()) {
118-
let depth = ctx
118+
let depth = 1 + ctx
119119
.nodes()
120120
.ancestors(node.id())
121121
.take_while(|node| !should_stop(node))

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ declare_oxc_lint!(
108108

109109
impl Rule for MaxNestedCallbacks {
110110
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
111-
if is_function_node(node) {
112-
let depth = ctx
111+
if is_callback(node, ctx) {
112+
let depth = 1 + ctx
113113
.semantic()
114114
.nodes()
115115
.ancestors(node.id())

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ impl Rule for NoCondAssign {
8989
}
9090
AstKind::AssignmentExpression(expr) if self.config == NoCondAssignConfig::Always => {
9191
let mut spans = vec![];
92-
for ancestor in ctx.nodes().ancestors(node.id()).skip(1) {
92+
for ancestor in ctx.nodes().ancestors(node.id()) {
9393
match ancestor.kind() {
9494
AstKind::IfStatement(if_stmt) => {
9595
spans.push(if_stmt.test.span());

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ fn remove_console<'c, 'a: 'c>(
158158
node: &AstNode<'a>,
159159
) -> RuleFix<'a> {
160160
let mut node_to_delete = node;
161-
for parent in ctx.nodes().ancestors(node.id()).skip(1) {
161+
for parent in ctx.nodes().ancestors(node.id()) {
162162
match parent.kind() {
163163
AstKind::ParenthesizedExpression(_)
164164
| AstKind::ExpressionStatement(_)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ fn is_constructor(node: &AstNode<'_>) -> bool {
6767
}
6868

6969
fn is_definitely_in_constructor(ctx: &LintContext, node_id: NodeId) -> bool {
70-
for ancestor_id in ctx.nodes().ancestor_ids(node_id).skip(1) {
70+
for ancestor_id in ctx.nodes().ancestor_ids(node_id) {
7171
match ctx.nodes().kind(ancestor_id) {
7272
AstKind::Function(_) => {
7373
return is_constructor(ctx.nodes().parent_node(ancestor_id));

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ impl Rule for NoDebugger {
5555
let Some(parent) = ctx
5656
.nodes()
5757
.ancestors(node.id())
58-
.skip(1)
5958
.find(|p| !matches!(p.kind(), AstKind::ParenthesizedExpression(_)))
6059
else {
6160
return fixer.delete(&stmt.span).with_message(REMOVE_DEBUGGER);

0 commit comments

Comments
 (0)