fix(rules): match flag-only negation patterns against =-joined tokens#175
fix(rules): match flag-only negation patterns against =-joined tokens#175
=-joined tokens#175Conversation
Flag-only negation patterns like `\!--pre` did not reject `=`-joined flags such as `--pre=pdftotext` because `--pre=pdftotext` was compared as a single token against `--pre`. The matcher now also extracts the flag portion before `=` and checks it against the negation pattern, so both `--pre value` and `--pre=value` are correctly rejected. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the pattern matching logic, specifically for flag-only negation rules. The change ensures that these negation patterns correctly identify and apply to command-line arguments where flags and their values are joined by an equals sign, thereby preventing unintended allowances. The update improves the robustness and predictability of rule evaluation, aligning the system's behavior with user expectations for flag negation. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #175 +/- ##
=======================================
Coverage 89.93% 89.94%
=======================================
Files 38 38
Lines 7232 7248 +16
=======================================
+ Hits 6504 6519 +15
- Misses 728 729 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request correctly fixes an issue where flag-only negation patterns did not match against =-joined tokens. The changes are well-implemented, with a new helper function to handle the matching logic and consistent application in both match_tokens_core and extract_placeholder_all. The documentation has been updated accordingly, and the new behavior is thoroughly tested with both unit and integration tests that adhere to the project's testing style guide.
Note: Security Review is unavailable for this PR.
…into single match_engine Two nearly-identical recursive matching functions (match_tokens_core for boolean matching, extract_placeholder_all for placeholder extraction) duplicated all PatternToken variant handling. Bug fixes applied to one function were frequently missed in the other (PR #175, #179, #180). Merge both into a single `match_engine` function parameterized by an optional `extract` tuple. Each PatternToken variant's logic now exists in exactly one place. Also introduce a flag normalization layer (`split_flag_equals`, `flag_aliases_match_token`) to centralize `--flag=value` splitting, replacing the ad-hoc `=`-joined handling scattered across match_flag_token_with_equals, FlagWithValue, and optional_flags_absent. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Why
!--predid not match=-joined arguments such as--pre=pdftotext--pre=pdftotextis treated as a single raw token, so comparing it against the negation pattern--preresulted in a mismatch, causing the negation to pass through (evaluated as allow)What
=) of=-joined command tokens when evaluating flag-only negation patterns