Consuming builders lack #[must_use]

Timing: Follow-up after trial/3 merges into main; defer implementation until then.

Review baseline: Local trial/3 at a094418baf6388e3eeca23821df481a7f0a8751c. This branch is not yet on the remote. Source locations below refer to that snapshot and should be rechecked after merge.

Problem

Consuming builder methods return a modified Self without marking the type or method #[must_use]. A discarded temporary builder chain can compile silently while accomplishing nothing. Reusing a moved builder is already rejected by Rust; the gap here is accidentally discarding the returned temporary.

Evidence

crates/lib/src/request.rs:403-422: RequestBuilder::url and method.
crates/sql/sql/src/ddl.rs:321-323: consuming add_field.
The review's opt-in Clippy pass reported return_self_not_must_use across consuming APIs.

Acceptance criteria

Review consuming builders and pure value-transforming methods for appropriate type-level or method-level #[must_use] coverage.
Avoid indiscriminately annotating side-effecting APIs where discarding the return is valid.
Preserve builder semantics and chaining compatibility.
Verify the chosen diagnostics with a representative accidentally discarded chain and rerun affected lint/compile checks.

Code examples and suggested direction

Method-level annotation preserves the existing consuming signature:

#[must_use = "the updated request builder must be used"]
pub fn method(mut self, method: impl Into<String>) -> Self {
    self.method = Some(method.into());
    self
}

Expected diagnostic example after annotation:

use stackpress_lib::Request;

// Should warn: the returned builder is discarded.
Request::builder().method("POST");

// Intentional discard stays explicit.
let _ = Request::builder().method("POST");

Type-level #[must_use] on a dedicated builder may cover the surface more consistently; choose the annotation level after reviewing actual call sites.

Suggested investigation and fix notes

Deferred until trial/3 is merged into main. Recheck the merged source and tests first. The snippets are examples/proposals, not an approved patch or a change to frozen contracts.

Inventory consuming methods returning Self and their builder types in Lib, SQL, and the affected facade. Distinguish pure builder updates from side-effecting methods.
Prefer a single type-level annotation when every value of that builder should be used. Use method-level annotations when the type also has legitimate discard cases.
Keep ownership/chaining signatures unchanged. Existing Result values are already must-use; do not add redundant annotations just to satisfy a count.
Check examples and generated consumers for intentional discards. Use let _ = ... only when intent is real, not to hide a caller bug.
Validate warning behavior under the supported compiler. This is a compiler diagnostic change, not a runtime feature; a small compile check is enough, without adding a runtime test that merely repeats the annotation.

Suggested focused checks:

cargo clippy -p stackpress-lib -p stackpress-sql --all-targets --locked -- -W clippy::return_self_not_must_use
cargo test -p stackpress-lib -p stackpress-sql --doc --locked

Compile the accidental-discard snippet with -D unused_must_use in a disposable probe to confirm rejection, and confirm the explicit-discard version still compiles.

Implementation workflow: After the merge, follow .agents/workflows/rust-programming.md: prove the affected behavior, complete the three test-verified review rounds, and record relevant formatting/lints/docs, feature/platform scope, and applicable development coverage. Preserve unrelated changes.

Partly fixed on trial/3 in 644795f (merged as 64a5b70): the lib portion is done. `#[must_use]` added to consuming lib builders; a `compile_fail` doctest with `deny(unused_must_use)` on a discarded `Request::builder().method("POST")` failed before and passes after. Server/CLI/transformer parts are in task 00010 lane 2; SQL and framework parts follow in a third pass. Leaving open.

Progress on trial/3 (merged as ff7a251): the server, CLI and transformer parts are now done, in addition to the earlier lib part. The SQL and UI-framework parts follow in a short final pass. Leaving open.

Now complete across all crates on trial/3 (final pass 28b3826, merged as 7031282; earlier parts in 644795f and 407b4d6). Not yet on main, so leaving open.

SQL and UI-framework consuming builders now carry #[must_use], with a compile_fail doctest on a discarded chain that failed before the annotation.
Coordinator-verified on the combined code: fmt, Clippy -D warnings, Rustdoc -D warnings, cargo +1.96.0 check, 470 workspace tests and 867 doctests pass; the profile UI check passes for both in-memory and SQLite backends, with all 16 browser proofs.

Fix owner review issues #11, #1, #2, #5 and lib parts of #4, #7 (task…

Fix owner review issues #10, #3, #8, #9, #13, #12 and host parts of #4,

Finish owner review issues #4, #6, #7 for SQL and framework; neutral …

Navigation Menu

Repository navigation

Consuming builders lack #[must_use] #4

Problem

Evidence

Acceptance criteria

Code examples and suggested direction

Suggested investigation and fix notes

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Consuming builders lack #[must_use] #4

Description

Problem

Evidence

Acceptance criteria

Code examples and suggested direction

Activity

cblanquera commented on Oct 26, 2026

Suggested investigation and fix notes

cblanquera commented on Oct 26, 2026

cblanquera commented on Oct 26, 2026

cblanquera commented on Oct 26, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions