Skip to content
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.

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

[bug] napi.findFiles() issue: SgNode for file A apparently references text from file B #1380

Closed
kyegupov opened this issue Aug 5, 2024 · 8 comments
Labels
bug Something isn't working Needs Reproduction

Comments

@kyegupov
Copy link

kyegupov commented Aug 5, 2024

Idea: use ast-grep napi programmatically.

💻 Code

  const matches = await ts.findInFiles(
    {
      paths: ["../src"],
      matcher: {
        rule: { pattern: "yup.object($OBJ)" },
      },
    },
    processMatch,
  );

function processMatch(err: any, sgNodes: SgNode[]) {
  for (const sgNode of sgNodes) {
    console.log(sgNode.getRoot().filename());
    const obj = sgNode.getMatch("OBJ")!;
    console.log(sgNode.range());
    console.log(obj.range());
     // THIS PRINTS CORRECT CODE:
    console.log(sgNode.text());
    // THIS PRINTS EITHER GARBAGE (text from some other file or some Chinese characters)
    // OR CRASHES (see the message below)
    console.log(obj.text());
  }
}

crash messages are like this:

thread '<unnamed>' panicked at crates/napi/src/doc.rs:95:36:
range end index 1768 out of range for slice of length 1349

🙁 Actual behavior

See comments in the code above

🙂 Expected behavior

The code above should print the correct source code.

@kyegupov kyegupov added the bug Something isn't working label Aug 5, 2024
@HerringtonDarkholme
Copy link
Member

HerringtonDarkholme commented Aug 5, 2024

Can you also share the file you are scanning?

ast-grep -p 'yup.object($OBJ)' -l ts

this command should help you find the file

@HerringtonDarkholme
Copy link
Member

There should be several ways to debug this issue:

  1. it could be a concurrency issue. Can you reproduce it reliably? How many files are you scanning?
  2. it could be a text encoding issue. Can you find the file you are using non-ascii code?

You can try reduce the issue by deleting files in the repo and find the minimal reproduction.

@harrisonzhu-db
Copy link

harrisonzhu-db commented Aug 15, 2024

running into a similar issue. i ran this on my codebase and got the same error message as the original issue.

I get a segfault when trying to getMatch an existing metavariable with a reproducible version w/ one file.

import { tsx } from '@ast-grep/napi';

// this file will parse itself for simplicity lol

const funcWithArg = (arg: string) => arg;
funcWithArg('hello world');

async function main() {
  tsx.findInFiles({ paths: [__filename], matcher: { rule: { pattern: 'funcWithArg($KEY)' } } }, (err, results) => {
    if (err) {
      console.error(err);
      return;
    }
    results.forEach((result) => {
      // prints `matched undefined`
      console.log(`matched ${result.getMatch('NONEXISTENTKEY')?.text()}`);
      // segfaults :/
      console.log(`matched ${result.getMatch('KEY')?.text()}`); 
    });
  });
}

main();

This gives

❯ bin/test-ast-grep
matched undefined
[1]    48095 segmentation fault  bin/test-ast-grep

@HerringtonDarkholme
Copy link
Member

@harrisonzhu-db would you like to provide a minimal reproduction? Usually it can be done by deleting scanned code.

it could be a concurrency issue. Can you reproduce it reliably? How many files are you scanning?
it could be a text encoding issue. Can you find the file you are using non-ascii code?

@harrisonzhu-db
Copy link

@harrisonzhu-db would you like to provide a minimal reproduction? Usually it can be done by deleting scanned code.

the bug should be self-contained within the code I pasted - the script is using ast-grep to scan itself, so that should be all you need

@HerringtonDarkholme
Copy link
Member

Thanks for the report. I found the root issue now.

@HerringtonDarkholme
Copy link
Member

Fixed in main branch

image

@harrisonzhu-db
Copy link

very awesome! thanks for this

tmeijn pushed a commit to tmeijn/dotfiles that referenced this issue Aug 20, 2024
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [ast-grep/ast-grep](https://github.com/ast-grep/ast-grep) | patch | `0.26.1` -> `0.26.2` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>ast-grep/ast-grep (ast-grep/ast-grep)</summary>

### [`v0.26.2`](https://github.com/ast-grep/ast-grep/blob/HEAD/CHANGELOG.md#0262)

[Compare Source](ast-grep/ast-grep@0.26.1...0.26.2)

-   fix: readopt matched metavar in napi [`#1380`](ast-grep/ast-grep#1380)
-   chore: update napi definition [`07e084a`](ast-grep/ast-grep@07e084a)
-   fix(deps): update dependency [@&#8203;swc/core](https://github.com/swc/core) to v1.7.11 [`835b06f`](ast-grep/ast-grep@835b06f)
-   fix(deps): update dependency [@&#8203;swc/core](https://github.com/swc/core) to v1.7.10 [`d7a3820`](ast-grep/ast-grep@d7a3820)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Needs Reproduction
Projects
None yet
Development

No branches or pull requests

3 participants