-
Notifications
You must be signed in to change notification settings - Fork 775
Invert the behaviour of NotBlank
#1573
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
Conversation
There was some inconsistent Markdown, and some links were out of sync.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1573 +/- ##
=========================================
Coverage 97.49% 97.49%
Complexity 988 988
=========================================
Files 205 205
Lines 2237 2237
=========================================
Hits 2181 2181
Misses 56 56 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the NotBlank validation rule by inverting its behavior and renaming it to Blank. The change leverages the existing not prefix functionality, making the library more consistent and flexible - users can now write not(blank()) instead of notBlank().
Key changes:
- Renamed
NotBlankclass toBlankand inverted its validation logic - Updated mixin interfaces to remove
notBlank()methods and addblank()methods, withnotBlank()now available throughnot(blank()) - Removed
notBlankfrom the prefix transformer's untransformed rules list - Comprehensively updated documentation and tests
Reviewed changes
Copilot reviewed 49 out of 53 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| library/Rules/Blank.php | Core rule implementation - class renamed, templates swapped, and logic inverted to validate blank values |
| library/Transformers/Prefix.php | Removed 'notBlank' from untransformed rules list |
| library/Mixins/*.php | Updated all mixin interfaces: removed notBlank() methods, added blank() methods, and added notBlank() to NotChain/NotBuilder |
| tests/unit/Rules/BlankTest.php | Renamed test class and swapped valid/invalid input providers to match inverted behavior |
| tests/unit/Transformers/PrefixTest.php | Removed notBlank test case |
| tests/feature/Rules/BlankTest.php | New feature test validating default and inverted templates |
| tests/feature/Rules/NotBlankTest.php | Deleted old feature test file |
| docs/rules/Blank.md | New documentation for Blank rule |
| docs/rules/NotBlank.md | Deleted old documentation |
| docs/rules/*.md | Updated cross-references from NotBlank to Blank and fixed table formatting |
| docs/09-list-of-rules-by-category.md | Updated rule lists and alphabetical ordering |
| bin/create-mixin | Updated exclusion lists and parameter handling improvements |
| bin/update-doc-links | Updated script to match new separator format |
Comments suppressed due to low confidence (1)
library/Rules/Blank.php:28
- The templates are in the wrong order. According to the Template attribute documentation and the feature test in tests/feature/Rules/BlankTest.php (lines 13 and 21), the first template is for the default/non-inverted case and the second is for the inverted case.
Since this is the Blank rule (which checks if something IS blank), the default template should be "must be blank" and the inverted template should be "must not be blank". The current order has them reversed.
This should be:
#[Template(
'{{subject}} must be blank',
'{{subject}} must not be blank',
)]
The feature test expects:
v::blank()->assert(1)to produce "1 must be blank" (default)v::not(v::blank())->assert(null)to produce "nullmust not be blank" (inverted)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Since we have the ability to use `not` as a prefix, having rules that start with not becomes a bit inflexible, verbose, and harder to understand. This commit will refactor the `NotBlank` rule by inverting its behaviour and renaming it to `Blank`. Although this is a breaking change, users will not feel it because "NotBlank" will still be available by using the `not` prefix followed by the `Blank` rule.
f023280 to
338c186
Compare
Since we have the ability to use
notas a prefix, having rules that start with not becomes a bit inflexible, verbose, and harder to understand.This commit will refactor the
NotBlankrule by inverting its behaviour and renaming it toBlank. Although this is a breaking change, users will not feel it because "NotBlank" will still be available by using thenotprefix followed by theBlankrule.