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] Using the historical ./ path prefix to files breaks file discovery #1530

Closed
1 task done
PaulDance opened this issue Oct 16, 2024 · 4 comments
Closed
1 task done
Labels
bug Something isn't working

Comments

@PaulDance
Copy link

PaulDance commented Oct 16, 2024

Please read the FAQ for the bug you encountered.

  • I have read the existing FAQ

⏯ Playground Link

https://ast-grep.github.io/playground.html#eyJtb2RlIjoiQ29uZmlnIiwibGFuZyI6InJ1c3QiLCJxdWVyeSI6ImNvbnNvbGUubG9nKCRNQVRDSCkiLCJyZXdyaXRlIjoibG9nZ2VyLmxvZygkTUFUQ0gpIiwic3RyaWN0bmVzcyI6InNtYXJ0Iiwic2VsZWN0b3IiOiIiLCJjb25maWciOiJpZDogdGVzdFxuc2V2ZXJpdHk6IHdhcm5pbmdcbmxhbmd1YWdlOiBSdXN0XG5cbmZpbGVzOlxuICAtIFwiLi8qKi8qLnJzXCJcblxucnVsZTpcbiAga2luZDogaW50ZWdlcl9saXRlcmFsIiwic291cmNlIjoiZm4gZigpIHtcbiAgICBnKDEyMyk7XG59XG4ifQ==

💻 Code

Example Rust file under a/b.rs:

fn f() {
    g(123);
}

Non-matching rule under rule.yml:

id: test
severity: warning
language: Rust

files:
  - "./**/*.rs"

rule:
  kind: integer_literal

Matching rule:

id: test
severity: warning
language: Rust

files:
  - "**/*.rs"

rule:
  kind: integer_literal

🙁 Actual behavior

When running ast-grep scan -r rule.yml from the directory containing a, nothing is matched and no output is printed. When running the rule with the ./ removed from the file path glob, the discovery works again and a match is output:

$ sg scan -r rule.yml
warning[test]: 
  ┌─ a/b.rs:2:7

2 │     g(123);
  │       ^^^

🙂 Expected behavior

Both rule versions should now be identical and emit the same matches, or at least the ./ prefix should be outright refused, otherwise any analysis using the historically-enforced path prefix becomes entirely silent without notice.

Additional information about the issue

  • Observed in both 0.27.0 and 0.28.0.
  • The issue cannot be reproduced from the Playground as it seems that the files attribute is completely ignored: using file: - "whateveridontexist" still matches just fine; only locally will it occur.
  • It is not specific to Rust: at least observed with Python as well.
  • From the changelog, I see in 0.25.4: "feat: consistent file path for search by removing ./ prefix" [bug] File path determined by scan is not consistent #1343, or more specifically 54044fd, that could be at the root of this.
  • The documentation about this still considers the prefix to be valid, although not enforced anymore like it did previously if I remember correctly.
@HerringtonDarkholme
Copy link
Member

HerringtonDarkholme commented Oct 20, 2024

I think filepath should not contain ./ prefix. I will update the doc to reflect the ./ change

Thanks for the report.

@PaulDance
Copy link
Author

Sounds acceptable! Maybe a warning or even an error if the specific ./ prefix is present would be a good idea? It would enable a bit more visibility for the change and should be complementary to #1531.

@HerringtonDarkholme
Copy link
Member

--tracing should help users debugging the issue. https://ast-grep.github.io/reference/cli/run.html#tracing-level

This should somewhat overlap with #1531

@HerringtonDarkholme
Copy link
Member

The --inspect flag should solve this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants