| Author | Andy Green <andy@warmcat.com> 2026-10-04 05:33 UTC | | Committer | Andy Green <andy@warmcat.com> 2026-10-05 06:32 UTC | | Tree | 46f9692cc98f77cd0b89ef9fc92eb076a3077411 Raw Patch | | | meta: the -Wextra of Rust, and AGENTS.md's rules as lints | meta: the -Wextra of Rust, and AGENTS.md's rules as lints
C lws builds with -Wall -Wextra -Werror. npro had the first and last:
rustc's and clippy's default warnings, denied in CI. Add the rest.
rustc's useful off-by-default lints: rust_2018_idioms, unreachable_pub,
unused_crate_dependencies, unused_qualifications, unused_lifetimes,
redundant_lifetimes, let_underscore_drop, trivial_numeric_casts,
missing_debug_implementations and missing_copy_implementations.
clippy's pedantic group, the nearest thing to -Wextra, and restriction
lints that make AGENTS.md's rules checks rather than comments:
- wildcard_enum_match_arm: no `_ =>` arm on an enum
- allow_attributes(_without_reason): a lint is silenced with
#[expect] and a reason, and #[expect] fails once the lint stops
firing, so a silencing cannot outlive its reason
- std_instead_of_core/alloc, alloc_instead_of_core: no_std hygiene
- large_stack_arrays, large_stack_frames: what overflows a
microcontroller's stack
- print_stdout/stderr, dbg_macro, exit, mem_forget, unwrap_in_result,
panic_in_result_fn, float_arithmetic, shadow_unrelated
- missing_const_for_fn, the one nursery lint worth having
and rustdoc's private_intra_doc_links, unescaped_backticks and
missing_crate_level_docs.
What they found, fixed:
- npro-test's reader imported fmt, NonZeroU64, Error and from_utf8 from
std where core has them, matched Error with a wildcard arm, and
reused `at` for an unrelated offset
- SeededRandom::next_u64, splitmix64 and two reader helpers can be
const fn
- the fuzz harness reused `v` and `e` for unrelated things, and could
use let...else; its finding() is now #[expect], not #[allow]
- sha1's test hex() built a string with format! per byte
and kept, each with #[expect] and why:
- Sha1 and SeededRandom are not Copy: a copy made by accident would
hash on from the middle of a message, or draw the same random twice
- sha1's compress() uses RFC 3174's single-letter names, which it is
checked against (its inner a, b, c, e for bytes are renamed)
- the smoke tests, like any integration test, see every dependency of
their crate and use npro-test only through npro-fuzz
All gates pass, the 1.85 build knows every lint, and clippy is clean
over the whole feature powerset. AGENTS.md says #[expect] where it
said #[allow], and points at the list.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019kg5Eemy68ZaqDBcUJQG6J
|
diff --git a/AGENTS.md b/AGENTS.md
index d8a1e65..3c77c20 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -125,8 +125,14 @@ We are very concerned about:
`arithmetic_side_effects` and `as_conversions` enabled in the core,
`cargo doc` with `missing_docs` denied on public items,
`cargo semver-checks` on release, and a build at the pinned
- `rust-version`. A lint is silenced only with an `#[allow]` on the one
- item, carrying a reason.
+ `rust-version`. Beyond those, the workspace denies clippy's `pedantic`
+ group and rustc's useful off-by-default lints (the `-Wextra` of Rust),
+ and lints that turn the rules here into checks: no wildcard arm on an
+ enum, no `std` where `core` or `alloc` will do, no large stack frames;
+ the list, and why each is there, is `[workspace.lints]` in Cargo.toml.
+ A lint is silenced only with an `#[expect]` (not `#[allow]`, which the
+ lints refuse) on the one item, carrying a reason: `#[expect]` fails
+ once the lint stops firing, so a silencing cannot outlive its reason.
- Tests are part of the feature. Every parser has a `cargo-fuzz` target
seeded from the C corpus; state machines get property tests; public
diff --git a/Cargo.toml b/Cargo.toml
index e802236..60b0372 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -15,9 +15,23 @@ unsafe_code = "forbid"
missing_docs = "deny"
unexpected_cfgs = "deny"
+# Off by default in rustc, the nearest thing to C's -Wextra. Each finds
+# something real: an API wider than it is reachable, a dependency nothing
+# uses, a type that could be Copy, idioms older editions allowed.
+rust_2018_idioms = { level = "deny", priority = -1 }
+unreachable_pub = "deny"
+unused_crate_dependencies = "deny"
+unused_qualifications = "deny"
+unused_lifetimes = "deny"
+redundant_lifetimes = "deny"
+let_underscore_drop = "deny"
+trivial_numeric_casts = "deny"
+missing_debug_implementations = "deny"
+missing_copy_implementations = "deny"
+
# What AGENTS.md asks of every crate: nothing that can panic on input, no
# unchecked arithmetic on lengths, no lossy integer casts. A lint is
-# silenced only by an #[allow] on the one item, with a reason.
+# silenced only by an #[expect] on the one item, with a reason.
[workspace.lints.clippy]
unwrap_used = "deny"
@@ -34,3 +48,43 @@ cast_sign_loss = "deny"
cast_possible_wrap = "deny"
missing_errors_doc = "deny"
must_use_candidate = "deny"
+
+# pedantic: clippy's -Wextra. Some of it is taste; a lint that fights the
+# house style is turned off here, once, with why.
+pedantic = { level = "deny", priority = -1 }
+
+# AGENTS.md's rules, as lints rather than comments:
+# - a match on state has no `_ =>` arm, so a new state is handled
+# everywhere or the build fails
+wildcard_enum_match_arm = "deny"
+# - a lint is silenced on the one item, with a reason, and with #[expect]
+# rather than #[allow]: #[expect] fails once the lint stops firing, so
+# a stale silencing cannot outlive what it was for
+allow_attributes_without_reason = "deny"
+allow_attributes = "deny"
+# - no_std: what only needs core or alloc does not reach for std
+std_instead_of_core = "deny"
+std_instead_of_alloc = "deny"
+alloc_instead_of_core = "deny"
+# - stack use that is fine on a server and overflows a microcontroller
+large_stack_arrays = "deny"
+large_stack_frames = "deny"
+# - a library does not print, exit, leak, or panic where it returns errors
+print_stdout = "deny"
+print_stderr = "deny"
+dbg_macro = "deny"
+exit = "deny"
+mem_forget = "deny"
+unwrap_in_result = "deny"
+panic_in_result_fn = "deny"
+# - protocol code has no floating point
+float_arithmetic = "deny"
+# - a name reused for something unrelated reads as the old thing
+shadow_unrelated = "deny"
+# - from nursery, which is otherwise left off as unfinished
+missing_const_for_fn = "deny"
+
+[workspace.lints.rustdoc]
+private_intra_doc_links = "deny"
+unescaped_backticks = "deny"
+missing_crate_level_docs = "deny"
diff --git a/crates/npro-core/src/random.rs b/crates/npro-core/src/random.rs
index 9791f94..cb5311d 100644
--- a/crates/npro-core/src/random.rs
+++ b/crates/npro-core/src/random.rs
@@ -71,6 +71,10 @@ impl core::error::Error for Unavailable {}
/// ```
#[cfg(feature = "replay")]
#[derive(Clone, Debug)]
+#[expect(
+ missing_copy_implementations,
+ reason = "a copy made by accident would draw the same random twice"
+)]
pub struct SeededRandom {
s: [u64; 4],
}
@@ -88,7 +92,7 @@ impl SeededRandom {
}
/// The next 64-bit word of the stream (C's `lws_xos()`).
- pub fn next_u64(&mut self) -> u64 {
+ pub const fn next_u64(&mut self) -> u64 {
let [s0, s1, s2, s3] = &mut self.s;
let result = s1.wrapping_mul(5).rotate_left(7).wrapping_mul(9);
let t = s1.wrapping_shl(17);
@@ -122,7 +126,7 @@ impl Random for SeededRandom {
/// reference splitmix64 mixes it after. This is C's, so that the streams
/// match; the textbook one would seed a different stream.
#[cfg(feature = "replay")]
-fn splitmix64(s: &mut u64) -> u64 {
+const fn splitmix64(s: &mut u64) -> u64 {
let mut r = *s;
*s = s.wrapping_add(0x9e37_79b9_7f4a_7c15);
r = (r ^ (r >> 30)).wrapping_mul(0xbf58_476d_1ce4_e5b9);
diff --git a/crates/npro-core/src/sha1.rs b/crates/npro-core/src/sha1.rs
index 7c0fa33..8692114 100644
--- a/crates/npro-core/src/sha1.rs
+++ b/crates/npro-core/src/sha1.rs
@@ -35,6 +35,10 @@ const H0: [u32; 5] = [
/// assert_eq!(h.finish(), Sha1::digest(b"abc"));
/// ```
#[derive(Clone, Debug)]
+#[expect(
+ missing_copy_implementations,
+ reason = "a copy made by accident would hash on from the middle of a message"
+)]
pub struct Sha1 {
state: [u32; 5],
/// Bytes of the current block taken so far.
@@ -126,13 +130,17 @@ impl Sha1 {
}
/// The compression function over the full block (RFC 3174 6.1).
+ #[expect(
+ clippy::many_single_char_names,
+ reason = "a to e, f, k and w are RFC 3174's names, which the code is checked against"
+ )]
fn compress(&mut self) {
// the message schedule, kept as the last 16 words: each round's
// word is w[0], and the next is made from w[13], w[8], w[2], w[0]
let mut w = [0u32; 16];
- for (d, c) in w.iter_mut().zip(self.block.chunks_exact(4)) {
- if let &[a, b, c, e] = c {
- *d = u32::from_be_bytes([a, b, c, e]);
+ for (word, bytes) in w.iter_mut().zip(self.block.chunks_exact(4)) {
+ if let &[b0, b1, b2, b3] = bytes {
+ *word = u32::from_be_bytes([b0, b1, b2, b3]);
}
}
@@ -173,7 +181,12 @@ mod tests {
use super::*;
fn hex(d: [u8; DIGEST_LEN]) -> String {
- d.iter().map(|b| format!("{b:02x}")).collect()
+ use core::fmt::Write;
+
+ d.iter().fold(String::new(), |mut s, b| {
+ write!(s, "{b:02x}").unwrap();
+ s
+ })
}
#[test]
diff --git a/crates/npro-fuzz/src/targets.rs b/crates/npro-fuzz/src/targets.rs
index 4ea70d6..3f014ac 100644
--- a/crates/npro-fuzz/src/targets.rs
+++ b/crates/npro-fuzz/src/targets.rs
@@ -10,7 +10,7 @@ use npro_test::{MAX_BYTES, MAX_STEPS, Transcript};
/// Reports a finding to whatever is driving the target: libFuzzer, which
/// treats a panic as a crash and keeps the input, or a smoke test.
-#[allow(
+#[expect(
clippy::panic,
reason = "a panic is how a fuzz target reports a finding to libFuzzer"
)]
@@ -72,9 +72,8 @@ enum Utf8End {
/// The oracle: how `text` ends according to `core::str`.
fn utf8_end_by_core(text: &[u8]) -> Utf8End {
- let e = match core::str::from_utf8(text) {
- Ok(_) => return Utf8End::Complete,
- Err(e) => e,
+ let Err(e) = core::str::from_utf8(text) else {
+ return Utf8End::Complete;
};
if e.error_len().is_none() {
return Utf8End::Partial;
@@ -86,7 +85,7 @@ fn utf8_end_by_core(text: &[u8]) -> Utf8End {
let from = e.valid_up_to();
let refused = (from..text.len()).find(|&i| {
text.get(..=i)
- .is_some_and(|p| core::str::from_utf8(p).is_err_and(|e| e.error_len().is_some()))
+ .is_some_and(|p| core::str::from_utf8(p).is_err_and(|pe| pe.error_len().is_some()))
});
Utf8End::InvalidAt(refused.unwrap_or(from))
}
@@ -102,15 +101,15 @@ pub fn utf8(data: &[u8]) {
let (ctl, text) = control(data);
let expect = utf8_end_by_core(text);
- let mut v = Utf8Validator::new();
+ let mut one_at_a_time = Utf8Validator::new();
let mut bytewise = None;
for (i, b) in text.iter().enumerate() {
- if v.feed(core::slice::from_ref(b)).is_err() {
+ if one_at_a_time.feed(core::slice::from_ref(b)).is_err() {
bytewise = Some(Utf8End::InvalidAt(i));
break;
}
}
- let bytewise = bytewise.unwrap_or(if v.at_boundary() {
+ let bytewise = bytewise.unwrap_or(if one_at_a_time.at_boundary() {
Utf8End::Complete
} else {
Utf8End::Partial
diff --git a/crates/npro-fuzz/tests/smoke.rs b/crates/npro-fuzz/tests/smoke.rs
index 0092347..a9fcbdd 100644
--- a/crates/npro-fuzz/tests/smoke.rs
+++ b/crates/npro-fuzz/tests/smoke.rs
@@ -4,6 +4,11 @@
//! is `scripts/fuzz.sh`. A failure here reproduces exactly, since the
//! inputs are the same every run.
+#![expect(
+ unused_crate_dependencies,
+ reason = "an integration test sees all of its crate's dependencies; this one drives npro-test only through npro-fuzz"
+)]
+
use std::fs;
use std::io;
use std::path::{Path, PathBuf};
@@ -52,7 +57,7 @@ fn index_below(r: &mut SeededRandom, n: usize) -> usize {
usize::try_from(below(r, n)).unwrap_or(0)
}
-fn byte(r: &mut SeededRandom) -> u8 {
+const fn byte(r: &mut SeededRandom) -> u8 {
let [b, ..] = r.next_u64().to_le_bytes();
b
}
diff --git a/crates/npro-test/src/transcript.rs b/crates/npro-test/src/transcript.rs
index 4171ad1..3e75dce 100644
--- a/crates/npro-test/src/transcript.rs
+++ b/crates/npro-test/src/transcript.rs
@@ -12,9 +12,9 @@
//! each once; lowercase hex; no string escapes; no recursion. Anything else
//! is a new format version, and refused.
+use core::{fmt, num::NonZeroU64};
use std::{
- fmt, fs, io,
- num::NonZeroU64,
+ fs, io,
path::{Path, PathBuf},
};
@@ -165,11 +165,23 @@ impl fmt::Display for Error {
}
}
-impl std::error::Error for Error {
- fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+impl core::error::Error for Error {
+ fn source(&self) -> Option<&(dyn core::error::Error + 'static)> {
match self {
Self::Io(e) => Some(e),
- _ => None,
+ Self::TooLarge
+ | Self::Syntax { .. }
+ | Self::UnknownFormat
+ | Self::UnknownKey { .. }
+ | Self::DuplicateKey(_)
+ | Self::MissingKey(_)
+ | Self::BadSide { .. }
+ | Self::BadHex { .. }
+ | Self::BadStep { .. }
+ | Self::Overflow { .. }
+ | Self::TooManySteps
+ | Self::TimeGoesBackwards { .. }
+ | Self::CaseMismatch => None,
}
}
}
@@ -219,11 +231,11 @@ impl Transcript {
"format" => once(&mut format, "format", c.string()?),
"case" => once(&mut case, "case", c.string()?),
"side" => {
- let at = c.value_offset();
+ let value_at = c.value_offset();
let s = match c.string()? {
"server" => Side::Server,
"client" => Side::Client,
- _ => return Err(Error::BadSide { offset: at }),
+ _ => return Err(Error::BadSide { offset: value_at }),
};
once(&mut side, "side", s)
}
@@ -344,11 +356,11 @@ impl<'a> Cursor<'a> {
self.b.get(self.pos).copied()
}
- fn bump(&mut self) {
+ const fn bump(&mut self) {
self.pos = self.pos.saturating_add(1);
}
- fn syntax(&self, expected: &'static str) -> Error {
+ const fn syntax(&self, expected: &'static str) -> Error {
Error::Syntax {
offset: self.pos,
expected,
@@ -395,7 +407,7 @@ impl<'a> Cursor<'a> {
.get(start..self.pos)
.ok_or_else(|| self.syntax("a string"))?;
self.bump();
- std::str::from_utf8(s).map_err(|_| Error::Syntax {
+ core::str::from_utf8(s).map_err(|_| Error::Syntax {
offset: start,
expected: "UTF-8",
})
|