Skip to content

Commit

Permalink
Add project-specific lints with ast-grep (vercel#5637)
Browse files Browse the repository at this point in the history
ast-grep[0] is a tool that uses tree-sitter[1] and a pattern language to
match against code using its ast. It implements code transformation,
querying, and linting.

Since clippy isn't extensible for project-specific lints, this adds a
first ast-grep lint disallowing `context` as a variable name. Currently
it's set to warning as usage is addressed.

To run, install ast-grep, then run `ast-grep scan`. Example output:

```
warning[no-context]: Don't name variables `context`.
    ┌─ ./crates/turbopack-ecmascript-hmr-protocol/src/lib.rs:132:9
    │
132 │     pub context: &'a str,
    │     ----^^^^^^^---------
    │
    = Use a more specific name, such as chunking_context, asset_context, etc.
```

[0] https://ast-grep.github.io
[1] https://tree-sitter.github.io/tree-sitter/
  • Loading branch information
wbinnssmith authored Aug 1, 2023
1 parent 01d4e02 commit f5c848b
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 0 deletions.
42 changes: 42 additions & 0 deletions .config/ast-grep/rule-tests/__snapshots__/no-context-snapshot.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions .config/ast-grep/rule-tests/no-context-test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
id: no-context
valid:
- "let chunking_context = ChunkingContext::new();"
- "struct Foo { chunking_context: Context };"
- "foo(|chunking_context| context)"
- "fn foo(chunking_context: ChunkingContext) -> u32 { 5 };"
invalid:
- "let context = ChunkingContext::new();"
- "struct Foo { context: Context };"
- "foo(|context| context)"
- "fn foo(context: ChunkingContext) -> u32 { 5 };"
Empty file.
20 changes: 20 additions & 0 deletions .config/ast-grep/rules/no-context.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
id: no-context
message: Don't name variables `context`.
note: Use a more specific name, such as chunking_context, asset_context, etc.
severity: warning
language: Rust
rule:
regex: \bcontext\b
any:
- all:
- inside:
any:
- kind: closure_parameters
- kind: parameter
- kind: function_type
- kind: let_declaration
- kind: identifier
- all:
- kind: field_identifier
- inside:
kind: field_declaration
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
.config/ast-grep/rule-tests/__snapshots__/** linguist-generated=true
crates/turbopack-tests/tests/snapshot/**/output/** linguist-generated=true
crates/turborepo-lockfiles/fixtures/*.lock text eol=lf
8 changes: 8 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,10 @@ jobs:
run: |
cargo groups clippy turborepo-libraries --features rustls-tls
- name: Run ast-grep lints
run: |
npx --package @ast-grep/cli -- ast-grep scan $(cargo groups list turborepo-libraries | awk '{ print $2 }' | tr '\n' ' ')
turbopack_rust_check:
needs: [determine_jobs]
# We test dependency changes only on main
Expand Down Expand Up @@ -569,6 +573,10 @@ jobs:
run: |
RUSTFLAGS="-D warnings -A deprecated" cargo groups clippy turbopack --features rustls-tls
- name: Run ast-grep lints
run: |
npx --package @ast-grep/cli -- ast-grep scan $(cargo groups list turbopack | awk '{ print $2 }' | tr '\n' ' ')
next_dev_check:
needs: [determine_jobs]
if: needs.determine_jobs.outputs.turbopack == 'true' || needs.determine_jobs.outputs.cargo_on_main == 'true'
Expand Down
6 changes: 6 additions & 0 deletions sgconfig.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
ruleDirs:
- .config/ast-grep/rules
testConfigs:
- testDir: .config/ast-grep/rule-tests
utilDirs:
- .config/ast-grep/rule-utils

0 comments on commit f5c848b

Please sign in to comment.