feat: support global excludes in rules.yaml config (feat #93)#94
Conversation
📝 WalkthroughWalkthroughAdds a ChangesGlobal Exclude Feature
Estimated code review effort: 2 (Simple) | ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/becwright/rules.py`:
- Around line 91-96: The runtime validation in rules.py for global_exclude only
checks that each item is a string, so empty glob patterns can slip through
despite the schema’s minLength contract. Update the validation in the
rules-loading path that builds global_exclude to reject empty strings as well,
keeping the check aligned with _to_rule and the surrounding RulesError handling
so invalid patterns fail fast before returning the rule list.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a3bc34be-b93a-42f1-9255-1e4d6440b2d5
📒 Files selected for processing (4)
schema/rules.schema.jsonsrc/becwright/rules.pytests/test_cli_and_git.pytests/test_rules.py
|
|
||
| global_exclude = data.get("global_exclude", []) | ||
| if not isinstance(global_exclude, list) or not all(isinstance(x, str) for x in global_exclude): | ||
| raise RulesError(f"{rules_path}: global_exclude must be a list of glob patterns.") | ||
|
|
||
| return [_to_rule(r, global_exclude) for r in data["rules"]] if data.get("rules") else [] |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Runtime validation should reject empty strings in global_exclude.
The schema enforces "minLength": 1 on each item, but the runtime check at line 93 only validates isinstance(x, str). An empty string glob pattern in global_exclude would pass runtime validation and could match all files, silently disabling every file-based rule. Align the runtime check with the schema contract.
🛡️ Proposed fix
global_exclude = data.get("global_exclude", [])
- if not isinstance(global_exclude, list) or not all(isinstance(x, str) for x in global_exclude):
+ if not isinstance(global_exclude, list) or not all(isinstance(x, str) and x for x in global_exclude):
raise RulesError(f"{rules_path}: global_exclude must be a list of glob patterns.")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| global_exclude = data.get("global_exclude", []) | |
| if not isinstance(global_exclude, list) or not all(isinstance(x, str) for x in global_exclude): | |
| raise RulesError(f"{rules_path}: global_exclude must be a list of glob patterns.") | |
| return [_to_rule(r, global_exclude) for r in data["rules"]] if data.get("rules") else [] | |
| global_exclude = data.get("global_exclude", []) | |
| if not isinstance(global_exclude, list) or not all(isinstance(x, str) and x for x in global_exclude): | |
| raise RulesError(f"{rules_path}: global_exclude must be a list of glob patterns.") | |
| return [_to_rule(r, global_exclude) for r in data["rules"]] if data.get("rules") else [] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/becwright/rules.py` around lines 91 - 96, The runtime validation in
rules.py for global_exclude only checks that each item is a string, so empty
glob patterns can slip through despite the schema’s minLength contract. Update
the validation in the rules-loading path that builds global_exclude to reject
empty strings as well, keeping the check aligned with _to_rule and the
surrounding RulesError handling so invalid patterns fail fast before returning
the rule list.
Description
This PR introduces support for a top-level
global_excludeblock in.bec/rules.yaml. This enables developers to define global ignore patterns (e.g., ignoringvendor/**,build/**, or third-party dependencies) across all file-based rules in one place, avoiding configuration duplication.Fixes #93
Key Changes
1. Global Exclude Configuration & Rules Parsing
src/becwright/rules.py:load_rules) to parse the top-levelglobal_excludelist._to_rule) to append the global excludes into each individual rule'sexcludelist during configuration loading.schema/rules.schema.json:global_excludeproperty (type:arrayofstrings) to the JSON Schema, ensuring editor autocompletion and structural validation.2. Testing
tests/test_rules.py: Added unit tests verifying merge behavior and type-checking validations of theglobal_excludearray.tests/test_cli_and_git.py: Added an integration testtest_check_respects_global_excludechecking that files in folders defined in the global ignore patterns are successfully skipped during check runs.Verification Results
The test suite runs fully green:
Summary by CodeRabbit