The daemon builds with 40 warnings, and nobody reads them #236

New Issue

2026-09-28T19:13:39-04:00

Salastil commented

2026-09-28 19:13:39 -04:00

The daemon compiles with 40 warnings on every build, and has for long enough that nobody reads them. The CI log for the last run on master reports them three times over — 41 for the release binary, 35 for the test binary, 41 again on the Windows cross-build — so roughly 120 lines of warning scroll past per green run.

That is the real cost. A build that is never quiet cannot tell you when something new appears: the compiler has already flagged one bug in this list and it sat there unread because it looked like the other thirty-nine.

Counted with cargo check --release --message-format=json on nobilis at bda039d:

lint	count
`dead_code`	30
`unused_imports`	4
`unused_variables`	2
`unreachable_patterns`	2
`unused_doc_comments`	1
`unused_assignments`	1

Not all of these mean "delete it"

Three kinds are mixed together here and they want different answers. Sorting them is most of the work; the edits are trivial once sorted.

Genuinely unused — delete. src/backend/matrix/protocol.rs:13-15 defines EVENT_ROOM_NAME, EVENT_ROOM_MEMBER and EVENT_ROOM_ENCRYPTION and nothing reads any of them; the event types are written as string literals at the call sites instead. Same for ONION_PORT in sneedchat/connect.rs:14 and EVENT_MEMBER_NEXT in matrix/calls.rs:135.

Test-only helpers that are not marked as such. sneedchat/protocol.rs:403's parse_whisper_command is called by six tests and by nothing else — the live path goes through parse_whisper_command_among. It should be #[cfg(test)], or the tests should call what the code calls. Worth checking each "never used" function for this before deleting it: a helper that only tests use is a different thing from one nobody uses.

Deliberate, and the warning is the wrong answer. irc/send.rs:312 is a _ => '?' arm with a comment saying why it is there: the irc crate may grow ChannelMode variants, and an unknown mode letter is better shown than swallowed. It is unreachable today. Deleting it would be correct now and wrong at the next dependency bump. That one wants #[allow(unreachable_patterns)] and the comment kept, not a deletion. matrix/crypto.rs:370 is the same shape and deserves the same look.

Half-wired features. Several of the never-used methods are one end of something that was built and never connected — runtime.rs:1810 alone lists six (set_discord_voice_state, discord_known_avatar, discord_voice_members, discord_mute_keys, matrix_power_levels, matrix_direct_rooms). Each is a question: was the caller removed, or never written? The answer decides between deleting the method and filing the gap.

The one that is a bug

matrix/auth.rs:452 — value assigned to refresh_token is never read. Benign as it stands (the initialiser is dead because the loop either assigns or bails), but it is in the device-grant sign-in path, and the refresh token going missing there is exactly the fault that killed an account at its first token expiry a fortnight ago. The compiler pointing anywhere near that code deserves a read rather than a scroll.

Full inventory

unused_imports (4)

src/backend/irc/mod.rs:44 — drafts::*
src/backend/irc/mod.rs:51 — who::*
src/backend/matrix/mod.rs:72 — relock::*
src/backend/matrix/mod.rs:74 — server::*

unused_variables (2)

src/backend/discord/gateway.rs:179 — account_id
src/backend/matrix/dehydration.rs:222 — state

unused_assignments (1)

src/backend/matrix/auth.rs:452 — refresh_token

unused_doc_comments (1)

src/rpc/methods.rs:1096

unreachable_patterns (2)

src/backend/irc/send.rs:312
src/backend/matrix/crypto.rs:370

dead_code (30)

src/audio.rs:461 — method is_muted
src/audio.rs:523 — method queued
src/audio.rs:635 — function route_output
src/backend/discord/voice.rs:676 — function is_connected
src/backend/discord/voicecrypto.rs:148 — function open
src/backend/irc/connect.rs:771 — function offered_caps
src/backend/irc/dcc.rs:475 — field total
src/backend/irc/metadata.rs:60 — function is_metadata_cap
src/backend/irc/presence.rs:363 — associated function ranked
src/backend/kick/api.rs:55 — field username
src/backend/kick/api.rs:422 — field id
src/backend/kick/api.rs:707 — field kind
src/backend/kick/api.rs:1588 — function predictions_recent
src/backend/matrix/calls.rs:135 — constant EVENT_MEMBER_NEXT
src/backend/matrix/calls.rs:580 — function should_offer
src/backend/matrix/crypto.rs:771 — method share_and_encrypt
src/backend/matrix/directs.rs:65 — function without
src/backend/matrix/protocol.rs:13 — constant EVENT_ROOM_NAME
src/backend/matrix/protocol.rs:14 — constant EVENT_ROOM_MEMBER
src/backend/matrix/protocol.rs:15 — constant EVENT_ROOM_ENCRYPTION
src/backend/matrix/server.rs:57 — methods speaks, has_unstable
src/backend/sneedchat/auth.rs:95 — method base
src/backend/sneedchat/connect.rs:14 — constant ONION_PORT
src/backend/sneedchat/postimg.rs:130 — function upload_to_postimg
src/backend/sneedchat/protocol.rs:30 — fields message_date, room_id
src/backend/sneedchat/protocol.rs:273 — method is_empty
src/backend/sneedchat/protocol.rs:403 — function parse_whisper_command
src/export.rs:81 — field root
src/runtime.rs:1810 — methods set_discord_voice_state, discord_known_avatar, discord_voice_members, discord_mute_keys, matrix_power_levels, matrix_direct_rooms
src/voicenote.rs:106 — field samples

Done when

cargo build --release and cargo test --release are both silent, and the CI log for a green run contains no warning: lines from nobilis.

Worth doing in that order and keeping: once the count is zero, a -D warnings in the workflow would stop it climbing back. Suggested as a follow-up rather than part of this — turning it on while 40 are outstanding just breaks the build.

cargo fix --bin nobilis -p nobilis claims six of these, which is a starting point and not a plan: it will happily delete the deliberate arms above.

The daemon compiles with **40 warnings** on every build, and has for long enough that nobody reads them. The CI log for the last run on `master` reports them three times over — 41 for the release binary, 35 for the test binary, 41 again on the Windows cross-build — so roughly 120 lines of warning scroll past per green run. That is the real cost. A build that is never quiet cannot tell you when something new appears: the compiler has already flagged one bug in this list and it sat there unread because it looked like the other thirty-nine. Counted with `cargo check --release --message-format=json` on `nobilis` at `bda039d`: | lint | count | | --- | --- | | `dead_code` | 30 | | `unused_imports` | 4 | | `unused_variables` | 2 | | `unreachable_patterns` | 2 | | `unused_doc_comments` | 1 | | `unused_assignments` | 1 | ## Not all of these mean "delete it" Three kinds are mixed together here and they want different answers. Sorting them is most of the work; the edits are trivial once sorted. **Genuinely unused — delete.** `src/backend/matrix/protocol.rs:13-15` defines `EVENT_ROOM_NAME`, `EVENT_ROOM_MEMBER` and `EVENT_ROOM_ENCRYPTION` and nothing reads any of them; the event types are written as string literals at the call sites instead. Same for `ONION_PORT` in `sneedchat/connect.rs:14` and `EVENT_MEMBER_NEXT` in `matrix/calls.rs:135`. **Test-only helpers that are not marked as such.** `sneedchat/protocol.rs:403`'s `parse_whisper_command` is called by six tests and by nothing else — the live path goes through `parse_whisper_command_among`. It should be `#[cfg(test)]`, or the tests should call what the code calls. Worth checking each "never used" function for this before deleting it: a helper that only tests use is a different thing from one nobody uses. **Deliberate, and the warning is the wrong answer.** `irc/send.rs:312` is a `_ => '?'` arm with a comment saying why it is there: the irc crate may grow `ChannelMode` variants, and an unknown mode letter is better shown than swallowed. It is unreachable *today*. Deleting it would be correct now and wrong at the next dependency bump. That one wants `#[allow(unreachable_patterns)]` and the comment kept, not a deletion. `matrix/crypto.rs:370` is the same shape and deserves the same look. **Half-wired features.** Several of the never-used methods are one end of something that was built and never connected — `runtime.rs:1810` alone lists six (`set_discord_voice_state`, `discord_known_avatar`, `discord_voice_members`, `discord_mute_keys`, `matrix_power_levels`, `matrix_direct_rooms`). Each is a question: was the caller removed, or never written? The answer decides between deleting the method and filing the gap. ## The one that is a bug `matrix/auth.rs:452` — `value assigned to refresh_token is never read`. Benign as it stands (the initialiser is dead because the loop either assigns or bails), but it is in the device-grant sign-in path, and *the refresh token going missing there is exactly the fault that killed an account at its first token expiry* a fortnight ago. The compiler pointing anywhere near that code deserves a read rather than a scroll. ## Full inventory ### unused_imports (4) - `src/backend/irc/mod.rs:44` — `drafts::*` - `src/backend/irc/mod.rs:51` — `who::*` - `src/backend/matrix/mod.rs:72` — `relock::*` - `src/backend/matrix/mod.rs:74` — `server::*` ### unused_variables (2) - `src/backend/discord/gateway.rs:179` — `account_id` - `src/backend/matrix/dehydration.rs:222` — `state` ### unused_assignments (1) - `src/backend/matrix/auth.rs:452` — `refresh_token` ### unused_doc_comments (1) - `src/rpc/methods.rs:1096` ### unreachable_patterns (2) - `src/backend/irc/send.rs:312` - `src/backend/matrix/crypto.rs:370` ### dead_code (30) - `src/audio.rs:461` — method `is_muted` - `src/audio.rs:523` — method `queued` - `src/audio.rs:635` — function `route_output` - `src/backend/discord/voice.rs:676` — function `is_connected` - `src/backend/discord/voicecrypto.rs:148` — function `open` - `src/backend/irc/connect.rs:771` — function `offered_caps` - `src/backend/irc/dcc.rs:475` — field `total` - `src/backend/irc/metadata.rs:60` — function `is_metadata_cap` - `src/backend/irc/presence.rs:363` — associated function `ranked` - `src/backend/kick/api.rs:55` — field `username` - `src/backend/kick/api.rs:422` — field `id` - `src/backend/kick/api.rs:707` — field `kind` - `src/backend/kick/api.rs:1588` — function `predictions_recent` - `src/backend/matrix/calls.rs:135` — constant `EVENT_MEMBER_NEXT` - `src/backend/matrix/calls.rs:580` — function `should_offer` - `src/backend/matrix/crypto.rs:771` — method `share_and_encrypt` - `src/backend/matrix/directs.rs:65` — function `without` - `src/backend/matrix/protocol.rs:13` — constant `EVENT_ROOM_NAME` - `src/backend/matrix/protocol.rs:14` — constant `EVENT_ROOM_MEMBER` - `src/backend/matrix/protocol.rs:15` — constant `EVENT_ROOM_ENCRYPTION` - `src/backend/matrix/server.rs:57` — methods `speaks`, `has_unstable` - `src/backend/sneedchat/auth.rs:95` — method `base` - `src/backend/sneedchat/connect.rs:14` — constant `ONION_PORT` - `src/backend/sneedchat/postimg.rs:130` — function `upload_to_postimg` - `src/backend/sneedchat/protocol.rs:30` — fields `message_date`, `room_id` - `src/backend/sneedchat/protocol.rs:273` — method `is_empty` - `src/backend/sneedchat/protocol.rs:403` — function `parse_whisper_command` - `src/export.rs:81` — field `root` - `src/runtime.rs:1810` — methods `set_discord_voice_state`, `discord_known_avatar`, `discord_voice_members`, `discord_mute_keys`, `matrix_power_levels`, `matrix_direct_rooms` - `src/voicenote.rs:106` — field `samples` ## Done when `cargo build --release` and `cargo test --release` are both silent, and the CI log for a green run contains no `warning:` lines from `nobilis`. Worth doing in that order and keeping: once the count is zero, a `-D warnings` in the workflow would stop it climbing back. Suggested as a follow-up rather than part of this — turning it on while 40 are outstanding just breaks the build. `cargo fix --bin nobilis -p nobilis` claims six of these, which is a starting point and not a plan: it will happily delete the deliberate arms above.

Salastil added the bug low priority labels 2026-09-28 19:13:39 -04:00

Salastil commented

2026-09-28 19:47:37 -04:00

Finished. Both builds are silent now — cargo build --release and cargo test --release each report zero warnings, where they reported 41 and 35.

d0efc2a09ba09e5b2f5f0a7e6b4cbbbe7dd2ba0e — https://git.salastil.com/Salastil/nobilis/commit/d0efc2a

The sort came out much as the issue guessed, and one part of it bit exactly as predicted. open, without and ServerResponse::is_empty all looked like orphans to a release build and all three broke the test build when deleted — a release dead_code cannot see test callers, so "never used" and "used only by tests" are indistinguishable until you try it. They are #[cfg(test)] now, along with three more found the same way.

What went where:

deleted — three event-type constants the call sites write as string literals anyway, ONION_PORT, EVENT_MEMBER_NEXT, a CAP helper superseded by the one beside it, a postimg wrapper, and six Runtime accessors duplicating the ones in use
#[cfg(test)] — nine, including the three above
#[allow(dead_code)] with a note — four fields across the Kick and Sneedchat wire structs. A struct that drops the fields it does not read stops describing what arrives
#[allow(unreachable_patterns)] — both arms, which guard against a dependency growing a variant. cargo fix would have deleted both, which is why it was not used

The line worth reading was matrix/auth.rs:452, as suspected: a refresh token assigned a value nothing reads. Harmless as written — the loop below either assigns it or bails — but it is in the device-grant path where a missing refresh token has already killed an account once, so it now declares without an initialiser and says why.

The -D warnings follow-up is worth doing now that the count is zero; deliberately not part of this.

Finished. Both builds are silent now — `cargo build --release` and `cargo test --release` each report zero warnings, where they reported 41 and 35. d0efc2a09ba09e5b2f5f0a7e6b4cbbbe7dd2ba0e — https://git.salastil.com/Salastil/nobilis/commit/d0efc2a The sort came out much as the issue guessed, and one part of it bit exactly as predicted. `open`, `without` and `ServerResponse::is_empty` all looked like orphans to a release build and all three broke the test build when deleted — a release `dead_code` cannot see test callers, so "never used" and "used only by tests" are indistinguishable until you try it. They are `#[cfg(test)]` now, along with three more found the same way. What went where: - **deleted** — three event-type constants the call sites write as string literals anyway, `ONION_PORT`, `EVENT_MEMBER_NEXT`, a CAP helper superseded by the one beside it, a postimg wrapper, and six `Runtime` accessors duplicating the ones in use - **`#[cfg(test)]`** — nine, including the three above - **`#[allow(dead_code)]` with a note** — four fields across the Kick and Sneedchat wire structs. A struct that drops the fields it does not read stops describing what arrives - **`#[allow(unreachable_patterns)]`** — both arms, which guard against a dependency growing a variant. `cargo fix` would have deleted both, which is why it was not used The line worth reading was `matrix/auth.rs:452`, as suspected: a refresh token assigned a value nothing reads. Harmless as written — the loop below either assigns it or bails — but it is in the device-grant path where a missing refresh token has already killed an account once, so it now declares without an initialiser and says why. The `-D warnings` follow-up is worth doing now that the count is zero; deliberately not part of this.

Salastil closed this issue

2026-09-28 19:47:37 -04:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Salastil/moho#236