test: prevent duplicated aliases from idiots#365
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds fail-fast validation when generating USAGE.md so redundant visible aliases (aliases that repeat the argument’s own primary flag name) are detected and reported as a structured error, with the CLI binary returning an appropriate exit code.
Changes:
- Changed
render_usage_md()to returnResult<String, RenderUsageMdError>and introducedRenderUsageMdError. - Added
reject_redundant_aliases()validation before rendering usage markdown. - Updated
pdu-usage-mdand the sync test to handle the newResultAPI.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/sync_usage_md.rs | Updates the sync test to unwrap the new Result from render_usage_md(). |
| src/usage_md.rs | Introduces a new error type, adds redundant-alias validation, and returns Ok(out) instead of a bare String. |
| cli/usage_md.rs | Handles render_usage_md() errors, prints to stderr, and returns success/failure exit codes. |
Performance Regression Reportscommit: a108ad4 There are no regressions. |
… own flag name A visible_alias matching the argument's own --long name (or a visible_short_alias matching its -short flag) is a coding mistake that produces redundant output in USAGE.md. The new validation runs at the start of render_usage_md() and exits with code 1 if any such redundancy is found. https://claude.ai/code/session_01TSnvxN1HBwwzhxfFShWAQ5
…ss::exit
Define a non-exhaustive `RenderUsageMdError` enum with
`derive_more::{Display, Error}` and make `render_usage_md` return
`Result<String, RenderUsageMdError>`. The callsites in cli/usage_md.rs
and tests/sync_usage_md.rs now handle the error appropriately.
https://claude.ai/code/session_01TSnvxN1HBwwzhxfFShWAQ5
- Split RedundantVisibleAlias into RedundantVisibleLongAlias(String) and RedundantVisibleShortAlias(char), moving message into #[display] - Rename validate_no_redundant_visible_aliases to check_visible_aliases - Use if-let-Err pattern in cli/usage_md.rs to match lib.rs style - Simplify test to use .unwrap() instead of .expect() - Fix doc comment to use plain English instead of backtick-wrapped flags https://claude.ai/code/session_01TSnvxN1HBwwzhxfFShWAQ5
…t, revert to match - Rename check_visible_aliases to reject_redundant_aliases - Use pipe-trait to flatten nested constructor and Err calls - Revert cli/usage_md.rs back to match pattern https://claude.ai/code/session_01TSnvxN1HBwwzhxfFShWAQ5
KSXGitHub
force-pushed
the
claude/fix-visible-alias-conflict-o1le1
branch
from
3609c60 to
224c3ec
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.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
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
This PR adds validation to detect and reject redundant visible aliases in CLI arguments that duplicate their own primary flag names. The
render_usage_md()function now returns aResulttype to communicate validation errors, and the CLI tool properly handles and reports these errors.Key Changes
New error type: Added
RenderUsageMdErrorenum with two variants:RedundantVisibleLongAlias: Detects when a visible alias matches the argument's own long flagRedundantVisibleShortAlias: Detects when a visible short alias matches the argument's own short flagValidation function: Implemented
reject_redundant_aliases()to iterate through all arguments and check for duplicate aliases before renderingAPI change: Modified
render_usage_md()to returnResult<String, RenderUsageMdError>instead ofString, allowing callers to handle validation errorsCLI error handling: Updated the
usage_mdbinary to:Resultreturn typeSUCCESSorFAILURE)Test updates: Updated the test to unwrap the
Resultfromrender_usage_md()Implementation Details
derive_morecrate for automaticDisplayandErrortrait implementationspipe_traitfor functional-style error construction in the validation functionhttps://claude.ai/code/session_01TSnvxN1HBwwzhxfFShWAQ5