fix(attribute): Better naming prefix CanBe for boolean attributes.#85
fix(attribute): Better naming prefix CanBe for boolean attributes.#85terabytesoftw merged 3 commits intomainfrom
CanBe for boolean attributes.#85Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR renames boolean attribute traits from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/Global/CanBeAutofocus.php (1)
23-26: 🛠️ Refactor suggestion | 🟠 MajorUsage example is inconsistent with other
CanBe*traits.The other renamed traits (e.g.,
CanBeDisabled,CanBeReadonly,CanBeRequired) all include anullexample alongsidetruein their usage snippets. Since the signature was widened tobool|nullin this PR, adding thenullcase here would keep documentation consistent.📝 Proposed fix
* Usage example: * ```php * $element->autofocus(true); + * $element->autofocus(null); * ```tests/Form/CanBeMultipleTest.php (1)
76-80:⚠️ Potential issue | 🔴 CriticalSame
getAttributedefault mismatch as inCanBeHiddenTest.Line 78 uses
''as the default forgetAttribute, but the'null'and'unset with null'provider cases expectnull. This will causeassertSame(null, '')to fail ifsetAttribute(null)removes the attribute. Usenullas the default to be consistent withCanBeRequiredTest.Proposed fix
self::assertSame( $expectedValue, - $instance->getAttribute(Attribute::MULTIPLE, ''), + $instance->getAttribute(Attribute::MULTIPLE, null), $message, );tests/Form/CanBeCheckedTest.php (1)
76-80:⚠️ Potential issue | 🔴 CriticalSame
getAttributedefault mismatch as inCanBeHiddenTestandCanBeMultipleTest.Line 78 uses
''as the default forgetAttribute, but the provider's'null'and'unset with null'cases expectnull. Usenullas the default to matchCanBeRequiredTest.Proposed fix
self::assertSame( $expectedValue, - $instance->getAttribute(Attribute::CHECKED, ''), + $instance->getAttribute(Attribute::CHECKED, null), $message, );
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (22)
CHANGELOG.mdsrc/CanBeDisabled.phpsrc/Form/CanBeChecked.phpsrc/Form/CanBeMultiple.phpsrc/Form/CanBeReadonly.phpsrc/Form/CanBeRequired.phpsrc/Global/CanBeAutofocus.phpsrc/Global/CanBeHidden.phptests/CanBeDisabledTest.phptests/Form/CanBeCheckedTest.phptests/Form/CanBeMultipleTest.phptests/Form/CanBeReadonlyTest.phptests/Form/CanBeRequiredTest.phptests/Global/CanBeAutofocusTest.phptests/Global/CanBeHiddenTest.phptests/Provider/DisabledProvider.phptests/Provider/Form/CheckedProvider.phptests/Provider/Form/MultipleProvider.phptests/Provider/Form/ReadonlyProvider.phptests/Provider/Form/RequiredProvider.phptests/Provider/Global/AutofocusProvider.phptests/Provider/Global/HiddenProvider.php
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-09T16:05:15.502Z
Learnt from: terabytesoftw
Repo: ui-awesome/html-attribute PR: 18
File: tests/Support/Provider/Global/ContentEditableProvider.php:32-33
Timestamp: 2026-01-09T16:05:15.502Z
Learning: Preserve the original copyright year in file headers when moving PHP source files between packages within the ui-awesome repositories (e.g., from ui-awesome/html-core to ui-awesome/html-attribute). Do not update the header year to the current year; keep the original creation year as stated in the header. This applies to all PHP files throughout the repo that are moved between packages.
Applied to files:
tests/Provider/Form/RequiredProvider.phpsrc/Global/CanBeHidden.phptests/Provider/DisabledProvider.phpsrc/Global/CanBeAutofocus.phptests/Global/CanBeHiddenTest.phptests/CanBeDisabledTest.phpsrc/Form/CanBeRequired.phpsrc/Form/CanBeReadonly.phptests/Form/CanBeRequiredTest.phptests/Provider/Form/ReadonlyProvider.phptests/Provider/Global/HiddenProvider.phptests/Form/CanBeReadonlyTest.phptests/Global/CanBeAutofocusTest.phptests/Form/CanBeMultipleTest.phpsrc/CanBeDisabled.phpsrc/Form/CanBeChecked.phpsrc/Form/CanBeMultiple.phptests/Provider/Form/CheckedProvider.phptests/Provider/Form/MultipleProvider.phptests/Form/CanBeCheckedTest.phptests/Provider/Global/AutofocusProvider.php
📚 Learning: 2026-02-08T20:05:36.407Z
Learnt from: terabytesoftw
Repo: ui-awesome/html-attribute PR: 80
File: src/HasValue.php:35-35
Timestamp: 2026-02-08T20:05:36.407Z
Learning: In the ui-awesome/html-attribute repository, for methods that accept multiple values of the same type (e.g., boolean true/false), document with a single representative example rather than exhaustively listing all variations. This keeps documentation concise while conveying the intended usage. Apply this consistently across PHP docblocks and examples.
Applied to files:
tests/Provider/Form/RequiredProvider.phpsrc/Global/CanBeHidden.phptests/Provider/DisabledProvider.phpsrc/Global/CanBeAutofocus.phptests/Global/CanBeHiddenTest.phptests/CanBeDisabledTest.phpsrc/Form/CanBeRequired.phpsrc/Form/CanBeReadonly.phptests/Form/CanBeRequiredTest.phptests/Provider/Form/ReadonlyProvider.phptests/Provider/Global/HiddenProvider.phptests/Form/CanBeReadonlyTest.phptests/Global/CanBeAutofocusTest.phptests/Form/CanBeMultipleTest.phpsrc/CanBeDisabled.phpsrc/Form/CanBeChecked.phpsrc/Form/CanBeMultiple.phptests/Provider/Form/CheckedProvider.phptests/Provider/Form/MultipleProvider.phptests/Form/CanBeCheckedTest.phptests/Provider/Global/AutofocusProvider.php
📚 Learning: 2026-02-07T13:44:00.931Z
Learnt from: terabytesoftw
Repo: ui-awesome/html-attribute PR: 78
File: tests/Global/HasDirTest.php:92-92
Timestamp: 2026-02-07T13:44:00.931Z
Learning: In PHP test methods that start with testThrowInvalidArgumentException, do not append 'Invalid' again in the method name suffix (e.g., prefer testThrowInvalidArgumentExceptionForSettingDirValue over testThrowInvalidArgumentExceptionForSettingInvalidDirValue). The exception type already communicates invalid input, so the repeated word is redundant. Apply this convention to all similar test methods under the tests/ directory.
Applied to files:
tests/Provider/Form/RequiredProvider.phptests/Provider/DisabledProvider.phptests/Global/CanBeHiddenTest.phptests/CanBeDisabledTest.phptests/Form/CanBeRequiredTest.phptests/Provider/Form/ReadonlyProvider.phptests/Provider/Global/HiddenProvider.phptests/Form/CanBeReadonlyTest.phptests/Global/CanBeAutofocusTest.phptests/Form/CanBeMultipleTest.phptests/Provider/Form/CheckedProvider.phptests/Provider/Form/MultipleProvider.phptests/Form/CanBeCheckedTest.phptests/Provider/Global/AutofocusProvider.php
🧬 Code graph analysis (7)
src/Global/CanBeHidden.php (1)
src/HasValue.php (1)
value(42-45)
src/Global/CanBeAutofocus.php (1)
src/HasValue.php (1)
value(42-45)
tests/CanBeDisabledTest.php (1)
tests/Provider/DisabledProvider.php (1)
DisabledProvider(15-60)
tests/Form/CanBeRequiredTest.php (1)
tests/Provider/Form/RequiredProvider.php (1)
RequiredProvider(15-60)
tests/Form/CanBeReadonlyTest.php (1)
tests/Provider/Form/ReadonlyProvider.php (1)
ReadonlyProvider(15-60)
tests/Form/CanBeMultipleTest.php (1)
tests/Provider/Form/MultipleProvider.php (1)
MultipleProvider(15-60)
tests/Form/CanBeCheckedTest.php (1)
tests/Provider/Form/CheckedProvider.php (1)
CheckedProvider(15-60)
🪛 PHPMD (2.15.0)
tests/CanBeDisabledTest.php
[warning] 66-66: Avoid excessively long variable names like $expectedRenderAttributes. Keep variable name length under 20. (undefined)
(LongVariable)
tests/Form/CanBeReadonlyTest.php
[warning] 66-66: Avoid excessively long variable names like $expectedRenderAttributes. Keep variable name length under 20. (undefined)
(LongVariable)
tests/Form/CanBeMultipleTest.php
[warning] 66-66: Avoid excessively long variable names like $expectedRenderAttributes. Keep variable name length under 20. (undefined)
(LongVariable)
tests/Form/CanBeCheckedTest.php
[warning] 66-66: Avoid excessively long variable names like $expectedRenderAttributes. Keep variable name length under 20. (undefined)
(LongVariable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: phpunit / PHP 8.5-ubuntu-latest
- GitHub Check: mutation / PHP 8.5-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: easy-coding-standard / PHP 8.5-ubuntu-latest
- GitHub Check: composer-require-checker / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.4-windows-2022
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.5-windows-2022
- GitHub Check: phpstan / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: phpunit / PHP 8.1-windows-2022
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: linter / Super Linter
🔇 Additional comments (20)
src/Form/CanBeMultiple.php (1)
18-18: LGTM! Clean trait rename with consistent documentation.src/Form/CanBeChecked.php (1)
18-18: LGTM! Consistent rename fromHasCheckedtoCanBeChecked.src/Form/CanBeReadonly.php (1)
18-18: LGTM! Consistent rename fromHasReadonlytoCanBeReadonly.src/CanBeDisabled.php (1)
18-18: LGTM! Consistent rename fromHasDisabledtoCanBeDisabled.src/Form/CanBeRequired.php (1)
18-18: LGTM! Consistent rename fromHasRequiredtoCanBeRequired.tests/Provider/Form/ReadonlyProvider.php (1)
8-8: LGTM! Docblock reference and PHPStan type annotation correctly updated to reflect theCanBeReadonlyrename and narrowed type.Also applies to: 18-18
tests/Form/CanBeReadonlyTest.php (1)
9-9: LGTM! All references correctly updated fromHasReadonlytoCanBeReadonly, and the$expectedValuetype properly narrowed tobool|null.Also applies to: 29-29, 34-34, 47-47, 65-65, 70-70
src/Global/CanBeAutofocus.php (1)
28-33: This review comment is based on an incorrect premise.The
CanBeAutofocustrait was newly created in this commit with thebool|nullsignature from the start—it was not widened from an existingboolparameter. This is a new trait, not a modification to existing code. Thebool|nullpattern is consistent with similar boolean attribute traits in the library (e.g.,CanBeHidden), and the signature is already well-tested with comprehensive coverage fortrue,false, andnullvalues, including attribute unsetting behavior.No downstream compatibility concerns apply to a newly created trait.
Likely an incorrect or invalid review comment.
tests/Provider/Global/HiddenProvider.php (1)
17-66: LGTM!The updated return type annotation and new null test cases are consistent with the nullable boolean support added to the
CanBeHiddentrait. Good coverage of both the standalonenullcase and unsetting an existing value withnull.src/Global/CanBeHidden.php (1)
18-37: LGTM!The nullable parameter addition is clean and consistent with the existing
HasValue::value()pattern. The docblock correctly documents all three states (true,false,null).tests/Provider/Form/MultipleProvider.php (1)
7-14: LGTM!Docblock reference correctly updated to
CanBeMultipleTest.tests/Form/CanBeRequiredTest.php (1)
1-87: LGTM!Clean rename from
HasRequiredtoCanBeRequired. ThegetAttributedefault ofnullon line 78 correctly aligns with the nullable test cases from the provider.tests/Provider/Form/RequiredProvider.php (1)
7-14: LGTM!Docblock reference correctly updated to
CanBeRequiredTest.tests/Global/CanBeHiddenTest.php (1)
76-80: ThegetAttributedefault value should match the expected value when attribute is null.Line 78 uses
''(empty string) as the default forgetAttribute, but the provider's'null'and'unset with null'test cases expect$expectedValue = null. This creates an inconsistency: ifsetAttribute(null)unsets the attribute,getAttribute(HIDDEN, '')would return''notnull, causing the assertion to fail.However,
CanBeRequiredTest(line 78) usesgetAttribute(..., null)as the default, which aligns with thenullexpected values in its provider. This inconsistency across the test suite suggests either:
- The default should be changed to
nullto matchCanBeRequiredTestand the expected test values, or- All similar tests have the same issue and need consistent updates
Change the default to
nullto be consistent with the expected value:self::assertSame( $expectedValue, - $instance->getAttribute(GlobalAttribute::HIDDEN, ''), + $instance->getAttribute(GlobalAttribute::HIDDEN, null), $message, );CHANGELOG.md (1)
58-58: LGTM!The changelog entry follows the established format and accurately describes the change.
tests/Provider/Form/CheckedProvider.php (1)
8-18: LGTM!Docblock reference and PHPStan return type annotation correctly updated to reflect the
CanBeCheckedrename and the narrowedbool|nulltype.tests/Provider/DisabledProvider.php (1)
8-18: LGTM!Docblock and type annotations consistently updated to match the
CanBeDisabledrename.tests/CanBeDisabledTest.php (1)
9-70: LGTM!Trait references, class name, anonymous class usages, and parameter types are all consistently updated for the
CanBeDisabledrename.tests/Provider/Global/AutofocusProvider.php (1)
37-64: LGTM!Good addition of
nullandunset with nulltest cases — these align with the updatedbool|nullparameter type and are consistent with the patterns in other providers (e.g.,CheckedProvider,DisabledProvider).tests/Global/CanBeAutofocusTest.php (1)
62-67: LGTM!Parameter types correctly widened to
bool|nullto match the updated provider data covering null scenarios.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Line 58: Update the changelog entry for Bug `#85` to be more specific: replace
the vague line "Better naming prefix for boolean attributes" with a descriptive
sentence referencing the concrete change (the introduction/standardization of
the "CanBe" prefix for boolean attributes), e.g. "Bug `#85`: Standardize boolean
attribute names to use the 'CanBe' prefix (`@terabytesoftw`)"; ensure the entry
still includes the bug number and author tag and matches the level of
detail/format used by other entries like those around lines with similar
entries.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: linter / Super Linter
- GitHub Check: linter / Super Linter
1 participant
Pull Request