Fix pre-existing clippy errors so cargo clippy --all-targets passes (#21)#25
Open
kriszyp wants to merge 2 commits into
Open
Fix pre-existing clippy errors so cargo clippy --all-targets passes (#21)#25kriszyp wants to merge 2 commits into
cargo clippy --all-targets passes (#21)#25kriszyp wants to merge 2 commits into
Conversation
…#21) 12 clippy errors under `#![deny(clippy::all)]` (clippy 1.94) failed the documented local lint on files untouched by feature branches: - sni.rs: 3× needless_borrow (extract_sni/compute_ja3 take slices) - proxy_conn.rs, http_listener.rs: 3× io_other_error → std::io::Error::other - http_listener.rs, listener.rs: 2× useless_conversion (TcpListener→TcpListener) - http_listener.rs, http_proxy.rs: 2× empty_line_after_doc_comment — these were module-level docs mis-written as `///`; converted to `//!` - proxy.rs: type_complexity → ParsedProtectionConfig type alias - proxy.rs: iter().cloned().collect() → to_vec() Also silences one unused-variable warning (excess → _excess). Mechanical; no behavior change. Formatting left as-is (repo has no rustfmt.toml and uses tabs; a blanket `cargo fmt` would churn the whole tree). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request performs several code cleanups and refactorings across multiple Rust files. Key changes include converting module-level doc comments to inner doc comments, simplifying TcpListener::from_std calls by removing redundant .into() conversions, utilizing the cleaner std::io::Error::other constructor, introducing a type alias ParsedProtectionConfig for improved readability, simplifying a vector cloning operation, and removing unnecessary references when passing struct fields. There are no review comments, and I have no additional feedback to provide.
Follow-ups from #21 review: - rustfmt.toml (hard_tabs = true): prevents cargo fmt from reformatting the tab-indented tree to spaces. Doesn't make `cargo fmt --check` pass clean — the codebase also hand-compacts some expressions (single-line if/else, condensed let-else) that rustfmt would still rewrite — so no fmt gate is added here; a full reformat is a separate decision. - rust-toolchain.toml pinning 1.94.1 + clippy/rustfmt components, so local and CI use the same toolchain. - New `lint` CI job running `cargo clippy --all-targets`. CI previously ran no clippy step at all (npm run build never invokes it), which is how #21's regressions went unnoticed; this makes clippy::all a required check going forward. - server.spec.ts: the status.json version assertion hardcoded '0.4.0' and went stale the moment package.json was bumped to 0.5.0. Now reads the version from package.json instead of a literal, so it can't drift again on the next bump. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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.
Closes #21.
cargo clippy --all-targetsfailed onmainwith 12 errors under# — the lint documented inCLAUDE.mdstarts red for any contributor running it locally, and a future CI toolchain bump would break the build. All are in files untouched by the current feature branches.Fixes (mechanical, no behavior change)
sni.rsneedless_borrow×3extract_sni/compute_ja3take slices — dropped the&proxy_conn.rs×2,http_listener.rs×1io_other_errorstd::io::Error::new(ErrorKind::Other, e)→std::io::Error::other(e)http_listener.rs,listener.rsuseless_conversionTcpListener::from_std(socket.into())→(socket)http_listener.rs,http_proxy.rsempty_line_after_doc_comment///; converted to//!(correct fix, not just deleting the blank line)proxy.rstype_complexityParsedProtectionConfigtype aliasproxy.rsiter().cloned().collect()to_vec()http_listener.rsexcess→_excessResult:
cargo clippy --all-targetsis clean. The remaining 17 warnings are all pre-existingdead_code-style ones on test/unused helpers — those don't gate underclippy::alland are left for a separate cleanup.Notes
cargo fmt. The repo has norustfmt.tomland uses tabs; rustfmt's default is spaces, so a blanket format churns all ~16 files. Edits are kept tab-clean. Suggested follow-up: add arustfmt.toml(hard_tabs = true) so localcargo fmtagrees with the tree, and pin/document the clippy version CI uses.server.spec.ts:142assertsstatus.version === '0.4.0'but the HEAD commit "Sync platform package versions to 0.5.0" bumpedpackage.jsonto0.5.0, sonpm testis red onmainindependent of this PR (this PR touches no TS/version files). Filing/fixing separately.🤖 Generated with Claude Code