-
Notifications
You must be signed in to change notification settings - Fork 775
Refactor the NotEmoji rule
#1576
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
7f338c9 to
dfe2abb
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1576 +/- ##
=========================================
Coverage 97.49% 97.49%
Complexity 987 987
=========================================
Files 205 205
Lines 2235 2235
=========================================
Hits 2179 2179
Misses 56 56 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dfe2abb to
7be8376
Compare
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 NotEmoji rule to Emoji, inverting its validation logic from "does not contain emoji" to "is an emoji (or sequence of emojis)". This change makes the rule more flexible and aligns with the validation framework's pattern of using the not() prefix for negative assertions. The implementation now uses a modern Unicode-based regex pattern with support for skin tone modifiers, ZWJ sequences, flags, and keycap sequences.
Key Changes:
- Renamed
NotEmojitoEmojiwith inverted validation logic (checks if input IS an emoji, not if it contains one) - Updated regex pattern from explicit Unicode ranges to a more maintainable pattern using
\p{Extended_Pictographic}property - Added comprehensive test coverage for emoji versions up to Emoji 16.0 (2024), including complex sequences
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
library/Rules/Emoji.php |
New rule implementation using Unicode property classes and supporting modern emoji sequences |
library/Rules/NotEmoji.php |
Removed old rule with hardcoded Unicode ranges |
library/Transformers/Prefix.php |
Updated to include emoji in rules to skip prefix transformation |
library/Mixins/*.php |
Updated all mixin interfaces to rename notEmoji() to emoji() |
tests/unit/Rules/EmojiTest.php |
New comprehensive test suite with 60+ emoji test cases covering various Unicode versions |
tests/unit/Rules/NotEmojiTest.php |
Removed old test file |
tests/feature/Rules/EmojiTest.php |
New feature tests for default and inverted templates |
tests/feature/Rules/NotEmojiTest.php |
Removed old feature tests |
tests/unit/Transformers/PrefixTest.php |
Updated test to reflect emoji as an untransformed rule name |
docs/rules/Emoji.md |
New documentation explaining emoji validation with examples and supported sequences |
docs/rules/NotEmoji.md |
Removed old documentation |
docs/rules/*.md |
Updated cross-references from NotEmoji to Emoji |
docs/09-list-of-rules-by-category.md |
Reorganized rule listings to replace NotEmoji with Emoji |
bin/create-mixin |
Removed NotEmoji from the deny list and added code formatting improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7be8376 to
d6cfca7
Compare
Since we have the ability to use `not` as a prefix, having rules that validate negative behaviour makes them a bit inflexible, verbose, and harder to understand. This commit will refactor the `NotEmoji` and rename it to `Emoji`. It will no longer check if the string contains emojis, but rather if the string is an emoji or not. I’m also adding support to more emojis, since the rule was a bit outdated. This change will make the validator more strict, but will make it useful in other scenarios. However, later on, I would like to create a rule called `has` which, could use a validator like `Emoji` to check if the input _has_ emojis. Assisted-by: Cursor (claude-4.5-opus-high)
d6cfca7 to
951c16e
Compare
Since we have the ability to use
notas a prefix, having rules that validate negative behaviour makes them a bit inflexible, verbose, and harder to understand.This commit will refactor the
NotEmojiand rename it toEmoji. It will no longer check if the string contains emojis, but rather if the string is an emoji or not. I’m also adding support to more emojis, since the rule was a bit outdated.This change will make the validator more strict, but will make it useful in other scenarios. However, later on, I would like to create a rule called
haswhich, could use a validator likeEmojito check if the input has emojis.