compat-eslint: lazy parent β defer parent resolution to first .parent read#96
Merged
Conversation
β¦ read `materialize(tsNode, ctx)` previously walked up the TS parent chain and built every ancestor top-down so the leaf could be returned with a fully-resolved parent. Rules that never read `.parent` (e.g. JSX-attribute counters, type- checker leaf reads) paid for the entire chain anyway. `LazyNode.parent` is now a lazy getter backed by a PARENT_UNSET sentinel; first read calls `resolveParent`, which walks the TS chain incrementally one ancestor per `.parent` step. Bottom-up `materialize` constructs ONLY the requested leaf with `parent: undefined`. Top-down construction is unchanged β slot getters still pass `this` as the parent, so the constructor sets `_parent` directly and `.parent` reads stay O(1). Three details that fall out of the design: - Routers that previously checked `parent._ts.kind` (BindingElement, Method/ Get/Set accessor) now read `tsNode.parent.kind` directly. The TS parent is the same node either way for these kinds, and routers must work without a resolved parent under the lazy-leaf path. - `convertChild` re-points a cached child's `_parent` when it's still PARENT_UNSET. Each TS node has exactly one ESTree parent, so the slot owner who's calling us IS that parent β without this, ChainExpression's `expression` slot leaves the inner Identifier pointing at PARENT_UNSET and `resolveParent` then drills to the wrong wrapper. - Parent getter prefers a side-effect parent over the `resolveParent` return value: wrappers like ChainExpression occupy the same TS slot as their inner expression, so resolveParent returns the outer wrapper, but the inner slot's `convertChild` already wrote the correct intermediate parent via the setter. Module-level `currentCtx` lets routers call `convertChild` with `parent: undefined` and still resolve ctx for the cache lookup. Set on entry to `materialize`, restored on exit. Numbers (Dify cold lint, 3 runs): - Wall: 13.55β13.88s β 10.96β11.85s (~18% faster) - User CPU: ~21s β ~17.5s (~17% faster) - Node count: 174,367 β 138,046 (~21% fewer, 60 kinds β 48 kinds) Tests: lazy-estree (incl. ~67k-assert dogfood parity sweep), predicate- coverage 152/152, bench (107Γ33 clean parity), dogfood (107Γ30, 0 divergences), scope-compat 24/24, selector-analysis, ts-ast-scan, compat- pipeline, jsx-react-x β all pass. Net β206 lines in lazy-estree.ts.
Encapsulate the cache-hit re-point. Replace the cast-based reach into
`_parent` from `convertChild` with a dedicated `_setParentIfUnset`
method on LazyNode. Same behaviour, but the PARENT_UNSET sentinel stays
an implementation detail.
Add three targeted tests for the subtle pieces of the lazy-parent path
that the parity sweep covers in aggregate but doesn't pin in isolation:
- ChainExpression: `arguments?.length` β Identifier.parent must be
MemberExpression, not the outer ChainExpression wrapper. This is
the regression that prompted the cache-hit re-point fix in the
first place.
- BindingElement collapse: `let [a] = [1]` β Identifier `a`'s parent
must be ArrayPattern, not itself (BindingElement collapses into the
inner Identifier; resolveParent's `parent._ts !== walker` check
keeps the walk going).
- ParenthesizedExpression collapse: `(x);` β Identifier `x`'s parent
must skip past the parens to ExpressionStatement.
A future "simplify" of the getter / cache-hit / collapse logic now
fails with a clear message instead of a vague type-mismatch row in the
67k-assert dogfood sweep.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
materialize(tsNode, ctx)previously walked up the TS parent chain and built every ancestor top-down so the leaf could be returned with a fully-resolved parent. Rules that never read.parent(e.g. JSX-attribute counters, type-checker leaf reads) paid for the entire chain anyway.LazyNode.parentis now a lazy getter backed by aPARENT_UNSETsentinel; first read callsresolveParent, which walks the TS chain incrementally β one ancestor per.parentstep. Bottom-upmaterializeconstructs ONLY the requested leaf withparent: undefined. Top-down construction is unchanged β slot getters still passthisas the parent, so the constructor sets_parentdirectly and.parentreads stay O(1).Three details that fell out of the design
tsNode.parent.kinddirectly. Routers that previously checkedparent._ts.kind(BindingElement, Method/Get/Set accessor) now read the TS parent β the same node either way for these kinds, and routers must work without a resolved parent under the lazy-leaf path.convertChildre-points a cached child's_parentwhen it's stillPARENT_UNSET. Each TS node has exactly one ESTree parent, so the slot owner who's calling us IS that parent. Without this, ChainExpression'sexpressionslot leaves the inner Identifier pointing at PARENT_UNSET andresolveParentdrills to the wrong wrapper. Caught byprefer-rest-paramsregressing onarguments?.length.resolveParentreturn value. Wrappers like ChainExpression occupy the same TS slot as their inner expression, so resolveParent returns the outer wrapper β but the inner slot'sconvertChildalready wrote the correct intermediate parent via the setter.Module-level
currentCtxlets routers callconvertChildwithparent: undefinedand still resolve ctx for the cache lookup. Set on entry tomaterialize, restored on exit.Numbers (Dify cold lint, 3 runs)
JSXAttribute 24kβ16.6k, JSXElement 15kβ10.4k, JSXOpeningElement 8kβ4.6k β those JSX ancestors are now built only when the rule actually reads
.parent.Net β206 lines in
lazy-estree.ts.Test plan
lazy-estree.testβ incl. ~67k-assert dogfood parity sweeppredicate-coverage152/152bench107 rules Γ 33 files β clean paritydogfood107 rules Γ 30 files β 0 divergences, 0 crashesscope-compat24/24,selector-analysis,ts-ast-scan,compat-pipeline,jsx-react-xPARITY β