Skip to content

fix: handle property accesses cases on resolve-binding-path #218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: handle property accesses cases on resolve-binding-path #218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3f9b1d9
fix: handle property accesses cases on resolve-binding-path
brunocroh Sep 28, 2025
e5dedd0
fix: add more test case to resolve-bind-path
brunocroh Sep 28, 2025
5abdc19
add ESM namespace test scenario
brunocroh Sep 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 121 additions & 16 deletions utils/src/ast-grep/resolve-binding-path.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import assert from "node:assert/strict";
import { describe, it } from "node:test";
import astGrep from "@ast-grep/napi";
import dedent from "dedent";
import type Js from "@codemod.com/jssg-types/langs/javascript";
import type { SgNode } from "@codemod.com/jssg-types/main";

import { resolveBindingPath } from "./resolve-binding-path.ts";

Expand All @@ -12,7 +14,7 @@ describe("resolve-binding-path", () => {
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const importStatement = rootNode.root().find({
const importStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "lexical_declaration",
},
Expand All @@ -23,13 +25,30 @@ describe("resolve-binding-path", () => {
assert.strictEqual(bindingPath, "util.types.isNativeError");
});

it("should be able to resolve binding path from namespace ESM import", () => {
const code = dedent`
import foo from "node:foo"
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const importStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "import_statement",
},
});

const bindingPath = resolveBindingPath(importStatement!, "$.bar");

assert.strictEqual(bindingPath, "foo.bar");
});

it("should be able to solve binding path when destructuring happen", () => {
const code = dedent`
const { types } = require('node:util');
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const importStatement = rootNode.root().find({
const importStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "variable_declarator",
},
Expand All @@ -46,7 +65,7 @@ describe("resolve-binding-path", () => {
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const requireStatement = rootNode.root().find({
const requireStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "variable_declarator",
},
Expand All @@ -63,7 +82,7 @@ describe("resolve-binding-path", () => {
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const importStatement = rootNode.root().find({
const importStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "variable_declarator",
},
Expand All @@ -80,7 +99,7 @@ describe("resolve-binding-path", () => {
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const functionDeclaration = rootNode.root().find({
const functionDeclaration = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "function_declaration",
},
Expand All @@ -95,7 +114,7 @@ describe("resolve-binding-path", () => {
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const importStatement = rootNode.root().find({
const importStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "import_statement",
},
Expand All @@ -112,7 +131,7 @@ describe("resolve-binding-path", () => {
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const importStatement = rootNode.root().find({
const importStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "import_statement",
},
Expand All @@ -129,7 +148,7 @@ describe("resolve-binding-path", () => {
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const importStatement = rootNode.root().find({
const importStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "import_statement",
},
Expand All @@ -146,7 +165,7 @@ describe("resolve-binding-path", () => {
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const importStatement = rootNode.root().find({
const importStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "import_statement",
},
Expand All @@ -163,7 +182,7 @@ describe("resolve-binding-path", () => {
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const requireStatement = rootNode.root().find({
const requireStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "variable_declarator",
},
Expand All @@ -180,7 +199,7 @@ describe("resolve-binding-path", () => {
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const importStatement = rootNode.root().find({
const importStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "lexical_declaration",
},
Expand All @@ -197,7 +216,7 @@ describe("resolve-binding-path", () => {
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const importStatement = rootNode.root().find({
const importStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "import_statement",
},
Expand All @@ -214,7 +233,7 @@ describe("resolve-binding-path", () => {
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const requireStatement = rootNode.root().find({
const requireStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "variable_declarator",
},
Expand All @@ -231,7 +250,7 @@ describe("resolve-binding-path", () => {
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const requireStatement = rootNode.root().find({
const requireStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "variable_declarator",
},
Expand All @@ -248,7 +267,7 @@ describe("resolve-binding-path", () => {
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const requireStatement = rootNode.root().find({
const requireStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "variable_declarator",
},
Expand All @@ -265,7 +284,8 @@ describe("resolve-binding-path", () => {
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const requireStatement = rootNode.root().find({

const requireStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "variable_declarator",
},
Expand All @@ -275,4 +295,89 @@ describe("resolve-binding-path", () => {

assert.strictEqual(bindingPath, "types.isNativeError");
});

it("should resolve correctly when have member-expression", () => {
const code = dedent`
const SlowBuffer = require('buffer').SlowBuffer;
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const requireStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "variable_declarator",
},
});

const bindingPath = resolveBindingPath(requireStatement!, "$.SlowBuffer");

assert.strictEqual(bindingPath, "SlowBuffer");
});

it("should resolve correctly when there are multiple property accesses", () => {
const code = dedent`
const variable = require('buffer').a.b;
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const requireStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "variable_declarator",
},
});

const bindingPath = resolveBindingPath(requireStatement!, "$.a.b");

assert.strictEqual(bindingPath, "variable");
});

it("should resolve correctly when there are multiple property accesses but not the entire path", () => {
const code = dedent`
const variable = require('buffer').a.b;
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const requireStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "variable_declarator",
},
});

const bindingPath = resolveBindingPath(requireStatement!, "$.a.b.c.d.e");

assert.strictEqual(bindingPath, "variable.c.d.e");
});

it("should resolve correctly when there are multiple property accesses and destructuring", () => {
const code = dedent`
const { c: { d } } = require('buffer').a.b;
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const requireStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "variable_declarator",
},
});

const bindingPath = resolveBindingPath(requireStatement!, "$.a.b.c.d.e");

assert.strictEqual(bindingPath, "d.e");
});

it("should resolve as undefined when property accesses is different than path to solve", () => {
const code = dedent`
const c = require('buffer').a.g.c;
`;

const rootNode = astGrep.parse(astGrep.Lang.JavaScript, code);
const requireStatement = (rootNode.root() as SgNode<Js>).find({
rule: {
kind: "variable_declarator",
},
});

const bindingPath = resolveBindingPath(requireStatement!, "$.a.b.c.d.e");

assert.strictEqual(bindingPath, undefined);
});
});
25 changes: 25 additions & 0 deletions utils/src/ast-grep/resolve-binding-path.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,31 @@ function resolveBindingPathRequire(node: SgNode<Js>, path: string) {
});
}

const propertyAccesses = activeNode.findAll({
rule: {
kind: "property_identifier",
inside: {
kind: "member_expression",
},
},
});

if (propertyAccesses.length) {
const pathArr = path.split(".");
let newPath = ["$"];
Copy link
Member

Choose a reason for hiding this comment

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

nit: I think newPath is actually constant?

let i = 0;

for (; i < propertyAccesses.length; i++) {
Comment on lines +83 to +85
Copy link
Member

Choose a reason for hiding this comment

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

i will actually be scoped to the parent of for automatically, so extracting it like this is unnecessary, but perhaps you did it to make it more obvious that the parent scoping is necessary/intended?

// pathArr[i+1] to skip the first element (which is $) that was used for binding replacement
if (propertyAccesses[i]?.text() !== pathArr[i + 1]) {
return undefined;
}
}

// Get the remaining path that was not used in propertyAccesses
path = newPath.concat(pathArr.splice(i + 1)).join(".");
}

activeNode = activeNode.child(0);

if (activeNode?.kind() === "identifier") {
Expand Down