cebamps
commented
4 months ago The second obstacle I found that stands in the way of supporting scope is in how Babel's type definitions do not fully match the expectations set by the ast-types scopePlugin. But that may be a limitation of ast-types itself, so I am unsure about creating a PR... You can find my fix here; let me know if this seems appropriate to submit: https://github.com/cebamps/astx/compare/fix/ast-types-init...cebamps:astx:fix/ast-types-scope
The issue is that the ast-types scopePlugin implicitly relies on the Pattern type being a supertype of Identifier (see assertion in scopePlugin, and the Identifier definition in ast-types), but @babel/types distinguishes Pattern and PatternLike, with Babel's Identifier only having the latter as a supertype.
Therefore, ast-types scope lookups simply fail the assertion in a simple case I tried:
Example test case using scope
```ts import { NodePath as AstTypesNodePath } from 'ast-types/lib/node-path' import { Astx } from '../../src' import { astxTestcase } from '../astxTestcase' import dedent from 'dedent-js' astxTestcase({ file: __filename, input: dedent` const x = 1, y = 2; function foo() { const y = 3; const target = x + y; } `, expected: dedent` const x = 1, y = 2; function foo() { const y = 3; const target = 1 + 3; } `, parsers: ['babel', 'babel/tsx'], astx: ({ astx }) => { for (const { $a, $b } of astx.find`const target = $a + $b`) { inlineNaively($a) inlineNaively($b) } }, }) /** kludge to gain access to scope */ type AstxWithScope = Astx & { path: AstTypesNodePath } // inspired by jscodeshift function getDeclarator(a: AstxWithScope) { const name = a.path.value.name const bindings = new Astx( a.context, a.path.scope?.lookup(name)?.getBindings()[name] ) return bindings .closest((astx) => astx.node.type === 'VariableDeclarator') .at(0) } function inlineNaively(a: Astx) { const val = getDeclarator(a as AstxWithScope).path.get('init').value a.replace(val) } ```Adding an extra relationship in babelAstTypes by adding an extra line def('Identifier').bases('Pattern'); makes it work, but I suspect this kind of ad-hoc definition tweak is not quite right for astx because it introduces a difference with the Babel type hierarchy. Perhaps it should instead fall on ast-types to make a few less assumptions about what the Pattern type contains?
jedwards1211
Hi!
First of all, for context: I am trying my hand at code mods and found astx after experimenting some with jscodeshift. This project is great! I love that pattern matching on the AST no longer gets in my way. Thank you :)
I have hit two obstacles trying to use ast-types's Scope functionality on matched NodePaths (see ast-types readme). I figure this is not something you have sought to support so far, but I have still managed to work around my immediate obstacles and have produced a couple of small changes to propose.
This PR is for the first issue, which I am more confident about addressing. It removes
fork.use(nodePathPlugin)inbabelAstTypes, which I believe is done too early and is also not needed since ast-types already handles that for us. (I will explain the second issue in a comment.)That early use of
nodePathPlugincaused thescopeproperty on NodePath instances to be alwaysnull. This is becausenodePathPluginusesscopePlugin, which in turns usestypesPluginat initialization time -- specifically thenamedTypesobject it provides. But whenscopePluginis initialized too early,namedTypesis still empty: it only gets populated when the definitions oftypesPluginare finalized, which ast-types gets in the right order.I could not find other parts of ast-types that are similarly sensitive to ordering, so I understand this is not much of an issue for astx until it supports scopes.
This PR first adds a test that witnesses the scope issue, then fixes the broken test by removing the
fork.use(nodePathPlugin)call.The test is a little odd because it checks that
scopeis non-null, though astx omitsscopewhen redefining the NodePath interface, so I have a type mismatch. Adding scope to that re-definition looks like a big change that I can't really see as a good idea from where I stand.