From 7ae0668b342b73e92a444ff23620d5875d057a46 Mon Sep 17 00:00:00 2001 From: Joshua Wink Date: Wed, 28 Oct 2026 10:20:43 -0500 Subject: [PATCH] feat(rust): a linter for Rust code that uses codeupipe packages/codeupipe-lint-rs: codeupipe-lint, a codeupipe pipeline of 11 Filters that carries a LintRun of its own in place of a Payload. It reads the source with syn and checks the rules that the Rust compiler does not: CUP-I000, A003, A005, I003, I005, I006, I007, and the warnings I008 and I009. The ids, the report, the exit codes, the markers and cup.config.json are those of the Python and TypeScript linters. It is a Cargo workspace of its own, so the runtime still builds offline. A new CI job runs fmt, clippy, the tests, the linter with --strict on all the Rust code of the repository, and Rust 1.75. The Rust code passes it: step types declared in test functions moved to the top of their files, and cup: allow-plain markers with a reason on the runtime's own types, the conformance runner and the linter's helpers. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 25 ++ .gitignore | 1 + README.md | 14 +- conformance/runners/rust/src/bench.rs | 4 + conformance/runners/rust/src/cup_ops/sink.rs | 1 + conformance/runners/rust/src/json.rs | 4 + conformance/runners/rust/src/main.rs | 7 + docs/rust-plan.md | 37 ++- packages/codeupipe-lint-rs/Cargo.lock | 123 ++++++++ packages/codeupipe-lint-rs/Cargo.toml | 27 ++ packages/codeupipe-lint-rs/LICENSE | 177 ++++++++++++ packages/codeupipe-lint-rs/NOTICE | 4 + packages/codeupipe-lint-rs/README.md | 161 +++++++++++ .../src/analyze_plain_logic_filter.rs | 125 ++++++++ .../src/analyze_primitive_names_filter.rs | 81 ++++++ .../src/analyze_primitives_per_file_filter.rs | 41 +++ .../src/analyze_stale_markers_filter.rs | 46 +++ .../src/analyze_state_reads_filter.rs | 220 +++++++++++++++ .../src/analyze_steps_filter.rs | 30 ++ .../src/apply_suppressions_filter.rs | 71 +++++ .../src/collect_files_filter.rs | 63 +++++ .../src/drop_disabled_filter.rs | 20 ++ packages/codeupipe-lint-rs/src/finding.rs | 28 ++ packages/codeupipe-lint-rs/src/fnmatch.rs | 68 +++++ .../src/format_report_filter.rs | 62 ++++ packages/codeupipe-lint-rs/src/lib.rs | 46 +++ packages/codeupipe-lint-rs/src/lint_paths.rs | 26 ++ .../codeupipe-lint-rs/src/lint_pipeline.rs | 33 +++ packages/codeupipe-lint-rs/src/lint_run.rs | 38 +++ packages/codeupipe-lint-rs/src/logic.rs | 48 ++++ packages/codeupipe-lint-rs/src/main.rs | 108 +++++++ packages/codeupipe-lint-rs/src/markers.rs | 169 +++++++++++ .../src/parse_files_filter.rs | 53 ++++ .../codeupipe-lint-rs/src/primitive_kind.rs | 182 ++++++++++++ packages/codeupipe-lint-rs/src/read_config.rs | 67 +++++ packages/codeupipe-lint-rs/src/rules.rs | 119 ++++++++ packages/codeupipe-lint-rs/src/source.rs | 53 ++++ .../codeupipe-lint-rs/src/step_visitor.rs | 266 ++++++++++++++++++ .../tests/anonymous_steps.rs | 177 ++++++++++++ packages/codeupipe-lint-rs/tests/cli.rs | 169 +++++++++++ .../codeupipe-lint-rs/tests/common/mod.rs | 69 +++++ .../codeupipe-lint-rs/tests/invariants.rs | 206 ++++++++++++++ .../codeupipe-lint-rs/tests/plain_logic.rs | 239 ++++++++++++++++ .../codeupipe-lint-rs/tests/stale_markers.rs | 91 ++++++ packages/codeupipe-rs/README.md | 6 +- packages/codeupipe-rs/src/map.rs | 2 + packages/codeupipe-rs/src/payload.rs | 1 + packages/codeupipe-rs/src/pipeline.rs | 3 + packages/codeupipe-rs/src/state.rs | 2 + packages/codeupipe-rs/src/value.rs | 2 + packages/codeupipe-rs/tests/pipeline.rs | 47 ++-- packages/codeupipe-rs/tests/state_per_run.rs | 13 +- packages/codeupipe-rs/tests/step_label.rs | 14 +- packages/codeupipe-rs/tests/typed_payload.rs | 31 +- 54 files changed, 3661 insertions(+), 59 deletions(-) create mode 100644 packages/codeupipe-lint-rs/Cargo.lock create mode 100644 packages/codeupipe-lint-rs/Cargo.toml create mode 100644 packages/codeupipe-lint-rs/LICENSE create mode 100644 packages/codeupipe-lint-rs/NOTICE create mode 100644 packages/codeupipe-lint-rs/README.md create mode 100644 packages/codeupipe-lint-rs/src/analyze_plain_logic_filter.rs create mode 100644 packages/codeupipe-lint-rs/src/analyze_primitive_names_filter.rs create mode 100644 packages/codeupipe-lint-rs/src/analyze_primitives_per_file_filter.rs create mode 100644 packages/codeupipe-lint-rs/src/analyze_stale_markers_filter.rs create mode 100644 packages/codeupipe-lint-rs/src/analyze_state_reads_filter.rs create mode 100644 packages/codeupipe-lint-rs/src/analyze_steps_filter.rs create mode 100644 packages/codeupipe-lint-rs/src/apply_suppressions_filter.rs create mode 100644 packages/codeupipe-lint-rs/src/collect_files_filter.rs create mode 100644 packages/codeupipe-lint-rs/src/drop_disabled_filter.rs create mode 100644 packages/codeupipe-lint-rs/src/finding.rs create mode 100644 packages/codeupipe-lint-rs/src/fnmatch.rs create mode 100644 packages/codeupipe-lint-rs/src/format_report_filter.rs create mode 100644 packages/codeupipe-lint-rs/src/lib.rs create mode 100644 packages/codeupipe-lint-rs/src/lint_paths.rs create mode 100644 packages/codeupipe-lint-rs/src/lint_pipeline.rs create mode 100644 packages/codeupipe-lint-rs/src/lint_run.rs create mode 100644 packages/codeupipe-lint-rs/src/logic.rs create mode 100644 packages/codeupipe-lint-rs/src/main.rs create mode 100644 packages/codeupipe-lint-rs/src/markers.rs create mode 100644 packages/codeupipe-lint-rs/src/parse_files_filter.rs create mode 100644 packages/codeupipe-lint-rs/src/primitive_kind.rs create mode 100644 packages/codeupipe-lint-rs/src/read_config.rs create mode 100644 packages/codeupipe-lint-rs/src/rules.rs create mode 100644 packages/codeupipe-lint-rs/src/source.rs create mode 100644 packages/codeupipe-lint-rs/src/step_visitor.rs create mode 100644 packages/codeupipe-lint-rs/tests/anonymous_steps.rs create mode 100644 packages/codeupipe-lint-rs/tests/cli.rs create mode 100644 packages/codeupipe-lint-rs/tests/common/mod.rs create mode 100644 packages/codeupipe-lint-rs/tests/invariants.rs create mode 100644 packages/codeupipe-lint-rs/tests/plain_logic.rs create mode 100644 packages/codeupipe-lint-rs/tests/stale_markers.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d41752a..de79c58 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -99,6 +99,31 @@ jobs: rustup toolchain install 1.75 --profile minimal cargo +1.75 test --offline -p codeupipe + rust-lint: + name: Rust linter + runs-on: ubuntu-latest + defaults: + run: + working-directory: packages/codeupipe-lint-rs + steps: + - uses: actions/checkout@v5 + # A workspace of its own: it downloads syn and serde_json from + # crates.io, at the versions in its Cargo.lock. + - name: Format + run: cargo fmt --check + - name: Clippy + run: cargo clippy --locked --all-targets -- -D warnings + - name: Tests + run: cargo test --locked + - name: The linter on the Rust code of this repository + run: >- + cargo run --locked -q -- --strict src tests ../codeupipe-rs/src + ../codeupipe-rs/examples ../codeupipe-rs/tests ../../conformance/runners/rust/src + - name: The oldest Rust that the crate supports (rust-version) + run: | + rustup toolchain install 1.75 --profile minimal + cargo +1.75 test --locked + export: name: Export against dynamic run runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index 70a8784..003e60c 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ scorecard/results/ node_modules/ /target/ +/packages/codeupipe-lint-rs/target/ diff --git a/README.md b/README.md index c41ab18..9426022 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ If you know Unix, these are close to things you know. | Hook | A git hook: code that runs before and after a step | | State | `$?` and `PIPESTATUS` | -This repository has six packages. +This repository has seven packages. | Package | Contents | Reference | |---|---|---| @@ -37,7 +37,8 @@ This repository has six packages. | `codeupipe` for TypeScript | The eight primitives. No dependencies | [packages/codeupipe-ts/README.md](packages/codeupipe-ts/README.md) | | `codeupipe-lint` for TypeScript | A linter for pipeline code in TypeScript | [packages/codeupipe-lint-ts/README.md](packages/codeupipe-lint-ts/README.md) | | `codeupipe-compile` for TypeScript | Writes a pipeline as plain TypeScript with no import of codeupipe | [packages/codeupipe-compile-ts/README.md](packages/codeupipe-compile-ts/README.md) | -| `codeupipe` for Rust | The primitives, 0.1.0. No dependencies. No compiler or linter yet | [packages/codeupipe-rs/README.md](packages/codeupipe-rs/README.md) | +| `codeupipe` for Rust | The primitives, 0.1.0. No dependencies. No compiler yet | [packages/codeupipe-rs/README.md](packages/codeupipe-rs/README.md) | +| `codeupipe-lint` for Rust | A linter for pipeline code in Rust, for the rules that the compiler does not check | [packages/codeupipe-lint-rs/README.md](packages/codeupipe-lint-rs/README.md) | ## Install @@ -205,7 +206,8 @@ An editable install needs pip 21.3 or later. | `packages/codeupipe-ts/` | The TypeScript runtime, with its tests and its example | | `packages/codeupipe-lint-ts/` | The TypeScript linter, with its tests | | `packages/codeupipe-compile-ts/` | The TypeScript compiler, with its tests | -| `packages/codeupipe-rs/` | The Rust runtime, with its tests and its example | +| `packages/codeupipe-rs/` | The Rust runtime, with its tests and its examples | +| `packages/codeupipe-lint-rs/` | The Rust linter, with its tests. A Cargo workspace of its own | | `conformance/` | The contract ([API.md](conformance/API.md)), the cases, the runners, the benchmark | | `tests/` | The tests for the two Python packages | | `examples/pricing/` | The example, with its test in `tests/examples/` | @@ -215,7 +217,9 @@ An editable install needs pip 21.3 or later. The root `pyproject.toml` is not a package. It holds the test settings. The root `package.json` is not a package: it makes the three TypeScript packages one npm workspace. The root `Cargo.toml` is not a package either: it makes -the Rust runtime and its conformance runner one Cargo workspace. +the Rust runtime and its conformance runner one Cargo workspace, which +builds with no download. The Rust linter needs `syn`, so it is a workspace +of its own. ## Other languages @@ -223,7 +227,7 @@ The contract and the conformance cases do not depend on a language. Python is the first runtime and TypeScript is the second ([packages/codeupipe-ts/](packages/codeupipe-ts/)), with its own linter and its own compiler. Rust is the third -([packages/codeupipe-rs/](packages/codeupipe-rs/)); its plan is +([packages/codeupipe-rs/](packages/codeupipe-rs/)), with its own linter; its plan is [docs/rust-plan.md](docs/rust-plan.md). A runtime in another language must implement the contract and add a conformance runner. diff --git a/conformance/runners/rust/src/bench.rs b/conformance/runners/rust/src/bench.rs index ed0ec8e..efb6900 100644 --- a/conformance/runners/rust/src/bench.rs +++ b/conformance/runners/rust/src/bench.rs @@ -19,6 +19,7 @@ fn reference_write(data: &mut HashMap, key: &str) { /// The reference workload: ten map writes behind ten function calls, with no /// framework. A pipeline's time is reported relative to it. +// cup: allow-plain the timing loop of the bench target: it measures the pipeline, so it is not a step of it fn reference() -> HashMap { let mut data = HashMap::from([("a".to_string(), 1.0)]); for key in REFERENCE_KEYS { @@ -27,6 +28,7 @@ fn reference() -> HashMap { data } +// cup: allow-plain the timing loop of the bench target: it measures the pipeline, so it is not a step of it fn timed(f: &mut impl FnMut()) -> Duration { let started = Instant::now(); for _ in 0..RUNS { @@ -38,6 +40,7 @@ fn timed(f: &mut impl FnMut()) -> Duration { /// (µs per run, time relative to the reference workload). µs per run is the /// best of REPEATS x RUNS. Each repeat times the reference and then `f`, /// back to back, and the median of the ratios is reported. +// cup: allow-plain the timing loop of the bench target: it measures the pipeline, so it is not a step of it fn measure(mut f: impl FnMut()) -> (f64, f64) { let mut reference_run = || { black_box(reference()); @@ -60,6 +63,7 @@ fn measure(mut f: impl FnMut()) -> (f64, f64) { } /// Time one scenario. `pipeline` is built from its steps; `input` is its input. +// cup: allow-plain the timing loop of the bench target: it measures the pipeline, so it is not a step of it pub fn bench(id: Value, pipeline: Pipeline, input: codeupipe::Payload) -> Value { let output = match pipeline.run(&input) { Ok(output) => output.to_dict(), diff --git a/conformance/runners/rust/src/cup_ops/sink.rs b/conformance/runners/rust/src/cup_ops/sink.rs index eb1f6d2..56c4273 100644 --- a/conformance/runners/rust/src/cup_ops/sink.rs +++ b/conformance/runners/rust/src/cup_ops/sink.rs @@ -29,6 +29,7 @@ pub fn set_run_again(run_again: Option) { RUN_AGAIN.with(|slot| slot.replace(run_again)); } +// cup: allow-plain the edge of the runner: it runs the pipeline again for the run_again op pub fn run_again(input: Map) -> Result<(), StepError> { // Taken out of the cell first: the run that it starts reads the cell too. let run_again = RUN_AGAIN.with(|slot| slot.borrow().clone()); diff --git a/conformance/runners/rust/src/json.rs b/conformance/runners/rust/src/json.rs index ca7f163..4ff8e5d 100644 --- a/conformance/runners/rust/src/json.rs +++ b/conformance/runners/rust/src/json.rs @@ -4,6 +4,7 @@ use codeupipe::{Map, Value}; /// Read one JSON value from `text`. +// cup: allow-plain the JSON reader and writer of the runner, which must not download a crate pub fn read(text: &str) -> Result { let mut reader = Reader { text, at: 0 }; let value = reader.value()?; @@ -21,6 +22,7 @@ pub fn write(value: &Value) -> String { out } +// cup: allow-plain the JSON reader and writer of the runner, which must not download a crate fn write_into(value: &Value, out: &mut String) { match value { Value::Null => out.push_str("null"), @@ -54,6 +56,7 @@ fn write_into(value: &Value, out: &mut String) { } } +// cup: allow-plain the JSON reader and writer of the runner, which must not download a crate fn write_string(s: &str, out: &mut String) { out.push('"'); for c in s.chars() { @@ -75,6 +78,7 @@ struct Reader<'a> { at: usize, } +// cup: allow-plain the JSON reader and writer of the runner, which must not download a crate impl Reader<'_> { fn problem(&self, what: &str) -> String { format!("not JSON at byte {}: {what}", self.at) diff --git a/conformance/runners/rust/src/main.rs b/conformance/runners/rust/src/main.rs index 4e390b5..e2e975c 100644 --- a/conformance/runners/rust/src/main.rs +++ b/conformance/runners/rust/src/main.rs @@ -79,6 +79,7 @@ impl UseFilter for AsValve<'_, P> { } } +// cup: allow-plain the entry of the runner: it builds each case's pipeline from the JSON vocabulary and reports the result fn add_filter(spec: &Map, to: impl UseFilter) -> Result<(), Unsupported> { match text(spec, "op")?.as_str() { "set" => to.add(SetFilter { @@ -122,6 +123,7 @@ fn add_filter(spec: &Map, to: impl UseFilter) -> Result<(), Unsupported> { Ok(()) } +// cup: allow-plain the entry of the runner: it builds each case's pipeline from the JSON vocabulary and reports the result fn add_tap(spec: &Map, pipeline: &mut Pipeline) -> Result<(), Unsupported> { match text(spec, "op")?.as_str() { "record" => pipeline.add_tap(RecordTap { @@ -136,6 +138,7 @@ fn add_tap(spec: &Map, pipeline: &mut Pipeline) -> Result<(), Unsupported> { Ok(()) } +// cup: allow-plain the entry of the runner: it builds each case's pipeline from the JSON vocabulary and reports the result fn add_valve(when: &Map, filter: &Map, pipeline: &mut Pipeline) -> Result<(), Unsupported> { match text(when, "op")?.as_str() { "equals" => { @@ -161,6 +164,7 @@ fn steps_of(spec: &Map) -> Result<&[Value], Unsupported> { .ok_or_else(|| Unsupported("'steps' is not a list".to_string())) } +// cup: allow-plain the entry of the runner: it builds each case's pipeline from the JSON vocabulary and reports the result fn build(steps: &[Value], pipeline: &mut Pipeline) -> Result<(), Unsupported> { for step in steps { let step = step @@ -210,6 +214,7 @@ fn names(labels: &[String], named: &impl Fn(&str) -> String) -> Value { ) } +// cup: allow-plain the entry of the runner: it builds each case's pipeline from the JSON vocabulary and reports the result fn run_case(spec: &Map) -> Result { let steps = steps_of(spec)?; let mut pipeline = Pipeline::new(); @@ -292,6 +297,7 @@ fn run_case(spec: &Map) -> Result { Ok(Value::Map(result)) } +// cup: allow-plain the entry of the runner: it builds each case's pipeline from the JSON vocabulary and reports the result fn bench_case(spec: &Map) -> Result { let mut pipeline = Pipeline::new(); build(steps_of(spec)?, &mut pipeline)?; @@ -302,6 +308,7 @@ fn bench_case(spec: &Map) -> Result { Ok(bench::bench(value(spec, "id")?, pipeline, input)) } +// cup: allow-plain the entry of the runner: it builds each case's pipeline from the JSON vocabulary and reports the result fn main() -> ExitCode { let target = std::env::args().nth(1).unwrap_or_default(); let run: fn(&Map) -> Result = match target.as_str() { diff --git a/docs/rust-plan.md b/docs/rust-plan.md index 788d399..e901659 100644 --- a/docs/rust-plan.md +++ b/docs/rust-plan.md @@ -46,7 +46,9 @@ The packages are beside the others, with `-rs` where TypeScript has `-ts`. A root `Cargo.toml` makes the Rust crates one Cargo workspace. It is not a package, as the root `package.json` is not a package. `Cargo.lock` is committed, since the workspace has a binary (the runner). `target/` goes in -`.gitignore`. +`.gitignore`. The linter (phase 4) is a workspace of its own, with its own +`Cargo.lock`: it needs `syn` from crates.io, and the root workspace builds +with no download. One primitive is in each file, and the file name is the type name in snake_case, as in Python: `DiscountFilter` is in `discount_filter.rs`. @@ -479,6 +481,37 @@ as `typescript:static` does, and the refusal tests pass. ## Phase 4: the linter +**Done** (2026-10-27): `packages/codeupipe-lint-rs/`, the crate +`codeupipe-lint` 0.1.0, with its own README. It is a codeupipe pipeline of +eleven Filters that carry a `LintRun` (a type of its own, D11), on `syn` +(`full`, `visit`) and `serde_json` for `cup.config.json`. The rules are +CUP-I000, A003, A005, I003, I005, I006, I007, and the warnings I008 and +I009; the arguments, the config file, the markers and the report are those +of the TypeScript linter. Decided on the way: + +- **The file name is snake_case** (`discount_filter.rs`), and a primitive is + a type that implements a primitive trait in the same file. +- **CUP-I008 is reported for each `impl`** of a type that is not a + primitive, not once for the type: in Rust the code of a type is in its + `impl` blocks, so the marker goes next to the code. Each `impl` of a + primitive (a helper method of a Filter, its `new`) is the primitive's code. +- **Comments are found between the tokens**, since a syntax tree has none. + Text in a doc comment or a string is not a marker. +- **A name is followed to its `let`** with the scopes of Rust: a later `let` + replaces it, a closure sees the function around it, a nested function does + not, and the types declared in a block are seen in all of it. + +CI runs it with `--strict` on all the Rust code of the repository (the +runtime, its examples and tests, the conformance runner, and the linter), as +the Python linter runs. That needed 45 `cup: allow-plain` markers (the +runtime's own types, the runner's JSON reader and entry, the linter's +visitors) and a fix in the runtime's tests: six step types declared in a +test function moved to the top of the file, and the test of a plain nested +Pipeline's label says why with `cup: allow-anonymous`. + +What follows is the plan as it was written. + + Rust's types already check several rules: | Rule | In Rust | @@ -505,7 +538,7 @@ correctness. The others are about form. | 1 | Core crate, example, README | About 600 lines with tests (TypeScript: 415 lines of source) | | 2 | Runner, `run.py`, bench, CI, documents | About 400 lines, most of it the JSON reader and the `cup_ops` | | 3 | Compiler and `rust:static`, if the benchmark says so | About 1,000 lines (TypeScript: 851 lines of source) | -| 4 | Linter | About 1,200 lines | +| 4 | Linter. **Done** | About 1,200 lines (written: 2,060 of source with its comments, 950 of tests) | Phases 1 and 2 are one release: `codeupipe` 0.1.0 for Rust, in the conformance matrix. Phases 3 and 4 are later versions. diff --git a/packages/codeupipe-lint-rs/Cargo.lock b/packages/codeupipe-lint-rs/Cargo.lock new file mode 100644 index 0000000..99a44fb --- /dev/null +++ b/packages/codeupipe-lint-rs/Cargo.lock @@ -0,0 +1,123 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "codeupipe" +version = "0.1.0" + +[[package]] +name = "codeupipe-lint" +version = "0.1.0" +dependencies = [ + "codeupipe", + "proc-macro2", + "serde_json", + "syn 2.0.119", +] + +[[package]] +name = "itoa" +version = "1.0.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" + +[[package]] +name = "memchr" +version = "2.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" + +[[package]] +name = "proc-macro2" +version = "1.0.107" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "985e7ec9bb745e6ce6535b544d84d6cd6f7ad8bd711c398938ae983b91a766d9" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "quote" +version = "1.0.47" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fbf4db142a473a8d80c26bbf18454ed458bf8d26c8219c331daecfdbd079001" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "serde" +version = "1.0.229" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4148590afebada386688f18773da617792bf2ef03ffc1e4cbd2b1d45b023e0ba" +dependencies = [ + "serde_core", +] + +[[package]] +name = "serde_core" +version = "1.0.229" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "67dca2c9c51e58a4791a4b1ed58308b39c64224d349a935ab5039aa360942a48" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.229" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e7a5d71263a5a7d47b41f6b3f06ba276f10cc18b0931f1799f710578e2309348" +dependencies = [ + "proc-macro2", + "quote", + "syn 3.0.6", +] + +[[package]] +name = "serde_json" +version = "1.0.151" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c841b55ecdae098c80dcae9cf767f6f8a0c2cdb3416bbef72181df4d0fe73f14" +dependencies = [ + "itoa", + "memchr", + "serde", + "serde_core", + "zmij", +] + +[[package]] +name = "syn" +version = "2.0.119" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "872831b642d1a07999a962a351ed35b955ea2cfc8f3862091e2a240a84f17297" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "syn" +version = "3.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8593e8e72159ed2257d083c7a454a85cbf854f37a0966d8d483aff8c8a3ebcee" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "unicode-ident" +version = "1.0.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d245f478577f809a851594d02313b640fb437e0bb33866753cff937863096954" + +[[package]] +name = "zmij" +version = "1.0.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29666d0abbfad1e3dc4dcf6144730dd3a3ab225bbbdac83319345b1b44ccfc1b" diff --git a/packages/codeupipe-lint-rs/Cargo.toml b/packages/codeupipe-lint-rs/Cargo.toml new file mode 100644 index 0000000..bf0df07 --- /dev/null +++ b/packages/codeupipe-lint-rs/Cargo.toml @@ -0,0 +1,27 @@ +[package] +name = "codeupipe-lint" +version = "0.1.0" +edition = "2021" +rust-version = "1.75" +description = "A linter for Rust code that uses codeupipe." +license = "Apache-2.0" +repository = "https://github.com/orchestrate-solutions/codeupipe" +readme = "README.md" +keywords = ["pipeline", "filter", "payload", "lint"] +# Not on crates.io yet. A version below 1.0 is not stable. +publish = false + +[[bin]] +name = "codeupipe-lint" +path = "src/main.rs" + +[dependencies] +codeupipe = { path = "../codeupipe-rs" } +# Lines and columns of the syntax tree, outside a procedural macro. +proc-macro2 = { version = "1.0.80", default-features = false, features = ["span-locations"] } +serde_json = "1.0.100" +syn = { version = "2.0.60", default-features = false, features = ["full", "parsing", "printing", "clone-impls", "visit"] } + +# A workspace of its own. The crates of the root workspace have no +# dependencies and build with no download; this one needs syn. +[workspace] diff --git a/packages/codeupipe-lint-rs/LICENSE b/packages/codeupipe-lint-rs/LICENSE new file mode 100644 index 0000000..f433b1a --- /dev/null +++ b/packages/codeupipe-lint-rs/LICENSE @@ -0,0 +1,177 @@ + + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS diff --git a/packages/codeupipe-lint-rs/NOTICE b/packages/codeupipe-lint-rs/NOTICE new file mode 100644 index 0000000..fd0d109 --- /dev/null +++ b/packages/codeupipe-lint-rs/NOTICE @@ -0,0 +1,4 @@ +codeupipe +Copyright 2026 Orchestrate LLC + +Licensed under the Apache License, Version 2.0. See LICENSE. diff --git a/packages/codeupipe-lint-rs/README.md b/packages/codeupipe-lint-rs/README.md new file mode 100644 index 0000000..a07ce94 --- /dev/null +++ b/packages/codeupipe-lint-rs/README.md @@ -0,0 +1,161 @@ +# codeupipe-lint for Rust + +A linter for Rust code that uses codeupipe. It reads the source and does +not run it. + +The linter is a codeupipe pipeline: [src/lint_pipeline.rs](src/lint_pipeline.rs). +Its steps carry a type of their own, `LintRun`, in place of a `Payload`, +because a run holds syntax trees. It depends on `codeupipe`, on `syn` to +parse the files, and on `serde_json` to read the config file. + +The version is below 1.0: the package is not stable yet, and a new version +can change in a way that is not compatible. It is not on crates.io yet. +Build it from this repository: + +```bash +cargo install --path packages/codeupipe-lint-rs +``` + +```bash +codeupipe-lint src # files or directories +codeupipe-lint --exclude '*/generated/*' src +codeupipe-lint --disable CUP-I005 src +codeupipe-lint --strict src # a warning fails the run too +codeupipe-lint --rules # list the rule ids +codeupipe-lint # paths come from cup.config.json +``` + +Each finding is one line, and the message names the fix: + +``` +path:line:col CUP-A005 unnamed-pipeline-step Pipeline is a plain `Pipeline::new()`. A nested pipeline must be (...) +``` + +Exit code: **0** no findings, **1** findings, **2** a path could not be +read or the arguments cannot be used. + +From Rust: `lint_paths(&paths, &LintOptions { exclude, disable, strict })` +returns the `LintRun` (`findings`, `suppressed`, `errors`, `report`, +`exit_code`). + +## Rules + +The ids are those of the Python and TypeScript linters. + +| id | slug | what | +|---|---|---| +| CUP-I000 | parse-error | the file has a syntax error | +| CUP-A003 | local-type-step | a step whose type is declared in a function | +| CUP-A005 | unnamed-pipeline-step | a plain `Pipeline::new()` nested as a step | +| CUP-I003 | state-read-in-step | a Filter (`call`) or a Predicate (`test`) reads the State of a pipeline | +| CUP-I005 | one-primitive-per-file | a second primitive at the top level of a file | +| CUP-I006 | primitive-file-name | the file name is not the type name in snake_case | +| CUP-I007 | primitive-role-suffix | the type name does not end with its role | +| CUP-I008 | logic-outside-primitive | **a warning.** A function, an `impl` of a type that is not a primitive, a trait, or a `const` has logic (`if`, `match`, a loop, `let ... else`) | +| CUP-I009 | marker-allows-nothing | **a warning.** A `cup: allow-...` comment that allowed no finding | + +A step is the argument of `add_filter`, `add_tap` or `add_pipeline` (on any +receiver), and the two arguments of `add_valve(predicate, filter)`. + +A type is a primitive when it implements `Filter`, `Tap`, `Predicate`, +`Hook` or `NamedPipeline`, in the same file. Its role is the trait +(`NamedPipeline` is the role `Pipeline`). + +## Rules that the compiler checks + +The types of the Rust runtime refuse these, so the linter has no rule for +them: + +| id | In Rust | +|---|---| +| CUP-A001, A002, A008 | A closure or a function is not a step and not a predicate: it does not implement the trait | +| CUP-A009 | An object literal does not exist in Rust | +| CUP-I004 | A Tap, a Predicate and a Hook get `&Payload`, so only a Filter can write | +| CUP-I001 | The data of a Payload is private | + +A `--disable` or a `lint.disable` with one of these ids is an error, as for +any id the linter does not have. + +## Logic that is not in a primitive (CUP-I008) + +A project that uses codeupipe writes its logic as Filters, Predicates, Taps, +Hooks and Pipelines. Logic in a plain function or a plain type works as +well, so nothing else shows the mistake. This rule shows it. + +- **It is a warning.** It is in the report and it is counted apart + (`No findings. 3 warnings.`). It does not make the exit code 1. With + `--strict`, a warning fails the run as a finding does. +- **Flagged:** a function, each `impl` of a type that is not a primitive + (an inherent `impl` or a trait such as `Display`), a trait with a default + method, and the first `const` or `static` whose value has logic. The + modules of a file are checked the same way. +- **Not flagged:** the code of a primitive (each `impl` of a type that + implements a primitive trait, so a helper method of a Filter is fine), + code with no logic, code for the tests only (`#[test]`, `#[cfg(test)]`), + and the files under `tests/`. +- **Not seen:** a loop that is a method call (`map`, `filter`, `for_each`), + `&&`, `||`, `?`, and code in a macro call. +- For logic that must be plain code (the edge to a file or a server, the + entry of a program), say why in a comment: `// cup: allow-plain `. + +## Turn a finding off + +- **One line.** A comment on the line of the finding, on the line of the + `add_...` call around it, or alone on the line directly above: + `// cup: allow-anonymous` for a CUP-A finding; + `// cup: allow-state-read`, `allow-shared-file`, `allow-name` or + `allow-plain` for a CUP-I finding. Any text can follow the marker. + A finding of a function or a type is on the line of its name or of its + `impl`, so the comment goes after the doc comment and the attributes: + + ```rust + /// Reads the config file. + #[inline] + // cup: allow-plain the edge of the program: it reads a file before the pipeline runs + pub fn read_config() -> Config { + ``` + + A comment that allows no finding is itself a finding (CUP-I009), so an + old one does not stay. Text in a doc comment or a string is not a marker. +- **Files.** `--exclude GLOB`, or `lint.exclude` in the config file. In a + glob, `*` is any text, a `/` too. +- **A rule, for the project.** `--disable RULE`, or `lint.disable` in the + config file. A rule id that does not exist is an error. + +## Config file + +`cup.config.json`, in the directory that the command runs from. The linter +reads the `lint` section and no other. + +```json +{"lint": {"paths": ["src"], "exclude": ["*/generated/*"], "disable": ["CUP-I005"]}} +``` + +The file is used only when no paths are given on the command line. A key in +`lint` that the linter does not read is an error. + +## Limits + +- A name is followed to the `let` that gave its value, in the same + function, and a later `let` of the same name replaces it. A parameter, a + pattern, a name from another file and the result of a call are not + followed, so a function that returns a step of a local type is not seen. +- A type is matched by its name in the file. The linter reads syntax only: + it does not resolve `use`, macros or types. +- The files under a `tests` directory are not checked by CUP-I005, I006, + I007 and I008. + +## Development + +The linter is a Cargo workspace of its own, apart from the root workspace, +so that the runtime builds with no download. In this directory: + +```bash +cargo test # the tests, and the linter on the Rust code of this repository +cargo clippy --all-targets -- -D warnings +cargo run -q -- --strict src tests ../codeupipe-rs/src ../codeupipe-rs/examples ../codeupipe-rs/tests ../../conformance/runners/rust/src +``` + +## License + +Apache License 2.0. diff --git a/packages/codeupipe-lint-rs/src/analyze_plain_logic_filter.rs b/packages/codeupipe-lint-rs/src/analyze_plain_logic_filter.rs new file mode 100644 index 0000000..3bb43c9 --- /dev/null +++ b/packages/codeupipe-lint-rs/src/analyze_plain_logic_filter.rs @@ -0,0 +1,125 @@ +use codeupipe::{Filter, StepError}; +use proc_macro2::Span; +use syn::spanned::Spanned; +use syn::visit::Visit; +use syn::Item; + +use crate::lint_run::LintRun; +use crate::logic::has_logic; +use crate::primitive_kind::{ + attributes_of, is_test_file, is_test_only, primitive_names, self_type_name, +}; +use crate::rules::{make_finding, LOGIC_OUTSIDE_PRIMITIVE}; +use crate::source::Source; + +const WHAT: &str = "has logic and is not a Filter, a Predicate, a Tap, a Hook or a Pipeline"; + +/// CUP-I008, a warning: logic that is not in a primitive. +/// +/// A project that uses codeupipe writes its logic as Filters, Predicates, +/// Taps, Hooks and Pipelines. Logic in a plain function or a plain type works +/// as well, so nothing else shows the mistake. This rule shows it. +/// +/// Logic is a decision or a loop: `if`, `match`, `for`, `while`, `loop`, +/// `let ... else`. Flagged: a function, an `impl` of a type that is not a +/// primitive (once for each `impl`), a trait with a default method, and the +/// first `const` or `static` whose value has logic. Modules of the file are +/// checked the same way. Not flagged: the code of a primitive (each `impl` +/// of a type that implements a primitive trait), code with no logic, code +/// for the tests only (`#[test]`, `#[cfg(test)]`), and the files of `tests/`. +/// +/// Reads `sources`. Writes `findings`. +pub struct AnalyzePlainLogicFilter; + +impl Filter for AnalyzePlainLogicFilter { + fn call(&self, run: &mut LintRun) -> Result<(), StepError> { + for source in run + .sources + .iter() + .filter(|source| !is_test_file(&source.path)) + { + let mut primitives = Vec::new(); + primitive_names(&source.tree.items, &mut primitives); + let mut module_code = None; + let mut found = Vec::new(); + Self::in_items( + &source.tree.items, + &primitives, + &mut found, + &mut module_code, + ); + found + .extend(module_code.map(|at| (at, "The code at the top of this file".to_string()))); + for (at, what) in found { + run.findings.push(Self::finding(source, at, &what)); + } + } + Ok(()) + } +} + +impl AnalyzePlainLogicFilter { + fn finding(source: &Source, at: Span, what: &str) -> crate::finding::Finding { + make_finding( + source, + at, + LOGIC_OUTSIDE_PRIMITIVE, + &format!("{what} {WHAT}"), + None, + ) + } + + /// The items of a file or a module with logic that is not in a primitive. + fn in_items( + items: &[Item], + primitives: &[String], + found: &mut Vec<(Span, String)>, + module_code: &mut Option, + ) { + for item in items + .iter() + .filter(|item| !is_test_only(attributes_of(item))) + { + match item { + Item::Fn(function) => { + if has_logic(|v| v.visit_block(&function.block)) { + found.push(( + function.sig.ident.span(), + format!("Function `{}`", function.sig.ident), + )); + } + } + Item::Impl(block) => { + let name = self_type_name(&block.self_ty).unwrap_or_default(); + if !primitives.contains(&name) && has_logic(|v| v.visit_item_impl(block)) { + found.push((block.impl_token.span(), format!("Type `{name}`"))); + } + } + Item::Trait(definition) => { + if has_logic(|v| v.visit_item_trait(definition)) { + found.push(( + definition.ident.span(), + format!("Trait `{}`", definition.ident), + )); + } + } + Item::Const(constant) => { + if module_code.is_none() && has_logic(|v| v.visit_expr(&constant.expr)) { + *module_code = Some(constant.span()); + } + } + Item::Static(value) => { + if module_code.is_none() && has_logic(|v| v.visit_expr(&value.expr)) { + *module_code = Some(value.span()); + } + } + Item::Mod(module) => { + if let Some((_, inner)) = &module.content { + Self::in_items(inner, primitives, found, module_code); + } + } + _ => {} + } + } + } +} diff --git a/packages/codeupipe-lint-rs/src/analyze_primitive_names_filter.rs b/packages/codeupipe-lint-rs/src/analyze_primitive_names_filter.rs new file mode 100644 index 0000000..7043a69 --- /dev/null +++ b/packages/codeupipe-lint-rs/src/analyze_primitive_names_filter.rs @@ -0,0 +1,81 @@ +use std::path::Path; + +use codeupipe::{Filter, StepError}; + +use crate::lint_run::LintRun; +use crate::primitive_kind::{is_test_file, primitives_in}; +use crate::rules::{make_finding, PRIMITIVE_FILE_NAME, PRIMITIVE_ROLE_SUFFIX}; + +/// CUP-I007: the type name of a primitive ends with its role +/// (`DiscountFilter`, `AuditTap`, `PrintStepsHook`, `AtLeastPredicate`, +/// `PricingPipeline`). +/// +/// CUP-I006: the file name is the type name in snake_case +/// (`discount_filter.rs`). +/// +/// Checked: the primitives at the top level of a file. Not checked: plain +/// types, and the files of `tests/`. +/// +/// Reads `sources`. Writes `findings`. +pub struct AnalyzePrimitiveNamesFilter; + +impl Filter for AnalyzePrimitiveNamesFilter { + fn call(&self, run: &mut LintRun) -> Result<(), StepError> { + for source in run + .sources + .iter() + .filter(|source| !is_test_file(&source.path)) + { + let file = Path::new(&source.path) + .file_name() + .unwrap_or_default() + .to_string_lossy(); + let (stem, extension) = file.split_at(file.find('.').unwrap_or(file.len())); + for primitive in primitives_in(&source.tree.items) { + let role = primitive.kind.name(); + let mut wanted = primitive.name.clone(); + if !wanted.ends_with(role) { + wanted.push_str(role); + let what = format!( + "{role} `{}` does not say what it is. Name it `{wanted}`", + primitive.name + ); + let finding = + make_finding(source, primitive.at, PRIMITIVE_ROLE_SUFFIX, &what, None); + run.findings.push(finding); + } + let snake = Self::snake_case(&wanted); + if stem != snake { + let what = format!("`{wanted}` is in `{file}`. Put it in `{snake}{extension}`"); + let finding = + make_finding(source, primitive.at, PRIMITIVE_FILE_NAME, &what, None); + run.findings.push(finding); + } + } + } + Ok(()) + } +} + +impl AnalyzePrimitiveNamesFilter { + /// `ParseHTMLFilter` gives `parse_html_filter`. + pub fn snake_case(name: &str) -> String { + let chars: Vec = name.chars().collect(); + let mut out = String::new(); + for (index, c) in chars.iter().enumerate() { + let before = index.checked_sub(1).map(|i| chars[i]); + let after = chars.get(index + 1); + let word_starts = c.is_uppercase() + && before.is_some_and(|b| { + b.is_lowercase() + || b.is_ascii_digit() + || (b.is_uppercase() && after.is_some_and(|a| a.is_lowercase())) + }); + if word_starts { + out.push('_'); + } + out.extend(c.to_lowercase()); + } + out + } +} diff --git a/packages/codeupipe-lint-rs/src/analyze_primitives_per_file_filter.rs b/packages/codeupipe-lint-rs/src/analyze_primitives_per_file_filter.rs new file mode 100644 index 0000000..07b0160 --- /dev/null +++ b/packages/codeupipe-lint-rs/src/analyze_primitives_per_file_filter.rs @@ -0,0 +1,41 @@ +use codeupipe::{Filter, StepError}; + +use crate::lint_run::LintRun; +use crate::primitive_kind::{is_test_file, primitives_in}; +use crate::rules::{make_finding, ONE_PRIMITIVE_PER_FILE}; + +/// CUP-I005: one primitive in each file. +/// +/// Flagged: each primitive at the top level of a file after the first. Not +/// flagged: plain types and functions beside the primitive, types in a +/// function or a module of the file, and the files of `tests/`. +/// +/// Reads `sources`. Writes `findings`. +pub struct AnalyzePrimitivesPerFileFilter; + +impl Filter for AnalyzePrimitivesPerFileFilter { + fn call(&self, run: &mut LintRun) -> Result<(), StepError> { + for source in run + .sources + .iter() + .filter(|source| !is_test_file(&source.path)) + { + let primitives = primitives_in(&source.tree.items); + let Some((first, rest)) = primitives.split_first() else { + continue; + }; + for other in rest { + let what = format!( + "{} `{}` shares this file with {} `{}`", + other.kind.name(), + other.name, + first.kind.name(), + first.name + ); + let finding = make_finding(source, other.at, ONE_PRIMITIVE_PER_FILE, &what, None); + run.findings.push(finding); + } + } + Ok(()) + } +} diff --git a/packages/codeupipe-lint-rs/src/analyze_stale_markers_filter.rs b/packages/codeupipe-lint-rs/src/analyze_stale_markers_filter.rs new file mode 100644 index 0000000..7a580e4 --- /dev/null +++ b/packages/codeupipe-lint-rs/src/analyze_stale_markers_filter.rs @@ -0,0 +1,46 @@ +use codeupipe::{Filter, StepError}; + +use crate::finding::Finding; +use crate::lint_run::LintRun; +use crate::rules::{rule, MARKER_ALLOWS_NOTHING}; + +/// Finds each `cup: allow-...` comment that allowed no finding (CUP-I009, a +/// warning). Such a comment stays when the code that it was for is gone or +/// has moved, or when its kind has a spelling error. +/// +/// Only a comment line that starts with the marker counts, so text that +/// names a marker in a string, a doc comment or the middle of a sentence is +/// not reported. +/// +/// Reads `sources` and `used_markers` (from ApplySuppressionsFilter). Writes +/// `findings`. +pub struct AnalyzeStaleMarkersFilter; + +impl Filter for AnalyzeStaleMarkersFilter { + fn call(&self, run: &mut LintRun) -> Result<(), StepError> { + let stale = rule(MARKER_ALLOWS_NOTHING).expect("a rule"); + for source in &run.sources { + let unused = source.markers.iter().filter(|marker| { + !run.used_markers + .iter() + .any(|(path, line)| *path == source.path && *line == marker.line) + }); + for marker in unused { + run.findings.push(Finding { + path: source.path.clone(), + line: marker.line, + col: marker.col, + rule: MARKER_ALLOWS_NOTHING.to_string(), + slug: stale.slug.to_string(), + message: format!( + "The comment `cup: {}` allows nothing. {}", + marker.kind, stale.fix + ), + call_line: None, + warning: true, + }); + } + } + Ok(()) + } +} diff --git a/packages/codeupipe-lint-rs/src/analyze_state_reads_filter.rs b/packages/codeupipe-lint-rs/src/analyze_state_reads_filter.rs new file mode 100644 index 0000000..ea7d7fb --- /dev/null +++ b/packages/codeupipe-lint-rs/src/analyze_state_reads_filter.rs @@ -0,0 +1,220 @@ +use std::collections::{HashMap, HashSet}; + +use codeupipe::{Filter, StepError}; +use syn::spanned::Spanned; +use syn::visit::{self, Visit}; +use syn::{Expr, GenericArgument, Member, PathArguments, Type}; + +use crate::finding::Finding; +use crate::lint_run::LintRun; +use crate::primitive_kind::{self_type_name, PrimitiveKind}; +use crate::rules::{make_finding, STATE_READ}; +use crate::source::{position, text_of, Source}; + +/// The methods of State that read it. +const STATE_MEMBERS: &[&str] = &["executed", "skipped", "errors", "timings"]; + +/// Types that hold a value of the type in them: a field of `Rc` holds a State. +const HOLDERS: &[&str] = &[ + "Rc", "Arc", "RefCell", "Cell", "Mutex", "RwLock", "Box", "Option", "Weak", +]; + +/// CUP-I003: a Filter (`call`) or a Predicate (`test`) that reads the State +/// of a pipeline. +/// +/// Flagged: `.state()` followed by a method of State (`executed`, +/// `skipped`, `errors`, `timings`); `.state()` when the name says +/// it is a pipeline (`self.pipeline.borrow().state()`); and a field of the +/// step that holds a State (`seen: Rc`). Not flagged: `self.state()` +/// (the step's own method), a State that the step makes, a Payload key named +/// "state", and anything in a Tap or a Hook. +/// +/// Reads `sources`. Writes `findings`. +pub struct AnalyzeStateReadsFilter; + +impl Filter for AnalyzeStateReadsFilter { + fn call(&self, run: &mut LintRun) -> Result<(), StepError> { + for source in &run.sources { + let mut fields = StateFields::default(); + fields.visit_file(&source.tree); + let mut steps = StepBodies { + source, + fields: &fields.by_type, + findings: Vec::new(), + }; + steps.visit_file(&source.tree); + run.findings.append(&mut steps.findings); + } + Ok(()) + } +} + +/// The fields of each struct of a file that hold a State. +#[derive(Default)] +struct StateFields { + by_type: HashMap>, +} + +impl<'ast> Visit<'ast> for StateFields { + fn visit_item_struct(&mut self, item: &'ast syn::ItemStruct) { + let held: HashSet = item + .fields + .iter() + .filter(|field| holds_state(&field.ty)) + .filter_map(|field| field.ident.as_ref().map(ToString::to_string)) + .collect(); + self.by_type.insert(item.ident.to_string(), held); + visit::visit_item_struct(self, item); + } +} + +// cup: allow-plain used by the analysis Filter: it reads a type of the syntax tree +fn holds_state(ty: &Type) -> bool { + match ty { + Type::Reference(reference) => holds_state(&reference.elem), + Type::Path(path) => path.path.segments.last().is_some_and(|last| { + let held = || { + match &last.arguments { + PathArguments::AngleBracketed(arguments) => arguments.args.iter().any(|argument| { + matches!(argument, GenericArgument::Type(inner) if holds_state(inner)) + }), + _ => false, + } + }; + last.ident == "State" || (HOLDERS.iter().any(|holder| last.ident == holder) && held()) + }), + _ => false, + } +} + +/// Finds the State reads in each `call` of a Filter and `test` of a Predicate. +struct StepBodies<'a> { + source: &'a Source, + fields: &'a HashMap>, + findings: Vec, +} + +// cup: allow-plain a visitor of a syntax tree: the tool that a Filter uses to read source code +impl<'ast> Visit<'ast> for StepBodies<'_> { + fn visit_item_impl(&mut self, item: &'ast syn::ItemImpl) { + let kind = item + .trait_ + .as_ref() + .and_then(|(_, path, _)| PrimitiveKind::of_trait(path)); + let method = match kind { + Some(PrimitiveKind::Filter) => "call", + Some(PrimitiveKind::Predicate) => "test", + _ => return visit::visit_item_impl(self, item), + }; + let none = HashSet::new(); + let fields = self_type_name(&item.self_ty) + .and_then(|name| self.fields.get(&name)) + .unwrap_or(&none); + let mut reads = Reads { + source: self.source, + fields, + found: Vec::new(), + }; + for inner in &item.items { + if let syn::ImplItem::Fn(function) = inner { + if function.sig.ident == method { + reads.visit_block(&function.block); + } + } + } + self.findings.append(&mut reads.found); + } +} + +/// The State reads in the body of one step. +struct Reads<'a> { + source: &'a Source, + fields: &'a HashSet, + found: Vec, +} + +// cup: allow-plain a visitor of a syntax tree: the tool that a Filter uses to read source code +impl Reads<'_> { + fn flag(&mut self, read: &dyn Spanned) { + let span = read.span(); + let at = position(span); + if self + .found + .iter() + .any(|finding| (finding.line, finding.col) == at) + { + return; + } + let what = format!( + "Step reads pipeline State (`{}`)", + text_of(&self.source.text, span) + ); + self.found + .push(make_finding(self.source, span, STATE_READ, &what, None)); + } +} + +// cup: allow-plain a visitor of a syntax tree: the tool that a Filter uses to read source code +impl<'ast> Visit<'ast> for Reads<'_> { + fn visit_item(&mut self, _: &'ast syn::Item) { + // An item in the body has its own scope. + } + + fn visit_expr_method_call(&mut self, call: &'ast syn::ExprMethodCall) { + if STATE_MEMBERS.iter().any(|member| call.method == member) && is_state_call(&call.receiver) + { + self.flag(&call.receiver); + } else if is_state_call_on(call) && names_a_pipeline(&call.receiver) { + self.flag(call); + } + visit::visit_expr_method_call(self, call); + } + + fn visit_expr_field(&mut self, field: &'ast syn::ExprField) { + let held = match &field.member { + Member::Named(name) => is_self(&field.base) && self.fields.contains(&name.to_string()), + Member::Unnamed(_) => false, + }; + if held { + self.flag(field); + } + visit::visit_expr_field(self, field); + } +} + +/// `.state()`, where `` is not `self`. +fn is_state_call(value: &Expr) -> bool { + matches!(value, Expr::MethodCall(call) if is_state_call_on(call)) +} + +fn is_state_call_on(call: &syn::ExprMethodCall) -> bool { + call.method == "state" && call.args.is_empty() && !is_self(&call.receiver) +} + +fn is_self(value: &Expr) -> bool { + matches!(value, Expr::Path(path) if path.path.is_ident("self")) +} + +/// The name of `value`, past the method calls on it, says it is a pipeline: +/// `self.pipeline.borrow().upgrade()`, `inner_pipeline`. +// cup: allow-plain a visitor of a syntax tree: the tool that a Filter uses to read source code +fn names_a_pipeline(value: &Expr) -> bool { + let name = match value { + Expr::MethodCall(call) => return names_a_pipeline(&call.receiver), + Expr::Paren(inner) => return names_a_pipeline(&inner.expr), + Expr::Reference(inner) => return names_a_pipeline(&inner.expr), + Expr::Unary(inner) => return names_a_pipeline(&inner.expr), + Expr::Field(field) => match &field.member { + Member::Named(name) => name.to_string(), + Member::Unnamed(_) => return false, + }, + Expr::Path(path) => path + .path + .segments + .last() + .map(|s| s.ident.to_string()) + .unwrap_or_default(), + _ => return false, + }; + name.to_lowercase().contains("pipeline") +} diff --git a/packages/codeupipe-lint-rs/src/analyze_steps_filter.rs b/packages/codeupipe-lint-rs/src/analyze_steps_filter.rs new file mode 100644 index 0000000..e012dbf --- /dev/null +++ b/packages/codeupipe-lint-rs/src/analyze_steps_filter.rs @@ -0,0 +1,30 @@ +use codeupipe::{Filter, StepError}; +use syn::visit::Visit; + +use crate::lint_run::LintRun; +use crate::step_visitor::StepVisitor; + +/// CUP-A003 and CUP-A005: a step whose type is declared in a function, and +/// a plain `Pipeline` nested as a step. +/// +/// A step is the argument of `add_filter`, `add_tap` or `add_pipeline` (on +/// any receiver), or one of the two arguments of `add_valve(predicate, +/// filter)`. A name is followed to the `let` that gave its value, in the +/// same function. A parameter and a name from another file are not followed. +/// +/// A closure or a function as a step (CUP-A001, A002, A008) is a compile +/// error in Rust, so it has no rule here. +/// +/// Reads `sources`. Writes `findings`. +pub struct AnalyzeStepsFilter; + +impl Filter for AnalyzeStepsFilter { + fn call(&self, run: &mut LintRun) -> Result<(), StepError> { + for source in &run.sources { + let mut visitor = StepVisitor::new(source); + visitor.visit_file(&source.tree); + run.findings.append(&mut visitor.findings); + } + Ok(()) + } +} diff --git a/packages/codeupipe-lint-rs/src/apply_suppressions_filter.rs b/packages/codeupipe-lint-rs/src/apply_suppressions_filter.rs new file mode 100644 index 0000000..48a3470 --- /dev/null +++ b/packages/codeupipe-lint-rs/src/apply_suppressions_filter.rs @@ -0,0 +1,71 @@ +use std::collections::HashMap; + +use codeupipe::{Filter, StepError}; + +use crate::lint_run::LintRun; +use crate::source::Marker; + +/// The markers that allow a CUP-A finding. +const ANONYMOUS: &[&str] = &["allow-anonymous"]; +/// The markers that allow a CUP-I finding. +const INVARIANT: &[&str] = &[ + "allow-state-read", + "allow-shared-file", + "allow-name", + "allow-plain", +]; + +/// Drops the findings that a comment allows. +/// +/// `// cup: allow-anonymous` allows a CUP-A finding. `// cup: allow-state-read`, +/// `allow-shared-file`, `allow-name` and `allow-plain` allow a CUP-I finding. +/// The comment is on the line of the finding, on the line of the `add_...` +/// call around it, or alone on the line directly above. Any text can follow +/// the marker. +/// +/// Reads `findings` and `sources`. Writes `findings` (kept), `suppressed` +/// (dropped), and `used_markers` (each comment that allowed a finding). +pub struct ApplySuppressionsFilter; + +impl Filter for ApplySuppressionsFilter { + fn call(&self, run: &mut LintRun) -> Result<(), StepError> { + let markers: HashMap<&str, &[Marker]> = run + .sources + .iter() + .map(|source| (source.path.as_str(), source.markers.as_slice())) + .collect(); + let mut kept = Vec::new(); + for finding in std::mem::take(&mut run.findings) { + let kinds = if finding.rule.starts_with("CUP-A") { + ANONYMOUS + } else { + INVARIANT + }; + let file = markers + .get(finding.path.as_str()) + .copied() + .unwrap_or_default(); + let allows = |marker: &&Marker| { + kinds.contains(&marker.kind.as_str()) + && (marker.line == finding.line + || Some(marker.line) == finding.call_line + || (marker.alone && marker.line + 1 == finding.line)) + }; + let places: Vec = file + .iter() + .filter(allows) + .map(|marker| marker.line) + .collect(); + if places.is_empty() { + kept.push(finding); + continue; + } + for line in places { + run.used_markers.push((finding.path.clone(), line)); + } + run.suppressed.push(finding); + } + run.findings = kept; + Ok(()) + } +} diff --git a/packages/codeupipe-lint-rs/src/collect_files_filter.rs b/packages/codeupipe-lint-rs/src/collect_files_filter.rs new file mode 100644 index 0000000..803a62c --- /dev/null +++ b/packages/codeupipe-lint-rs/src/collect_files_filter.rs @@ -0,0 +1,63 @@ +use std::fs; +use std::path::Path; + +use codeupipe::{Filter, StepError}; + +use crate::fnmatch::fnmatch; +use crate::lint_run::LintRun; + +/// Directories that hold no source of the project: version control, build +/// output, and dependencies. +const SKIP_DIRS: &[&str] = &[".git", "target", "node_modules"]; + +/// The Rust files under the given paths. +/// +/// Reads `paths` (files or directories) and `exclude` (globs matched against +/// the path of each file). Writes `files`, and `errors` for a path that is +/// not there. Directories are walked in the order of their names. +pub struct CollectFilesFilter; + +impl Filter for CollectFilesFilter { + fn call(&self, run: &mut LintRun) -> Result<(), StepError> { + let mut found = Vec::new(); + for root in run.paths.clone() { + let path = Path::new(&root); + if path.is_dir() { + Self::walk(path, &mut found); + } else if path.is_file() { + found.push(root); + } else { + run.errors + .push(format!("{root}: no such file or directory")); + } + } + let exclude = &run.exclude; + run.files = found + .into_iter() + .filter(|file| file.ends_with(".rs")) + .filter(|file| !exclude.iter().any(|pattern| fnmatch(file, pattern))) + .collect(); + Ok(()) + } +} + +impl CollectFilesFilter { + /// The files under `directory`, in the order of their names. + fn walk(directory: &Path, found: &mut Vec) { + let Ok(entries) = fs::read_dir(directory) else { + return; + }; + let mut entries: Vec<_> = entries.flatten().map(|entry| entry.path()).collect(); + entries.sort(); + for path in entries { + let name = path.file_name().unwrap_or_default().to_string_lossy(); + if path.is_dir() && !SKIP_DIRS.contains(&name.as_ref()) { + Self::walk(&path, found); + } else if path.is_file() { + // `./src/x.rs` is `src/x.rs`. + let path = path.strip_prefix(".").unwrap_or(&path); + found.push(path.to_string_lossy().into_owned()); + } + } + } +} diff --git a/packages/codeupipe-lint-rs/src/drop_disabled_filter.rs b/packages/codeupipe-lint-rs/src/drop_disabled_filter.rs new file mode 100644 index 0000000..589c84a --- /dev/null +++ b/packages/codeupipe-lint-rs/src/drop_disabled_filter.rs @@ -0,0 +1,20 @@ +use codeupipe::{Filter, StepError}; + +use crate::lint_run::LintRun; + +/// Drops the findings of the rules that the project switched off, from those +/// that are reported and from those that a comment allowed. +/// +/// Reads `disable`. Writes `findings` and `suppressed`. +pub struct DropDisabledFilter; + +impl Filter for DropDisabledFilter { + fn call(&self, run: &mut LintRun) -> Result<(), StepError> { + let disable = &run.disable; + run.findings + .retain(|finding| !disable.contains(&finding.rule)); + run.suppressed + .retain(|finding| !disable.contains(&finding.rule)); + Ok(()) + } +} diff --git a/packages/codeupipe-lint-rs/src/finding.rs b/packages/codeupipe-lint-rs/src/finding.rs new file mode 100644 index 0000000..34435aa --- /dev/null +++ b/packages/codeupipe-lint-rs/src/finding.rs @@ -0,0 +1,28 @@ +/// One finding, at `path:line:col` (line and column start at 1). +/// +/// `call_line` is the line of the `add_...` call around the finding, when it +/// is a different line: a comment that suppresses can be on that line too. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Finding { + pub path: String, + pub line: usize, + pub col: usize, + pub rule: String, + pub slug: String, + pub message: String, + pub call_line: Option, + /// A warning is reported and does not fail the run, unless the run is strict. + pub warning: bool, +} + +// cup: allow-plain the text of one finding, for the report +impl Finding { + /// `path:line:col RULE slug message`, with `warning: ` before the message of a warning. + pub fn format(&self) -> String { + let warning = if self.warning { "warning: " } else { "" }; + format!( + "{}:{}:{} {} {} {}{}", + self.path, self.line, self.col, self.rule, self.slug, warning, self.message + ) + } +} diff --git a/packages/codeupipe-lint-rs/src/fnmatch.rs b/packages/codeupipe-lint-rs/src/fnmatch.rs new file mode 100644 index 0000000..b98d181 --- /dev/null +++ b/packages/codeupipe-lint-rs/src/fnmatch.rs @@ -0,0 +1,68 @@ +/// Whether `path` matches `pattern`, with the rules of a shell glob as +/// Python's fnmatch has them: `*` is any text (a `/` too), `?` is one +/// character, `[abc]` is one of the characters, `[a-z]` a range, and `[!a]` +/// any character but those. +pub fn fnmatch(path: &str, pattern: &str) -> bool { + let path: Vec = path.chars().collect(); + let pattern: Vec = pattern.chars().collect(); + matches(&path, &pattern) +} + +// cup: allow-plain shared by the collect Filter and its tests: a glob match, as Python's fnmatch has it +fn matches(path: &[char], pattern: &[char]) -> bool { + match pattern.split_first() { + None => path.is_empty(), + Some(('*', rest)) => (0..=path.len()).any(|skip| matches(&path[skip..], rest)), + Some(('?', rest)) => !path.is_empty() && matches(&path[1..], rest), + Some(('[', rest)) => match (path.split_first(), rest.iter().position(|c| *c == ']')) { + (Some((c, path_rest)), Some(end)) => { + let (negated, class) = match rest[..end].split_first() { + Some(('!', class)) => (true, class), + _ => (false, &rest[..end]), + }; + in_class(*c, class) != negated && matches(path_rest, &rest[end + 1..]) + } + (Some((c, path_rest)), None) => *c == '[' && matches(path_rest, rest), + (None, _) => false, + }, + Some((p, rest)) => path.first() == Some(p) && matches(&path[1..], rest), + } +} + +// cup: allow-plain shared by the collect Filter and its tests: a glob match, as Python's fnmatch has it +fn in_class(c: char, class: &[char]) -> bool { + let mut at = 0; + while at < class.len() { + if at + 2 < class.len() && class[at + 1] == '-' { + if class[at] <= c && c <= class[at + 2] { + return true; + } + at += 3; + } else { + if class[at] == c { + return true; + } + at += 1; + } + } + false +} + +#[cfg(test)] +mod tests { + use super::fnmatch; + + #[test] + fn matches_as_a_shell_glob() { + assert!(fnmatch("src/generated/x.rs", "*/generated/*")); + assert!(fnmatch("a/b.rs", "*.rs")); + assert!(!fnmatch("a/b.rs", "*.ts")); + assert!(fnmatch("x1.rs", "x?.rs")); + assert!(fnmatch("xb.rs", "x[abc].rs")); + assert!(!fnmatch("xd.rs", "x[abc].rs")); + assert!(fnmatch("xd.rs", "x[!abc].rs")); + assert!(fnmatch("x5.rs", "x[0-9].rs")); + assert!(fnmatch("x[.rs", "x[.rs")); + assert!(!fnmatch("discount.rs", "*/discount.rs")); + } +} diff --git a/packages/codeupipe-lint-rs/src/format_report_filter.rs b/packages/codeupipe-lint-rs/src/format_report_filter.rs new file mode 100644 index 0000000..bc7fc2e --- /dev/null +++ b/packages/codeupipe-lint-rs/src/format_report_filter.rs @@ -0,0 +1,62 @@ +use codeupipe::{Filter, StepError}; + +use crate::lint_run::LintRun; + +/// The findings as sorted report lines, and the exit code. +/// +/// Reads `findings`, `suppressed`, `errors` and `strict`. Writes `findings` +/// (sorted), `report` (text for stdout), `error_report` (text for stderr), +/// and `exit_code`: 0 no findings, 1 findings, 2 a path could not be read, +/// so the check is not complete. +/// +/// A warning is in the report and is counted apart. It does not make the +/// exit code 1, unless the run is strict. +pub struct FormatReportFilter; + +impl Filter for FormatReportFilter { + fn call(&self, run: &mut LintRun) -> Result<(), StepError> { + run.findings.sort_by(|a, b| { + (&a.path, a.line, a.col, &a.rule).cmp(&(&b.path, b.line, b.col, &b.rule)) + }); + let warnings = run + .findings + .iter() + .filter(|finding| finding.warning) + .count(); + let real = run.findings.len() - warnings; + let mut summary = match real { + 0 => "No findings".to_string(), + 1 => "1 finding".to_string(), + _ => format!("{real} findings"), + }; + if !run.suppressed.is_empty() { + summary += &format!(" ({} suppressed)", run.suppressed.len()); + } + summary += "."; + match warnings { + 0 => {} + 1 => summary += " 1 warning.", + _ => summary += &format!(" {warnings} warnings."), + } + let mut lines: Vec = run + .findings + .iter() + .map(|finding| finding.format()) + .collect(); + lines.push(summary); + run.report = lines.join("\n"); + run.error_report = run + .errors + .iter() + .map(|error| format!("error: {error}")) + .collect::>() + .join("\n"); + let fails = real > 0 || (warnings > 0 && run.strict); + run.exit_code = match (run.errors.is_empty(), fails) { + (false, _) => 2, + (true, true) => 1, + (true, false) => 0, + }; + Ok(()) + } +} diff --git a/packages/codeupipe-lint-rs/src/lib.rs b/packages/codeupipe-lint-rs/src/lib.rs new file mode 100644 index 0000000..3d052f0 --- /dev/null +++ b/packages/codeupipe-lint-rs/src/lib.rs @@ -0,0 +1,46 @@ +//! A linter for Rust code that uses codeupipe. It reads the source and does +//! not run it. +//! +//! The linter is a codeupipe pipeline ([`LintPipeline`]) that carries a type +//! of its own, [`LintRun`], in place of a `Payload`: a run holds syntax +//! trees, which are not JSON-shaped values. +//! +//! ```no_run +//! use codeupipe_lint::{lint_paths, LintOptions}; +//! +//! let out = lint_paths(&["src".to_string()], &LintOptions::default())?; +//! println!("{}", out.report); +//! # Ok::<(), codeupipe::StepError>(()) +//! ``` + +mod analyze_plain_logic_filter; +mod analyze_primitive_names_filter; +mod analyze_primitives_per_file_filter; +mod analyze_stale_markers_filter; +mod analyze_state_reads_filter; +mod analyze_steps_filter; +mod apply_suppressions_filter; +mod collect_files_filter; +mod drop_disabled_filter; +mod finding; +mod fnmatch; +mod format_report_filter; +mod lint_paths; +mod lint_pipeline; +mod lint_run; +mod logic; +mod markers; +mod parse_files_filter; +mod primitive_kind; +mod read_config; +mod rules; +mod source; +mod step_visitor; + +pub use finding::Finding; +pub use lint_paths::{lint_paths, LintOptions}; +pub use lint_pipeline::LintPipeline; +pub use lint_run::LintRun; +pub use read_config::{read_lint_config, LintConfig, CONFIG_NAME}; +pub use rules::{Rule, RULES}; +pub use source::{Marker, Source}; diff --git a/packages/codeupipe-lint-rs/src/lint_paths.rs b/packages/codeupipe-lint-rs/src/lint_paths.rs new file mode 100644 index 0000000..5ad5515 --- /dev/null +++ b/packages/codeupipe-lint-rs/src/lint_paths.rs @@ -0,0 +1,26 @@ +use codeupipe::{Pipeline, StepError}; + +use crate::lint_pipeline::LintPipeline; +use crate::lint_run::LintRun; + +/// How a run is done: the files that are left out, the rules that are off, +/// and whether a warning fails the run. +#[derive(Clone, Debug, Default)] +pub struct LintOptions { + pub exclude: Vec, + pub disable: Vec, + pub strict: bool, +} + +/// Run the linter over `paths` (files or directories). The run that comes +/// back holds `findings`, `suppressed`, `errors`, `report`, `error_report` +/// and `exit_code`. +pub fn lint_paths(paths: &[String], options: &LintOptions) -> Result { + Pipeline::named(LintPipeline).run(&LintRun { + paths: paths.to_vec(), + exclude: options.exclude.clone(), + disable: options.disable.clone(), + strict: options.strict, + ..LintRun::default() + }) +} diff --git a/packages/codeupipe-lint-rs/src/lint_pipeline.rs b/packages/codeupipe-lint-rs/src/lint_pipeline.rs new file mode 100644 index 0000000..7d8f1fc --- /dev/null +++ b/packages/codeupipe-lint-rs/src/lint_pipeline.rs @@ -0,0 +1,33 @@ +use codeupipe::{NamedPipeline, Pipeline}; + +use crate::analyze_plain_logic_filter::AnalyzePlainLogicFilter; +use crate::analyze_primitive_names_filter::AnalyzePrimitiveNamesFilter; +use crate::analyze_primitives_per_file_filter::AnalyzePrimitivesPerFileFilter; +use crate::analyze_stale_markers_filter::AnalyzeStaleMarkersFilter; +use crate::analyze_state_reads_filter::AnalyzeStateReadsFilter; +use crate::analyze_steps_filter::AnalyzeStepsFilter; +use crate::apply_suppressions_filter::ApplySuppressionsFilter; +use crate::collect_files_filter::CollectFilesFilter; +use crate::drop_disabled_filter::DropDisabledFilter; +use crate::format_report_filter::FormatReportFilter; +use crate::lint_run::LintRun; +use crate::parse_files_filter::ParseFilesFilter; + +/// The linter, as a codeupipe pipeline: `Pipeline::named(LintPipeline)`. +pub struct LintPipeline; + +impl NamedPipeline for LintPipeline { + fn add_steps(&self, pipeline: &mut Pipeline) { + pipeline.add_filter(CollectFilesFilter); // paths to files + pipeline.add_filter(ParseFilesFilter); // files to syntax trees (CUP-I000) + pipeline.add_filter(AnalyzeStepsFilter); // CUP-A003, A005 + pipeline.add_filter(AnalyzeStateReadsFilter); // CUP-I003 + pipeline.add_filter(AnalyzePrimitivesPerFileFilter); // CUP-I005 + pipeline.add_filter(AnalyzePrimitiveNamesFilter); // CUP-I006, CUP-I007 + pipeline.add_filter(AnalyzePlainLogicFilter); // CUP-I008, a warning + pipeline.add_filter(ApplySuppressionsFilter); + pipeline.add_filter(AnalyzeStaleMarkersFilter); // CUP-I009, a warning + pipeline.add_filter(DropDisabledFilter); // last, so that a comment for a rule that is off is not stale + pipeline.add_filter(FormatReportFilter); + } +} diff --git a/packages/codeupipe-lint-rs/src/lint_run.rs b/packages/codeupipe-lint-rs/src/lint_run.rs new file mode 100644 index 0000000..0d8adbe --- /dev/null +++ b/packages/codeupipe-lint-rs/src/lint_run.rs @@ -0,0 +1,38 @@ +use codeupipe::PipelineData; + +use crate::finding::Finding; +use crate::source::Source; + +/// What the lint pipeline carries from step to step: the input of a run, +/// and what each step adds. +#[derive(Clone, Default)] +pub struct LintRun { + /// Files or directories to check. + pub paths: Vec, + /// Globs matched against the path of each file; a match is not checked. + pub exclude: Vec, + /// Rule ids that are switched off. + pub disable: Vec, + /// A warning fails the run too. + pub strict: bool, + /// The Rust files under `paths` (CollectFilesFilter). + pub files: Vec, + /// The files that parse (ParseFilesFilter). + pub sources: Vec, + /// The findings that are reported. + pub findings: Vec, + /// The findings that a comment allowed (ApplySuppressionsFilter). + pub suppressed: Vec, + /// Each comment that allowed a finding, as (path, line). + pub used_markers: Vec<(String, usize)>, + /// A path that could not be read: the check is not complete. + pub errors: Vec, + /// The report for stdout (FormatReportFilter). + pub report: String, + /// The errors for stderr. + pub error_report: String, + /// 0 no findings, 1 findings, 2 a path could not be read. + pub exit_code: i32, +} + +impl PipelineData for LintRun {} diff --git a/packages/codeupipe-lint-rs/src/logic.rs b/packages/codeupipe-lint-rs/src/logic.rs new file mode 100644 index 0000000..49483dd --- /dev/null +++ b/packages/codeupipe-lint-rs/src/logic.rs @@ -0,0 +1,48 @@ +use syn::visit::{self, Visit}; + +/// Whether `node` has a decision or a loop: `if`, `match`, `for`, `while`, +/// `loop`, or `let ... else`. A loop that is a method call (`map`, +/// `filter`, `for_each`), `&&`, `||` and `?` are not seen, and nor is code +/// in a macro call. +/// +/// `walk` gives the visitor the node: `has_logic(|v| v.visit_block(block))`. +pub fn has_logic(walk: impl FnOnce(&mut LogicVisitor)) -> bool { + let mut visitor = LogicVisitor { found: false }; + walk(&mut visitor); + visitor.found +} + +pub struct LogicVisitor { + found: bool, +} + +impl<'ast> Visit<'ast> for LogicVisitor { + fn visit_expr_if(&mut self, _: &'ast syn::ExprIf) { + self.found = true; + } + + fn visit_expr_match(&mut self, _: &'ast syn::ExprMatch) { + self.found = true; + } + + fn visit_expr_for_loop(&mut self, _: &'ast syn::ExprForLoop) { + self.found = true; + } + + fn visit_expr_while(&mut self, _: &'ast syn::ExprWhile) { + self.found = true; + } + + fn visit_expr_loop(&mut self, _: &'ast syn::ExprLoop) { + self.found = true; + } + + fn visit_local(&mut self, local: &'ast syn::Local) { + let diverges = local + .init + .as_ref() + .is_some_and(|init| init.diverge.is_some()); + self.found |= diverges; + visit::visit_local(self, local); + } +} diff --git a/packages/codeupipe-lint-rs/src/main.rs b/packages/codeupipe-lint-rs/src/main.rs new file mode 100644 index 0000000..d0bcd69 --- /dev/null +++ b/packages/codeupipe-lint-rs/src/main.rs @@ -0,0 +1,108 @@ +//! codeupipe-lint [--exclude GLOB] [--disable RULE] [--strict] [--config FILE] [--rules] [paths...] +//! +//! With no paths, they come from cup.config.json. Exit code: 0 no findings, +//! 1 findings, 2 a path could not be read or the arguments cannot be used. +//! A warning does not make the exit code 1, unless --strict is given. + +use std::process::ExitCode; + +use codeupipe_lint::{lint_paths, read_lint_config, LintOptions, CONFIG_NAME, RULES}; + +/// The arguments of the command. +#[derive(Default)] +struct Args { + paths: Vec, + exclude: Option>, + disable: Option>, + config: Option, + rules: bool, + strict: bool, +} + +// cup: allow-plain the entry of the program: it reads the arguments, runs the pipeline and prints the result +fn main() -> ExitCode { + match run(std::env::args().skip(1).collect()) { + Ok(code) => ExitCode::from(code), + Err(error) => { + eprintln!("error: {error}"); + ExitCode::from(2) + } + } +} + +// cup: allow-plain the entry of the program: it reads the arguments, runs the pipeline and prints the result +fn run(argv: Vec) -> Result { + let mut args = parse(argv)?; + if args.rules { + for rule in RULES { + println!("{} {}", rule.id, rule.slug); + } + return Ok(0); + } + if args.paths.is_empty() { + // The file stands in for the arguments only when no paths are given: + // a path that someone names is checked, whatever the file excludes. + let lint = read_lint_config(args.config.as_deref())?.unwrap_or_default(); + args.paths = lint.paths.unwrap_or_default(); + args.exclude = args.exclude.or(lint.exclude); + args.disable = args.disable.or(lint.disable); + } + if args.paths.is_empty() { + return Err(format!( + "give at least one path, or list them under lint.paths in {CONFIG_NAME}" + )); + } + let disable = args.disable.unwrap_or_default(); + let unknown: Vec<&str> = disable + .iter() + .filter(|id| !RULES.iter().any(|rule| rule.id == id.as_str())) + .map(String::as_str) + .collect(); + if !unknown.is_empty() { + return Err(format!( + "no such rule: {} (the rules are listed by --rules)", + unknown.join(", ") + )); + } + let options = LintOptions { + exclude: args.exclude.unwrap_or_default(), + disable, + strict: args.strict, + }; + let out = lint_paths(&args.paths, &options).map_err(|error| error.to_string())?; + println!("{}", out.report); + if !out.error_report.is_empty() { + eprintln!("{}", out.error_report); + } + Ok(out.exit_code as u8) +} + +// cup: allow-plain the entry of the program: it reads the arguments, runs the pipeline and prints the result +fn parse(argv: Vec) -> Result { + let mut args = Args::default(); + let mut argv = argv.into_iter(); + while let Some(arg) = argv.next() { + let (name, inline) = match arg.split_once('=') { + Some((name, value)) if name.starts_with("--") => { + (name.to_string(), Some(value.to_string())) + } + _ => (arg.clone(), None), + }; + let mut value = || { + inline + .clone() + .or_else(|| argv.next()) + .ok_or_else(|| format!("option '{name}' needs a value")) + }; + match name.as_str() { + "--exclude" => args.exclude.get_or_insert_with(Vec::new).push(value()?), + "--disable" => args.disable.get_or_insert_with(Vec::new).push(value()?), + "--config" => args.config = Some(value()?), + "--rules" => args.rules = true, + "--strict" => args.strict = true, + _ if name.starts_with("--") => return Err(format!("unknown option '{name}'")), + _ => args.paths.push(arg), + } + } + Ok(args) +} diff --git a/packages/codeupipe-lint-rs/src/markers.rs b/packages/codeupipe-lint-rs/src/markers.rs new file mode 100644 index 0000000..0b80b28 --- /dev/null +++ b/packages/codeupipe-lint-rs/src/markers.rs @@ -0,0 +1,169 @@ +//! The `cup: allow-...` comments of a file. +//! +//! A syntax tree has no comments, so they are found between the tokens: the +//! text between two tokens is space and comments only. A doc comment is a +//! token (an attribute), so text in it is documentation, not a marker, and +//! so is text in a string. + +use proc_macro2::{LineColumn, TokenStream, TokenTree}; + +use crate::source::Marker; + +/// Each line of a comment of `text` that starts with `cup: allow-...`. +// cup: allow-plain used by the parse Filter: it finds the comments of a file, which a syntax tree does not keep +pub fn markers_of(text: &str) -> Vec { + let Ok(tokens) = text.parse::() else { + return Vec::new(); + }; + let lines = Lines::new(text); + let mut taken = Vec::new(); + spans_of(tokens, &mut taken); + let mut taken: Vec<(usize, usize)> = taken + .into_iter() + .map(|(start, end)| (lines.offset(start), lines.offset(end))) + .collect(); + taken.sort_unstable(); + let mut markers = Vec::new(); + let mut at = 0; + for (start, end) in taken { + comments_in(text, &lines, at, start, &mut markers); + at = at.max(end); + } + comments_in(text, &lines, at, text.len(), &mut markers); + markers +} + +/// Where each token starts and ends. +// cup: allow-plain used by the parse Filter: it finds the comments of a file, which a syntax tree does not keep +fn spans_of(tokens: TokenStream, out: &mut Vec<(LineColumn, LineColumn)>) { + for token in tokens { + if let TokenTree::Group(group) = &token { + out.push((group.span_open().start(), group.span_open().end())); + out.push((group.span_close().start(), group.span_close().end())); + spans_of(group.stream(), out); + } else { + out.push((token.span().start(), token.span().end())); + } + } +} + +/// The comments in `text[from..to]`, text that holds space and comments only. +// cup: allow-plain used by the parse Filter: it finds the comments of a file, which a syntax tree does not keep +fn comments_in(text: &str, lines: &Lines, from: usize, to: usize, out: &mut Vec) { + let mut at = from; + while at < to { + let rest = &text[at..to]; + let end = if rest.starts_with("//") { + at + rest.find('\n').unwrap_or(rest.len()) + } else if rest.starts_with("/*") { + at + block_comment_length(rest) + } else { + at += rest.chars().next().map_or(1, char::len_utf8); + continue; + }; + markers_in_comment(text, lines, at, end, out); + at = end; + } +} + +/// The length of the block comment at the start of `text`. Block comments nest. +// cup: allow-plain used by the parse Filter: it finds the comments of a file, which a syntax tree does not keep +fn block_comment_length(text: &str) -> usize { + let mut depth = 0; + let mut at = 0; + while at < text.len() { + let rest = &text[at..]; + if rest.starts_with("/*") { + depth += 1; + at += 2; + } else if rest.starts_with("*/") { + depth -= 1; + at += 2; + if depth == 0 { + return at; + } + } else { + at += rest.chars().next().map_or(1, char::len_utf8); + } + } + text.len() +} + +/// Each line of the comment `text[from..to]` that starts with a marker. +// cup: allow-plain used by the parse Filter: it finds the comments of a file, which a syntax tree does not keep +fn markers_in_comment(text: &str, lines: &Lines, from: usize, to: usize, out: &mut Vec) { + let mut at = from; + for (index, piece) in text[from..to].split('\n').enumerate() { + let line = lines.line_of(at); + let line_start = lines.starts[line - 1]; + let lead = piece.len() - strip_comment_lead(piece).len(); + if let Some(kind) = marker_kind(&piece[lead..]) { + out.push(Marker { + line, + col: text[line_start..at + lead].chars().count() + 1, + kind, + // The first line of the comment may have code before it. + alone: index > 0 || text[line_start..at].trim().is_empty(), + }); + } + at += piece.len() + 1; + } +} + +/// `piece` without the space and the comment characters at its start: `//`, +/// `/*`, or the `*` of a line inside a block comment. +// cup: allow-plain used by the parse Filter: it finds the comments of a file, which a syntax tree does not keep +fn strip_comment_lead(piece: &str) -> &str { + let piece = piece.trim_start(); + let piece = if piece.starts_with("//") { + piece.trim_start_matches('/') + } else if let Some(rest) = piece.strip_prefix("/*") { + rest.trim_start_matches('*') + } else if piece.starts_with('*') && !piece.starts_with("*/") { + &piece[1..] + } else { + piece + }; + piece.trim_start() +} + +/// `allow-plain` for `cup: allow-plain the reason`. +fn marker_kind(text: &str) -> Option { + let rest = text.strip_prefix("cup:")?.trim_start(); + rest.starts_with("allow-").then(|| { + rest.chars() + .take_while(|c| c.is_alphanumeric() || *c == '_' || *c == '-') + .collect() + }) +} + +/// The byte offset where each line of a text starts. +struct Lines<'a> { + text: &'a str, + starts: Vec, +} + +impl<'a> Lines<'a> { + fn new(text: &'a str) -> Self { + let breaks = text.match_indices('\n').map(|(at, _)| at + 1); + Lines { + text, + starts: std::iter::once(0).chain(breaks).collect(), + } + } + + /// The byte offset of a line (from 1) and column (from 0, in characters). + fn offset(&self, at: LineColumn) -> usize { + let start = self.starts[at.line - 1]; + start + + self.text[start..] + .char_indices() + .nth(at.column) + .map_or(self.text.len() - start, |(index, _)| index) + } + + /// The line (from 1) of a byte offset. + fn line_of(&self, offset: usize) -> usize { + self.starts.partition_point(|start| *start <= offset) + } +} diff --git a/packages/codeupipe-lint-rs/src/parse_files_filter.rs b/packages/codeupipe-lint-rs/src/parse_files_filter.rs new file mode 100644 index 0000000..160fcbc --- /dev/null +++ b/packages/codeupipe-lint-rs/src/parse_files_filter.rs @@ -0,0 +1,53 @@ +use std::fs; + +use codeupipe::{Filter, StepError}; + +use crate::finding::Finding; +use crate::lint_run::LintRun; +use crate::markers::markers_of; +use crate::rules::{rule, PARSE_ERROR}; +use crate::source::{position, Source}; + +/// Reads and parses each file. +/// +/// Reads `files`. Writes `sources` (the files that parse, with their +/// `cup: allow-...` comments), a CUP-I000 finding for a file with a syntax +/// error, and `errors` for a file that cannot be read. +pub struct ParseFilesFilter; + +impl Filter for ParseFilesFilter { + fn call(&self, run: &mut LintRun) -> Result<(), StepError> { + for path in run.files.clone() { + let text = match fs::read_to_string(&path) { + Ok(text) => text, + Err(error) => { + run.errors.push(format!("{path}: {error}")); + continue; + } + }; + match syn::parse_file(&text) { + Ok(tree) => run.sources.push(Source { + markers: markers_of(&text), + path, + text, + tree, + }), + Err(error) => { + let (line, col) = position(error.span()); + let parse_error = rule(PARSE_ERROR).expect("a rule"); + run.findings.push(Finding { + path, + line, + col, + rule: PARSE_ERROR.to_string(), + slug: parse_error.slug.to_string(), + message: format!("{error}. {}", parse_error.fix), + call_line: None, + warning: false, + }); + } + } + } + Ok(()) + } +} diff --git a/packages/codeupipe-lint-rs/src/primitive_kind.rs b/packages/codeupipe-lint-rs/src/primitive_kind.rs new file mode 100644 index 0000000..d3bc3f8 --- /dev/null +++ b/packages/codeupipe-lint-rs/src/primitive_kind.rs @@ -0,0 +1,182 @@ +use std::collections::HashMap; +use std::path::Path; + +use proc_macro2::Span; +use syn::spanned::Spanned; +use syn::{Attribute, Item, Meta, Type}; + +/// The primitives that a type can be. A type is a primitive when it +/// implements the trait: `impl Filter for DiscountFilter`. A named pipeline +/// implements `NamedPipeline`. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub enum PrimitiveKind { + Pipeline, + Hook, + Filter, + Tap, + Predicate, +} + +// cup: allow-plain shared by the analysis Filters: it says which primitive a type is, from its trait +impl PrimitiveKind { + /// The kind of the trait at the end of `path` (`codeupipe::Filter` is a Filter). + pub fn of_trait(path: &syn::Path) -> Option { + let name = path.segments.last()?.ident.to_string(); + [ + ("NamedPipeline", PrimitiveKind::Pipeline), + ("Hook", PrimitiveKind::Hook), + ("Filter", PrimitiveKind::Filter), + ("Tap", PrimitiveKind::Tap), + ("Predicate", PrimitiveKind::Predicate), + ] + .into_iter() + .find(|(trait_name, _)| *trait_name == name) + .map(|(_, kind)| kind) + } + + /// The role word: `Filter`, `Pipeline`, ... + pub fn name(self) -> &'static str { + match self { + PrimitiveKind::Pipeline => "Pipeline", + PrimitiveKind::Hook => "Hook", + PrimitiveKind::Filter => "Filter", + PrimitiveKind::Tap => "Tap", + PrimitiveKind::Predicate => "Predicate", + } + } +} + +/// A type at the top level of a file that is a primitive. +pub struct Primitive { + pub name: String, + pub kind: PrimitiveKind, + /// The name in the declaration of the type, or in the first `impl` of a + /// primitive trait when the type is declared in another file. + pub at: Span, +} + +/// The name of the type that an `impl` is for: `DiscountFilter` for +/// `impl Filter for DiscountFilter` and for `impl

Filter

for DiscountFilter

`. +// cup: allow-plain shared by the analysis Filters: it says which primitive a type is, from its trait +pub fn self_type_name(self_ty: &Type) -> Option { + match self_ty { + Type::Path(path) => path.path.segments.last().map(|s| s.ident.to_string()), + _ => None, + } +} + +/// The primitives among `items`, in the order of the file. A type that +/// implements two primitive traits is one primitive, of the first kind of +/// Pipeline, Hook, Filter, Tap, Predicate. +// cup: allow-plain shared by the analysis Filters: it says which primitive a type is, from its trait +pub fn primitives_in(items: &[Item]) -> Vec { + let mut declared = HashMap::new(); + let mut found: Vec = Vec::new(); + for item in items + .iter() + .filter(|item| !is_test_only(attributes_of(item))) + { + match item { + Item::Struct(item) => { + declared.insert(item.ident.to_string(), item.ident.span()); + } + Item::Enum(item) => { + declared.insert(item.ident.to_string(), item.ident.span()); + } + Item::Impl(item) => { + let kind = item + .trait_ + .as_ref() + .and_then(|(_, path, _)| PrimitiveKind::of_trait(path)); + let (Some(kind), Some(name)) = (kind, self_type_name(&item.self_ty)) else { + continue; + }; + match found.iter_mut().find(|primitive| primitive.name == name) { + Some(primitive) => primitive.kind = primitive.kind.min(kind), + None => found.push(Primitive { + name, + kind, + at: item.self_ty.span(), + }), + } + } + _ => {} + } + } + for primitive in &mut found { + if let Some(at) = declared.get(&primitive.name) { + primitive.at = *at; + } + } + found.sort_by_key(|primitive| { + let start = primitive.at.start(); + (start.line, start.column) + }); + found +} + +/// The names of the types that are primitives, at any level of the file. +// cup: allow-plain shared by the analysis Filters: it says which primitive a type is, from its trait +pub fn primitive_names(items: &[Item], out: &mut Vec) { + for item in items { + match item { + Item::Impl(item) => { + let is_primitive = item + .trait_ + .as_ref() + .and_then(|(_, path, _)| PrimitiveKind::of_trait(path)) + .is_some(); + out.extend(self_type_name(&item.self_ty).filter(|_| is_primitive)); + } + Item::Mod(item) => { + if let Some((_, items)) = &item.content { + primitive_names(items, out); + } + } + _ => {} + } + } +} + +/// A file under a `tests` directory: an integration test of Cargo. +pub fn is_test_file(path: &str) -> bool { + let path = Path::new(path); + path.parent() + .is_some_and(|parent| parent.components().any(|part| part.as_os_str() == "tests")) +} + +/// The attributes of an item. +// cup: allow-plain shared by the analysis Filters: it says which primitive a type is, from its trait +pub fn attributes_of(item: &Item) -> &[Attribute] { + match item { + Item::Const(item) => &item.attrs, + Item::Enum(item) => &item.attrs, + Item::Fn(item) => &item.attrs, + Item::Impl(item) => &item.attrs, + Item::Mod(item) => &item.attrs, + Item::Static(item) => &item.attrs, + Item::Struct(item) => &item.attrs, + Item::Trait(item) => &item.attrs, + Item::Type(item) => &item.attrs, + Item::Union(item) => &item.attrs, + Item::Use(item) => &item.attrs, + _ => &[], + } +} + +/// `#[test]`, or `#[cfg(test)]` and the like: code for the tests only. +// cup: allow-plain shared by the analysis Filters: it says which primitive a type is, from its trait +pub fn is_test_only(attributes: &[Attribute]) -> bool { + attributes.iter().any(|attribute| match &attribute.meta { + Meta::Path(path) => path.is_ident("test"), + Meta::List(list) => { + list.path.is_ident("cfg") + && list + .tokens + .clone() + .into_iter() + .any(|token| token.to_string() == "test") + } + Meta::NameValue(_) => false, + }) +} diff --git a/packages/codeupipe-lint-rs/src/read_config.rs b/packages/codeupipe-lint-rs/src/read_config.rs new file mode 100644 index 0000000..5713618 --- /dev/null +++ b/packages/codeupipe-lint-rs/src/read_config.rs @@ -0,0 +1,67 @@ +use std::fs; +use std::path::Path; + +use serde_json::Value; + +/// The file that holds the settings of the project's tools. +pub const CONFIG_NAME: &str = "cup.config.json"; +const LINT_KEYS: &[&str] = &["paths", "exclude", "disable"]; + +/// The `lint` section of the config file. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct LintConfig { + pub paths: Option>, + pub exclude: Option>, + pub disable: Option>, +} + +/// The `lint` section of the config file, checked. `None` when the file is +/// not there. With `path` given, a file that is not there is an error. +/// +/// A key that the linter does not read is an error, so the file cannot hold +/// a setting that does nothing. Other sections of the file are for other +/// tools. +// cup: allow-plain the edge of the program: it reads the config file before the pipeline runs +pub fn read_lint_config(path: Option<&str>) -> Result, String> { + let name = path.unwrap_or(CONFIG_NAME); + if !Path::new(name).exists() { + return match path { + None => Ok(None), + Some(_) => Err(format!("{name}: no such file")), + }; + } + let text = fs::read_to_string(name).map_err(|error| format!("{name}: {error}"))?; + let config: Value = serde_json::from_str(&text) + .map_err(|error| format!("{name} is not valid JSON: {error}"))?; + let Value::Object(config) = config else { + return Err(format!("{name} must hold a JSON object")); + }; + let lint = match config.get("lint") { + None => return Ok(Some(LintConfig::default())), + Some(Value::Object(lint)) => lint, + Some(_) => return Err(format!("{name}: lint must be an object")), + }; + let mut out = LintConfig::default(); + for (key, value) in lint { + let strings: Option> = value.as_array().and_then(|items| { + items + .iter() + .map(|item| item.as_str().map(String::from)) + .collect() + }); + let slot = match key.as_str() { + "paths" => &mut out.paths, + "exclude" => &mut out.exclude, + "disable" => &mut out.disable, + _ => { + return Err(format!( + "{name}: lint: unknown key '{key}' (the keys are {})", + LINT_KEYS.join(", ") + )) + } + }; + *slot = + Some(strings.ok_or_else(|| format!("{name}: lint.{key} must be a list of strings"))?); + } + Ok(Some(out)) +} diff --git a/packages/codeupipe-lint-rs/src/rules.rs b/packages/codeupipe-lint-rs/src/rules.rs new file mode 100644 index 0000000..0254ef0 --- /dev/null +++ b/packages/codeupipe-lint-rs/src/rules.rs @@ -0,0 +1,119 @@ +use proc_macro2::Span; + +use crate::finding::Finding; +use crate::source::{position, Source}; + +pub const PARSE_ERROR: &str = "CUP-I000"; +pub const LOCAL_TYPE_STEP: &str = "CUP-A003"; +pub const UNNAMED_PIPELINE: &str = "CUP-A005"; +pub const STATE_READ: &str = "CUP-I003"; +pub const ONE_PRIMITIVE_PER_FILE: &str = "CUP-I005"; +pub const PRIMITIVE_FILE_NAME: &str = "CUP-I006"; +pub const PRIMITIVE_ROLE_SUFFIX: &str = "CUP-I007"; +pub const LOGIC_OUTSIDE_PRIMITIVE: &str = "CUP-I008"; +pub const MARKER_ALLOWS_NOTHING: &str = "CUP-I009"; + +/// One rule: its id, its short name, what to do about a finding, and +/// whether a finding is a warning. +pub struct Rule { + pub id: &'static str, + pub slug: &'static str, + pub fix: &'static str, + pub warning: bool, +} + +const NAMED_TYPE: &str = "Declare it at module level as a named type (a Valve predicate: a type that implements Predicate) \ +so it can be tested on its own."; + +/// The rules, in the order of their ids. The ids and names are those of the +/// Python and TypeScript linters. A rule that the Rust compiler checks +/// (CUP-A001, A002, A008, A009, I004) is not here. +pub const RULES: &[Rule] = &[ + Rule { + id: LOCAL_TYPE_STEP, + slug: "local-type-step", + fix: NAMED_TYPE, + warning: false, + }, + Rule { + id: UNNAMED_PIPELINE, + slug: "unnamed-pipeline-step", + fix: "A nested pipeline must be a named pipeline (a type that implements NamedPipeline, nested as \ +Pipeline::named(UserOnboarding)), named like a filter. A plain Pipeline can only run at the top level.", + warning: false, + }, + Rule { + id: PARSE_ERROR, + slug: "parse-error", + fix: "Fix the syntax error so the file can be checked.", + warning: false, + }, + Rule { + id: STATE_READ, + slug: "state-read-in-step", + fix: "A Filter's result and a Valve's decision must depend only on the Payload. Put what the step needs \ +into the Payload, or observe State from a Tap or a Hook.", + warning: false, + }, + Rule { + id: ONE_PRIMITIVE_PER_FILE, + slug: "one-primitive-per-file", + fix: "Give each Filter, Predicate, Tap, Hook and named Pipeline its own file, so it can be tested alone \ +and used again. The pipeline file is where they come together.", + warning: false, + }, + Rule { + id: PRIMITIVE_FILE_NAME, + slug: "primitive-file-name", + fix: "The file name of a primitive is its type name in snake_case, so one name gives the other.", + warning: false, + }, + Rule { + id: PRIMITIVE_ROLE_SUFFIX, + slug: "primitive-role-suffix", + fix: "A primitive's type name ends with its role (Filter, Tap, Hook, Predicate, Pipeline), so the name \ +tells what the type is.", + warning: false, + }, + Rule { + id: LOGIC_OUTSIDE_PRIMITIVE, + slug: "logic-outside-primitive", + fix: "Write it as a primitive, so that the pipeline has the logic and it can be tested as a step. For \ +logic that must be plain code, say why: // cup: allow-plain ", + warning: true, + }, + Rule { + id: MARKER_ALLOWS_NOTHING, + slug: "marker-allows-nothing", + fix: "Remove it, or put it on the line of the finding that it is for. A comment that allows nothing says \ +that the code has an exception that it does not have.", + warning: true, + }, +]; + +/// The rule with the id `id`. +pub fn rule(id: &str) -> Option<&'static Rule> { + RULES.iter().find(|rule| rule.id == id) +} + +/// A Finding of `rule` at `at`. Its message reads `. `. +pub fn make_finding( + source: &Source, + at: Span, + id: &str, + what: &str, + call_line: Option, +) -> Finding { + let (line, col) = position(at); + let rule = rule(id).expect("a rule of this linter"); + Finding { + path: source.path.clone(), + line, + col, + rule: id.to_string(), + slug: rule.slug.to_string(), + message: format!("{what}. {}", rule.fix), + call_line: call_line.filter(|call_line| *call_line != line), + warning: rule.warning, + } +} diff --git a/packages/codeupipe-lint-rs/src/source.rs b/packages/codeupipe-lint-rs/src/source.rs new file mode 100644 index 0000000..b0cb826 --- /dev/null +++ b/packages/codeupipe-lint-rs/src/source.rs @@ -0,0 +1,53 @@ +use proc_macro2::Span; + +/// One parsed file. +#[derive(Clone)] +pub struct Source { + pub path: String, + pub text: String, + pub tree: syn::File, + /// The `cup: allow-...` comments of the file. + pub markers: Vec, +} + +/// A comment line that starts with `cup: allow-...`. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Marker { + pub line: usize, + pub col: usize, + /// `allow-plain`, `allow-name`, ...: the word after `cup:`. + pub kind: String, + /// The line holds the comment and no code. + pub alone: bool, +} + +/// The line and column where `span` starts, from 1. +pub fn position(span: Span) -> (usize, usize) { + let start = span.start(); + (start.line, start.column + 1) +} + +/// The text of `span` in `text`. The lines of a span over several lines are +/// joined with their indent taken away: `self.pipeline.state()`. +// cup: allow-plain shared by the analysis Filters: the text of a node, for a message +pub fn text_of(text: &str, span: Span) -> String { + let (start, end) = (span.start(), span.end()); + text.lines() + .enumerate() + .skip(start.line - 1) + .take(end.line + 1 - start.line) + .map(|(index, line)| { + let to = (index + 1 == end.line).then_some(end.column); + let from = (index + 1 == start.line).then_some(start.column); + let taken: String = line + .chars() + .take(to.unwrap_or(usize::MAX)) + .skip(from.unwrap_or(0)) + .collect(); + match from { + Some(_) => taken, + None => taken.trim_start().to_string(), + } + }) + .collect() +} diff --git a/packages/codeupipe-lint-rs/src/step_visitor.rs b/packages/codeupipe-lint-rs/src/step_visitor.rs new file mode 100644 index 0000000..3869b5b --- /dev/null +++ b/packages/codeupipe-lint-rs/src/step_visitor.rs @@ -0,0 +1,266 @@ +//! Finds the steps that CUP-A003 and CUP-A005 are about, with the names in +//! scope at each `add_...` call. + +use syn::spanned::Spanned; +use syn::visit::{self, Visit}; +use syn::{Expr, Item, Pat, Stmt}; + +use crate::finding::Finding; +use crate::rules::{make_finding, LOCAL_TYPE_STEP, UNNAMED_PIPELINE}; +use crate::source::{position, text_of, Source}; + +/// How far a name is followed, from a binding to the value it was given. +const FOLLOW: usize = 8; + +/// A block, a function or a closure: the names that it declares. +#[derive(Default)] +struct Scope { + /// The types declared in a block. A block's items are seen in all of it. + types: Vec, + /// The bindings, in order: a `let` with its value, or a parameter or a + /// pattern with none. + bindings: Vec<(String, Option)>, + /// A function: it does not see the bindings of the code around it. + function: bool, +} + +/// What is wrong with a step: the rule, and words that say what the value is. +struct Verdict { + rule: &'static str, + what: String, +} + +/// Visits a file and finds the steps of CUP-A003 and CUP-A005. +pub struct StepVisitor<'a> { + source: &'a Source, + scopes: Vec, + pub findings: Vec, +} + +// cup: allow-plain a visitor of a syntax tree: the tool that a Filter uses to read source code +impl<'a> StepVisitor<'a> { + pub fn new(source: &'a Source) -> Self { + StepVisitor { + source, + scopes: Vec::new(), + findings: Vec::new(), + } + } + + fn in_scope(&mut self, function: bool, types: Vec, walk: impl FnOnce(&mut Self)) { + self.scopes.push(Scope { + types, + function, + bindings: Vec::new(), + }); + walk(self); + self.scopes.pop(); + } + + fn bind(&mut self, name: String, value: Option) { + if let Some(scope) = self.scopes.last_mut() { + scope.bindings.push((name, value)); + } + } + + /// The value that the binding `name` was given: `None` when the name is + /// not bound, `Some(None)` when it is bound with no value to follow. + fn binding(&self, name: &str) -> Option> { + for scope in self.scopes.iter().rev() { + if let Some((_, value)) = scope.bindings.iter().rev().find(|(bound, _)| bound == name) { + return Some(value.as_ref()); + } + if scope.function { + return None; + } + } + None + } + + /// The type `name` is declared in a block of a function. + fn is_local_type(&self, name: &str) -> bool { + self.scopes + .iter() + .any(|scope| scope.types.iter().any(|declared| declared == name)) + } + + fn check_call(&mut self, call: &syn::ExprMethodCall) { + let roles: &[&str] = match call.method.to_string().as_str() { + "add_filter" => &["Filter"], + "add_tap" => &["Tap"], + "add_pipeline" => &["Pipeline"], + "add_valve" => &["Valve predicate", "Valve filter"], + _ => &[], + }; + let call_line = position(call.method.span()).0; + for (role, argument) in roles.iter().zip(&call.args) { + if let Some(verdict) = self.classify(argument, role, 0) { + let what = format!("{role} {}", verdict.what); + let finding = make_finding( + self.source, + argument.span(), + verdict.rule, + &what, + Some(call_line), + ); + self.findings.push(finding); + } + } + } + + /// What is wrong with `value` as a step of `role`, if anything. + fn classify(&self, value: &Expr, role: &str, depth: usize) -> Option { + if depth > FOLLOW { + return None; + } + match unwrap(value) { + Expr::Path(path) if path.qself.is_none() && path.path.segments.len() == 1 => { + let name = path.path.segments[0].ident.to_string(); + match self.binding(&name) { + Some(Some(bound)) => { + self.classify(bound, role, depth + 1) + .map(|verdict| Verdict { + rule: verdict.rule, + what: format!("`{name}` {}", verdict.what), + }) + } + Some(None) => None, + None => self.local_type(&name), + } + } + Expr::Struct(value) => self.type_of_path(&value.path), + Expr::Call(call) => { + let Expr::Path(function) = &*call.func else { + return None; + }; + let names: Vec = function + .path + .segments + .iter() + .map(|s| s.ident.to_string()) + .collect(); + let names: Vec<&str> = names.iter().map(String::as_str).collect(); + match names.as_slice() { + [.., "Pipeline", "new" | "default"] if role == "Pipeline" => Some(Verdict { + rule: UNNAMED_PIPELINE, + what: format!("is a plain `{}`", text_of(&self.source.text, call.span())), + }), + [.., "Pipeline", "named"] => call + .args + .first() + .and_then(|named| self.classify(named, "named", depth + 1)), + // `LocalTap::new()`, and `LocalTap(1)` for a tuple struct. + [name, _] | [name] => self.local_type(name), + _ => None, + } + } + Expr::MethodCall(call) if call.method == "clone" && call.args.is_empty() => { + self.classify(&call.receiver, role, depth + 1) + } + _ => None, + } + } + + fn type_of_path(&self, path: &syn::Path) -> Option { + match path.segments.len() { + 1 => self.local_type(&path.segments[0].ident.to_string()), + _ => None, + } + } + + fn local_type(&self, name: &str) -> Option { + self.is_local_type(name).then(|| Verdict { + rule: LOCAL_TYPE_STEP, + what: format!("is an instance of `{name}`, a type that is declared in a function"), + }) + } +} + +/// `value` without the parentheses and the `&` around it. +// cup: allow-plain a visitor of a syntax tree: the tool that a Filter uses to read source code +fn unwrap(value: &Expr) -> &Expr { + match value { + Expr::Paren(inner) => unwrap(&inner.expr), + Expr::Group(inner) => unwrap(&inner.expr), + Expr::Reference(inner) => unwrap(&inner.expr), + _ => value, + } +} + +/// The name of a pattern that binds one name: `x`, `mut x`, `x: T`. +// cup: allow-plain a visitor of a syntax tree: the tool that a Filter uses to read source code +fn name_of(pattern: &Pat) -> Option { + match pattern { + Pat::Ident(ident) if ident.subpat.is_none() => Some(ident.ident.to_string()), + Pat::Type(typed) => name_of(&typed.pat), + _ => None, + } +} + +// cup: allow-plain a visitor of a syntax tree: the tool that a Filter uses to read source code +impl<'ast> Visit<'ast> for StepVisitor<'_> { + fn visit_block(&mut self, block: &'ast syn::Block) { + let types = block + .stmts + .iter() + .filter_map(|stmt| match stmt { + Stmt::Item(Item::Struct(item)) => Some(item.ident.to_string()), + Stmt::Item(Item::Enum(item)) => Some(item.ident.to_string()), + Stmt::Item(Item::Union(item)) => Some(item.ident.to_string()), + _ => None, + }) + .collect(); + self.in_scope(false, types, |me| visit::visit_block(me, block)); + } + + fn visit_item_fn(&mut self, item: &'ast syn::ItemFn) { + self.in_scope(true, Vec::new(), |me| visit::visit_item_fn(me, item)); + } + + fn visit_impl_item_fn(&mut self, item: &'ast syn::ImplItemFn) { + self.in_scope(true, Vec::new(), |me| visit::visit_impl_item_fn(me, item)); + } + + fn visit_trait_item_fn(&mut self, item: &'ast syn::TraitItemFn) { + self.in_scope(true, Vec::new(), |me| visit::visit_trait_item_fn(me, item)); + } + + fn visit_expr_closure(&mut self, closure: &'ast syn::ExprClosure) { + self.in_scope(false, Vec::new(), |me| { + visit::visit_expr_closure(me, closure) + }); + } + + fn visit_arm(&mut self, arm: &'ast syn::Arm) { + self.in_scope(false, Vec::new(), |me| visit::visit_arm(me, arm)); + } + + fn visit_expr_for_loop(&mut self, for_loop: &'ast syn::ExprForLoop) { + self.in_scope(false, Vec::new(), |me| { + visit::visit_expr_for_loop(me, for_loop) + }); + } + + fn visit_local(&mut self, local: &'ast syn::Local) { + if let Some(init) = &local.init { + self.visit_expr(&init.expr); + if let Some((_, diverge)) = &init.diverge { + self.visit_expr(diverge); + } + } + match name_of(&local.pat) { + Some(name) => self.bind(name, local.init.as_ref().map(|init| (*init.expr).clone())), + None => self.visit_pat(&local.pat), + } + } + + fn visit_pat_ident(&mut self, pattern: &'ast syn::PatIdent) { + self.bind(pattern.ident.to_string(), None); + visit::visit_pat_ident(self, pattern); + } + + fn visit_expr_method_call(&mut self, call: &'ast syn::ExprMethodCall) { + self.check_call(call); + visit::visit_expr_method_call(self, call); + } +} diff --git a/packages/codeupipe-lint-rs/tests/anonymous_steps.rs b/packages/codeupipe-lint-rs/tests/anonymous_steps.rs new file mode 100644 index 0000000..8285ea3 --- /dev/null +++ b/packages/codeupipe-lint-rs/tests/anonymous_steps.rs @@ -0,0 +1,177 @@ +// CUP-A003 and CUP-A005: a step whose type has no place a test can reach, +// and a plain Pipeline nested as a step. A closure or a function as a step +// (CUP-A001, A002, A008) is a compile error in Rust, so it has no rule here. +mod common; + +use common::{disable, lint_in}; + +const HEAD: &str = "use codeupipe::{Filter, NamedPipeline, Payload, Pipeline, Predicate, StepError, Tap}; +struct AddFilter; +impl Filter for AddFilter { fn call(&self, _payload: &mut Payload) -> Result<(), StepError> { Ok(()) } } +struct IsOnPredicate; +impl Predicate for IsOnPredicate { fn test(&self, _payload: &Payload) -> bool { true } } +struct InnerPipeline; +impl NamedPipeline for InnerPipeline { fn add_steps(&self, _pipeline: &mut Pipeline) {} } +"; + +fn build(body: &str) -> common::Linted { + let text = format!("{HEAD}{body}"); + lint_in( + &[("build.rs", &text)], + disable(&["CUP-I005", "CUP-I006", "CUP-I008"]), + ) +} + +#[test] +fn named_types_at_module_level_are_clean() { + let out = build( + "fn build() { + let mut pipeline = Pipeline::new(); + pipeline.add_filter(AddFilter); + pipeline.add_valve(IsOnPredicate, AddFilter); + let kept = AddFilter; + pipeline.add_filter(kept.clone()); + pipeline.add_tap(audit::AuditTap::new()); + pipeline.add_pipeline(Pipeline::named(InnerPipeline)); +} +", + ); + assert_eq!(out.found, Vec::::new()); + assert_eq!(out.exit_code, 0); + assert_eq!(out.report, "No findings."); +} + +#[test] +fn a003_a_type_that_is_declared_in_a_function() { + let out = build( + "fn build() { + struct LocalFilter; + let mut pipeline = Pipeline::new(); + pipeline.add_filter(LocalFilter); + let kept = LocalFilter; + pipeline.add_filter(kept); + pipeline.add_valve(IsOnPredicate, LocalFilter {}); + pipeline.add_tap(LocalTap::new()); + pipeline.add_pipeline(Pipeline::named(LocalPipeline)); + struct LocalTap; + struct LocalPipeline; + fn helper(pipeline: &mut Pipeline) { + pipeline.add_filter(LocalFilter); + } +} +", + ); + assert_eq!( + out.found, + [ + "build.rs:11 CUP-A003", + "build.rs:13 CUP-A003", + "build.rs:14 CUP-A003", + "build.rs:15 CUP-A003", + "build.rs:16 CUP-A003", + "build.rs:20 CUP-A003" + ] + ); + assert!(out.findings[0].message.starts_with( + "Filter is an instance of `LocalFilter`, a type that is declared in a function. Declare it" + )); + assert!(out.findings[1] + .message + .starts_with("Filter `kept` is an instance of `LocalFilter`,")); + assert!(out.findings[2].message.starts_with("Valve filter is")); + assert_eq!(out.exit_code, 1); +} + +#[test] +fn a005_a_plain_pipeline_as_a_step() { + let out = build( + "fn build() { + let mut pipeline = Pipeline::new(); + pipeline.add_pipeline(Pipeline::new()); + let mut inner = Pipeline::new(); + inner.add_filter(AddFilter); + pipeline.add_pipeline(inner); + pipeline.add_pipeline(Pipeline::named(InnerPipeline)); + pipeline.add_pipeline(Pipeline::::default()); +} +", + ); + assert_eq!( + out.found, + [ + "build.rs:10 CUP-A005", + "build.rs:13 CUP-A005", + "build.rs:15 CUP-A005" + ] + ); + assert!(out.findings[0] + .message + .starts_with("Pipeline is a plain `Pipeline::new()`. A nested pipeline must be")); + assert!(out.findings[1] + .message + .starts_with("Pipeline `inner` is a plain `Pipeline::new()`.")); + assert!(out.findings[2] + .message + .starts_with("Pipeline is a plain `Pipeline::::default()`.")); +} + +#[test] +fn a_name_from_elsewhere_a_parameter_and_a_name_bound_again_are_not_flagged() { + let out = build( + "use other::Imported; +fn build(parameter: Pipeline) { + let mut pipeline = Pipeline::new(); + pipeline.add_filter(Imported); + pipeline.add_pipeline(parameter); + let inner = Pipeline::new(); + let inner = Pipeline::named(InnerPipeline); + pipeline.add_pipeline(inner); + pipeline.add_filter(other::LocalFilter); + let make = |inner: Pipeline| pipeline.add_pipeline(inner); + for inner in list { pipeline.add_pipeline(inner); } +} +", + ); + assert_eq!(out.found, Vec::::new()); +} + +#[test] +fn the_add_steps_of_a_named_pipeline_is_checked_too() { + let out = lint_in( + &[( + "pricing_pipeline.rs", + "use codeupipe::{NamedPipeline, Pipeline}; +pub struct PricingPipeline; +impl NamedPipeline for PricingPipeline { + fn add_steps(&self, pipeline: &mut Pipeline) { + pipeline.add_pipeline(Pipeline::new()); + } +} +", + )], + Default::default(), + ); + assert_eq!(out.found, ["pricing_pipeline.rs:5 CUP-A005"]); +} + +#[test] +fn allow_anonymous_suppresses_a_finding_on_its_line_the_line_above_or_the_line_of_the_call() { + let out = build( + "fn build() { + struct LocalFilter; + let mut pipeline = Pipeline::new(); + pipeline.add_pipeline(Pipeline::new()); // cup: allow-anonymous a test of the refusal + // cup: allow-anonymous + pipeline.add_pipeline(Pipeline::new()); + pipeline.add_valve( // cup: allow-anonymous + IsOnPredicate, + LocalFilter, + ); + pipeline.add_pipeline(Pipeline::new()); +} +", + ); + assert_eq!(out.found, ["build.rs:18 CUP-A005"]); + assert_eq!(out.suppressed, 3); + assert!(out.report.ends_with("1 finding (3 suppressed).")); +} diff --git a/packages/codeupipe-lint-rs/tests/cli.rs b/packages/codeupipe-lint-rs/tests/cli.rs new file mode 100644 index 0000000..805792a --- /dev/null +++ b/packages/codeupipe-lint-rs/tests/cli.rs @@ -0,0 +1,169 @@ +// The command: its exit codes, arguments, config file, and the linter on +// the Rust code of this repository. +mod common; + +use std::path::Path; +use std::process::Command; + +use codeupipe_lint::LintOptions; +use common::{lint_in, project}; + +const BAD: &str = "pub struct Discount; +impl Filter for Discount { fn call(&self, _p: &mut Payload) -> Result<(), StepError> { Ok(()) } } +"; +const GOOD: &str = "pub struct SetFilter; +impl Filter for SetFilter { fn call(&self, _p: &mut Payload) -> Result<(), StepError> { Ok(()) } } +"; + +struct Ran { + code: i32, + out: String, + err: String, +} + +fn run(cwd: &Path, args: &[&str]) -> Ran { + let done = Command::new(env!("CARGO_BIN_EXE_codeupipe-lint")) + .args(args) + .current_dir(cwd) + .output() + .unwrap(); + Ran { + code: done.status.code().unwrap(), + out: String::from_utf8(done.stdout).unwrap(), + err: String::from_utf8(done.stderr).unwrap(), + } +} + +#[test] +fn exit_0_with_no_findings_exit_1_with_findings_one_line_for_each() { + let root = project(&[("discount.rs", BAD), ("set_filter.rs", GOOD)]); + let clean = run(&root, &["set_filter.rs"]); + assert_eq!(clean.code, 0); + assert_eq!(clean.out.trim(), "No findings."); + let bad = run(&root, &["."]); + assert_eq!(bad.code, 1); + assert!( + bad.out + .starts_with("discount.rs:1:12 CUP-I006 primitive-file-name "), + "{}", + bad.out + ); + assert!(bad.out.ends_with("\n2 findings.\n")); +} + +#[test] +fn exit_2_for_a_path_that_is_not_there_a_rule_that_is_not_known_and_no_paths() { + let root = project(&[]); + let missing = run(&root, &["nothing-here"]); + assert_eq!(missing.code, 2); + assert!(missing + .err + .contains("error: nothing-here: no such file or directory")); + let unknown = run(&root, &["--disable", "CUP-X999", "."]); + assert_eq!(unknown.code, 2); + assert!(unknown.err.contains("no such rule: CUP-X999")); + // A rule that the compiler checks in Rust is not a rule of this linter. + assert_eq!(run(&root, &["--disable", "CUP-A001", "."]).code, 2); + assert_eq!(run(&root, &[]).code, 2); + assert_eq!(run(&root, &["--nope", "."]).code, 2); +} + +#[test] +fn rules_lists_the_rule_ids() { + let listed = run(&project(&[]), &["--rules"]); + assert_eq!(listed.code, 0); + assert_eq!( + listed.out, + "CUP-A003 local-type-step +CUP-A005 unnamed-pipeline-step +CUP-I000 parse-error +CUP-I003 state-read-in-step +CUP-I005 one-primitive-per-file +CUP-I006 primitive-file-name +CUP-I007 primitive-role-suffix +CUP-I008 logic-outside-primitive +CUP-I009 marker-allows-nothing +" + ); +} + +#[test] +fn exclude_and_disable() { + let root = project(&[("discount.rs", BAD)]); + assert_eq!( + run( + &root, + &[ + "--exclude", + "*/discount.rs", + "--exclude", + "discount.rs", + "." + ] + ) + .code, + 0 + ); + assert_eq!( + run(&root, &["--disable", "CUP-I006", "--disable=CUP-I007", "."]).code, + 0 + ); + assert_eq!(run(&root, &["--disable", "CUP-I006", "."]).code, 1); +} + +#[test] +fn with_no_paths_they_come_from_cup_config_json() { + let root = project(&[ + ("discount.rs", BAD), + ( + "cup.config.json", + r#"{"lint": {"paths": ["."], "disable": ["CUP-I006", "CUP-I007"]}, "deploy": {}}"#, + ), + ]); + assert_eq!(run(&root, &[]).code, 0); + assert_eq!(run(&root, &["discount.rs"]).code, 1); + std::fs::write(root.join("cup.config.json"), r#"{"lint": {"path": ["."]}}"#).unwrap(); + let wrong = run(&root, &[]); + assert_eq!(wrong.code, 2); + assert!(wrong.err.contains("unknown key 'path'"), "{}", wrong.err); + std::fs::write(root.join("cup.config.json"), r#"{"lint": {"paths": "."}}"#).unwrap(); + assert!(run(&root, &[]) + .err + .contains("lint.paths must be a list of strings")); + assert!(run(&root, &["--config", "other.json"]) + .err + .contains("other.json: no such file")); +} + +#[test] +fn target_and_git_are_not_read() { + let out = lint_in( + &[("target/debug/discount.rs", BAD), (".git/discount.rs", BAD)], + LintOptions::default(), + ); + assert_eq!(out.found, Vec::::new()); +} + +#[test] +fn the_linter_is_clean_on_the_rust_code_of_this_repository() { + let here = Path::new(env!("CARGO_MANIFEST_DIR")); + let done = run( + here, + &[ + "--strict", + "src", + "tests", + "../codeupipe-rs/src", + "../codeupipe-rs/examples", + "../codeupipe-rs/tests", + "../../conformance/runners/rust/src", + ], + ); + // Each plain logic of this repository says why, with `cup: allow-plain`. + assert!( + done.out.starts_with("No findings (") && !done.out.contains("warning"), + "{}", + done.out + ); + assert_eq!(done.code, 0); +} diff --git a/packages/codeupipe-lint-rs/tests/common/mod.rs b/packages/codeupipe-lint-rs/tests/common/mod.rs new file mode 100644 index 0000000..abcb339 --- /dev/null +++ b/packages/codeupipe-lint-rs/tests/common/mod.rs @@ -0,0 +1,69 @@ +// Helpers for the tests: write source text to a new directory and lint it. +#![allow(dead_code)] + +use std::fs; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicUsize, Ordering}; + +use codeupipe_lint::{lint_paths, Finding, LintOptions}; + +static MADE: AtomicUsize = AtomicUsize::new(0); + +/// A new, empty directory with `files` in it (name, text). +pub fn project(files: &[(&str, &str)]) -> PathBuf { + let root = std::env::temp_dir().join(format!( + "cup-lint-rs-{}-{}", + std::process::id(), + MADE.fetch_add(1, Ordering::SeqCst) + )); + let _ = fs::remove_dir_all(&root); + fs::create_dir_all(&root).unwrap(); + for (name, text) in files { + let path = root.join(name); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, text).unwrap(); + } + root +} + +pub struct Linted { + /// One string for each finding: `file:line rule`. + pub found: Vec, + pub findings: Vec, + pub suppressed: usize, + pub report: String, + pub exit_code: i32, +} + +pub fn lint_in(files: &[(&str, &str)], options: LintOptions) -> Linted { + let root = project(files); + let out = lint_paths(&[root.to_string_lossy().into_owned()], &options).unwrap(); + let found = out + .findings + .iter() + .map(|f| format!("{}:{} {}", relative(&root, &f.path), f.line, f.rule)) + .collect(); + let _ = fs::remove_dir_all(&root); + Linted { + found, + findings: out.findings, + suppressed: out.suppressed.len(), + report: out.report, + exit_code: out.exit_code, + } +} + +pub fn disable(rules: &[&str]) -> LintOptions { + LintOptions { + disable: rules.iter().map(|rule| rule.to_string()).collect(), + ..LintOptions::default() + } +} + +fn relative(root: &Path, path: &str) -> String { + Path::new(path) + .strip_prefix(root) + .unwrap() + .to_string_lossy() + .into_owned() +} diff --git a/packages/codeupipe-lint-rs/tests/invariants.rs b/packages/codeupipe-lint-rs/tests/invariants.rs new file mode 100644 index 0000000..7110ddb --- /dev/null +++ b/packages/codeupipe-lint-rs/tests/invariants.rs @@ -0,0 +1,206 @@ +// CUP-I003, I005, I006, I007, and CUP-I000. A write to the Payload outside a +// Filter (CUP-I004) is a compile error in Rust, so it has no rule here. +mod common; + +use codeupipe_lint::LintOptions; +use common::lint_in; + +#[test] +fn i003_a_filter_or_a_predicate_that_reads_state() { + let out = lint_in( + &[ + ( + "count_filter.rs", + "use std::rc::Rc; +use codeupipe::{Filter, Payload, Pipeline, State, StepError}; +pub struct CountFilter { + pipeline: Rc, + seen: Rc, + states: Vec, +} +impl Filter for CountFilter { + fn call(&self, payload: &mut Payload) -> Result<(), StepError> { + payload.insert(\"a\", self.pipeline.state().executed().len()); + payload.insert(\"b\", self.seen.skipped().len()); + payload.insert(\"c\", self.states.len()); + payload.insert(\"d\", payload.get(\"state\").is_some()); + let state = State::new(); + payload.insert(\"e\", state.executed().len() + self.state()); + Ok(()) + } +} +", + ), + ( + "ran_predicate.rs", + "pub struct RanPredicate { holder: Holder } +impl Predicate for RanPredicate { + fn test(&self, _payload: &Payload) -> bool { !self.holder.state().executed().is_empty() } +} +", + ), + ( + "watch_tap.rs", + "pub struct WatchTap { pipeline: Rc } +impl Tap for WatchTap { + fn observe(&self, _payload: &Payload) -> Result<(), StepError> { + println!(\"{:?}\", self.pipeline.state().executed()); + Ok(()) + } +} +", + ), + ], + LintOptions::default(), + ); + assert_eq!( + out.found, + [ + "count_filter.rs:10 CUP-I003", + "count_filter.rs:11 CUP-I003", + "ran_predicate.rs:3 CUP-I003" + ] + ); + assert!(out.findings[0] + .message + .starts_with("Step reads pipeline State (`self.pipeline.state()`). A Filter's result")); + assert!(out.findings[1] + .message + .starts_with("Step reads pipeline State (`self.seen`).")); +} + +const FILTER: &str = "fn call(&self, _payload: &mut Payload) -> Result<(), StepError> { Ok(()) }"; +const TAP: &str = "fn observe(&self, _payload: &Payload) -> Result<(), StepError> { Ok(()) }"; + +#[test] +fn i005_one_primitive_in_each_file() { + let set_filter = format!( + "pub struct SetFilter; +impl Filter for SetFilter {{ {FILTER} }} +struct Helper; +pub struct SeeTap; +impl Tap for SeeTap {{ {TAP} }} +fn make() {{ struct LocalFilter; impl Filter for LocalFilter {{ {FILTER} }} }} +" + ); + let two = format!( + "struct AFilter;\nimpl Filter for AFilter {{ {FILTER} }}\nstruct BFilter;\nimpl Filter for BFilter {{ {FILTER} }}\n" + ); + let out = lint_in( + &[("set_filter.rs", &set_filter), ("tests/steps.rs", &two)], + LintOptions::default(), + ); + assert_eq!( + out.found, + ["set_filter.rs:4 CUP-I005", "set_filter.rs:4 CUP-I006"] + ); + assert!(out.findings[0] + .message + .starts_with("Tap `SeeTap` shares this file with Filter `SetFilter`.")); +} + +#[test] +fn i006_the_file_name_is_the_type_name_in_snake_case() { + let out = lint_in( + &[ + ( + "discount.rs", + &format!("pub struct DiscountFilter;\nimpl Filter for DiscountFilter {{ {FILTER} }}\n"), + ), + ( + "parse_html_filter.rs", + &format!("pub struct ParseHTMLFilter;\nimpl Filter for ParseHTMLFilter {{ {FILTER} }}\n"), + ), + ( + "pricing_pipeline.rs", + "pub struct PricingPipeline;\nimpl NamedPipeline for PricingPipeline { fn add_steps(&self, _pipeline: &mut Pipeline) {} }\n", + ), + ( + "order_filter.rs", + &format!("impl codeupipe::Filter

for OrderFilter

{{ {FILTER} }}\n"), + ), + ], + LintOptions::default(), + ); + assert_eq!(out.found, ["discount.rs:1 CUP-I006"]); + assert!(out.findings[0] + .message + .starts_with("`DiscountFilter` is in `discount.rs`. Put it in `discount_filter.rs`.")); +} + +#[test] +fn i007_the_type_name_ends_with_its_role() { + let out = lint_in( + &[ + ( + "discount.rs", + &format!("pub struct Discount;\nimpl Filter for Discount {{ {FILTER} }}\n"), + ), + ( + "audit.rs", + &format!("pub struct Audit;\nimpl codeupipe::Tap for Audit {{ {TAP} }}\n"), + ), + ( + "at_least.rs", + "pub struct AtLeast;\nimpl Predicate for AtLeast { fn test(&self, _p: &Payload) -> bool { true } }\n", + ), + ( + "print_steps.rs", + "pub struct PrintSteps;\nimpl Hook for PrintSteps {}\n", + ), + ( + "pricing.rs", + "pub struct Pricing;\nimpl NamedPipeline for Pricing { fn add_steps(&self, _pipeline: &mut Pipeline) {} }\n", + ), + ( + "point.rs", + "pub struct Point;\nimpl Point { fn call(&self, a: i32, b: i32) {} fn test(&self) {} }\n", + ), + ], + LintOptions::default(), + ); + assert_eq!( + out.found, + [ + "at_least.rs:1 CUP-I006", + "at_least.rs:1 CUP-I007", + "audit.rs:1 CUP-I006", + "audit.rs:1 CUP-I007", + "discount.rs:1 CUP-I006", + "discount.rs:1 CUP-I007", + "pricing.rs:1 CUP-I006", + "pricing.rs:1 CUP-I007", + "print_steps.rs:1 CUP-I006", + "print_steps.rs:1 CUP-I007" + ] + ); + assert!(out.findings[1] + .message + .starts_with("Predicate `AtLeast` does not say what it is. Name it `AtLeastPredicate`.")); +} + +#[test] +fn the_comment_markers_suppress_the_invariant_findings() { + let out = lint_in( + &[( + "discount.rs", + &format!( + "// cup: allow-name\n#[derive(Clone)]\n// cup: allow-name a name from the paper\npub struct Discount;\nimpl Filter for Discount {{ {FILTER} }}\n" + ), + )], + LintOptions::default(), + ); + // The first marker is not directly above the struct, so it allows nothing. + assert_eq!(out.found, ["discount.rs:1 CUP-I009"]); + assert_eq!(out.suppressed, 2); +} + +#[test] +fn a_file_with_a_syntax_error_is_i000() { + let out = lint_in(&[("broken.rs", "pub struct {{{\n")], LintOptions::default()); + assert_eq!(out.found, ["broken.rs:1 CUP-I000"]); + assert!(out.findings[0] + .message + .ends_with("Fix the syntax error so the file can be checked.")); + assert_eq!(out.exit_code, 1); +} diff --git a/packages/codeupipe-lint-rs/tests/plain_logic.rs b/packages/codeupipe-lint-rs/tests/plain_logic.rs new file mode 100644 index 0000000..68cb9af --- /dev/null +++ b/packages/codeupipe-lint-rs/tests/plain_logic.rs @@ -0,0 +1,239 @@ +// CUP-I008, a warning: logic that is not in a primitive. +mod common; + +use codeupipe_lint::LintOptions; +use common::{disable, lint_in, Linted}; + +fn warnings(out: &Linted) -> Vec { + out.findings + .iter() + .filter(|f| f.warning) + .map(|f| f.message.split(" has logic").next().unwrap().to_string()) + .collect() +} + +#[test] +fn a_function_with_a_decision_that_is_not_in_a_primitive_is_a_warning() { + let out = lint_in( + &[( + "who_is_due.rs", + "pub fn who_is_due(entries: &[String]) -> &'static str { if entries.is_empty() { return \"say\"; } \"model\" } +pub fn pick(list: &[i32]) -> Option { for each in list { if *each > 0 { return Some(*each); } } None } +pub fn choose(flag: bool) -> &'static str { match flag { true => \"a\", false => \"b\" } } +pub fn wait() { loop { break; } } +pub fn plain(a: i32, b: i32) -> i32 { a + b } +pub fn double(list: &[i32]) -> Vec { list.iter().map(|n| n * 2).collect() } +pub fn first(list: &[i32]) -> i32 { let Some(x) = list.first() else { return 0 }; *x } +", + )], + LintOptions::default(), + ); + assert_eq!( + out.found, + [ + "who_is_due.rs:1 CUP-I008", + "who_is_due.rs:2 CUP-I008", + "who_is_due.rs:3 CUP-I008", + "who_is_due.rs:4 CUP-I008", + "who_is_due.rs:7 CUP-I008" + ] + ); + assert_eq!( + warnings(&out), + [ + "Function `who_is_due`", + "Function `pick`", + "Function `choose`", + "Function `wait`", + "Function `first`" + ] + ); + assert!(out.findings[0].message.contains( + "is not a Filter, a Predicate, a Tap, a Hook or a Pipeline. Write it as a primitive" + )); +} + +#[test] +fn a_type_or_a_trait_that_is_not_a_primitive_and_code_at_the_top_of_a_file() { + let out = lint_in( + &[ + ( + "record_store.rs", + "pub struct RecordStore { files: Vec } +impl RecordStore { + pub fn read(&self, id: &str) -> Vec { if id.is_empty() { return vec![]; } self.files.clone() } +} +pub struct Point { x: f64 } +impl Point { pub fn double(&self) -> f64 { self.x * 2.0 } } +impl std::fmt::Display for Point { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { write!(f, \"{}\", self.x) } } +pub trait Choose { fn choose(&self, a: bool) -> i32 { if a { 1 } else { 2 } } } +impl Default for Elsewhere { fn default() -> Self { match 1 { _ => Elsewhere } } } +", + ), + ( + "main.rs", + "const LIMIT: usize = if cfg!(debug_assertions) { 1 } else { 2 }; +fn main() { let args: Vec = std::env::args().collect(); if args.is_empty() { std::process::exit(2); } } +", + ), + ( + "helpers.rs", + "pub mod inner { + pub fn pick(a: bool) -> i32 { if a { 1 } else { 2 } } +} +", + ), + ], + LintOptions::default(), + ); + assert_eq!( + out.found, + [ + "helpers.rs:2 CUP-I008", + "main.rs:1 CUP-I008", + "main.rs:2 CUP-I008", + "record_store.rs:2 CUP-I008", + "record_store.rs:8 CUP-I008", + "record_store.rs:9 CUP-I008" + ] + ); + assert_eq!( + warnings(&out), + [ + "Function `pick`", + "The code at the top of this file", + "Function `main`", + "Type `RecordStore`", + "Trait `Choose`", + "Type `Elsewhere`" + ] + ); +} + +#[test] +fn logic_in_a_primitive_is_where_it_must_be_and_types_and_tests_have_no_finding() { + let out = lint_in( + &[ + ( + "pick_filter.rs", + "pub struct PickFilter { on: bool } +impl PickFilter { pub fn new(on: bool) -> Self { if on { PickFilter { on } } else { PickFilter { on: false } } } } +impl Filter for PickFilter { + fn call(&self, payload: &mut Payload) -> Result<(), StepError> { + if payload.has(\"a\") { payload.insert(\"b\", 1); } + for _ in 0..2 {} + Ok(()) + } +} +", + ), + ( + "is_on_predicate.rs", + "pub struct IsOnPredicate;\nimpl Predicate for IsOnPredicate { fn test(&self, p: &Payload) -> bool { match p.get(\"on\") { Some(_) => true, None => false } } }\n", + ), + ( + "see_tap.rs", + "pub struct SeeTap;\nimpl Tap for SeeTap { fn observe(&self, p: &Payload) -> Result<(), StepError> { if p.has(\"x\") { println!(\"x\"); } Ok(()) } }\n", + ), + ( + "log_hook.rs", + "pub struct LogHook;\nimpl Hook for LogHook { fn before(&self, step: Option<&str>, _p: &Payload) { if let Some(step) = step { println!(\"{step}\"); } } }\n", + ), + ( + "main_pipeline.rs", + "pub struct MainPipeline;\nimpl NamedPipeline for MainPipeline { fn add_steps(&self, pipeline: &mut Pipeline) { if std::env::var(\"X\").is_ok() { pipeline.observe(true, false); } } }\n", + ), + ( + "types.rs", + "pub enum Due { Say, Model } +pub trait Model { fn reply(&self, text: &str) -> String; } +pub const LIMIT: usize = 20000; +pub type Names = Vec; +#[test] +fn checks() { if LIMIT > 1 { return; } } +#[cfg(test)] +mod tests { + fn helper(a: bool) -> i32 { if a { 1 } else { 2 } } +} +", + ), + ], + LintOptions::default(), + ); + assert_eq!(out.found, Vec::::new()); +} + +#[test] +fn a_warning_does_not_fail_the_run_unless_the_run_is_strict() { + let files = [( + "helper.rs", + "pub fn pick(a: bool) -> i32 { if a { 1 } else { 2 } }\n", + )]; + let soft = lint_in(&files, LintOptions::default()); + assert_eq!(soft.exit_code, 0); + assert!(soft.report.contains( + "helper.rs:1:8 CUP-I008 logic-outside-primitive warning: Function `pick` has logic" + )); + assert!(soft.report.ends_with("\nNo findings. 1 warning.")); + let strict = lint_in( + &files, + LintOptions { + strict: true, + ..LintOptions::default() + }, + ); + assert_eq!(strict.exit_code, 1); +} + +#[test] +fn a_warning_and_a_finding_are_counted_apart() { + let out = lint_in( + &[( + "discount.rs", + "pub struct Discount; +impl Filter for Discount { fn call(&self, _p: &mut Payload) -> Result<(), StepError> { Ok(()) } } +pub fn pick(a: bool) -> i32 { if a { 1 } else { 2 } } +", + )], + LintOptions::default(), + ); + assert_eq!(out.exit_code, 1); + assert!(out.report.ends_with("\n2 findings. 1 warning.")); +} + +#[test] +fn allow_plain_with_a_reason_suppresses_the_warning_and_test_files_are_not_checked() { + let out = lint_in( + &[ + ( + "helper.rs", + "/// Picks one. +#[inline] +// cup: allow-plain a pure helper of two Predicates +pub fn pick(a: bool) -> i32 { if a { 1 } else { 2 } } +", + ), + ( + "tests/support.rs", + "pub fn pick(a: bool) -> i32 { if a { 1 } else { 2 } }\n", + ), + ], + LintOptions::default(), + ); + assert_eq!(out.found, Vec::::new()); + assert_eq!(out.suppressed, 1); + assert_eq!(out.report, "No findings (1 suppressed)."); +} + +#[test] +fn the_rule_can_be_switched_off() { + let out = lint_in( + &[( + "helper.rs", + "pub fn pick(a: bool) -> i32 { if a { 1 } else { 2 } }\n", + )], + disable(&["CUP-I008"]), + ); + assert_eq!(out.found, Vec::::new()); + assert_eq!(out.report, "No findings."); +} diff --git a/packages/codeupipe-lint-rs/tests/stale_markers.rs b/packages/codeupipe-lint-rs/tests/stale_markers.rs new file mode 100644 index 0000000..8751339 --- /dev/null +++ b/packages/codeupipe-lint-rs/tests/stale_markers.rs @@ -0,0 +1,91 @@ +// CUP-I009, a warning: a `cup: allow-...` comment that allowed no finding. +mod common; + +use codeupipe_lint::LintOptions; +use common::{disable, lint_in}; + +#[test] +fn a_comment_that_allows_nothing_is_a_warning() { + let out = lint_in( + &[( + "add.rs", + "// cup: allow-plain it had a loop before +pub fn add(a: i32, b: i32) -> i32 { a + b } +pub const ONE: i32 = 1; // cup: allow-state-read nothing here reads State +/* + * cup: allow-plian a kind that does not exist + */ +pub const TWO: i32 = 2; +// cup: allow-outside-write Rust has no such rule +pub const THREE: i32 = 3; +", + )], + LintOptions::default(), + ); + assert_eq!( + out.found, + [ + "add.rs:1 CUP-I009", + "add.rs:3 CUP-I009", + "add.rs:5 CUP-I009", + "add.rs:8 CUP-I009" + ] + ); + assert!(out.findings[0].warning); + assert!(out.findings[0] + .message + .starts_with("The comment `cup: allow-plain` allows nothing. Remove it")); + assert_eq!(out.findings[1].col, 28); + assert_eq!(out.exit_code, 0); + let strict = lint_in( + &[( + "add.rs", + "// cup: allow-plain no reason\npub const ONE: i32 = 1;\n", + )], + LintOptions { + strict: true, + ..LintOptions::default() + }, + ); + assert_eq!(strict.exit_code, 1); +} + +#[test] +fn a_comment_that_allows_a_finding_is_not_reported_and_text_that_only_names_a_marker_is_not_a_comment( +) { + let out = lint_in( + &[( + "pick.rs", + "// cup: allow-plain the entry of the program +pub fn pick(list: &[i32]) -> Option { for each in list { if *each > 0 { return Some(*each); } } None } +pub fn choose(flag: bool) -> i32 { if flag { 1 } else { 2 } } // cup: allow-plain a small helper +/// cup: allow-plain in a doc comment is text of the documentation +pub const HELP: &str = \"say why: // cup: allow-plain \"; +pub const RAW: &str = r#\"// cup: allow-plain\"#; +pub const SLASH: char = '/'; // a char, then cup: allow-plain is not at the start +", + )], + LintOptions::default(), + ); + assert_eq!(out.found, Vec::::new()); + assert_eq!(out.suppressed, 2); +} + +#[test] +fn a_comment_for_a_rule_that_is_switched_off_is_not_reported_and_the_rule_can_be_switched_off() { + let files = [( + "pick.rs", + "// cup: allow-plain the entry\npub fn pick(flag: bool) -> i32 { if flag { 1 } else { 2 } }\n", + )]; + let off = lint_in(&files, disable(&["CUP-I008"])); + assert_eq!(off.found, Vec::::new()); + assert_eq!(off.suppressed, 0); + let quiet = lint_in( + &[( + "one.rs", + "// cup: allow-plain no reason\npub const ONE: i32 = 1;\n", + )], + disable(&["CUP-I009"]), + ); + assert_eq!(quiet.found, Vec::::new()); +} diff --git a/packages/codeupipe-rs/README.md b/packages/codeupipe-rs/README.md index d4bc603..31cd7fb 100644 --- a/packages/codeupipe-rs/README.md +++ b/packages/codeupipe-rs/README.md @@ -4,8 +4,10 @@ codeupipe is a library for writing a program as a pipeline of small steps. This is the Rust runtime. It has no dependencies. It implements the same contract as the Python and TypeScript packages: -[conformance/API.md](../../conformance/API.md). The conformance runner for -Rust is not written yet ([docs/rust-plan.md](../../docs/rust-plan.md), phase 2). +[conformance/API.md](../../conformance/API.md), and passes all its +conformance cases (`python conformance/run.py rust:run`). The linter for +Rust is [packages/codeupipe-lint-rs/](../codeupipe-lint-rs/); a compiler is +not planned yet ([docs/rust-plan.md](../../docs/rust-plan.md)). The version is below 1.0: the package is not stable yet, and a new version can change in a way that is not compatible. diff --git a/packages/codeupipe-rs/src/map.rs b/packages/codeupipe-rs/src/map.rs index 954a1ed..7573cd7 100644 --- a/packages/codeupipe-rs/src/map.rs +++ b/packages/codeupipe-rs/src/map.rs @@ -20,6 +20,7 @@ pub struct Map { index: Option, usize>>, } +// cup: allow-plain a primitive itself: the framework makes the primitives, so it is not written with them impl Map { pub fn new() -> Self { Map::default() @@ -113,6 +114,7 @@ impl fmt::Debug for Map { } } +// cup: allow-plain a primitive itself: the framework makes the primitives, so it is not written with them impl, V: Into> FromIterator<(K, V)> for Map { fn from_iter>(entries: I) -> Self { let mut map = Map::new(); diff --git a/packages/codeupipe-rs/src/payload.rs b/packages/codeupipe-rs/src/payload.rs index e44aee0..3cb00be 100644 --- a/packages/codeupipe-rs/src/payload.rs +++ b/packages/codeupipe-rs/src/payload.rs @@ -14,6 +14,7 @@ pub struct Payload { lineage: Vec, } +// cup: allow-plain a primitive itself: the framework makes the primitives, so it is not written with them impl Payload { /// An empty Payload. pub fn new() -> Self { diff --git a/packages/codeupipe-rs/src/pipeline.rs b/packages/codeupipe-rs/src/pipeline.rs index 339e6fb..a99c2d4 100644 --- a/packages/codeupipe-rs/src/pipeline.rs +++ b/packages/codeupipe-rs/src/pipeline.rs @@ -64,6 +64,7 @@ struct Runs { spare: RefCell>, } +// cup: allow-plain a primitive itself: the framework makes the primitives, so it is not written with them impl Runs { fn start(&self) -> u64 { let ticket = self.started.get() + 1; @@ -114,6 +115,7 @@ impl

Pipeline

{ } } +// cup: allow-plain a primitive itself: the framework makes the primitives, so it is not written with them impl Pipeline

{ /// A pipeline with no name. Nested, its label is `Pipeline`. pub fn new() -> Self { @@ -308,6 +310,7 @@ impl Pipeline

{ } /// Run one step that Hooks see. `Ok(false)` is a Valve that skipped its Filter. +// cup: allow-plain a primitive itself: the framework makes the primitives, so it is not written with them fn run_gated(gated: &Gated

, payload: &mut P) -> Result { match gated { Gated::Filter(filter) => filter.call(payload).map(|()| true), diff --git a/packages/codeupipe-rs/src/state.rs b/packages/codeupipe-rs/src/state.rs index d9efd87..4dbb247 100644 --- a/packages/codeupipe-rs/src/state.rs +++ b/packages/codeupipe-rs/src/state.rs @@ -30,6 +30,7 @@ struct Labels { len: usize, } +// cup: allow-plain a primitive itself: the framework makes the primitives, so it is not written with them impl Labels { fn push(&mut self, label: &str) { match self.all.get_mut(self.len) { @@ -57,6 +58,7 @@ impl fmt::Debug for Labels { } } +// cup: allow-plain a primitive itself: the framework makes the primitives, so it is not written with them impl State { /// An empty State, for a caller that gives the State of a run /// (`pipeline.run_with_state(&input, &mut state)`). diff --git a/packages/codeupipe-rs/src/value.rs b/packages/codeupipe-rs/src/value.rs index 87a20c6..8b1667d 100644 --- a/packages/codeupipe-rs/src/value.rs +++ b/packages/codeupipe-rs/src/value.rs @@ -16,6 +16,7 @@ pub enum Value { Map(Map), } +// cup: allow-plain a primitive itself: the framework makes the primitives, so it is not written with them impl Value { pub fn is_null(&self) -> bool { matches!(self, Value::Null) @@ -58,6 +59,7 @@ impl Value { } /// Written as JSON is, so that a printed Payload reads like the data. +// cup: allow-plain a primitive itself: the framework makes the primitives, so it is not written with them impl fmt::Debug for Value { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { diff --git a/packages/codeupipe-rs/tests/pipeline.rs b/packages/codeupipe-rs/tests/pipeline.rs index 3f2c5e6..22ad5bf 100644 --- a/packages/codeupipe-rs/tests/pipeline.rs +++ b/packages/codeupipe-rs/tests/pipeline.rs @@ -369,26 +369,27 @@ fn each_run_starts_with_a_new_state() { assert!(pipeline.state().skipped().is_empty()); } -#[test] -fn a_shorter_run_after_a_longer_one_keeps_nothing_of_it() { - struct FailIfAskedFilter; - impl Filter for FailIfAskedFilter { - fn call(&self, payload: &mut Payload) -> Result<(), StepError> { - if payload.get("fail") == Some(&Value::Bool(true)) { - return Err("asked to fail".into()); - } - Ok(()) +struct FailIfAskedFilter; +impl Filter for FailIfAskedFilter { + fn call(&self, payload: &mut Payload) -> Result<(), StepError> { + if payload.get("fail") == Some(&Value::Bool(true)) { + return Err("asked to fail".into()); } + Ok(()) } - struct FailIfAskedTap; - impl Tap for FailIfAskedTap { - fn observe(&self, payload: &Payload) -> Result<(), StepError> { - if payload.get("tap_fail") == Some(&Value::Bool(true)) { - return Err("asked to fail".into()); - } - Ok(()) +} +struct FailIfAskedTap; +impl Tap for FailIfAskedTap { + fn observe(&self, payload: &Payload) -> Result<(), StepError> { + if payload.get("tap_fail") == Some(&Value::Bool(true)) { + return Err("asked to fail".into()); } + Ok(()) } +} + +#[test] +fn a_shorter_run_after_a_longer_one_keeps_nothing_of_it() { let mut pipeline = Pipeline::new(); pipeline.observe(true, false); pipeline.add_filter(FailIfAskedFilter); @@ -437,15 +438,15 @@ fn a_shorter_run_after_a_longer_one_keeps_nothing_of_it() { assert!(state.errors().is_empty()); } -#[test] -fn a_failure_inside_a_nested_pipeline_is_reported_as_the_nesting_step() { - struct FailingInnerPipeline; - impl NamedPipeline for FailingInnerPipeline { - fn add_steps(&self, pipeline: &mut Pipeline) { - pipeline.add_filter(FailFilter); - } +struct FailingInnerPipeline; +impl NamedPipeline for FailingInnerPipeline { + fn add_steps(&self, pipeline: &mut Pipeline) { + pipeline.add_filter(FailFilter); } +} +#[test] +fn a_failure_inside_a_nested_pipeline_is_reported_as_the_nesting_step() { let hook = RecordHook::default(); let mut pipeline = Pipeline::new(); pipeline.add_pipeline(Pipeline::named(FailingInnerPipeline)); diff --git a/packages/codeupipe-rs/tests/state_per_run.rs b/packages/codeupipe-rs/tests/state_per_run.rs index 9a538c4..e6a71fe 100644 --- a/packages/codeupipe-rs/tests/state_per_run.rs +++ b/packages/codeupipe-rs/tests/state_per_run.rs @@ -139,14 +139,15 @@ fn a_state_that_was_read_does_not_change_with_the_next_run() { assert_eq!(pipeline.state().skipped(), ["MarkFilter"]); } +struct FailFilter; +impl Filter for FailFilter { + fn call(&self, _payload: &mut Payload) -> Result<(), StepError> { + Err("boom".into()) + } +} + #[test] fn a_failed_run_with_the_callers_state_keeps_the_steps_before_the_failure() { - struct FailFilter; - impl Filter for FailFilter { - fn call(&self, _payload: &mut Payload) -> Result<(), StepError> { - Err("boom".into()) - } - } let mut pipeline = Pipeline::new(); pipeline.add_filter(SetFilter); pipeline.add_filter(FailFilter); diff --git a/packages/codeupipe-rs/tests/step_label.rs b/packages/codeupipe-rs/tests/step_label.rs index ec6d9d9..6ea9956 100644 --- a/packages/codeupipe-rs/tests/step_label.rs +++ b/packages/codeupipe-rs/tests/step_label.rs @@ -66,6 +66,7 @@ fn a_valve_has_the_label_of_its_filter() { fn a_nested_pipeline_has_the_label_of_its_type() { let mut pipeline = Pipeline::new(); pipeline.add_pipeline(Pipeline::named(InnerPipeline)); + // cup: allow-anonymous the label of a plain nested pipeline is what this test checks pipeline.add_pipeline(Pipeline::new()); assert_eq!(pipeline.labels(), ["InnerPipeline", "Pipeline"]); } @@ -86,14 +87,15 @@ fn a_type_defined_in_a_function_works() { assert_eq!(pipeline.labels(), ["LocalFilter"]); } +struct WrapFilter(PhantomData); +impl Filter for WrapFilter { + fn call(&self, _payload: &mut Payload) -> Result<(), StepError> { + Ok(()) + } +} + #[test] fn a_generic_type_has_its_name_without_the_arguments() { - struct WrapFilter(PhantomData); - impl Filter for WrapFilter { - fn call(&self, _payload: &mut Payload) -> Result<(), StepError> { - Ok(()) - } - } let mut pipeline = Pipeline::new(); pipeline.add_filter(WrapFilter::>>( PhantomData, diff --git a/packages/codeupipe-rs/tests/typed_payload.rs b/packages/codeupipe-rs/tests/typed_payload.rs index 8786724..25b85c2 100644 --- a/packages/codeupipe-rs/tests/typed_payload.rs +++ b/packages/codeupipe-rs/tests/typed_payload.rs @@ -173,23 +173,24 @@ fn a_failure_is_the_same_as_with_a_payload() { ); } -#[test] -fn lineage_is_kept_only_by_a_type_that_keeps_it() { - #[derive(Clone, Default)] - struct Traced { - steps: Vec, - } - impl PipelineData for Traced { - fn stamp(&mut self, label: &str) { - self.steps.push(label.to_string()); - } +#[derive(Clone, Default)] +struct Traced { + steps: Vec, +} +impl PipelineData for Traced { + fn stamp(&mut self, label: &str) { + self.steps.push(label.to_string()); } - struct NoopFilter; - impl Filter for NoopFilter { - fn call(&self, _traced: &mut Traced) -> Result<(), StepError> { - Ok(()) - } +} +struct NoopFilter; +impl Filter for NoopFilter { + fn call(&self, _traced: &mut Traced) -> Result<(), StepError> { + Ok(()) } +} + +#[test] +fn lineage_is_kept_only_by_a_type_that_keeps_it() { let mut traced = Pipeline::new(); traced.add_filter(NoopFilter); traced.observe(false, true);