Skip to content

style: fix clippy and deprecation warnings in test and bench targets - #763

Open
JamBalaya56562 wants to merge 1 commit into
jdx:mainfrom
JamBalaya56562:fix-clippy-test-bench-targets
Open

style: fix clippy and deprecation warnings in test and bench targets#763
JamBalaya56562 wants to merge 1 commit into
jdx:mainfrom
JamBalaya56562:fix-clippy-test-bench-targets

Conversation

@JamBalaya56562

@JamBalaya56562 JamBalaya56562 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

lint:clippy runs cargo clippy --all --all-features -- -D warnings, which does not cover test, bench or example targets. Nothing has been linting them, so they had drifted — --all-targets currently reports 17 errors on main. This PR fixes all of them. No behaviour changes.

File Count Lint
lib/benches/parse.rs 13 criterion::black_box is deprecated in favour of std::hint::black_box
lib/src/parse.rs (test module) 2 clippy::unnecessary_get_then_check
cli/tests/shell_completions_integration.rs 2 clippy::manual_contains

The bench fix is a one-line import change — use std::hint::black_box; — so all 13 call sites stay as they are.

After this, cargo clippy --all --all-features --all-targets -- -D warnings is clean.

I deliberately left mise.toml alone: adding --all-targets to lint:clippy would keep this from drifting again, but that is a CI policy change and felt like a separate conversation. Happy to send it as a follow-up if you want it.

Verified with cargo test -p usage-lib --all-features, cargo bench -p usage-lib --no-run (the bench builds and links), and cargo fmt --all -- --check. The shell-completion integration tests only compile-check locally — they need Unix shells — so CI is the real check there.


This pull request was generated by Claude Code.

Summary by CodeRabbit

  • Tests

    • Improved shell completion and mount-related regression test assertions without changing test behavior.
  • Chores

    • Updated benchmark utilities to use the standard library’s recommended optimization helper.

`lint:clippy` runs `cargo clippy --all --all-features -- -D warnings`, which
does not cover test, bench or example targets. Nothing was linting them, so
they had drifted:

- `lib/benches/parse.rs` used `criterion::black_box`, deprecated in favour of
  `std::hint::black_box`. Importing it from `std::hint` fixes all 13 call sites
  without touching them.
- `lib/src/parse.rs` used `get(k).is_none()` where `!contains_key(k)` says the
  same thing (`clippy::unnecessary_get_then_check`).
- `cli/tests/shell_completions_integration.rs` used `iter().any(|l| *l == x)`
  where `contains(&x)` is equivalent and cheaper (`clippy::manual_contains`).

`cargo clippy --all --all-features --all-targets -- -D warnings` is now clean.
No behaviour changes.
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The changes simplify two test assertions and replace Criterion’s black_box import with std::hint::black_box. Benchmark registrations and test behavior remain unchanged.

Changes

Test and benchmark cleanup

Layer / File(s) Summary
Direct standard-library checks
cli/tests/shell_completions_integration.rs, lib/src/parse.rs, lib/benches/parse.rs
Zsh completion checks use contains, flag absence uses contains_key, and the benchmark imports black_box from std::hint.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Suggested reviewers: jdx

Poem

A rabbit checks each row with care,
Finds keys and flags laid bare.
black_box hops to standard ground,
While simpler checks go round and round.
Three tiny tweaks, neat as a hare.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the changes that fix Clippy and deprecation warnings in test and benchmark targets.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@JamBalaya56562
JamBalaya56562 marked this pull request as ready for review August 1, 2026 13:22
@greptile-apps

greptile-apps Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR resolves Clippy and deprecation warnings in test and benchmark targets without changing production behavior.

  • Replaces manual iterator equality checks with contains in shell-completion tests.
  • Replaces deprecated criterion::black_box with std::hint::black_box in parser benchmarks.
  • Replaces map get(...).is_none() assertions with contains_key in parser tests.

Confidence Score: 5/5

The PR appears safe to merge with no actionable defects identified.

The assertion rewrites preserve membership semantics, and the benchmark helper replacement is supported by the repository’s declared Rust toolchain.

Important Files Changed

Filename Overview
cli/tests/shell_completions_integration.rs Simplifies two exact-string membership assertions while preserving their behavior.
lib/benches/parse.rs Uses the standard-library black-box helper, which is available under the crate’s declared Rust version.
lib/src/parse.rs Rewrites two map-absence assertions using the equivalent contains_key API.

Reviews (1): Last reviewed commit: "style: fix clippy and deprecation warnin..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant