Skip to content

fix: simplify required-unless predicates - #1326

Merged
jdx merged 1 commit into
mainfrom
fix/clippy-required-unless
Aug 25, 2026
Merged

fix: simplify required-unless predicates#1326
jdx merged 1 commit into
mainfrom
fix/clippy-required-unless

Conversation

@jdx

@jdx jdx commented Aug 25, 2026

Copy link
Copy Markdown
Owner

Summary

  • simplify the equivalent required_unless predicates for arguments and flags
  • keep the existing behavior while satisfying current clippy nonminimal_bool

Validation

  • cargo fmt --check -- lib/src/parse.rs
  • cargo clippy -p usage-lib --no-default-features -- -D warnings

Note

Low Risk
Refactor-only change to equivalent boolean logic in CLI validation; no intended behavior change.

Overview
Rewrites how required_unless is computed when validating missing args and flags in parse.rs. The old form (lists non-empty) && !(unless satisfied) is replaced with a single negated OR: not (unless_any || unless_all || no unless rules defined).

Behavior is unchanged—only the boolean shape changes so clippy::nonminimal_bool passes with -D warnings. The same logic is applied in both the argument and flag missing-value checks.

Reviewed by Cursor Bugbot for commit 2c3e278. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Bug Fixes
    • Corrected requiredness validation for positional arguments and flags.
    • Missing-value errors are now reported consistently when no conditional exceptions are configured.
    • Updated handling ensures required-unless conditions correctly reflect whether any exception selectors match.

@jdx
jdx enabled auto-merge (squash) August 25, 2026 21:59
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: e6e022d9-78ef-4400-a03c-d7cb12ec77d7

📥 Commits

Reviewing files that changed from the base of the PR and between a5dfa3d and 2c3e278.

📒 Files selected for processing (1)
  • lib/src/parse.rs

Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The parser changes required_unless evaluation for positional arguments and flags. The condition is now true when no unless_any or unless_all selector matches, including when both declaration lists are empty.

Changes

Requiredness evaluation

Layer / File(s) Summary
Required-unless checks
lib/src/parse.rs
Positional argument and flag checks now treat required_unless as true when neither selector group matches, including empty declaration lists.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 2c3e2

This is a localized refactor of existing CLI validation logic intended to preserve behavior while satisfying clippy; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: jambalaya56562

Poem

A rabbit checks the parser line by line
Empty lists now follow a clearer sign
Flags and arguments share the rule
Unless selectors guide the tool
Requiredness hops into place just fine

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: simplifying the required_unless predicates.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
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.

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.

@jdx
jdx merged commit de8ad65 into main Aug 25, 2026
8 of 9 checks passed
@jdx
jdx deleted the fix/clippy-required-unless branch August 25, 2026 22:12
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