|
| 1 | +--- |
| 2 | +doc-type: issue |
| 3 | +issue-type: task |
| 4 | +status: open |
| 5 | +priority: p2 |
| 6 | +github-issue: 1786 |
| 7 | +spec-path: docs/issues/open/1786-tighten-lint-config.md |
| 8 | +branch: "1786-tighten-lint-config" |
| 9 | +related-pr: 1784 |
| 10 | +last-updated-utc: 2026-05-15 08:00 |
| 11 | +semantic-links: |
| 12 | + skill-links: |
| 13 | + - create-issue |
| 14 | + related-artifacts: |
| 15 | + - Cargo.toml |
| 16 | + - .cargo/config.toml |
| 17 | +--- |
| 18 | + |
| 19 | +<!-- skill-link: create-issue --> |
| 20 | + |
| 21 | +# Issue #1786 - Migrate lint configuration to `[workspace.lints]` in Cargo.toml |
| 22 | + |
| 23 | +## Goal |
| 24 | + |
| 25 | +Replace the ad-hoc lint configuration spread across `.cargo/config.toml` RUSTFLAGS and |
| 26 | +`torrust-linting` command-line arguments with a single authoritative `[workspace.lints]` |
| 27 | +section in `Cargo.toml`, following the idiomatic Cargo approach used in `torrust-index`. |
| 28 | + |
| 29 | +## Background |
| 30 | + |
| 31 | +Lint enforcement is currently split across three places: |
| 32 | + |
| 33 | +1. **`.cargo/config.toml` RUSTFLAGS** — carries rust-group denials (`-D warnings`, |
| 34 | + `-D future-incompatible`, `-D rust-2018-idioms`, etc.). These apply to every cargo |
| 35 | + invocation (build, test, check) but are invisible without reading the config file. |
| 36 | + |
| 37 | +2. **`torrust-linting` clippy runner** — passes `-D clippy::correctness`, |
| 38 | + `-D clippy::suspicious`, `-D clippy::complexity`, `-D clippy::perf`, |
| 39 | + `-D clippy::style`, `-D clippy::pedantic` on the command line. These are only |
| 40 | + active when the linter tool runs; `cargo clippy` invoked directly does not |
| 41 | + apply them. |
| 42 | + |
| 43 | +3. **`[lints.clippy]` on the root `[package]`** — the root `Cargo.toml` already has a |
| 44 | + `[lints.clippy]` section for the main binary package only; this is _not_ a |
| 45 | + `[workspace.lints]` and does not propagate to other workspace members. It also |
| 46 | + contains `needless_return = "allow"` with a `# temp allow this lint` comment, |
| 47 | + suggesting it was added as a temporary workaround rather than a deliberate policy |
| 48 | + decision. The original reason and whether the underlying callsites have since been |
| 49 | + fixed is unknown; this must be investigated before the section is migrated or removed. |
| 50 | + |
| 51 | +This fragmentation was raised in PR #1784 review by @da2ce7, who referenced the |
| 52 | +`torrust-index` configuration as the target state. |
| 53 | + |
| 54 | +Cargo 1.64+ supports `[workspace.lints]`, the idiomatic way to declare workspace-wide |
| 55 | +lint policy in a single, visible, version-controlled location. |
| 56 | + |
| 57 | +## Scope |
| 58 | + |
| 59 | +### In Scope |
| 60 | + |
| 61 | +- Add `[workspace.lints.rust]` to the root `Cargo.toml` with the lint groups currently |
| 62 | + expressed as RUSTFLAGS. |
| 63 | +- Add `[workspace.lints.clippy]` to the root `Cargo.toml` with the clippy groups |
| 64 | + currently passed by `torrust-linting`, plus `nursery = "warn"` as suggested in the |
| 65 | + PR review. |
| 66 | +- Remove the now-redundant lint entries from `RUSTFLAGS` in `.cargo/config.toml`. |
| 67 | +- Remove the root `[lints.clippy]` package-level section (superseded by workspace lints). |
| 68 | +- Fix any new warnings or errors that surface once `nursery = "warn"` and |
| 69 | + `all = "deny"` take effect (expected to be small; most lints are already enforced). |
| 70 | +- Investigate the `needless_return = "allow"` entry (see T7 below) and resolve it. |
| 71 | +- Coordinate with `torrust-linting`: either remove the redundant `-D clippy::X` flags |
| 72 | + from the clippy runner (cleaner) or document that they are intentional redundancy |
| 73 | + (safety net). A follow-up PR to `torrust-linting` may be needed. |
| 74 | + |
| 75 | +### Out of Scope |
| 76 | + |
| 77 | +- Changes to any other lint policy beyond migrating the existing set. |
| 78 | +- Enabling additional deny-level lints beyond what is listed in the Background section. |
| 79 | +- Changes to `torrust-linting` beyond removing the now-redundant clippy group flags. |
| 80 | +- MSRV changes (tracked separately in #1787). |
| 81 | + |
| 82 | +## Implementation Plan |
| 83 | + |
| 84 | +Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`. |
| 85 | + |
| 86 | +| ID | Status | Task | Notes / Expected Output | |
| 87 | +| --- | ------ | ------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------ | |
| 88 | +| T1 | TODO | Add `[workspace.lints.rust]` to root `Cargo.toml` | Mirrors current RUSTFLAGS entries; `rust-2024-compatibility` added | |
| 89 | +| T2 | TODO | Add `[workspace.lints.clippy]` to root `Cargo.toml` | Matches torrust-index config; `nursery = "warn"`, `all = "deny"` | |
| 90 | +| T3 | TODO | Remove redundant RUSTFLAGS lint entries from `.cargo/config.toml` | Only lint-related entries removed; other rustflags (e.g. `-D unused`) migrated too | |
| 91 | +| T4 | TODO | Remove root `[lints.clippy]` package section from `Cargo.toml` | Superseded by `[workspace.lints.clippy]` | |
| 92 | +| T5 | TODO | Fix any new lint failures from `nursery = "warn"` / `all = "deny"` | `cargo clippy --workspace --all-targets --all-features` must pass cleanly | |
| 93 | +| T6 | TODO | Update `torrust-linting` to remove redundant `-D clippy::X` flags | Open a separate PR in `torrust-linting`; document decision if deferred | |
| 94 | +| T7 | TODO | Investigate and resolve `needless_return = "allow"` in `Cargo.toml` | See Background; decide: fix callsites and remove the allow, or keep it with documented rationale | |
| 95 | +| T8 | TODO | Verify all quality gates pass | `linter all`, doc tests, full test suite, pre-push hook | |
| 96 | + |
| 97 | +## Progress Tracking |
| 98 | + |
| 99 | +### Workflow Checkpoints |
| 100 | + |
| 101 | +- [ ] Spec drafted in `docs/issues/drafts/` |
| 102 | +- [x] Spec reviewed and approved by user/maintainer |
| 103 | +- [x] GitHub issue created and issue number added to this spec |
| 104 | +- [ ] Implementation completed |
| 105 | +- [ ] Automatic verification completed (`linter all`, relevant tests, and pre-push checks) |
| 106 | +- [ ] Manual verification scenarios executed and recorded (status + evidence) |
| 107 | +- [ ] Acceptance criteria reviewed after implementation and updated with evidence |
| 108 | +- [ ] Reviewer validated acceptance criteria and updated checkboxes |
| 109 | +- [ ] Committer verified spec progress is up to date before commit |
| 110 | +- [ ] Issue closed and spec moved from `docs/issues/open/` to `docs/issues/closed/` |
| 111 | + |
| 112 | +### Progress Log |
| 113 | + |
| 114 | +- 2026-05-15 07:00 UTC - Agent - Spec drafted, triggered by @da2ce7 review comment on PR #1784 |
| 115 | +- 2026-05-15 08:00 UTC - Agent - GitHub issue #1786 created; spec moved from drafts/ to open/ |
| 116 | + |
| 117 | +## Acceptance Criteria |
| 118 | + |
| 119 | +- [ ] AC1: `[workspace.lints.rust]` in `Cargo.toml` covers all groups previously in RUSTFLAGS |
| 120 | +- [ ] AC2: `[workspace.lints.clippy]` in `Cargo.toml` covers all groups previously passed by `torrust-linting`, plus `nursery = "warn"` and `all = "deny"` |
| 121 | +- [ ] AC3: `.cargo/config.toml` no longer contains lint-related RUSTFLAGS entries |
| 122 | +- [ ] AC4: The root package `[lints.clippy]` section is removed |
| 123 | +- [ ] AC5: `cargo clippy --workspace --all-targets --all-features` exits `0` with no warnings |
| 124 | +- [ ] AC6: `linter all` exits `0` |
| 125 | +- [ ] AC7: All tests pass (`cargo test --workspace --all-targets --all-features`) |
| 126 | +- [ ] AC8: Pre-push hook passes |
| 127 | +- [ ] AC9: The `needless_return` allow is either removed (callsites fixed) or kept with a documented rationale replacing the `# temp allow this lint` comment |
| 128 | +- [ ] AC10: Manual verification scenarios are executed and documented (status + evidence) |
| 129 | +- [ ] AC11: Acceptance criteria are re-reviewed after implementation and reflect actual behavior |
| 130 | + |
| 131 | +## Verification Plan |
| 132 | + |
| 133 | +### Automatic Checks |
| 134 | + |
| 135 | +- `linter all` |
| 136 | +- `cargo clippy --workspace --all-targets --all-features` |
| 137 | +- `cargo test --doc --workspace` |
| 138 | +- `cargo test --tests --benches --examples --workspace --all-targets --all-features` |
| 139 | +- Pre-push hook (full gate) |
| 140 | + |
| 141 | +### Manual Verification Scenarios |
| 142 | + |
| 143 | +Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`. |
| 144 | + |
| 145 | +| ID | Scenario | Command/Steps | Expected Result | Status | Evidence | |
| 146 | +| --- | ------------------------------------------------------------- | ------------------------------------------------------------ | ------------------------------------------- | ------ | -------- | |
| 147 | +| M1 | Direct `cargo clippy` enforces workspace lints without linter | `cargo clippy --workspace --all-targets --all-features` | Exits 0; pedantic/nursery lints applied | TODO | | |
| 148 | +| M2 | `cargo build` no longer picks up redundant lint RUSTFLAGS | `cargo build --workspace` (inspect output for lint warnings) | No spurious warnings from removed RUSTFLAGS | TODO | | |
| 149 | +| M3 | `linter all` still passes with the new configuration | `linter all` | Exits 0 | TODO | | |
| 150 | + |
| 151 | +### Acceptance Verification |
| 152 | + |
| 153 | +| AC ID | Status (`TODO`/`DONE`) | Evidence | |
| 154 | +| ----- | ---------------------- | -------- | |
| 155 | +| AC1 | TODO | | |
| 156 | +| AC2 | TODO | | |
| 157 | +| AC3 | TODO | | |
| 158 | +| AC4 | TODO | | |
| 159 | +| AC5 | TODO | | |
| 160 | +| AC6 | TODO | | |
| 161 | +| AC7 | TODO | | |
| 162 | +| AC8 | TODO | | |
| 163 | +| AC9 | TODO | | |
| 164 | +| AC10 | TODO | | |
| 165 | +| AC11 | TODO | | |
| 166 | + |
| 167 | +## Risks and Trade-offs |
| 168 | + |
| 169 | +- **`nursery = "warn"` may surface many warnings**: nursery lints are experimental and |
| 170 | + can be noisy. Fixing them is not mandatory for CI to pass (warn, not deny), but a |
| 171 | + large warning count degrades signal quality. Monitor after enabling. |
| 172 | +- **`torrust-linting` coordination**: if the redundant `-D` flags are left in the linter |
| 173 | + after workspace lints are added, they remain harmless (idempotent) but add confusion. |
| 174 | + Cleaning them up requires a separate PR to `torrust-linting`. |
| 175 | + |
| 176 | +## References |
| 177 | + |
| 178 | +- Related PRs: #1784 |
| 179 | +- Suggested by: @da2ce7 in PR #1784 review |
| 180 | +- Reference config: `torrust-index` workspace `Cargo.toml` |
| 181 | +- Related issue: #1787 (evaluate MSRV bump) |
0 commit comments