From 1d9a9fef76c095bd7b3b5f42e2d621e848248c4b Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 19 Dec 2024 12:01:53 +0100 Subject: [PATCH 1/6] CI: fix rust formatting `cargo fmt --all` should be avoided in CI as that will include `rust/ast-generator` which has sources provided by bazel (`bazel run //rust/ast-generator:inject_sources` can provide those sources in-tree). Now the formatting checks are limited to the sources that trigger the jobs, and a check is added to `rust/ast-generator`. --- .github/workflows/ql-for-ql-tests.yml | 2 +- .github/workflows/ruby-build.yml | 2 +- .github/workflows/rust.yml | 30 +++++++++++++++++-- .../workflows/tree-sitter-extractor-test.yml | 6 ++-- 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ql-for-ql-tests.yml b/.github/workflows/ql-for-ql-tests.yml index bc247165198a..4502dded53f4 100644 --- a/.github/workflows/ql-for-ql-tests.yml +++ b/.github/workflows/ql-for-ql-tests.yml @@ -40,7 +40,7 @@ jobs: ql/target key: ${{ runner.os }}-${{ steps.os_version.outputs.version }}-qltest-cargo-${{ hashFiles('ql/rust-toolchain.toml', 'ql/**/Cargo.lock') }} - name: Check formatting - run: cd ql; cargo fmt --all -- --check + run: cd ql; cargo fmt -- --check - name: Build extractor run: | cd ql; diff --git a/.github/workflows/ruby-build.yml b/.github/workflows/ruby-build.yml index b1ae2e2b7db1..343e896151c2 100644 --- a/.github/workflows/ruby-build.yml +++ b/.github/workflows/ruby-build.yml @@ -79,7 +79,7 @@ jobs: key: ${{ runner.os }}-${{ steps.os_version.outputs.version }}-ruby-rust-cargo-${{ hashFiles('ruby/extractor/rust-toolchain.toml', 'ruby/extractor/**/Cargo.lock') }} - name: Check formatting if: steps.cache-extractor.outputs.cache-hit != 'true' - run: cd extractor && cargo fmt --all -- --check + run: cd extractor && cargo fmt -- --check - name: Build if: steps.cache-extractor.outputs.cache-hit != 'true' run: cd extractor && cargo build --verbose diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index e5f4bb2140f5..75d3adc803be 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -23,22 +23,46 @@ permissions: contents: read jobs: - rust-code: + rust-ast-generator: runs-on: ubuntu-latest + defaults: + run: + working-directory: rust/ast-generator steps: - name: Checkout uses: actions/checkout@v4 + - name: Inject sources + shell: bash + run: | + bazel run //rust/ast-generator:inject_sources - name: Format - working-directory: rust/extractor shell: bash run: | cargo fmt --check - name: Compilation - working-directory: rust/extractor shell: bash run: cargo check - name: Clippy + shell: bash + run: | + cargo clippy --fix + git diff --exit-code + rust-code: + runs-on: ubuntu-latest + defaults: + run: working-directory: rust/extractor + steps: + - name: Checkout + uses: actions/checkout@v4 + - name: Format + shell: bash + run: | + cargo fmt --check + - name: Compilation + shell: bash + run: cargo check + - name: Clippy shell: bash run: | cargo clippy --fix diff --git a/.github/workflows/tree-sitter-extractor-test.yml b/.github/workflows/tree-sitter-extractor-test.yml index acc68e7ec2c7..9a71e1fc7c54 100644 --- a/.github/workflows/tree-sitter-extractor-test.yml +++ b/.github/workflows/tree-sitter-extractor-test.yml @@ -32,17 +32,17 @@ jobs: steps: - uses: actions/checkout@v4 - name: Check formatting - run: cargo fmt --all -- --check + run: cargo fmt -- --check - name: Run tests run: cargo test --verbose fmt: - runs-on: ubuntu-latest + runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - name: Check formatting run: cargo fmt --check clippy: - runs-on: ubuntu-latest + runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - name: Run clippy From 7f5b8fdcecce199e96145c1b272abe8cd7a7abce Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 19 Dec 2024 12:22:40 +0100 Subject: [PATCH 2/6] Rust: remove clippy warnings --- .github/workflows/rust.yml | 6 ++---- rust/ast-generator/BUILD.bazel | 2 +- rust/ast-generator/src/main.rs | 5 ++--- rust/lint.py | 10 ++++++---- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 75d3adc803be..c898843889d1 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -45,8 +45,7 @@ jobs: - name: Clippy shell: bash run: | - cargo clippy --fix - git diff --exit-code + cargo clippy -- -D warnings rust-code: runs-on: ubuntu-latest defaults: @@ -65,8 +64,7 @@ jobs: - name: Clippy shell: bash run: | - cargo clippy --fix - git diff --exit-code + cargo clippy -- -D warnings rust-codegen: runs-on: ubuntu-latest steps: diff --git a/rust/ast-generator/BUILD.bazel b/rust/ast-generator/BUILD.bazel index b9a4baf018d6..b5956d096067 100644 --- a/rust/ast-generator/BUILD.bazel +++ b/rust/ast-generator/BUILD.bazel @@ -75,7 +75,7 @@ write_file( 'DST_DIR="$(dirname "$(rlocation "$1")")"', 'mkdir -p "$DST_DIR/src/codegen/grammar"', ] + [ - 'cp "$(rlocation "$%s")" "$DST_DIR/%s"' % item + 'cp -f --no-preserve=mode "$(rlocation "$%s")" "$DST_DIR/%s"' % item for item in enumerate(_codegen_outs, 2) ], is_executable = True, diff --git a/rust/ast-generator/src/main.rs b/rust/ast-generator/src/main.rs index a0e524689c12..2cc77fe5fff1 100644 --- a/rust/ast-generator/src/main.rs +++ b/rust/ast-generator/src/main.rs @@ -9,8 +9,7 @@ use std::env; use ungrammar::Grammar; fn project_root() -> PathBuf { - let dir = - env::var("CARGO_MANIFEST_DIR").unwrap().to_owned(); + let dir = env::var("CARGO_MANIFEST_DIR").unwrap().to_owned(); PathBuf::from(dir).parent().unwrap().to_owned() } @@ -593,7 +592,7 @@ impl Translator<'_> {{ fn main() -> std::io::Result<()> { let grammar = PathBuf::from("..").join(env::args().nth(1).expect("grammar file path required")); let grammar: Grammar = fs::read_to_string(&grammar) - .expect(&format!("Failed to parse grammar file: {}", grammar.display())) + .unwrap_or_else(|_| panic!("Failed to parse grammar file: {}", grammar.display())) .parse() .expect("Failed to parse grammar"); let mut grammar = codegen::grammar::lower(&grammar); diff --git a/rust/lint.py b/rust/lint.py index 3a231c157df3..bb3109a0f501 100755 --- a/rust/lint.py +++ b/rust/lint.py @@ -10,9 +10,11 @@ cargo = shutil.which("cargo") assert cargo, "no cargo binary found on `PATH`" -fmt = subprocess.run([cargo, "fmt", "--all", "--quiet"], cwd=this_dir) +runs = [] +runs.append(subprocess.run([cargo, "fmt", "--all", "--quiet"], cwd=this_dir)) + for manifest in this_dir.rglob("Cargo.toml"): if not manifest.is_relative_to(this_dir / "ql") and not manifest.is_relative_to(this_dir / "integration-tests"): - clippy = subprocess.run([cargo, "clippy", "--fix", "--allow-dirty", "--allow-staged", "--quiet"], - cwd=manifest.parent) -sys.exit(fmt.returncode or clippy.returncode) + runs.append(subprocess.run([cargo, "clippy", "--fix", "--allow-dirty", "--allow-staged", "--quiet", "--", "-D", "warnings"], + cwd=manifest.parent)) +sys.exit(max(r.returncode for r in runs)) From 110d3994ea35c0bf5b17378af95eced4bc4ab5f7 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 19 Dec 2024 12:23:36 +0100 Subject: [PATCH 3/6] Rust: fix workflow --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index c898843889d1..c7f5a9d7f514 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -34,7 +34,7 @@ jobs: - name: Inject sources shell: bash run: | - bazel run //rust/ast-generator:inject_sources + bazel run //rust/ast-generator:inject-sources - name: Format shell: bash run: | From df39610029f168bb0d10b58d2f764d8993539bd9 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 19 Dec 2024 12:29:27 +0100 Subject: [PATCH 4/6] Rust: skip injected sources in clippy and fmt checks --- .../ast-generator/patches/rust-analyzer.patch | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/rust/ast-generator/patches/rust-analyzer.patch b/rust/ast-generator/patches/rust-analyzer.patch index dceb60fbf4c2..e799e90f48a4 100644 --- a/rust/ast-generator/patches/rust-analyzer.patch +++ b/rust/ast-generator/patches/rust-analyzer.patch @@ -1,16 +1,19 @@ diff --git a/xtask/src/codegen/grammar.rs b/xtask/src/codegen/grammar.rs -index e7534582f2..8bc9237737 100644 +index e7534582f2..49c96f1be3 100644 --- a/xtask/src/codegen/grammar.rs +++ b/xtask/src/codegen/grammar.rs -@@ -3,6 +3,7 @@ +@@ -3,7 +3,9 @@ //! Specifically, it generates the `SyntaxKind` enum and a number of newtype //! wrappers around `SyntaxNode` which implement `syntax::AstNode`. +-#![allow(clippy::disallowed_types)] +#![allow(warnings)] - #![allow(clippy::disallowed_types)] ++#![allow(clippy)] ++#![cfg_attr(any(), rustfmt::skip)] use std::{ -@@ -23,7 +24,7 @@ use crate::{ + collections::{BTreeSet, HashSet}, +@@ -23,7 +25,7 @@ use crate::{ project_root, }; @@ -19,7 +22,7 @@ index e7534582f2..8bc9237737 100644 use self::ast_src::{AstEnumSrc, AstNodeSrc, AstSrc, Cardinality, Field, KindsSrc}; pub(crate) fn generate(check: bool) { -@@ -624,7 +625,7 @@ fn pluralize(s: &str) -> String { +@@ -624,7 +626,7 @@ fn pluralize(s: &str) -> String { } impl Field { @@ -28,7 +31,7 @@ index e7534582f2..8bc9237737 100644 matches!(self, Field::Node { cardinality: Cardinality::Many, .. }) } fn token_kind(&self) -> Option { -@@ -636,7 +637,7 @@ impl Field { +@@ -636,7 +638,7 @@ impl Field { _ => None, } } @@ -37,7 +40,7 @@ index e7534582f2..8bc9237737 100644 match self { Field::Token(name) => { let name = match name.as_str() { -@@ -682,7 +683,7 @@ impl Field { +@@ -682,7 +684,7 @@ impl Field { } } } @@ -46,7 +49,7 @@ index e7534582f2..8bc9237737 100644 match self { Field::Token(_) => format_ident!("SyntaxToken"), Field::Node { ty, .. } => format_ident!("{}", ty), -@@ -699,7 +700,7 @@ fn clean_token_name(name: &str) -> String { +@@ -699,7 +701,7 @@ fn clean_token_name(name: &str) -> String { } } @@ -55,3 +58,16 @@ index e7534582f2..8bc9237737 100644 let mut res = AstSrc { tokens: "Whitespace Comment String ByteString CString IntNumber FloatNumber Char Byte Ident" +diff --git a/xtask/src/codegen/grammar/ast_src.rs b/xtask/src/codegen/grammar/ast_src.rs +index 9269d15423..babe7ca1bf 100644 +--- a/xtask/src/codegen/grammar/ast_src.rs ++++ b/xtask/src/codegen/grammar/ast_src.rs +@@ -1,5 +1,8 @@ + //! Defines input for code generation process. + ++#![allow(clippy)] ++#![cfg_attr(any(), rustfmt::skip)] ++ + use quote::ToTokens; + + use crate::codegen::grammar::to_upper_snake_case; From 2ff0394a10d3ce30ffad31bc51346fbd41ee5bce Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 19 Dec 2024 15:58:34 +0100 Subject: [PATCH 5/6] Rust: add `--no-deps` to `clippy` checks --- .github/workflows/rust.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index c7f5a9d7f514..cc880072555e 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -45,7 +45,7 @@ jobs: - name: Clippy shell: bash run: | - cargo clippy -- -D warnings + cargo clippy --no-deps -- -D warnings rust-code: runs-on: ubuntu-latest defaults: @@ -64,7 +64,7 @@ jobs: - name: Clippy shell: bash run: | - cargo clippy -- -D warnings + cargo clippy --no-deps -- -D warnings rust-codegen: runs-on: ubuntu-latest steps: From 8e28d99a629fdc794da6207e93abdcfcac5d1545 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Fri, 20 Dec 2024 10:42:19 +0100 Subject: [PATCH 6/6] QL for QL: accept test changes --- ql/ql/test/dataflow/getAStringValue/getAStringValue.expected | 2 -- 1 file changed, 2 deletions(-) diff --git a/ql/ql/test/dataflow/getAStringValue/getAStringValue.expected b/ql/ql/test/dataflow/getAStringValue/getAStringValue.expected index 48de9172b362..e69de29bb2d1 100644 --- a/ql/ql/test/dataflow/getAStringValue/getAStringValue.expected +++ b/ql/ql/test/dataflow/getAStringValue/getAStringValue.expected @@ -1,2 +0,0 @@ -failures -testFailures