From 2a6ddd47f4b41293e7ed06a5967ca67cb593dc14 Mon Sep 17 00:00:00 2001 From: Milan Hauth Date: Sat, 5 Nov 2022 16:49:17 +0100 Subject: [PATCH] fix(codemod): replace only local references --- .../src/transforms/replaceObjectBinding.ts | 24 +++--- .../src/utils/forEachLocalReference.ts | 82 +++++++++++++++++++ .../transforms/replaceObjectBinding.test.ts | 72 ++++++++++++++-- 3 files changed, 162 insertions(+), 16 deletions(-) create mode 100644 packages/codemod/src/utils/forEachLocalReference.ts diff --git a/packages/codemod/src/transforms/replaceObjectBinding.ts b/packages/codemod/src/transforms/replaceObjectBinding.ts index 2d993362c..4d3325bde 100644 --- a/packages/codemod/src/transforms/replaceObjectBinding.ts +++ b/packages/codemod/src/transforms/replaceObjectBinding.ts @@ -1,9 +1,9 @@ import capitalize from "../utils/capitalize"; -//import checkNodeScope, { NodeScope } from "../utils/checkNodeScope"; -import { NodeScope } from "../utils/checkNodeScope"; +import checkNodeScope, { NodeScope } from "../utils/checkNodeScope"; import isStaticValue from "../utils/isStaticValue"; import renameIdentifiers from "../utils/renameIdentifiers"; import hasAncestorType from "../utils/hasAncestorType"; +import forEachLocalReference from "../utils/forEachLocalReference"; import { Identifier, @@ -37,23 +37,24 @@ function renameObjectBinding(object: ObjectBindingJson, varName: string) { ? `${varName}[${object.nameNode.getText()}]` : `${varName}.${object.nameNode.getText()}`; const excludeSelf = true; - - for (const ref of node.findReferencesAsNodes()) { - if (excludeSelf && node === ref) continue; - if (hasAncestorType(ref)) continue; + forEachLocalReference(node, ref => { + if (excludeSelf && node === ref) return; + if (hasAncestorType(ref)) return; const parent = ref.getParent(); - // keep JSX attribute names if ( parent && - parent.isKind(ts.SyntaxKind.JsxAttribute) && + ( + parent.isKind(ts.SyntaxKind.PropertyAssignment) || + parent.isKind(ts.SyntaxKind.JsxAttribute) + ) && parent.getNameNode() == ref - ) continue; + ) return; if (Node.isShorthandPropertyAssignment(parent)) { parent.replaceWithText(`${node.getText()}: ${name}`); } else if (Node.isIdentifier(ref)) { ref.replaceWithText(name); } - } + }); } } @@ -247,7 +248,8 @@ function generateSentences(name: string, objects: ObjectBindingJson[]) { function replaceObjectBinding( node: ObjectBindingPattern, options: ReplaceObjectBindingOptions = { - scopes: ["component-top-level", "jsx"], + //scopes: ["component-top-level", "jsx"], + scopes: [], } ) { if (node.wasForgotten()) return; diff --git a/packages/codemod/src/utils/forEachLocalReference.ts b/packages/codemod/src/utils/forEachLocalReference.ts new file mode 100644 index 000000000..55a232f65 --- /dev/null +++ b/packages/codemod/src/utils/forEachLocalReference.ts @@ -0,0 +1,82 @@ +import { + ForEachDescendantTraversalControl, + Identifier, + Node, + ts, +} from "ts-morph" + +export function isScope(node: Node): boolean { + return ( + node.isKind(ts.SyntaxKind.Block) || + node.isKind(ts.SyntaxKind.FunctionDeclaration) || + node.isKind(ts.SyntaxKind.FunctionExpression) || + node.isKind(ts.SyntaxKind.ArrowFunction) || + // TODO more? + false + ) +} + +export function isDeclaration(node: Identifier): boolean { + const parent = node.getParent() + if (!parent) return false + return ( + ( + parent.isKind(ts.SyntaxKind.VariableDeclaration) || + parent.isKind(ts.SyntaxKind.Parameter) || + parent.isKind(ts.SyntaxKind.BindingElement) || + // TODO more? + false + ) && + parent.getNameNode() == node + ) +} + +/** + * Get the innermost scope which contains a given node + * @see https://github.com/mysticatea/eslint-utils/blob/master/src/get-innermost-scope.js + */ +export function getInnermostScope(node: Node, initialScope?: Node): Node | undefined { + if (!initialScope) initialScope = node.getSourceFile() + const location = node.getPos() + let scope = initialScope + initialScope.forEachDescendant(node => { + if ( + isScope(node) && + node.getPos() <= location && + location < node.getEnd() + ) { + scope = node + } + }) + return scope +} + +/** + * Invoke the cbNode callback for each reference in the local scope + * @see forEachDescendant + * @see https://github.com/dsherret/ts-morph/issues/1351 + */ +export default function forEachLocalReference(node: Identifier, + cbNode: (( + node: Node, + traversal: ForEachDescendantTraversalControl + ) => void) +) { + const scope = getInnermostScope(node) + if (!scope) return + const name = node.getText() + scope.forEachDescendant((node, traversal) => { + if ( + node.isKind(ts.SyntaxKind.Identifier) && + node.getText() == name + ) { + if (isDeclaration(node)) { + // name was redeclared + traversal.skip() + } + else { + cbNode(node, traversal) + } + } + }) +} diff --git a/packages/codemod/test/transforms/replaceObjectBinding.test.ts b/packages/codemod/test/transforms/replaceObjectBinding.test.ts index 56b40f0b8..b2e946aa9 100644 --- a/packages/codemod/test/transforms/replaceObjectBinding.test.ts +++ b/packages/codemod/test/transforms/replaceObjectBinding.test.ts @@ -7,7 +7,10 @@ const t = (code: string) => transform(code, [ (source) => { findObjectBindingPatterns(source).forEach((node) => - replaceObjectBinding(node, {}) + replaceObjectBinding( + node + //{} + ) ); }, ]); @@ -94,11 +97,11 @@ describe("replaceObjectBinding", () => { } `) ).toMatchInlineSnapshot(` - "function Component(params_0) { - var x = f(params_0); - var x = f(params_0); + "function Component(params) { + var x = f(params); + var x = f(params); // TODO(milahu): sort names - var x = f({ name2: params_0.name2, name1: params_0.name1, ...params_0.rest }); + var x = f({ name2: params.name2, name1: params.name1, ...params.rest }); } " `); @@ -166,4 +169,63 @@ describe("replaceObjectBinding", () => { " `); }); + it("keeps object prop names in complex code", () => { + expect( + t(` + import * as React from 'react' + type RendererProps = { + expanded: boolean + } + type Renderer = (props: RendererProps) => JSX.Element + function Expander({ expanded }: { expanded: boolean }) { + return
{expanded}
+ } + export const DefaultRenderer: Renderer = ({ + expanded = false, + }) => { + return ( + + ) + } + type ExplorerProps = Partial & { + renderer?: Renderer + defaultExpanded?: true | Record + } + export default function Explorer({ + defaultExpanded, + renderer = DefaultRenderer, + }: ExplorerProps) { + const [expanded, setExpanded] = React.useState(Boolean(defaultExpanded)) + return renderer({ + expanded, + }) + } + `) + ).toMatchInlineSnapshot(` + "import * as React from "react"; + type RendererProps = { + expanded: boolean; + }; + type Renderer = (props: RendererProps) => JSX.Element; + function Expander(params) { + return
{params.expanded}
; + } + export const DefaultRenderer: Renderer = (params) => { + return ; + }; + type ExplorerProps = Partial & { + renderer?: Renderer; + defaultExpanded?: true | Record; + }; + export default function Explorer(params) { + const [expanded, setExpanded] = React.useState( + Boolean(params.defaultExpanded) + ); + return params.renderer({ + expanded, + }); + } + " + `); + }); });