fix: improve key naming convention detection for dot notation#60
fix: improve key naming convention detection for dot notation#60konradmichalik merged 2 commits intomainfrom
Conversation
WalkthroughThe Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/Command/ValidateTranslationCommand.php(2 hunks)src/Config/ConfigReader.php(1 hunks)src/Enum/KeyNamingConvention.php(2 hunks)src/Validator/KeyNamingConventionValidator.php(9 hunks)tests/src/Validator/KeyNamingConventionValidatorTest.php(5 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/Config/ConfigReader.php
- src/Command/ValidateTranslationCommand.php
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Validator/KeyNamingConventionValidator.php
- tests/src/Validator/KeyNamingConventionValidatorTest.php
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Enum/KeyNamingConvention.php (1)
src/Validator/KeyNamingConventionValidator.php (1)
getAvailableConventions(408-424)
🔇 Additional comments (3)
src/Enum/KeyNamingConvention.php (3)
65-78: LGTM! Well-structured validation with clear error messaging.The enhanced
fromStringmethod correctly implements the two-stage validation:
- Validates the convention exists as an enum value
- Explicitly rejects
dot.notationas non-configurableThe error messages appropriately use
getConfigurableConventions()to guide users toward valid options, which aligns with the design goal of treatingdot.notationas detection-only.
87-88: LGTM! Clean refactoring using static closure.The method now uses a more concise approach with
array_mapand a static closure while maintaining the same functionality - returning all enum values as strings includingdot.notation.
90-102: LGTM! Well-documented method with clear purpose.The new
getConfigurableConventionsmethod effectively separates the concerns:
- Provides a clean API for getting user-configurable conventions
- Excludes
dot.notationconsistently with the validation logic- Includes comprehensive documentation explaining why
dot.notationis excludedThe implementation correctly filters out
dot.notationwhile reusinggetAvailableConventions()for maintainability.
| * | ||
| * @return array<string> | ||
| */ | ||
| public static function getConfigurableConventions(): array |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify consistency with validator's getAvailableConventions method.
There appears to be a potential inconsistency between this enum's approach and the validator's getAvailableConventions method. The relevant code snippet shows the validator manually excluding dot.notation in a loop, while this enum provides a dedicated getConfigurableConventions method.
🏁 Script executed:
#!/bin/bash
# Description: Check if the validator should use the enum's getConfigurableConventions method
# Expected: The validator's getAvailableConventions method might be redundant now
# Search for usage of getAvailableConventions in the validator
rg -A 10 -B 5 "getAvailableConventions" src/Validator/KeyNamingConventionValidator.php
# Search for any other usages of getAvailableConventions that might need updating
rg -A 5 -B 5 "getAvailableConventions" --type phpLength of output: 6299
Use enum’s getConfigurableConventions in the validator for a single source of truth
The validator’s getAvailableConventions currently duplicates the “exclude dot.notation” logic that’s already encapsulated in KeyNamingConvention::getConfigurableConventions(). To avoid divergence and keep things DRY, switch the validator to use the enum’s method directly.
Locations to update:
- src/Validator/KeyNamingConventionValidator.php: replace the manual loop over KeyNamingConvention::cases() with KeyNamingConvention::getConfigurableConventions().
Proposed diff:
public static function getAvailableConventions(): array
{
- $conventions = [];
- foreach (KeyNamingConvention::cases() as $convention) {
- // Exclude dot.notation from configurable conventions
- if ('dot.notation' === $convention->value) {
- continue;
- }
-
- $conventions[$convention->value] = [
- 'pattern' => $convention->getPattern(),
- 'description' => $convention->description(),
- ];
- }
- return $conventions;
+ $conventions = [];
+ foreach (KeyNamingConvention::getConfigurableConventions() as $value) {
+ $convention = KeyNamingConvention::from($value);
+ $conventions[$value] = [
+ 'pattern' => $convention->getPattern(),
+ 'description' => $convention->description(),
+ ];
+ }
+ return $conventions;
}Tests covering getAvailableConventions will continue to pass without modification.
📝 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.
| public static function getConfigurableConventions(): array | |
| public static function getAvailableConventions(): array | |
| { | |
| $conventions = []; | |
| foreach (KeyNamingConvention::getConfigurableConventions() as $value) { | |
| $convention = KeyNamingConvention::from($value); | |
| $conventions[$value] = [ | |
| 'pattern' => $convention->getPattern(), | |
| 'description' => $convention->description(), | |
| ]; | |
| } | |
| return $conventions; | |
| } |
🤖 Prompt for AI Agents
In src/Validator/KeyNamingConventionValidator.php, replace the manual iteration
over KeyNamingConvention::cases() in the getAvailableConventions method with a
direct call to KeyNamingConvention::getConfigurableConventions() to reuse the
existing logic and avoid duplication. This change ensures the validator uses the
enum’s single source of truth for configurable conventions.
Summary by CodeRabbit
Bug Fixes
New Features
Tests