Break down findings from review #002 into dependency-ordered fix tasks: Critical/High: - fix/acme-contact-and-challenge (C1+C2): Add acme_contact field, wire to ACME, remove unused challenge_config, add validation rule 19 - fix/remove-health-and-hardcode-https (W5+W14+ADR-022): Remove /health from main listener, hardcode X-Forwarded-Proto to https - fix/config-reload-static-drift (C4): Use ArcSwap<StaticConfig> so reload diffs against last config, not startup config - fix/access-logging (W13): Wire up log_request! macro for every proxied request with client_ip, host, method, path, status, upstream, duration_ms Medium: - fix/graceful-shutdown (W1+W7): Join HTTPS tasks with timeout instead of abort, add shutdown signal to admin socket and eviction task - fix/connect-timeout (W4): Wire upstream_connect_timeout_secs to enforce separate connect timeout Low/Independent: - fix/token-bucket-nanosecond (W6): Use as_nanos() instead of as_millis() - fix/normalize-host-ipv6 (S3): Handle IPv6 bracket notation in normalize_host - fix/http-port-validation (S1): Validate http_port in range 0 or 1-65535 - fix/integration-test-toml (S10): Fix double-nested listeners.listeners.sites - fix/logging-test-global-subscriber (W9): Use try_init() to avoid test conflicts - fix/fragile-error-detection (W3): Add typed error matching or documented string match - fix/add-code-comments (C3,W8,W10,W11,S9): Document correct-but-non-obvious behaviors - fix/request-timeout-scope (S8): Document full-request timeout scope - fix/clean-dead-code (S4+S2): Remove dead_code annotations, add #[non_exhaustive] Review gate: - review/post-fix-review: Verify all fixes against architecture spec
2.6 KiB
2.6 KiB
id, name, status, depends_on, scope, risk, impact, level, review_findings
| id | name | status | depends_on | scope | risk | impact | level | review_findings | |||
|---|---|---|---|---|---|---|---|---|---|---|---|
| fix/clean-dead-code | Remove dead_code annotations and add | pending |
|
narrow | low | component | implementation |
|
Description
Many public items are annotated with #[allow(dead_code)] because they're defined but not yet used by the binary crate. After the ACME contact and challenge fix (which wires up previously unused ACME code), most of these annotations should be removable.
Additionally, public enums (TlsMode, ProxyError, AdminSocketError, ValidationError) should have #[non_exhaustive] to allow future expansion without breaking changes.
Changes Required
Remove #[allow(dead_code)] annotations:
- After
fix/acme-contact-and-challengeis complete,build_acme_challenge_configandTlsMode::Acme.challenge_configwill be removed entirely (not just un-annotated) - Run
cargo checkand remove#[allow(dead_code)]annotations from items that are now used - Items that are still genuinely unused after the ACME fix should keep the annotation but with a TODO comment explaining why
Add #[non_exhaustive] to public enums:
src/tls/acceptor.rs:49—TlsModeenum (may gain modes like"letsencrypt"or"auto")src/proxy/error.rs:5—ProxyErrorenum (may gainUpstreamTlserror handling)src/admin/socket.rs:15—AdminSocketErrorenum (may gain new error variants)src/config/validation.rs:10—ValidationErrorenum (new validation rules may be added)
Files to Check
src/tls/acceptor.rs— lines 14, 33, 48, 58src/tls/acme.rs— lines 9, 11, 15, 23, 55src/config/static_config.rs— lines 4, 31, 44, 49, 54, 56, 70, 76, 86, 91
Acceptance Criteria
#[allow(dead_code)]annotations removed from items that are now used- Remaining
#[allow(dead_code)]annotations have TODO comments explaining why #[non_exhaustive]added toTlsMode,ProxyError,AdminSocketError,ValidationErrorcargo checkpasses (no unused code warnings for annotated items)cargo clippypasses with no warnings
References
- docs/reviews/002-implementation-review.md — S4, S2 findings
- src/tls/acceptor.rs — TlsMode enum
- src/proxy/error.rs — ProxyError enum
- src/admin/socket.rs — AdminSocketError enum
- src/config/validation.rs — ValidationError enum
Notes
This task depends on fix/acme-contact-and-challenge because the ACME fix removes some dead code (challenge_config) and wires up other code that was previously unused.
Summary
To be filled on completion