Skip to content

fix(config): scope the dead_code suppressions to the fields that are unread - #300

Merged
argszero merged 1 commit into
mainfrom
fix/stale-dead-code-allowances
Sep 25, 2026
Merged

argszero merged 1 commit into
mainfrom
fix/stale-dead-code-allowances

Conversation

@argszero

Copy link
Copy Markdown
Owner

Summary

Six #[allow(dead_code)] attributes sat at struct level in src/config.rs (lines 133, 153, 167, 176, 192, 201) — Config, Points, Provider, Plan, Endpoint, Model. A struct-level attribute silences every field of its struct, including fields added later, and one of them (Config) still carried a reason naming a development stage that landed long ago.

Measured with the compiler, in the one build configuration where a dead_code suppression can matter — a non-test build (RUSTFLAGS="-D warnings" cargo build):

attribute what it actually suppresses
Config (:133) nothing
Points (:153) display_name, symbol
Provider (:167) name, country
Plan (:176) nothing
Endpoint (:192) nothing
Model (:201) nothing

With every allow stripped, rustc names exactly four fields: display_name, symbol, name, country.

Two notes on how this was established, because both matter for reading the diff:

  • A static read of the crate gets Provider.name wrong. A plain identifier search credits it with reads that in fact belong to Plan.name (gateway.rs reads Plan.name when it builds the /api/plans payload). The compiler names Provider.name as unread; the compiler is right and the lexical count is not — so the suppression for that field moves to the field too.
  • The four fields are deliberately left alone semantically. They are 配了却没人用 knobs, and wiring or deleting them is a separate product decision (the provider display name currently has more than one source, and which one is authoritative is an open question). This PR only moves where the suppression sits; it adds no wiring and deletes no field.

Why this survived at all: CI's only warning gate is cargo clippy --all-targets -- -D warnings, and --all-targets compiles the test targets. The parse tests in config.rs assert on every field (assert_eq!(cfg.points.display_name, "点数"), p.country == "CN", …) — so in CI's view every one of these fields is read. The gate is structurally blind to the whole class "configured in TOML, never read by production": it goes red only when a field is read from nowhere at all, not when the only reader is a test.

Related Issue

None — the repository has no open issues (all three are CLOSED). This change comes from this task's own reconnaissance: an audit of the repository's suppressed compiler diagnostics, motivated by the observation that a suppression outlives the condition it was written for.

Changes

  • Drop the four attributes that suppress nothing (Config, Plan, Endpoint, Model). Four of the six structs are covered by the compiler again, and future fields added to them can no longer be silenced by an attribute nobody revisits.
  • Move the remaining suppression onto the fields it belongs to: Points::display_name, Points::symbol, Provider::name, Provider::country.
  • Replace the reason comments with ones that state the invariant. The removed comment on Config claimed “points/providers/plans 由后续 P0 网关/定价阶段消费” — that stage landed long ago, which is exactly how a stale reason keeps a wide suppressor alive.
  • Untouched, deliberately: the two #[allow(clippy::too_many_arguments)] (src/billing.rs:58, src/routes/admin_models.rs:74). Both are genuine shape suppressions (flat price fields, kept in request-struct order) and both already state their reason.
  • No field added, removed or retyped, so config/config.example.toml and the example data are unchanged.

Tests

  • cargo test — 392 passed, 0 failed (identical to the baseline: the change is an attribute block, and no test observes which attribute carries a suppression)
  • cargo fmt --check — rc=0
  • cargo clippy --all-targets -- -D warnings — rc=0
  • New unit tests — not applicable: a suppression's reach is a compile-time fact with no runtime trace, and the repository's own warning gate cannot see the class (that is the finding above). The measurement is the A/B below.

A/B (compiler legs)

tmp/r153ab/ab.py — four legs, each with its own CARGO_TARGET_DIR, each RUSTFLAGS="-D warnings" cargo build, BASE = src/config.rs at acb8815, FIXED = this branch:

# attributes: BASE struct-level=6  FIXED struct-level=0 / any-level=4
[l1-fixed-asis]        rc=0 unread=[]
[l2-fixed-stripped]    rc=1 unread=['country', 'display_name', 'name', 'symbol']
[l3-base-asis]         rc=0 unread=[]
[l4-base-structgone]   rc=1 unread=['country', 'display_name', 'name', 'symbol']

L1 FIXED as-is: clean build (the four field attributes carry the suppression)  — as declared
L2 FIXED, allows stripped: exactly the unread fields are named                  — as declared
L3 BASE as-is: clean build                                                     — as declared
L4 BASE, struct-level allows gone: SAME field set                              — as declared

RESULT: 4 as declared, 0 contrary

L4 is the decisive leg: it removes all six attributes from the original file and gets a build failure naming the same four fields. If any of the four attributes being dropped here had been suppressing something, its field would appear in that list — so each of the four was inert, and the narrowed form covers exactly what the six covered, no more and no less. L2 == L4 is the statement that this PR moved the suppression without moving the set of unread fields.

Checklist

  • 分支命名符合约定 (fix/)
  • Commit message 使用 Conventional Commits 格式
  • 单一职责,改动最小化

…unread

Six `#[allow(dead_code)]` attributes sat on the config.rs structs, so each one
silenced every field of its struct -- including fields added later. Two of them
suppressed something (Points, Provider); the other four (Config, Plan,
Endpoint, Model) silenced nothing, and the reason comment on `Config` still
named a development stage that landed long ago.

Measured with the compiler in the one configuration where a dead_code
suppression can matter, a non-test build (`RUSTFLAGS="-D warnings" cargo
build`): with every allow stripped, rustc names exactly `display_name`,
`symbol`, `name` and `country`.

So: drop the four inert attributes, and move the suppression for the remaining
four fields onto the fields themselves, with reasons that state the invariant
instead of a stage that is over.

CI cannot see this class -- its only warning gate is
`clippy --all-targets -- -D warnings`, and `--all-targets` compiles the parse
tests, which read every field. No behaviour change: no config field is added,
removed or retyped, so config/config.example.toml is untouched.
@argszero
argszero merged commit a270e12 into main Sep 25, 2026
1 check passed
@argszero
argszero deleted the fix/stale-dead-code-allowances branch September 25, 2026 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant