From dd35e087253b313974d2b252b719704d57a9baab Mon Sep 17 00:00:00 2001 From: Will Binns-Smith Date: Fri, 28 Jul 2023 15:34:57 -0700 Subject: [PATCH 1/4] Add project-specific lints with ast-grep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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/ --- .../__snapshots__/no-context-snapshot.yml | 42 +++++++++++++++++++ .../ast-grep/rule-tests/no-context-test.yml | 11 +++++ .config/ast-grep/rules/no-context.yml | 20 +++++++++ .gitattributes | 1 + sgconfig.yml | 6 +++ 5 files changed, 80 insertions(+) create mode 100644 .config/ast-grep/rule-tests/__snapshots__/no-context-snapshot.yml create mode 100644 .config/ast-grep/rule-tests/no-context-test.yml create mode 100644 .config/ast-grep/rules/no-context.yml create mode 100644 sgconfig.yml diff --git a/.config/ast-grep/rule-tests/__snapshots__/no-context-snapshot.yml b/.config/ast-grep/rule-tests/__snapshots__/no-context-snapshot.yml new file mode 100644 index 0000000000000..ff36ca0094d1f --- /dev/null +++ b/.config/ast-grep/rule-tests/__snapshots__/no-context-snapshot.yml @@ -0,0 +1,42 @@ +id: no-context +snapshots: + "fn foo(context: ChunkingContext) -> u32 { 5 };": + labels: + - source: context + style: primary + start: 7 + end: 14 + - source: "context: ChunkingContext" + style: secondary + start: 7 + end: 31 + foo(|context| context): + labels: + - source: context + style: primary + start: 5 + end: 12 + - source: "|context|" + style: secondary + start: 4 + end: 13 + let context = ChunkingContext::new();: + labels: + - source: context + style: primary + start: 4 + end: 11 + - source: let context = ChunkingContext::new(); + style: secondary + start: 0 + end: 37 + "struct Foo { context: Context };": + labels: + - source: context + style: primary + start: 13 + end: 20 + - source: "context: Context" + style: secondary + start: 13 + end: 29 diff --git a/.config/ast-grep/rule-tests/no-context-test.yml b/.config/ast-grep/rule-tests/no-context-test.yml new file mode 100644 index 0000000000000..ed8ebf148087b --- /dev/null +++ b/.config/ast-grep/rule-tests/no-context-test.yml @@ -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 };" diff --git a/.config/ast-grep/rules/no-context.yml b/.config/ast-grep/rules/no-context.yml new file mode 100644 index 0000000000000..b7f496e459454 --- /dev/null +++ b/.config/ast-grep/rules/no-context.yml @@ -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 diff --git a/.gitattributes b/.gitattributes index af8824eb9b77e..bd24553281a47 100644 --- a/.gitattributes +++ b/.gitattributes @@ -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 diff --git a/sgconfig.yml b/sgconfig.yml new file mode 100644 index 0000000000000..ae6f77b0db445 --- /dev/null +++ b/sgconfig.yml @@ -0,0 +1,6 @@ +ruleDirs: + - .config/ast-grep/rules +testConfigs: + - testDir: .config/ast-grep/rule-tests +utilDirs: + - .config/ast-grep/rule-utils From daa61bb59864c75cf20cda777afa5dd9414eea28 Mon Sep 17 00:00:00 2001 From: Will Binns-Smith Date: Fri, 28 Jul 2023 15:40:00 -0700 Subject: [PATCH 2/4] fixup! Add project-specific lints with ast-grep --- .config/ast-grep/rule-utils/.gitkeep | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 .config/ast-grep/rule-utils/.gitkeep diff --git a/.config/ast-grep/rule-utils/.gitkeep b/.config/ast-grep/rule-utils/.gitkeep new file mode 100644 index 0000000000000..e69de29bb2d1d From c82491e856658ec6d5855f3d21d5a417ac7881fd Mon Sep 17 00:00:00 2001 From: Will Binns-Smith Date: Mon, 31 Jul 2023 10:04:43 -0700 Subject: [PATCH 3/4] Run in CI --- .github/workflows/test.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7d94b321dd3b5..1a0905967b45e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -518,6 +518,10 @@ jobs: run: | cargo groups clippy turborepo-libraries --features rustls-tls + - name: Run ast-grep lints + run: | + pnpm --package=@ast-grep/cli dlx 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 @@ -569,6 +573,10 @@ jobs: run: | RUSTFLAGS="-D warnings -A deprecated" cargo groups clippy turbopack --features rustls-tls + - name: Run ast-grep lints + run: | + pnpm --package=@ast-grep/cli dlx 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' From b49048e7d454949f2665cfa4426450e207e43fff Mon Sep 17 00:00:00 2001 From: Will Binns-Smith Date: Mon, 31 Jul 2023 10:21:53 -0700 Subject: [PATCH 4/4] fixup! Run in CI --- .github/workflows/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1a0905967b45e..3b55ca38c9740 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -520,7 +520,7 @@ jobs: - name: Run ast-grep lints run: | - pnpm --package=@ast-grep/cli dlx ast-grep scan $(cargo groups list turborepo-libraries | awk '{ print $2 }' | tr '\n' ' ') + npx --package @ast-grep/cli -- ast-grep scan $(cargo groups list turborepo-libraries | awk '{ print $2 }' | tr '\n' ' ') turbopack_rust_check: needs: [determine_jobs] @@ -575,7 +575,7 @@ jobs: - name: Run ast-grep lints run: | - pnpm --package=@ast-grep/cli dlx ast-grep scan $(cargo groups list turbopack | awk '{ print $2 }' | tr '\n' ' ') + npx --package @ast-grep/cli -- ast-grep scan $(cargo groups list turbopack | awk '{ print $2 }' | tr '\n' ' ') next_dev_check: needs: [determine_jobs]