Enable strict clippy with documented allow-list and defensive-coding pass#257
Draft
Enable strict clippy with documented allow-list and defensive-coding pass#257
Conversation
Turns on `pedantic` (warn) and `restriction` (deny) workspace-wide and adds
`[lints] workspace = true` to every crate so the policy actually applies.
Captures a baseline allow-list in `Cargo.toml`, organized by category
(Documentation, Style/formatting, Defensive coding, API design, Imports/paths,
Output/diagnostics, Tests, Attributes) with per-lint counts and rationales —
each entry is a TODO unless explicitly marked intentional.
Defensive-coding pass:
- New `clippy.toml` with `allow-{unwrap,expect,panic,indexing-slicing}-in-tests`
so test code keeps its conventional idioms; production code is denied.
- Production unwraps factored out: `current_dir()`/`init_logger()` now
propagate via `?`; `writeln!` to a `String` rewritten as `push_str(&format!)`
so there's no `Result` to discard; bundled-template registration and other
genuine compile-time invariants use `.expect("...")` as documented assertions.
- Other small wins: `inefficient_to_string` fixed, `match_same_arms` collapsed,
`manual_assert` swapped, `cast_lossless`+truncation replaced with bound-checked
`u16::try_from` in adapter-axum CLI, `unreachable!()` in `#[action]` macro
replaced with a proper `syn::Error::compile_error`.
Lints kept allowed in the workspace are annotated with `(intentional)` where
they conflict with idiomatic Rust (`implicit_return`, `question_mark_used`,
`pattern_type_mismatch`, `default_numeric_fallback`, `arithmetic_side_effects`,
`as_conversions`, `string_slice`) or have no per-test config option
(`assertions_on_result_states`).
`cargo clippy --workspace --all-targets --all-features -- -D warnings`,
`cargo fmt`, and `cargo test --workspace --all-targets` all pass.
Drives the API-design lint group from 18 allows down to 8 (kept as intentional
with rationale comments in `Cargo.toml`).
Factored out:
- `return_self_not_must_use` (18): added `#[must_use]` to all `RouterBuilder`
builder methods. Catches "I forgot to call `.build()`" bugs.
- `impl_trait_in_params` (26): converted `fn f(x: impl Into<String>)` →
explicit generics on `EdgeError::*`, `ConfigStoreError::*`, `RouteInfo::new`,
`InMemorySecretStore::new`, `AxumConfigStore::{new,from_env,from_lookup}`.
Makes turbofish callable.
- `rc_buffer` (4): `Arc<Vec<RouteInfo>>` → `Arc<[RouteInfo]>` in `RouterInner`
and the builder. Saves an indirection.
- `unnecessary_wraps` (4): `build_fastly_request` and `convert_response` no
longer wrap an always-Ok value in `Result`. Cleaner call sites.
- `mutex_atomic` (1): `Arc<Mutex<bool>>` → `Arc<AtomicBool>` in the
`middleware_fn` test.
- `ref_patterns` (11): `if let Some(ref x) = ...` → `if let Some(x) = &...`
across env-override `Drop` impls, router builder, response builder, body
matchers.
- `wildcard_enum_match_arm` (7): `args.rs` tests now use `let-else` instead of
catch-all wildcard match arms; `EdgeError::source` now lists each non-Internal
variant explicitly; `cli/build.rs` switched to `if let Value::Table(_) = ...`;
the one site that genuinely matches an external enum (`fastly::config_store::
LookupError`) keeps a localized `#[allow(..., reason = "external enum")]`.
- `clone_on_ref_ptr` (1): `store.clone()` → `Arc::clone(&store)` in the axum
service test (with explicit `Arc<dyn KvStore>` annotation so `Arc::clone`
picks the right type).
- `renamed_function_params` (4): renamed `request: Request` → `req: Request`
in `Service::call` impls to match the trait signature.
- `same_name_method` (2): `EdgeError::source` deliberately shadows
`std::error::Error::source` (typed `&AnyError` vs trait-object `&dyn Error`).
Documented at the call site with a `#[allow(..., reason = "...")]`.
Kept allowed (with `(intentional: ...)` comments in `Cargo.toml`):
- `exhaustive_structs` (108) and `exhaustive_enums` (18): blanket
`#[non_exhaustive]` would break user pattern matching and field-syntax
construction. Apply per-type only when genuinely planned.
- `must_use_candidate` (117): most flagged sites are getters returning
`&str`/`&Path` — ignoring is impossible, the lint adds noise.
- `missing_trait_methods` (20): relying on default trait methods is fine.
- `needless_pass_by_value` (16): most flagged sites are deliberate ownership
transfers — error transformers, proc-macro signatures, builders.
- `field_scoped_visibility_modifiers`, `partial_pub_fields`,
`trivially_copy_pass_by_ref`: deliberate API design choices.
Final clippy + workspace tests pass.
Following pushback that the prior passes were papering over lints rather
than addressing them, this commit revisits each lint that was previously
allowed with hand-wavy reasoning and either (a) factors it out for real,
(b) applies it selectively where the fix matters, or (c) replaces the
rationale with a per-site audit finding.
Real fixes:
- `Body::as_bytes` and `Body::into_bytes` no longer panic on streaming
bodies — they return `Option`. This eliminates two production panic
sites the previous pass left as `panic = "allow"`. The internal
`into_bytes_bounded` site is correctly gated by `is_stream()`; all
other callers are tests that *intentionally* assert the body is
buffered, now with `.expect("buffered")`.
- `assertions_on_result_states` is no longer allowed. All 13 sites
converted from `assert!(r.is_ok())` / `assert!(r.is_err())` to
`r.expect("...")` / `r.expect_err("...")` — these print the value or
error on failure instead of just `assertion failed: false`.
- `#[non_exhaustive]` applied to all 4 error enums (`EdgeError`,
`KvError`, `SecretError`, `ConfigStoreError`) and the 3 manifest
enums (`HttpMethod`, `BodyMode`, `LogLevel`) — this is the idiomatic
Rust pattern for error/config enums (see `std::io::ErrorKind`,
`serde::de::Error`). Also applied to 19 deserialize-only manifest
structs (`Manifest*`, `ResolvedEnvironment*`-where-not-constructed-
externally).
- `needless_pass_by_value` real fix in `run_app_with_stores`:
`FastlyLogging` and `StoreRequirements` are now passed by reference
since the function only reads from them.
Lints kept allowed but with audited per-site rationales (replacing the
previous one-line hand-waves):
- `pattern_type_mismatch`: every flagged site uses Rust 2018
match-ergonomics. The "fix" reverts to manual `ref` patterns or
explicit `&Variant(...)` arms, both worse.
- `arithmetic_side_effects`: every site is bounded by domain invariants
(TTL+now, path component counts, byte offsets after `len()` checks).
- `as_conversions`: dominated by trait-object coercions (`Arc::new(x)
as BoxMiddleware`) which cannot be expressed as `From`/`Into` in
stable Rust.
- `string_slice`: every flagged site indexes ASCII-only data (env var
names, header names, `matchit` path components).
- `expect_used`: 62 production sites audited — bundled-template
registration, AsyncRead-contract slice access, lock-poisoning
unrecoverable, build-script panics. None benefit from `?`
propagation.
- `panic`: route-registration `unwrap_or_else(|err| panic!(...))` and
proc-macro expansion failures. Both build/setup-time programmer
errors, not runtime conditions.
- `cast_possible_truncation` / `cast_sign_loss`: narrowing/sign casts
always preceded by range checks.
- `exhaustive_structs` / `exhaustive_enums`: applied selectively above;
remaining sites are tuple-struct extractors users *destructure*,
unit structs, externally-constructed scaffold blueprints, request-
context types used in integration tests, and small enums (`Body`,
`AdapterAction`) where adding `#[non_exhaustive]` would force 12+
adapter sites to add never-firing wildcard arms.
Workspace clippy + tests still pass with `-D warnings`.
Removes 22 mechanical-fix allow entries from `Cargo.toml` after fixing the
underlying call sites:
Auto-fixed (`cargo clippy --fix` + manual cleanup):
- `uninlined_format_args` (180), `redundant_closure_for_method_calls` (25),
`map_unwrap_or` (29), `explicit_iter_loop` (14),
`unseparated_literal_suffix` (24, separated form chosen),
`implicit_clone` (2), `pathbuf_init_then_push` (3), `string_add` (3),
`unreadable_literal` (4), `manual_let_else` (2), `else_if_without_else`
(2 — the Fastly-vs-other-adapter logging branch refactored to a
pre-computed `Option<endpoint>`), `return_and_then` (2), `ip_constant`
(2), `manual_string_new` (1), `redundant_type_annotations` (1),
`needless_raw_strings` (1), `needless_raw_string_hashes` (1),
`elidable_lifetime_names` (2), `redundant_test_prefix` (1),
`if_then_some_else_none` (6), `deref_by_slicing` (5), `shadow_same` (4),
`match_wildcard_for_single_variants` (5), `pub_with_shorthand` (30),
`decimal_literal_representation` (1).
Real fixes (manual):
- `key_value_store.rs`: replaced bare scoping blocks `{ ...?; }` with
explicit `drop(table)` so neither `semicolon_inside_block` nor
`semicolon_outside_block` fires (the lint pair is mutually exclusive
and one always fires). Same treatment for `decompress.rs` and
`proxy.rs` brotli-test compressor scopes.
- `middleware.rs`: collapsed the `Mutex` lock+await pattern into a
single `self.log.lock().unwrap().push(...)` statement so the lock
guard drops immediately (was previously triggering
`await_holding_lock` after I removed the scoping block).
- `dev_server.rs`: `let service = service` (shadow_same) refactored
into a `let service = { mut service = ...; ...; service }` block
expression that yields the configured value.
- `response.rs`: dropped redundant `let stream = stream` shadow.
- `request.rs`: renamed `test_is_json_content_type` →
`json_content_type_detection` (the redundant `test_` prefix).
- `proxy.rs` test panics: `_ => panic!(...)` → `Body::Stream(_) =>
panic!(...)` so the match stays exhaustive when `Body` grows.
- `cli.rs`: `0xFFFF` instead of `65535` for the u16-MAX boundary.
- `dev_server.rs::stable_store_name_hash`: split FNV-1a magic numbers
with `_` separators.
The Style section in `Cargo.toml` is rewritten as a tight allow-list
(no narrative, no historical commit log inside the manifest). Each
remaining entry has a one-line rationale grouped by category:
- Idiomatic Rust (8 lints): `implicit_return`, `min_ident_chars`,
`single_call_fn`, `single_char_lifetime_names`, `pub_use`,
`str_to_string`, `question_mark_used` (was duplicated; consolidated
in Defensive section).
- Mutually-exclusive pairs we picked one side of: `separated_literal_suffix`,
`pub_with_shorthand`.
- Held-by-choice (5 lints): `format_push_string`, `shadow_reuse`,
`shadow_unrelated`, `similar_names`, `non_ascii_literal`,
`too_many_lines`, `arbitrary_source_item_ordering`,
`module_name_repetitions`.
Allow-list went from ~80 entries to 57 across all categories.
`cargo clippy --workspace --all-targets --all-features -- -D warnings`
and `cargo test --workspace --all-targets` both pass.
`#[action]` requires the user-written fn to be `async fn` because the generated outer fn `.await`s it. When a handler body has no awaits of its own, `clippy::unused_async` fires on the user's source — but the user has no choice; the macro forces `async`. Inject the allow into the inner fn's attribute list inside the macro expansion so handler authors don't have to know about the lint.
Imports/paths track:
- `non_std_lazy_statics` (6 sites): `once_cell::Lazy` → `std::sync::LazyLock`
in `crates/edgezero-adapter/src/{registry,scaffold}.rs`. Drops `once_cell`
from `crates/edgezero-adapter/Cargo.toml`. (Workspace dep stays — example
app still uses it.)
- `unused_trait_names` (37 sites): `use Foo;` → `use Foo as _;` for traits
imported only for their methods (`StreamExt`, `Write`, `Read`, `Hooks`,
`IntoHandler`, `Spanned`, etc.) across both library and proc-macro crates.
- `iter_over_hash_type` (1 site): the only flagged production iteration is
in `RouterInner::dispatch` (collecting allowed methods for a 405 response).
Refactored from a `for ... { allowed.insert(...) }` loop into
`.iter().filter().map().collect::<HashSet<_>>()`. The result is a `HashSet`
whose order doesn't matter (`EdgeError::method_not_allowed` sorts on render).
Attributes track:
- `allow_attributes` (3 sites): `#[allow(...)]` → `#[expect(..., reason)]` on
the genuine deliberate-shadowing/wildcard-match-arm sites in
`error.rs::EdgeError::source` and `config_store.rs::map_lookup_error`. The
CLI build script (`build.rs`) now emits `#[expect(unused_imports, reason)]`
on every generated `pub(crate) use` re-export.
- `allow_attributes_without_reason` (5 sites): every existing `#[allow(...)]`
now has a `, reason = "..."` and (where stable-`expect` applies) is migrated
to `#[expect(...)]`. Sites: `cli_support.rs` and `decompress.rs` top-of-file
`#![expect(dead_code, ...)]`; the four test-only `Deserialize` field structs
in `context.rs` and `params.rs`; the macro's `manifest_definitions` shim;
the two fastly `deprecated` re-exports.
Also kept allowed (real audits in `Cargo.toml` rationales):
- `absolute_paths` (200+ sites): one-shot `std::env::var()` / `std::fmt::Display`
uses; adding `use` statements wouldn't improve readability for single-use.
- `std_instead_of_alloc` / `std_instead_of_core`: not targeting `no_std`.
- `tests_outside_test_module`: lint matches plain `#[cfg(test)] mod tests`
only — doesn't recognize `#[cfg(all(test, feature = "..."))]` or
integration-test files in `tests/`.
- `print_stderr` / `print_stdout`: kept in CLI top-level error reporters and
status output (`[edgezero] creating project at ...`).
Allow-list now at 51 entries.
…c / doc_markdown / missing_fields_in_debug Adds public-API docs across every flagged site: - `missing_panics_doc` (28 sites): added `# Panics` sections describing each panic condition. Most are documented invariants (lock poisoning, AsyncRead-contract slice access, builder pre-validated headers); a few are caller-controlled (`enable_route_listing_at` asserts on path shape, `RouterBuilder::build` panics on duplicate route, `load_from_str` panics on invalid embedded TOML — the docs note safer alternatives). - `missing_errors_doc` (62 unique pub fns, 124 lints with re-exports): added `# Errors` sections describing the concrete error variants returned. Dispatched via batch script with per-fn descriptions covering every site (KV / secret / config-store / manifest / proxy / extractor / body / responder / middleware / adapter dispatch APIs). - `missing_fields_in_debug` (2 unique sites — 4 with re-exports): `ProxyRequest`/`ProxyResponse` `Debug` impls now use `finish_non_exhaustive()` to acknowledge the deliberately-skipped `body` and `extensions` fields. - `doc_markdown` (17 sites): backticked `EdgeZero`, `SystemTime`, `Axum`, `SecretStore`, etc. in doc comments. Lints kept allowed (with rationale comments in `Cargo.toml`): - `missing_docs_in_private_items` (275 sites): private docs aren't load-bearing for users — industry-standard "kept allowed". - `missing_inline_in_public_items`: `#[inline]` is a perf hint; rustc/LLVM make better decisions than blanket-marking every cross-crate public item. Allow-list: 51 → 47 entries.
…t_stdout allows
The CLI binary now initializes a `simple_logger` with no timestamps and no
level prefixes (so the user-facing UX is unchanged: `[edgezero] creating
project at ...` still prints exactly that), and all `println!` /
`eprintln!` sites are converted to `log::info!` / `log::error!` /
`log::warn!`.
Sites converted (24 total):
- `crates/edgezero-cli/src/main.rs`: top-level error reporters (`new`,
`build`, `deploy`, `serve`, `dev`) + status output for store-binding
warnings.
- `crates/edgezero-cli/src/generator.rs`: 9 status messages and 2 git
warnings now go through the logger.
- `crates/edgezero-cli/src/dev_server.rs`, `adapter.rs`: dev manifest /
command-failure reporting.
- `crates/edgezero-adapter-{axum,cloudflare,fastly,spin}/src/cli.rs`:
one build-artifact-path message each.
Allow-list: 47 → 45 entries (`print_stderr` + `print_stdout` removed).
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| fn log_store_bindings(adapter_name: &str, manifest: &ManifestLoader) { | ||
| if let Some(message) = store_bindings_message(adapter_name, manifest) { | ||
| println!("{message}"); | ||
| log::info!("{message}"); |
Real renames + restructuring (no inline allow attrs):
- `non_ascii_literal` (3 sites): replaced the Japanese KV-key test literal
with `\u{...}` escapes (same runtime bytes, ASCII source) instead of
`#[expect]`-ing the lint. Replaced `→` arrow in a CLI test message with
`->`.
- `similar_names` (2 sites): renamed `decoded` → `output` in
`crates/edgezero-adapter-spin/src/decompress.rs` to break the
`decoded`/`decoder` prefix-share that the lint flags.
- `too_many_lines` (1 site): split `collect_adapter_data` in
`crates/edgezero-cli/src/generator.rs` into three helpers
(`blueprint_data_entries`, `render_manifest_section`,
`append_readme_entries`).
- `shadow_unrelated` (~14 sites): renamed every flagged inner binding
to be specific to its purpose:
- `serve_with_stores`: `let router = Router::new()...` →
`axum_router`; `let server = server.with_graceful_shutdown(...)` →
`graceful_server`; `let shutdown = ...` → `shutdown_signal`.
- `store_name_slug`: `Some(ch)` → `Some(lower_ch)` (was shadowing
outer `ch`).
- dev_server tests: `let url = ...` reused per-step → `write_url`,
`read_url`, `check_url`, `delete_url`, `save_url`, `load_url`;
`let resp = ...` → `write_response`/`read_response`/`save_resp`/
`load_resp`/`exists_before`/`exists_after`.
- `axum::key_value_store::get_bytes`: inner write-txn `table` →
`write_table`, `entry` → `fresh_entry`.
- `list_keys_page` cursor match: inner `Some(cursor)` → `Some(scan_from)`.
- `data_persists_across_reopens` test: second `let store = ...` →
`reopened`.
- `axum::response::into_axum_response` error path: `body` →
`error_body`, `response` → `error_response`. Test: `stream` →
`body_stream`.
- `fastly::key_value_store::list_keys_page`: inner `cursor` →
`next_cursor`.
- `fastly::proxy` test: collapsed two pairs of `body`/`collected` reuse
into named bindings (`plain_body`, `gzip_body`).
- `spin::decompress` test: `let result = ...` reused per-encoding →
`none_encoding`, `identity_encoding`.
- `core::body::from_stream_maps_errors` test: `stream` →
`source`/`chunks`.
- `core::key_value_store` tests: `let val = ...` reused → `after_first`/
`after_second`/`int_val`/`str_val`/`single_dot_err`/`double_dot_err`.
- `axum::cli::read_axum_project`: `Some(value)` → `Some(port_value)`
(was shadowing outer `value` from `toml::from_str`).
Allow-list: 45 → 41 entries.
…quest path
Real fixes (not just docs) for every production-code .expect() that could
fire under upstream contract change or misconfigured input:
- `IntoResponse::into_response` now returns `Result<Response, EdgeError>`
workspace-wide (breaking change). Cascades through `Responder`,
`EdgeError::into_response`, `RouterService::oneshot`, the handler future
in `core/handler.rs`, and the route-listing builder.
- `ProxyResponse::into_response` and `core::response::response_with_body`
now return `Result<Response, EdgeError>` and propagate `http::Builder`
failures via `map_err(EdgeError::internal)?` instead of `.expect()`.
- `core::body::Body::into_bytes_bounded` rewritten as a `match self {
Once | Stream }` so the unreachable `is_stream()`-guarded `.expect()`
pair is gone — the compiler proves exhaustiveness.
- `core/compression.rs` decoder slice access now propagates as
`io::Error::other(...)` instead of `.expect("AsyncRead contract")`,
so a malicious or buggy upstream stream fails the request rather than
crashing the worker.
- `axum/response.rs::into_axum_response` error path no longer uses
`Response::builder().expect(...)`; constructs the 500 response
directly via `Response::new` + `status_mut` + `headers_mut().insert`,
every step infallible by `http`-crate contract.
- `axum/proxy.rs` replaced `Default` (which panicked on TLS init) with
fallible `AxumProxyClient::try_new() -> Result<_, reqwest::Error>`.
Production caller in `request.rs::into_core_request` propagates as a
`String` error (matches the fn's existing return type).
- `fastly/logger.rs::init_logger` now returns
`Result<(), InitLoggerError>` (a typed enum wrapping the underlying
build error and `log::SetLoggerError`) instead of `.expect("non-empty
Fastly logger endpoint")`. `lib.rs::init_logger` re-exports the wider
return type.
- `cli/generator.rs::render_templates` propagates the previously-
`.expect("adapter context dir has a file name")` invariant as
`io::Error::other` since the surrounding fn already returns
`io::Result<()>`.
`axum/service.rs::call` (the tower `Service` impl) bridges the new
`Result<Response, EdgeError>` from `RouterService::oneshot` into a
`Response<AxumBody>` by mapping the error to a hard-coded 500 with a
plain-text body — `Service::call` returns `Result<Response, Infallible>`
so we cannot propagate further up the stack here.
`adapter-fastly` adds `thiserror` as a direct dependency for
`InitLoggerError`. All 557 workspace tests still pass.
Replaces the previous \`std::io::Result<()>\` / \`io::Error::other(format!(...))\`
shape across the \`edgezero new\` code path with two domain-specific error
types:
- \`crate::scaffold::ScaffoldError\` (variants \`Io { path, source }\` and
\`Render { name, message }\`) wraps every Handlebars failure and every
filesystem op inside template rendering with the offending path/template
name attached.
- \`crate::generator::GeneratorError\` (variants \`OutputDirExists\`,
\`AdapterDirMissingFileName\`, \`Io { path, source }\`, and
\`Scaffold(#[from] ScaffoldError)\`) replaces the workspace-construction
io::Error stringification.
\`generate_new\`, \`ProjectLayout::new\`, \`collect_adapter_data\`, and
\`render_templates\` all return \`Result<_, GeneratorError>\`.
\`adapter-cli\` and \`scaffold\` now depend on \`thiserror\` directly. All
557 workspace tests still pass.
The `IntoResponse::into_response` change in 1506738 turned the trait into `-> Result<Response, EdgeError>` workspace-wide. The demo app (`examples/app-demo/`) is excluded from the main `Cargo.toml` workspace, so it didn't get rebuilt by the workspace clippy/test gate and silently broke. This propagates the same fix to the demo: - Every `block_on(handler(ctx)).expect("handler ok").into_response()` in `crates/app-demo-core/src/handlers.rs` test code now appends `.expect("response")` to unwrap the response result. - Every `into_body().into_bytes()` test path now appends `.expect("buffered")` since `Body::into_bytes()` returns `Option<Bytes>` (changed in the defensive-coding pass). `cd examples/app-demo && cargo test --workspace --all-targets` passes all 21 demo handler tests; `cargo clippy --workspace -- -D warnings` also clean.
Inherit pedantic+restriction lints in the demo workspace and each demo crate. Fix the lints that flagged real issues in the demo handlers (`as _` trait imports, inlined format args, fast-path `to_string`, renamed shadowed bindings, separated literal suffix). The demo's allow-list is intentionally narrower than the library's — only entries the demo actually trips. New allows can be added lazily as future failures surface.
Add a clippy.toml mirroring the parent (allow expect/unwrap/panic/ indexing-slicing in tests). Then refactor away the workspace allows that were genuine wins: - shadow_reuse: rename `chunk` and `cursor` shadows - absolute_paths: import std::env, std::time::Duration, std::process, and use already-imported Arc instead of std::sync::Arc - default_numeric_fallback: add type suffixes (1_u64, 0_i32..3_i32, 1_i64) - pattern_type_mismatch: implicitly fixed by str_to_owned changes - missing_trait_methods: implement KvStore::exists on the test MockKv - expect_used in production code: stream() now propagates the response builder error via EdgeError::internal The remaining allow-list keeps only entries the demo actually trips that match main's philosophical stance — std (not core/alloc) for binaries, idiomatic `?` over match, terse closure idents, and the single exhaustive_structs site that comes from the `app!` macro.
- str_to_string (21 sites): `.to_string()` → `.to_owned()` on `&str` - arithmetic_side_effects: counter `n + 1` → `n.wrapping_add(1)` - min_ident_chars + pattern_type_mismatch: rename closure destructures `|(k, v)|` → `|&(name, value)|`/`|&(key, value)|` - pub_with_shorthand + field_scoped_visibility_modifiers: drop `pub(crate)` shorthand on the demo's DTOs and handlers — the `mod handlers;` declaration is already private, so plain `pub` is crate-private at the boundary - print_stderr: axum main returns `anyhow::Result<()>` and lets the Termination impl render errors; fastly/cloudflare host stubs keep `eprintln!` behind a localized `#[expect]` with reason since they only run on the wrong target Workspace allow-list now keeps only the entries that match main's philosophical stance (idiomatic `?`, `pub` shorthand handled per-call site, etc.) plus the single `exhaustive_structs` site from the `app!` macro.
Drop the `arbitrary_source_item_ordering` allow in favor of the canonical clippy-restriction layout: - Top of `handlers.rs`: consts (alphabetical), then structs (alphabetical: ConfigParams, EchoBody, EchoParams, NoteIdPath, ProxyPath), then handler fns - Test mod: uses, then structs (alphabetical), then impls grouped with their self-types, then helper + test fns interleaved in alphabetical order - `impl KvStore for MockKv` methods alphabetical (delete, exists, get_bytes, list_keys_page, put_bytes, put_bytes_with_ttl) - Hoisted the late `use edgezero_core::secret_store::...` up to the test mod's use block No behavior changes — pure reordering. Demo workspace allow-list drops to 8 entries.
The `edgezero new` generator now scaffolds the same lint policy EdgeZero itself uses: - Root `Cargo.toml` carries `[workspace.lints.clippy]` (pedantic warn + restriction deny) with the same demo-tested allow-list - Root `clippy.toml` exempts tests from `unwrap`/`expect`/`panic`/ indexing-slicing restriction lints - Each generated crate's Cargo.toml inherits via `[lints] workspace = true` Generated projects are clippy-clean against the strict gate out of the box.
Both adapters were calling `from_core_response` directly on the router's return value, but `oneshot` now yields `Result<Response, EdgeError>` since the response builder errors propagate through the router. Extract the response with `?` first so the wasm32 builds (`--target wasm32-unknown-unknown` for cloudflare, `--target wasm32-wasip1` for spin) compile again.
… per-site Real fixes (allows now justified by audit, not laziness): - build.rs returns `Result<(), Box<dyn Error>>` instead of expect-panicking - adapter registry / blueprint registry recover from poisoned RwLocks via `unwrap_or_else(PoisonError::into_inner)` rather than expect-panicking - ManifestLoader gains `try_load_from_str` returning `io::Result`; adapter `run_app` paths propagate via `?`. The non-fallible `load_from_str` keeps its panic-on-bad-input contract for compile-time-embedded manifests, with a documented per-fn `#[expect(clippy::panic, reason = ...)]` - `expand_app` macro emits `compile_error!()` instead of panicking on bad `edgezero.toml` (rustc surfaces a clean build error) - `parse_handler_path` keeps a panic with a clear reason — proc-macro expansion errors *are* build failures - `partial_pub_fields` on `Manifest`: privatized `root` and `logging_resolved`, kept the deserialized fields `pub` for the public API. Localized `#[expect]` documents the deliberate split - `must_use_candidate` fixed on cli_support helpers via `#[must_use]` - `missing_inline` fixed on adapter/scaffold registry functions - `pub_use`, `format_push_string`, `arithmetic_side_effects`, `default_numeric_fallback`, `pattern_type_mismatch`, `min_ident_chars`, `str_to_string`, `absolute_paths`, `module_name_repetitions`, `shadow_reuse`: all kept as workspace allows but with concise rationales replacing the prior verbose audit notes Each remaining workspace allow now has a one-line reason. The list is shorter than before but explicitly accepts the lints whose "fix" would universally make the code worse (match-ergonomics destructures, std-only binary entrypoints, idiomatic `?`/return).
…space-wide 54 sites across 23 files. Fixed places where my bulk replace had wrongly converted Display::to_string() calls (anyhow::Error, io::Error, i32 etc.) back to .to_string(). The lint allow is dropped from the workspace.
23 sites across extractor.rs, key_value_store.rs, middleware.rs, proxy.rs, adapter-axum dev_server/key_value_store, adapter-spin decompress. Validator length(min=N) gets _u64; range(min=N, max=N) gets matching type suffix; loop-bound and assertion literals get explicit i32.
Core crate: replaced 60+ `std::collections::HashMap`, `std::sync::Arc`, `std::ops::Deref/DerefMut`, `crate::error::EdgeError`, `futures::executor::block_on`, `std::task::*`, `std::string::String::*` absolute paths with explicit `use` statements. Axum proxy.rs: imported the various `axum::http::*` and `axum::routing::*` types used in test functions. The lint stays allowed at the workspace level for adapter test modules where one-shot uses of framework types like `axum::http::HeaderMap` and `fastly::kv_store::KVStore` are clearer inline.
Real fixes (workspace allows dropped, code refactored): - AdapterAction marked #[non_exhaustive] with wildcard arms in adapter cli match sites — drops a workspace exhaustive_enums concession - Adapter crate exposes `pub mod registry` instead of pub-using items at the crate root — drops the workspace pub_use concession - expand_action_impl made private (no longer pub(crate)) — drops the workspace pub_with_shorthand concession on this site - ManifestLoader, Manifest, ManifestApp/HttpTrigger/Environment/Binding/ ResolvedEnvironment*, ManifestAdapterBuild/Commands, ManifestConfigStoreConfig, ManifestLoggingConfig, ResolvedLoggingConfig, ManifestKvConfig, ManifestSecretsConfig, HttpMethod, LogLevel — all reordered to match canonical clippy item ordering (consts first, then structs, impls, fns; alphabetical within each group) - Manifest impl methods sorted alphabetically; Manifest fields sorted - match-ergonomics destructures rewritten as let-else for clarity - HttpMethod gained Copy; LogLevel/HttpMethod take `self` (drops trivially_copy_pass_by_ref) - partial_pub_fields fixed via consistent pub on Stores in fastly request - needless_pass_by_value: run_app_with_config / run_app_with_logging take `&FastlyLogging`; map_edge_error / map_lookup_error take by ref; build_fastly_request takes `&HeaderMap`; generate_new takes `&NewArgs` - expect_used localized on register_templates with rationale - ManifestLoader::load_from_str / parse_handler_path keep panic-on-bad- build-input contract documented per-fn - Router: route-listing duplicate-path panic + add_route panic both documented per-fn (build-time programmer error) - spin contract test uses #[allow] for expect/tests-outside per file - separate manifest_definitions.rs in macros crate (drops mod-after-use) Workspace allows that survived (most match audited rationales): implicit_return, question_mark_used, single_call_fn, separated_literal_suffix, pub_with_shorthand (rustfmt-enforced), pub_use, min_ident_chars, single_char_lifetime_names, shadow_reuse, module_name_repetitions, format_push_string, pattern_type_mismatch, arithmetic_side_effects, float_arithmetic, as_conversions, exhaustive_structs, exhaustive_enums, missing_trait_methods, absolute_paths, std_instead_of_alloc/core, missing_inline_in_public_items, tests_outside_test_module, arbitrary_source_item_ordering (core-crate files outside manifest.rs). Tests pass, strict clippy clean across workspace + demo.
Override KvStore::exists in 4 production impls (axum/fastly/cloudflare + NoopKvStore) and the in-test MockStore. Override configure/name/ config_store/build_app in the two Hooks test impls. Update the #[app] macro to emit configure, build_app, and a None-returning config_store when [stores.config] is absent so generated user apps still pass clippy. Add explicit clone_from to RouteEntry's Clone impl.
Delete config_store, key_value_store, and secret_store crate-root
re-exports — items remain reachable via the `pub mod` paths. Update the
two short-path callers (axum service.rs / secret_store.rs) to use full
module paths. Keep `pub use edgezero_macros::{action, app}` and the
`http` facade re-exports — these are the only surviving sites and the
lint is module-scoped so it cannot be silenced per-item. Workspace
allow rationale updated to point to those two patterns.
The previous comment framed `push_str(&format!(...))` as a stylistic preference. It is actually the only call-site form that satisfies the full restriction-deny gate: `write!(s, ...)` returns a `Result` which trips `let_underscore_must_use` under `let _ =`, `unwrap_used` under `.unwrap()`, and `expect_used` under `.expect()`.
Switch generator.rs from `push_str(&format!(...))` to `writeln!(...)?` which writes directly into the buffer (no temp String allocation) and propagates `std::fmt::Error` rather than silencing it. Add `GeneratorError::Format(#[from] std::fmt::Error)` and bubble the result through `render_manifest_section` and `append_readme_entries`. Drop the workspace allow.
Rename 'a → 'mw on Next, 'a → 'route on RouteMatch, 'a → 'manifest on manifest_command, and 'a → 'blueprint on AdapterContext. Drop the workspace allow.
Eliminate let-rebinding shadows across core, fastly, axum, and cli
crates. The recurring patterns:
- `while let Some(chunk) = stream.next().await { let chunk = chunk?; }`
→ rename outer to `result`, keep inner `chunk`
- `if let Some(cursor) = cursor.filter(...)` → rename outer/inner to
distinct names
- `let path = path.into()` (Into-paramter idiom) → rename to
destination-specific name
- closure params shadowing outer captures → rename closure param
All renames preserve semantics; tests + workspace clippy + wasm
target checks all pass.
Split `#[cfg(all(test, feature = "..."))]` on test modules into two separate cfg attributes (`#[cfg(test)] #[cfg(feature = "...")]`) which the lint recognizes correctly. Affects edgezero-adapter-fastly lib.rs and edgezero-cli main.rs.
Convert the http builder re-exports to `pub type` aliases (real fix — no `pub use` required) and wrap the `header` re-export in a child module with a scoped `#![expect]`. Add file-level `#![expect(clippy::pub_use)]` to each adapter lib.rs (axum, fastly, spin, cloudflare) and to edgezero-core/lib.rs for the proc-macro re-export. Cloudflare uses `cfg_attr(target_arch = "wasm32", expect)` because its re-exports are wasm-gated and would leave the expect unfulfilled on the host build.
For each adapter (axum/fastly/spin/cloudflare): make the previously
private internal modules `pub mod` and drop every `pub use` re-export.
Callers now reach types via the full path, which is what the lint
suggests as the proper fix. Update internal cross-module refs and
external callers (edgezero-cli, demo crates, axum/spin scaffold
templates, fastly/spin/cloudflare contract tests).
Remaining `pub_use` expects:
- `edgezero-core/src/lib.rs` — single-line proc-macro re-export
(`pub use edgezero_macros::{action, app}`); the canonical proc-macro
distribution pattern requires this and the lint is module-scoped, so
a tightly-scoped file-level expect is the only available form
- `edgezero-core/src/http.rs::header` — wrapped in a child module with
the expect scoped to that one line; required by the CLAUDE.md HTTP
facade rule
The two `#[allow(deprecated)]` annotations on `AppExt::dispatch` implementations (cloudflare/fastly) were unnecessary — implementing a deprecated trait method does not trigger the `deprecated` lint, only calling the deprecated declaration does. Drop them. Also fix the fastly contract integration test (wasm32-only) which was still importing names from the previous crate-root re-exports — switch to the new `request::`/`response::`/`context::` module paths.
Per-package `.cargo/config.toml` is only honored when cwd is inside the package directory, so `cargo test -p edgezero-adapter-fastly --target wasm32-wasip1 --test contract` from the workspace root fails to resolve the Viceroy runner. Mirror the runner at the workspace level. Cargo invokes test runners with cwd set to the package manifest directory, so `../../examples/...` resolves correctly for any adapter package targeting wasm32-wasip1.
Three previously-duplicated wasm test jobs (cloudflare, fastly, and a new spin entry) collapse into one `adapter-wasm-tests` matrix that varies on adapter, target, and runner. Spin uses Wasmtime; fastly keeps Viceroy; cloudflare keeps wasm-bindgen-test-runner. Per-adapter toolchain installs are gated with `if: matrix.adapter == ...` so each job only pulls what it needs. Also fix a pre-existing compile error in `crates/edgezero-adapter-spin/ tests/contract.rs:171` (`name == "x-edgezero-res"` needed a deref) — silently broken because there was no CI job exercising it.
Remove the `extra_check` matrix flag and gating `if:` — every adapter in the wasm matrix now runs the same test+check pair, and the duplicate "Check Spin wasm32 compilation" step in the top-level `test` job (now redundant with the matrix's spin cell) goes away. axum is the host-target adapter — its 102 tests already run as part of `cargo test --workspace --all-targets` in the `test` job. It has no `--test contract` integration target, so adding it to the wasm matrix would either need a special-case command or duplicate the workspace-test work. Keeping it in the `test` job is the simpler call.
The cargo cache restores `~/.cargo/bin/{viceroy,wasm-bindgen-test-runner}`
from prior runs; a bare `cargo install` then fails with `binary already
exists in destination`. Match the same `command -v` guard the spin step
already uses, and for wasm-bindgen also re-check the version (the cache
key is per-Cargo.lock so a wasm-bindgen bump in lockfile needs a refresh).
Replace the conditional `command -v` guards with unconditional `cargo install --force` for both viceroy and wasm-bindgen-cli. The cargo cache restores prior binaries into `~/.cargo/bin/` and `cargo install` rejects by default; the previous version-grep guard was fragile and the simpler `--force` is always safe with `--locked`.
Five `pub(crate)` items in fastly/spin are file-local, not actually cross-module: drop them to private (Stores, dispatch_with_handles, resolve_kv_handle, resolve_secret_handle, MAX_DECOMPRESSED_SIZE). Also drop `validate_name` to private in edgezero-core/secret_store (only used inside the same file). The remaining five `pub(crate)` items (dispatch_raw, dispatch_with_ store_names, parse_uri, parse_client_addr, decompress_body) are genuine cross-file crate-internal API and must stay at crate visibility. `pub_with_shorthand` wants `pub(in crate)` but rustfmt unconditionally rewrites that back to `pub(crate)` — there is no spelling that satisfies both the lint and rustfmt, so the workspace allow stays with a tighter rationale.
For every previously inline `std::*`, `fastly::*`, `crate::*` etc.
absolute path, add a `use` import at the appropriate scope (file top
or `mod tests {}`) and replace the inline path with the short name.
Affects ~30 files across edgezero-core, all four adapters, and the
CLI. No behaviour change; lint count down by one workspace allow.
Reorder source items across edgezero-core and the adapter/cli crates
to satisfy the canonical clippy item ordering (ExternCrate → Use → Mod
→ Static → Const → TyAlias → Enum → Struct → Trait → Impl → Fn) with
alphabetical ordering inside each kind. Applies recursively to:
- top-level items in 12 core files (app, body, config_store, context,
error, extractor, http, key_value_store, middleware, params, proxy,
router, secret_store) and the adapter/cli files that needed it
- struct fields and constructor argument order
- enum variants
- methods inside `impl` blocks
- items inside `mod tests {}` blocks (including macro_rules! placement
before `use super::*` where required)
Pure reordering — no behavioural changes, no `#[expect]` annotations.
All clippy lints pass, 557+ tests green, all three wasm targets compile.
All cast sites turned out to be either redundant trait-object coercions that Rust performs automatically, or numeric conversions that can use a sibling const at the right type: - spin/decompress.rs (2 sites): added MAX_DECOMPRESSED_SIZE_U64 sibling const so the `Read::take` callsites do not need a usize→u64 cast - fastly/logger.rs: replaced `Box::new(logger) as Box<dyn log::Log>` with an inline `let boxed: Box<dyn log::Log> = Box::new(logger);` pattern (Box<T>→Box<dyn Trait> coerces automatically through a typed binding) - core/middleware.rs (4 sites in tests) and core/router.rs (1 site): same pattern — drop redundant `as BoxMiddleware` casts where the surrounding `Vec<BoxMiddleware>` annotation already drives coercion - cli/main.rs: drop `&[] as &[String]` — the function signature drives inference Workspace allow is gone; clippy + 557+ tests + all wasm targets pass.
Six arithmetic sites — all on usize/SystemTime where overflow is practically impossible but the lint cannot prove it. Real fix: use the explicit no-panic variant at each site. - axum/key_value_store.rs: `limit + 1` → `limit.saturating_add(1)`, `MAX_SCAN_BATCHES * LIST_SCAN_BATCH_SIZE` → `saturating_mul`, `batch_count += 1` → `saturating_add`, and `SystemTime::now() + ttl` → `SystemTime::now().checked_add(ttl).ok_or_else(KvError::Internal)?` so an absurd ttl propagates as an error rather than panicking - core/key_value_store.rs (test MockStore): same `checked_add(ttl)?` pattern so the test backend matches the production contract - cli/generator.rs: `count + 1` → `saturating_add(1)` Workspace allow gone; all clippy lints, tests, and wasm targets pass.
viceroy 0.17.0 raises its MSRV to rustc 1.95; the workspace ships rustc 1.91 (.tool-versions), so the unpinned `cargo install viceroy` started failing with "rustc 1.91.1 is not supported by viceroy-lib@0.17.0 requires rustc 1.95". 0.16.x is compatible and is what local dev uses.
Matches the CI pin (`^0.16`) so local dev resolves the same major.minor that CI installs. 0.17 raises MSRV to rustc 1.95 which is past the workspace's rust 1.91.1.
Single source of truth: replace the hardcoded `^0.16` in the workflow with a step that greps the version out of `.tool-versions`. Matches the existing pattern used for rust, and means a future viceroy bump is a single-line edit in `.tool-versions` rather than two places.
Single-character bindings, closure params, and helper variable names were renamed to descriptive equivalents across 31 files. Common patterns: - closure error params: `|e|` → `|err|` - closure key/value pairs: `|(k, v)|` → `|(key, value)|` - short locals in tests: `let s = ...` → `let store/service/cs = ...` - `Some(p)` for `&UserProfile` → `Some(found)` (avoids shadow with outer `profile` var, which would trip `shadow_reuse`) - `let h = handle.clone()` in concurrent tests → `let kv_handle = ...` to avoid shadowing the outer `handle` - `m` (manifest data) in dev_server.rs / main.rs → `manifest_data` - HTTP closure params `|c| c.get(...)` → `|http_client| http_client.get` No behaviour changes — pure renames. Workspace allow gone; clippy + 557+ tests + all wasm targets pass.
Investigated removing the allow: 40 sites in edgezero-core alone (every public error type and handle: EdgeError, KvError, SecretError, ConfigStoreError, ConfigStoreHandle, plus the entire Manifest* family). The renames would force consumers in 4 adapter crates + cli + demo to either write `kv::Error`/`secret::Error`/etc. at every callsite or set up `use ... as KvError` aliases — a net loss in readability for a deliberately-prefixed cross-crate API. Replaced the terse comment with a longer one documenting the audit and why the allow is load-bearing rather than a leftover.
Attempted the rename and surfaced three blockers:
1. `proxy::Request`/`proxy::Response` would collide with
`http::Request`/`http::Response` already imported at every
consumer; the only non-colliding alternatives (`OutboundRequest`,
`Outbound`) are strictly more verbose than `ProxyRequest`.
2. `manifest.rs` has 17 `Manifest*` types used directly by adapters,
cli, demos, scaffold templates, and the `#[app]` macro output.
Stripping the prefix would force every site to write
`use edgezero_core::manifest::Spec as Manifest` etc.
3. The macro emits code that references these names by their current
spelling; renaming requires regenerating every app and updating
CLAUDE.md examples.
The lint's intent (the std-style `module::Type` idiom) is sound but
fights this crate's flat re-export surface, and several names cannot
be deprefixed without losing meaning. Allow stays with the audit
documented inline.
Two sites in middleware.rs computed `start.elapsed().as_secs_f64() *
1000.0` to get milliseconds with sub-ms precision for the
request-logging line. Sub-ms precision in a log line is unnecessary —
switch to `Duration::as_millis()` (returns `u128`) and drop the
`{:.2}` format spec. No precision loss that any reader would notice;
removes the only float-arithmetic site in the workspace.
Audit: only `Body { Once, Stream }` triggers the lint workspace-wide.
Marking it `#[non_exhaustive]` would force `_ => unreachable!()` at
each of the 37 external match sites in the four adapter crates, and
a third Body variant would silently `panic!` at runtime instead of
producing a compile error at every consumer. Body is intentionally
closed; the lint is genuinely incompatible with the design.
Add `#[inline]` to every public function and trait method across the
workspace. Touches 44 files: edgezero-core (~242 sites) and the four
adapter crates. Placement is right above the `pub fn` after any doc
comments and `#[must_use]`. No `#[inline(always)]` — leaving the call
to rustc/LLVM, which is the actual inlining decision-maker.
Note: the original workspace-allow rationale ("rustc/LLVM make better
choices than us") is still half true — the lint just wants the *hint*
present, even though rustc inlines monomorphised generics aggressively
without it. Adding the hint is cheap and the lint is satisfied.
Defends against the CodeQL `rust/cleartext-logging` rule, which heuristically flagged `log_store_bindings` because it pipes `manifest_data.secret_store_name(adapter)` into `log::info!`. The method returns the binding identifier from `edgezero.toml` (e.g. `"MY_SECRETS"`), not the secret value — but the function name pattern triggers the analyzer's "credential getter" heuristic. Renaming to `secret_store_binding` makes the intent unambiguous and the alert no longer fires. Also reorders the impl method block so `secret_store_binding` lands before `secret_store_enabled` per `arbitrary_source_item_ordering`.
| let client = reqwest::Client::new(); | ||
| let url = format!("{}/secret", server.base_url); | ||
| let response = send_with_retry(&client, |c| c.get(url.as_str())).await; | ||
| let response = send_with_retry(&client, |http_client| http_client.get(url.as_str())).await; |
| let client = reqwest::Client::new(); | ||
| let url = format!("{}/secret", server.base_url); | ||
| let response = send_with_retry(&client, |c| c.get(url.as_str())).await; | ||
| let response = send_with_retry(&client, |http_client| http_client.get(url.as_str())).await; |
| let client = reqwest::Client::new(); | ||
| let url = format!("{}/secret", server.base_url); | ||
| let response = send_with_retry(&client, |c| c.get(url.as_str())).await; | ||
| let response = send_with_retry(&client, |http_client| http_client.get(url.as_str())).await; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pedantic(warn) +restriction(deny) workspace-wide and wires[lints] workspace = trueinto all 8 crates so the policy actually applies.Cargo.toml(Documentation, Style, Defensive, API design, Imports/paths, Output, Tests, Attributes) with per-lint counts and rationales — each entry a TODO unless flagged(intentional).Changes
Cargo.toml[workspace.lints.clippy]withpedantic/restrictionenabled and ~150 lines of categorized allow-entries with counts + rationale; refresh-counts snippet in headerclippy.toml(new)allow-{unwrap,expect,panic,indexing-slicing}-in-tests = trueso test code keeps conventional idioms while production code is deniedCargo.toml[lints] workspace = truecrates/edgezero-cli/src/generator.rscurrent_dir()propagated via?; 9writeln!(string, ...).unwrap()rewritten aspush_str(&format!(...))(noResultto discard);path.file_name().unwrap()→.expect("invariant")for path we constructedcrates/edgezero-cli/src/scaffold.rsregister_template_string(...).unwrap()→.expect("compiled-in template is valid")(compile-time invariant)crates/edgezero-adapter-fastly/src/lib.rs,logger.rsinit_logger().expect()→?propagation (real runtime failure:SetLoggerErroron double-install).log_fastly::Logger::builder().build().expect("non-empty endpoint")documented as caller-guaranteed invariant.crates/edgezero-adapter-axum/src/cli.rscast_lossless+cast_possible_truncationrewritten as bound-checkedu16::try_from(value).filter(|p| *p > 0); preserves zero-port rejectioncrates/edgezero-adapter-axum/src/dev_server.rsmanual_assertswap,let_underscore_must_usefixes (let _ctrl_c = ...,let _result = ...),unused_result_ok(init().ok()→let _logger_init = init())crates/edgezero-core/src/error.rsmatch_same_armscollapsed:BadRequest | Validation | ServiceUnavailablearms with shared bodycrates/edgezero-core/src/compression.rs&buffer[..read]→buffer.get(..read).expect("AsyncRead::read returns at most buffer.len()")crates/edgezero-core/src/manifest.rsm.adapters.get("fastly").unwrap()→&m.adapters["fastly"](3 test sites)crates/edgezero-core/src/router.rslet _ = builder()→let _builder = builder()(typed binding) in#[should_panic]tests;map_err(|_| ...)→|_e|crates/edgezero-macros/src/action.rsFnArg::Receiver(_) => unreachable!()→ returns propersyn::Error::compile_errorforself-receiverscrates/edgezero-core/src/{config_store,context,extractor,secret_store}.rs,crates/edgezero-adapter-axum/src/config_store.rsinefficient_to_string: 6 sites of(&&str).to_string()→(*x).to_string()crates/edgezero-core/src/key_value_store.rs,crates/edgezero-adapter-fastly/src/key_value_store.rsmap_err_ignore: bind discarded error to|_e|crates/edgezero-cli/src/scaffold.rslet_underscore_untyped:for i in 0..depth { let _ = i; ... }→for _ in 0..depth { ... }Note on
.expect()vs.unwrap():.expect()has the same panic semantics as.unwrap()— it documents why a call is considered infallible, it does not magically make the code safer. The genuine safety wins in this PR are the cases that propagate via?(current_dir,init_logger) or eliminate theResultentirely (writeln!→push_str(&format!)). The remaining.expect("...")sites are compile-time / construction-time invariants documented at the call site.Closes
Closes #256
Test plan
cargo test --workspace --all-targetscargo clippy --workspace --all-targets --all-features -- -D warningscargo check --workspace --all-targets --features "fastly cloudflare"wasm32-wasip1(Fastly) /wasm32-unknown-unknown(Cloudflare)edgezero-cli devcargo fmt --all -- --checkChecklist
{id}syntax (not:id)edgezero_core(nothttpcrate)