Skip to content

Conversation

@camchenry
Copy link
Member

No description provided.

@github-actions github-actions bot added A-linter Area - Linter C-performance Category - Solution not expected to change functional behavior, only performance labels Aug 25, 2025
Copy link
Member Author

camchenry commented Aug 25, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@codspeed-hq
Copy link

codspeed-hq bot commented Aug 25, 2025

CodSpeed Instrumentation Performance Report

Merging #13287 will improve performances by 71.95%

Comparing 08-25-perf_linter_generate_node_type_info_for_lint_rules_with_let.else_ (918b957) with main (2f9a6ac)1

Summary

⚡ 4 improvements
✅ 30 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
linter[RadixUIAdoptionSection.jsx] 2.6 ms 1.5 ms +71.95%
linter[binder.ts] 147.6 ms 87.4 ms +68.9%
linter[cal.com.tsx] 1,215 ms 739.9 ms +64.22%
linter[react.development.js] 52.6 ms 32.6 ms +61.49%

Footnotes

  1. No successful run was found on 08-16-wip_skip_rules_that_do_not_have_any_relevant_node_types (52a4734) during the generation of this report, so main (2f9a6ac) was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@camchenry camchenry force-pushed the 08-25-perf_linter_generate_node_type_info_for_lint_rules_with_let.else_ branch from 7497edb to ce94493 Compare August 25, 2025 04:16
@camchenry camchenry force-pushed the 08-25-perf_linter_generate_node_type_info_for_lint_rules_with_let.else_ branch from ce94493 to e673f22 Compare August 25, 2025 20:36
@camchenry camchenry force-pushed the 08-16-wip_skip_rules_that_do_not_have_any_relevant_node_types branch from cb6d1d0 to ce94926 Compare August 25, 2025 20:36
@camchenry camchenry changed the base branch from 08-16-wip_skip_rules_that_do_not_have_any_relevant_node_types to graphite-base/13287 August 25, 2025 21:38
@camchenry camchenry force-pushed the 08-25-perf_linter_generate_node_type_info_for_lint_rules_with_let.else_ branch from e673f22 to 8b57262 Compare August 26, 2025 01:45
@camchenry camchenry force-pushed the graphite-base/13287 branch from ce94926 to 697d103 Compare August 26, 2025 01:45
@camchenry camchenry changed the base branch from graphite-base/13287 to 08-16-wip_skip_rules_that_do_not_have_any_relevant_node_types August 26, 2025 01:45
@camchenry camchenry changed the base branch from 08-16-wip_skip_rules_that_do_not_have_any_relevant_node_types to graphite-base/13287 August 26, 2025 02:45
@camchenry camchenry force-pushed the 08-25-perf_linter_generate_node_type_info_for_lint_rules_with_let.else_ branch from 8b57262 to 13af599 Compare August 26, 2025 02:46
@camchenry camchenry force-pushed the graphite-base/13287 branch 2 times, most recently from efc0e7a to 284c7db Compare August 26, 2025 02:50
@camchenry camchenry force-pushed the 08-25-perf_linter_generate_node_type_info_for_lint_rules_with_let.else_ branch from 13af599 to b272bd6 Compare August 26, 2025 02:50
@camchenry camchenry changed the base branch from graphite-base/13287 to 08-16-wip_skip_rules_that_do_not_have_any_relevant_node_types August 26, 2025 02:51
@camchenry camchenry changed the base branch from 08-16-wip_skip_rules_that_do_not_have_any_relevant_node_types to graphite-base/13287 August 26, 2025 02:52
@camchenry camchenry force-pushed the graphite-base/13287 branch from 284c7db to 52a4734 Compare August 26, 2025 02:52
@camchenry camchenry force-pushed the 08-25-perf_linter_generate_node_type_info_for_lint_rules_with_let.else_ branch from b272bd6 to 918b957 Compare August 26, 2025 02:52
@camchenry camchenry changed the base branch from graphite-base/13287 to 08-16-wip_skip_rules_that_do_not_have_any_relevant_node_types August 26, 2025 02:52
Comment on lines +280 to +289
// Ensure there are no top-level `if` statements before this let-else
if func
.block
.stmts
.iter()
.take(idx)
.any(|s| matches!(s, Stmt::Expr(Expr::If(_), _)))
{
return None;
}
Copy link
Member

@overlookmotel overlookmotel Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this check enough? This is a contrived example, but something like this would produce the wrong result:

let is_var_decl = matches!(node.kind(), AstKind::VariableDeclaration(_));
for ancestor_kind in ctx.ancestor_kinds(node.id()) {
    if matches!(ancestor_kind, AstKind::Function(_)) {
        if is_var_decl {
            ctx.diagnostic( /* ... */ );
            return;
        }
        break;
    }
}

let AstKind::Class(decl) = node.kind() else { return };

Maybe safer to bail out if let AstKind::Blah(_) = node.kind() else { return }; isn't the first statement.

Once all the common patterns are all covered, we'll be able to see which rules remain that aren't being identified, and match on more patterns to catch them.

i.e. start conservative and expand from there, rather than risk false positives from being too liberal initially.

@camchenry camchenry changed the base branch from 08-16-wip_skip_rules_that_do_not_have_any_relevant_node_types to graphite-base/13287 August 30, 2025 04:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter C-performance Category - Solution not expected to change functional behavior, only performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants