fix(traverse): `ChildScopeCollector` visit all scopes #10292

overlookmotel · 2025-04-07T13:45:14Z

Fix ChildScopeCollector so it doesn't miss out scopes in some cases.

Previously when a struct has its own scope, the visitor does not visit any of the struct's fields. But this is incorrect where the type's scope does not cover all of its fields (the scope starts late / ends early). If the fields outside of the scope may themselves have scopes, then those scopes are direct children of the starting point.

These types are:

Class (decorators is outside Class's scope).
SwitchStatement (discriminant is outside SwitchStatement's scope).
TSConditionalType (check_type and false_type are outside TSConditionalType's scope).

Additionally, make the visitor more efficient by not bothering to visit any struct fields / enum variants which don't contain a scope anywhere in their dependencies.

overlookmotel · 2025-04-07T13:45:28Z

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 · 2025-04-07T13:55:27Z

CodSpeed Instrumentation Performance Report

Merging #10292 will not alter performance

_{Comparing 04-05-fix_traverse_childscopecollector_visit_all_scopes (1e683f9) with main (521de23)}

Summary

✅ 36 untouched benchmarks

Dunqing · 2025-04-09T09:53:49Z

Merge activity

Apr 9, 5:53 AM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
Apr 9, 5:53 AM EDT: Dunqing added this pull request to the Graphite merge queue.
Apr 9, 10:23 AM UTC: The Graphite merge queue couldn't merge this PR because it timed out.
Apr 9, 8:09 AM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
Apr 9, 8:24 AM EDT: overlookmotel added this pull request to the Graphite merge queue.
Apr 9, 8:25 AM EDT: Merged by the Graphite merge queue.

…rator (#10289) Pure refactor. Move methods of `VisitBuilder` which generate `visitor.visit_*(...)` statements to be free functions, so that they can be exported and used by other generators. Preparatory work for #10292.

) Pure refactor. Make functions in `Visit` generator which generate `visitor.visit_*(...)` statements generic over `VisitorOutputs`. `VisitorOutputs` trait represents a set of outputs. In generator for `Visit` and `VisitMut`, there are 2 outputs for the 2 traits. But other generators need to be able to just generate `Visit` alone. Preparatory work for #10292.

Fixes #9702. Fix `ChildScopeCollector` so it doesn't miss out scopes in some cases. Previously when a struct has its own scope, the visitor does not visit any of the struct's fields. But this is incorrect where the type's scope does not cover all of its fields (the scope starts late / ends early). If the fields outside of the scope may themselves have scopes, then those scopes are direct children of the starting point. These types are: * `Class` (`decorators` is outside `Class`'s scope). * `SwitchStatement` (`discriminant` is outside `SwitchStatement`'s scope). * `TSConditionalType` (`check_type` and `false_type` are outside `TSConditionalType`'s scope). Additionally, make the visitor more efficient by not bothering to visit any struct fields / enum variants which don't contain a scope anywhere in their dependencies.

@camchenry

commit e419aaf Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed Apr 9 09:54:18 2025 +0000 refactor(ast_tools): move limit for inlining to a const in `Visit` generator (#10291) Pure refactor. `Visit` generator adds `#[inline]` attr to visitor methods containing 5 or less statements. Make this limit a `const`, rather than having it inline in multiple places. commit a605247 Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed Apr 9 09:54:17 2025 +0000 refactor(ast_tools): make visit call generator functions generic (#10290) Pure refactor. Make functions in `Visit` generator which generate `visitor.visit_*(...)` statements generic over `VisitorOutputs`. `VisitorOutputs` trait represents a set of outputs. In generator for `Visit` and `VisitMut`, there are 2 outputs for the 2 traits. But other generators need to be able to just generate `Visit` alone. Preparatory work for #10292. commit 411610f Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed Apr 9 09:54:17 2025 +0000 refactor(ast_tools): change methods to free functions in `Visit` generator (#10289) Pure refactor. Move methods of `VisitBuilder` which generate `visitor.visit_*(...)` statements to be free functions, so that they can be exported and used by other generators. Preparatory work for #10292. commit 267922b Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed Apr 9 09:16:59 2025 +0000 perf(transformer/jsx): speed up decoding `JSXText` strings (#9741) close: #10249 Speed up transforming `JSXText` nodes by: 1. Avoiding allocations (construct strings directly in arena, with no intermediate `String`s). 2. Where no HTML entity decoding or string concatenation is required, reuse a slice of source text, rather than generating a new `Atom` and copying string data. commit 11647e8 Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed Apr 9 09:16:59 2025 +0000 test(transformer/jsx): tests for `JSXText` strings (#10324) Add tests for transforming `JSXText` and `JSXAttributeValue`. Tests cover: * Whitespace and line breaks. * HTML entities ([`HTMLCharacterReference`](https://facebook.github.io/jsx/#sec-HTMLCharacterReference)). commit b54fb3e Author: Yuji Sugiura <6259812+leaysgur@users.noreply.github.com> Date: Wed Apr 9 17:13:24 2025 +0900 fix(estree): Rename `TSInstantiationExpression`.`type_parameters` to `type_arguments` (#10327) Part of #9705 commit 9734152 Author: Yuji Sugiura <6259812+leaysgur@users.noreply.github.com> Date: Wed Apr 9 16:14:25 2025 +0900 fix(ast): Handle `TSThisType` in `TSTypePredicate` (#10328) Part of #9705 When given this code: ```ts interface X { y(): this is { z: 1 } } ``` Currently the parser treats `this` inside the type annotation as `Identifier` named `this`, but it should be `TSThisType`. commit 81867c4 Author: camc314 <18101008+camc314@users.noreply.github.com> Date: Wed Apr 9 04:39:13 2025 +0000 fix(linter): fix stack overflow in react/exhaustive deps (#10322) fixes #10319 commit a95ba40 Author: Sysix <3897725+Sysix@users.noreply.github.com> Date: Tue Apr 8 22:33:17 2025 +0000 refactor(language_server): make server more error resistance by falling back to default config (#10257) When the client does provide us with an invalid config path / file, the server should not crash. Related oxc-project/oxc-zed#10 #10123 commit e0b6c8c Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Tue Apr 8 22:05:34 2025 +0000 fix(transformer/react): correct comment (#10323) Fix comment in function for decoding HTML entities in `JSXText` elements. Hex code escape is `ሴ` not `&x1234;`. Also reformat comments in this function. commit 294d24b Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Tue Apr 8 16:15:13 2025 +0000 refactor(ast/estree): simplify serialization for `JSXOpeningFragment` (#10316) Follow-on after #10208. We can remove the `JSXOpeningFragmentAttributes` serializer, and use `TsEmptyArray` instead. This results in `attributes?: []` in the TS type def, which seems reasonable since a JSX fragment can't have attributes. Really this field shouldn't exist at all! It can probably be considered a mistake in `acorn-jsx` that it does. commit cc07efd Author: therewillbecode <tomw08@gmail.com> Date: Tue Apr 8 16:55:36 2025 +0100 fix(ast/estree): Fix `JSXOpeningFragment` (#10208) Fix `JSXOpeningFragment` node estree TS serialization by skipping `selfClosing` and `attributes` fields when AST is TS. Part of our broader work to align our AST's ESTree output with that of TS-ESLint's. Relates to #9705 --------- Co-authored-by: overlookmotel <theoverlookmotel@gmail.com> commit 48ed6a1 Author: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Tue Apr 8 13:07:27 2025 +0000 fix(ast/estree): fix span for `TemplateElement` in TS AST (#10315) Part of #9705. TS-ESLint differs from Acorn in the `span` of `TemplateElement`. TS-ESLint includes the preceding `` ` `` or `}` and following `${` or `` ` `` in the span. ```js const template = `abc${x}def${x}ghi`; // Acorn: ^^^ ^^^ ^^^ // TS-ESLint: ^^^^^^ ^^^^^^ ^^^^^ ``` Make the span follow TS-ESLint in the TS AST. commit 48c711a Author: 翠 / green <green@sapphi.red> Date: Tue Apr 8 19:53:14 2025 +0900 fix(minifier): panic when compressing `a ? b() : b()` (#10311) found in vitejs/rolldown-vite#104 commit 4268b23 Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Date: Tue Apr 8 17:13:17 2025 +0800 chore(deps): lock file maintenance rust crates (#10300) commit eba5fcf Author: oxc-bot <boshen_chen@qq.com> Date: Tue Apr 8 13:52:15 2025 +0800 release(crates): v0.63.0 (#10309) commit 52ea978 Author: Ulrich Stark <8657779+ulrichstark@users.noreply.github.com> Date: Tue Apr 8 06:07:48 2025 +0200 refactor(linter): update comments, improve tests, add variant All to LintFilterKind (#10259) I split this PR into easily reviewable commits. While working on this PR, I noticed two possible follow up changes: `LintFilterKind::Rule` and `LintFilterKind::Generic` behave exactly the same in `ConfigStoreBuilder::with_filter` because `LintFilterKind::Rule` never actually uses or checks for the plugin it stores. Should I include the check for plugin? The proposed fifth `LintFilterKind` isn't implemented yet (see: `// TODO: plugin + category? e.g -A react:correctness)`). Should I add it? commit 9aaba69 Author: Sub <9058689+zubhav@users.noreply.github.com> Date: Tue Apr 8 08:07:31 2025 +0400 fix(linter): nested configuration directory resolution (#10157) Fixes #10156 I'm not very familiar with the inner workings of this project - this was a fix based on what I could understand after exploring the codebase for the first time. @camchenry It would be great to get your eyes on this since you are familiar with the nested config implementation. commit e0057c3 Author: Sysix <3897725+Sysix@users.noreply.github.com> Date: Tue Apr 8 01:49:06 2025 +0000 perf(language_server): only restart internal linter once when multiple config changes detected (#10256)

Fixes #9702. Fix `ChildScopeCollector` so it doesn't miss out scopes in some cases. Previously when a struct has its own scope, the visitor does not visit any of the struct's fields. But this is incorrect where the type's scope does not cover all of its fields (the scope starts late / ends early). If the fields outside of the scope may themselves have scopes, then those scopes are direct children of the starting point. These types are: * `Class` (`decorators` is outside `Class`'s scope). * `SwitchStatement` (`discriminant` is outside `SwitchStatement`'s scope). * `TSConditionalType` (`check_type` and `false_type` are outside `TSConditionalType`'s scope). Additionally, make the visitor more efficient by not bothering to visit any struct fields / enum variants which don't contain a scope anywhere in their dependencies.

This was referenced Apr 7, 2025

refactor(ast_tools): change methods to free functions in Visit generator #10289

Merged

refactor(ast_tools): make visit call generator functions generic #10290

Merged

overlookmotel mentioned this pull request Apr 7, 2025

refactor(ast_tools): move limit for inlining to a const in Visit generator #10291

Merged

github-actions bot added A-ast-tools Area - AST tools C-bug Category - Bug labels Apr 7, 2025

overlookmotel marked this pull request as ready for review April 7, 2025 14:01

overlookmotel mentioned this pull request Apr 7, 2025

Traverse: Fix and improve design of ChildScopeCollector #9702

Closed

Dunqing approved these changes Apr 9, 2025

View reviewed changes

Dunqing added the 0-merge Merge with Graphite Merge Queue label Apr 9, 2025

graphite-app bot force-pushed the 04-07-refactor_ast_tools_move_limit_for_inlining_to_a_const_in_visit_generator branch from 851650b to e419aaf Compare April 9, 2025 09:55

graphite-app bot force-pushed the 04-05-fix_traverse_childscopecollector_visit_all_scopes branch from 5ae1615 to 98da4ac Compare April 9, 2025 09:56

graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Apr 9, 2025

Base automatically changed from 04-07-refactor_ast_tools_move_limit_for_inlining_to_a_const_in_visit_generator to main April 9, 2025 10:02

overlookmotel force-pushed the 04-05-fix_traverse_childscopecollector_visit_all_scopes branch from 98da4ac to 37ea7ec Compare April 9, 2025 12:09

overlookmotel added the 0-merge Merge with Graphite Merge Queue label Apr 9, 2025

overlookmotel force-pushed the 04-05-fix_traverse_childscopecollector_visit_all_scopes branch from 37ea7ec to 1e683f9 Compare April 9, 2025 12:18

graphite-app bot merged commit 1e683f9 into main Apr 9, 2025
29 checks passed

graphite-app bot deleted the 04-05-fix_traverse_childscopecollector_visit_all_scopes branch April 9, 2025 12:25

graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Apr 9, 2025

oxc-bot mentioned this pull request Apr 17, 2025

release(crates): v0.64.0 #10459

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(traverse): `ChildScopeCollector` visit all scopes #10292

fix(traverse): `ChildScopeCollector` visit all scopes #10292

Uh oh!

overlookmotel commented Apr 7, 2025 •

edited

Loading

Uh oh!

overlookmotel commented Apr 7, 2025 •

edited

Loading

Uh oh!

codspeed-hq bot commented Apr 7, 2025 •

edited

Loading

Uh oh!

Dunqing commented Apr 9, 2025 •

edited by graphite-app bot

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

fix(traverse): ChildScopeCollector visit all scopes #10292

fix(traverse): ChildScopeCollector visit all scopes #10292

Uh oh!

Conversation

overlookmotel commented Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

overlookmotel commented Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

codspeed-hq bot commented Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Instrumentation Performance Report

Merging #10292 will not alter performance

Summary

Uh oh!

Dunqing commented Apr 9, 2025 • edited by graphite-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix(traverse): `ChildScopeCollector` visit all scopes #10292

fix(traverse): `ChildScopeCollector` visit all scopes #10292

overlookmotel commented Apr 7, 2025 •

edited

Loading

overlookmotel commented Apr 7, 2025 •

edited

Loading

codspeed-hq bot commented Apr 7, 2025 •

edited

Loading

Dunqing commented Apr 9, 2025 •

edited by graphite-app bot

Loading