feat(quality): lint guardrails, rustfmt baseline, and CI quality gate#222
Merged
Conversation
vpavlin
force-pushed
the
feat/190-lint-guardrails
branch
2 times, most recently
from
ac92ee1 to
6202381
Compare
…gate Introduces a quality gate that runs on every PR and push to main: - **Formatting** (`cargo fmt --all --check`) — enforces the rustfmt.toml baseline (trailing commas, imports granularity, max width 100). - **Clippy** — workspace-wide with a clippy.toml (MSRV 1.88, test allowances, domain idents). `spel` and `spel-ffi-compile-test` are excluded because their transitive deps (`logos-blockchain-poq`, `logos-blockchain-cryptarchia-engine`) use unstable features on stable 1.88. - **Unit tests** — same exclusions as clippy. New files: `.github/workflows/quality.yml`, `clippy.toml`, `rustfmt.toml` Workspace-level lint config added to every `Cargo.toml`: `let_underscore_must_use = "deny"` — forces explicit handling of must-use results (fixed two call sites: `drop(attr.parse_nested_meta(…))` and `.ok()` on `fs::remove_dir_all`). All source files re-formatted under the new `rustfmt.toml` baseline (trailing commas, 100-char width, imports granularity = "module"). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Prevents CI from resolving ruint ≥ 1.18.0, which requires rustc 1.90 and breaks the quality gate (pinned to 1.88). - `Cargo.lock` — workspace lock file (previously gitignored); pins ruint to 1.17.0 via `cargo update ruint --precise 1.17.0`. - `tests/e2e/fixture_program/Cargo.lock` — the fixture program is a separate workspace and runs its own dep resolution when e2e tests shell out to `cargo build`; committing its lock file prevents it from independently picking up ruint 1.18. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
vpavlin
force-pushed
the
feat/190-lint-guardrails
branch
from
0ae55d5 to
e681260
Compare
Comment on lines
21
to
27
| let ctx2 = ctx; // Copy | ||
| let ctx3 = ctx.clone(); // Clone | ||
| let ctx3 = ctx; // Clone | ||
|
|
Comment on lines
+17
to
+20
| # Rollout: lints start as `warn` (visible but not blocking). | ||
| # Promote to `deny` once the codebase is clean; meanwhile CI runs with | ||
| # `-- -D warnings` only on the clean subset (see quality.yml). | ||
| # ──────────────────────────────────────────────────────────────────────────── |
Comment on lines
+23
to
+28
| - uses: actions/checkout@v4 | ||
| - uses: dtolnay/rust-toolchain@stable | ||
| with: | ||
| toolchain: 1.88.0 | ||
| components: rustfmt | ||
| - uses: Swatinem/rust-cache@v2 |
- Split test_program_context_clone_copy into two tests: the original had `ctx; // Clone` (copy, not a Clone call) — now test_program_context_clone explicitly calls `.clone()` and test_program_context_copy tests Copy - Fix misleading Cargo.toml comment that claimed CI runs with -D warnings; currently warnings are collected but not errors (flip noted for later) - PR description updated to clarify rust-toolchain.toml is absent by design Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The script was removed from the logos-blockchain/logos-blockchain main branch after the May 25 CI run, causing every job that installs circuits to fail with "bash: line 1: 404:: command not found". Pin all 9 occurrences to commit 1da154c7 — the same rev already used by the workspace's logos-blockchain transitive dependency. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 51 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
spel-framework/src/lib.rs:28
- In
prelude,pub use spel_framework_core::prelude::*;already re-exportsSpelError,SpelResult,AutoClaim,SpelOutput, and (when thehostfeature is enabled)Message/WitnessSet. Re-exporting those names again in the same module will fail to compile with E0252 (name defined multiple times). Keep the single glob re-export and only add items not covered by the core prelude.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #190
Summary
[workspace.lints]in rootCargo.toml):unused_must_useandlet_underscore_must_useas deny;unwrap_used,expect_used,indexing_slicingas warn; noise reduction withmodule_name_repetitions = allow,must_use_candidate = allowrustfmt.toml: edition 2021,max_width = 100, Unix newlines,use_field_init_shorthand,use_try_shorthandclippy.toml:msrv = "1.88", test allow-list (allow-unwrap-in-tests,allow-expect-in-tests, etc.),doc-valid-identsfor domain terms (SPEL, IDL, FFI, PDA, ...).github/workflows/quality.yml: three jobs —fmt --check,clippy --all-targets,cargo test; toolchain pinned to 1.88.0 viadtolnay/rust-toolchainin the workflow (norust-toolchain.toml— that would pin globally and break the existing CI workflow which needs rustc 1.90+ for transitive deps); all memberCargo.tomlfiles get[lints] workspace = truecargo fmt --all(line-length normalisation across the workspace)cargo clippy --fixauto-fixes: uninlined format args,map_or->is_ok_and,let _ = must_use->drop()/.ok(), unnecessary qualificationsKnown limitation
spel(CLI) andspel-ffi-compile-testare excluded from the clippy and test CI jobs because their transitive deps (logos-blockchain-poq,logos-blockchain-cryptarchia-engine) use unstable features in array lengths, non-constf64::floor) that do not compile on stable 1.88. These crates are excluded via--excludeuntil upstream fixes land.Test plan
cargo fmt --all --checkpassescargo clippy --workspace --all-targets --exclude spel --exclude spel-ffi-compile-testpasses (warnings only, no errors)cargo test --workspace --all-targets --exclude spel --exclude spel-ffi-compile-testpassesGenerated with Claude Code