-
Notifications
You must be signed in to change notification settings - Fork 0
fix(rules): allow flag-only negation to pass with empty command tokens #179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e31144c
8c1d4b3
6202fa0
c29ca3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -222,23 +222,29 @@ fn match_tokens_core<'a>( | |
| } | ||
|
|
||
| PatternToken::Negation(inner) => { | ||
| if cmd_tokens.is_empty() { | ||
| return false; | ||
| } | ||
| // Flag-only negations (all inner elements start with `-`, excluding | ||
| // bare `--`) use order-independent matching: scan the entire command | ||
| // token list. Other negations check only the positional token. | ||
| let negation_passed = if is_flag_only_negation(inner) { | ||
| !cmd_tokens | ||
| if is_flag_only_negation(inner) { | ||
| // Flag-only negations scan the entire command token list for | ||
| // the forbidden flag (order-independent). When no tokens remain | ||
| // the flag is trivially absent, so the negation passes. | ||
| let negation_passed = !cmd_tokens | ||
| .iter() | ||
| .any(|t| match_flag_token_with_equals(inner, t, definitions)) | ||
| } else { | ||
| !match_single_token(inner, cmd_tokens[0], definitions) | ||
| }; | ||
| if negation_passed { | ||
| match_tokens_core(rest, &cmd_tokens[1..], definitions, steps, captures) | ||
| .any(|t| match_flag_token_with_equals(inner, t, definitions)); | ||
| if negation_passed { | ||
| // Flag-only negations do not consume a positional token. | ||
| match_tokens_core(rest, cmd_tokens, definitions, steps, captures) | ||
| } else { | ||
| false | ||
| } | ||
| } else { | ||
| false | ||
| if cmd_tokens.is_empty() { | ||
| return false; | ||
| } | ||
| let negation_passed = !match_single_token(inner, cmd_tokens[0], definitions); | ||
| if negation_passed { | ||
| match_tokens_core(rest, &cmd_tokens[1..], definitions, steps, captures) | ||
| } else { | ||
| false | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
224
to
249
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The logic for handling flag-only and positional negations has some duplication. You can refactor this to reduce redundancy and make the distinction between the two cases (the number of tokens consumed) more explicit. This will improve maintainability. PatternToken::Negation(inner) => {
let (negation_passed, consumed) = if is_flag_only_negation(inner) {
// Flag-only negations scan the entire command token list.
// If no tokens remain, the flag is trivially absent.
// They do not consume a positional token.
let passed = !cmd_tokens
.iter()
.any(|t| match_flag_token_with_equals(inner, t, definitions));
(passed, 0)
} else {
// Positional negations check only the next token.
if cmd_tokens.is_empty() {
return false;
}
let passed = !match_single_token(inner, cmd_tokens[0], definitions);
(passed, 1)
};
if negation_passed {
match_tokens_core(rest, &cmd_tokens[consumed..], definitions, steps, captures)
} else {
false
}
} |
||
|
|
||
|
|
@@ -497,25 +503,35 @@ fn extract_placeholder_all<'a>( | |
| } | ||
|
|
||
| PatternToken::Negation(inner) => { | ||
| if cmd_tokens.is_empty() { | ||
| return Ok(()); | ||
| } | ||
| let negation_passed = if is_flag_only_negation(inner) { | ||
| !cmd_tokens | ||
| if is_flag_only_negation(inner) { | ||
| let negation_passed = !cmd_tokens | ||
| .iter() | ||
| .any(|t| match_flag_token_with_equals(inner, t, definitions)) | ||
| .any(|t| match_flag_token_with_equals(inner, t, definitions)); | ||
| if negation_passed { | ||
| extract_placeholder_all( | ||
| rest, | ||
| cmd_tokens, | ||
| definitions, | ||
| steps, | ||
| captured, | ||
| all_candidates, | ||
| )?; | ||
| } | ||
| } else { | ||
| !match_single_token(inner, cmd_tokens[0], definitions) | ||
| }; | ||
| if negation_passed { | ||
| extract_placeholder_all( | ||
| rest, | ||
| &cmd_tokens[1..], | ||
| definitions, | ||
| steps, | ||
| captured, | ||
| all_candidates, | ||
| )?; | ||
| if cmd_tokens.is_empty() { | ||
| return Ok(()); | ||
| } | ||
| let negation_passed = !match_single_token(inner, cmd_tokens[0], definitions); | ||
| if negation_passed { | ||
| extract_placeholder_all( | ||
| rest, | ||
| &cmd_tokens[1..], | ||
| definitions, | ||
| steps, | ||
| captured, | ||
| all_candidates, | ||
| )?; | ||
| } | ||
| } | ||
| Ok(()) | ||
| } | ||
|
Comment on lines
505
to
537
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to PatternToken::Negation(inner) => {
let (negation_passed, consumed) = if is_flag_only_negation(inner) {
let passed = !cmd_tokens
.iter()
.any(|t| match_flag_token_with_equals(inner, t, definitions));
(passed, 0)
} else {
if cmd_tokens.is_empty() {
return Ok(());
}
let passed = !match_single_token(inner, cmd_tokens[0], definitions);
(passed, 1)
};
if negation_passed {
extract_placeholder_all(
rest,
&cmd_tokens[consumed..],
definitions,
steps,
captured,
all_candidates,
)?;
}
Ok(())
} |
||
|
|
@@ -1028,6 +1044,12 @@ mod tests { | |
| "sort --reverse=true file.txt", | ||
| true | ||
| )] | ||
| // Flag-only negation with empty command tokens (no arguments after command) | ||
| #[case::flag_negation_empty_tokens_single("sort !-o *", "sort", true)] | ||
| #[case::flag_negation_empty_tokens_alt("sort !-o|--output|--compress-program *", "sort", true)] | ||
| #[case::flag_negation_empty_tokens_find("find !-delete *", "find", true)] | ||
| // Positional negation with empty tokens should still be false | ||
| #[case::positional_negation_empty_tokens("kubectl !describe *", "kubectl", false)] | ||
| fn negation_matching( | ||
| #[case] pattern_str: &str, | ||
| #[case] command_str: &str, | ||
|
|
@@ -1496,8 +1518,33 @@ mod tests { | |
| #[case::negation_before_cmd( | ||
| "run !--dry-run <cmd>", | ||
| "run --verbose echo hello", | ||
| vec![vec!["--verbose", "echo", "hello"]], | ||
| )] | ||
| #[case::positional_negation_before_cmd( | ||
| "run !exec <cmd>", | ||
| "run start echo hello", | ||
| vec![vec!["echo", "hello"]], | ||
| )] | ||
| #[case::positional_negation_empty_tokens( | ||
| "run !exec <cmd>", | ||
| "run", | ||
| Vec::<Vec<&str>>::new(), | ||
| )] | ||
| #[case::flag_negation_empty_tokens_before_cmd( | ||
| "run !--dry-run <cmd>", | ||
| "run", | ||
| Vec::<Vec<&str>>::new(), | ||
| )] | ||
| #[case::flag_negation_rejected_before_cmd( | ||
| "run !--dry-run <cmd>", | ||
| "run --dry-run echo hello", | ||
| Vec::<Vec<&str>>::new(), | ||
| )] | ||
| #[case::positional_negation_rejected_before_cmd( | ||
| "run !exec <cmd>", | ||
| "run exec echo hello", | ||
| Vec::<Vec<&str>>::new(), | ||
| )] | ||
| fn extract_placeholder_cases( | ||
| #[case] pattern_str: &str, | ||
| #[case] command_str: &str, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.