Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 46 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,55 @@ This command will:
### Other available commands
Check the `composer.json` file to see all available commands.

## Code Style Conventions

### Variable Naming

- **Reflection variables**: Use `$reflClass` for `ClassReflection` objects (not `$classReflection`)
- **Method reflection**: Use `$reflMethod` for method reflections
- **Property reflection**: Use descriptive names like `$propertyRefl`
- **Attribute nodes**: Use `$attribute` for single attributes, `$livePropAttribute` for specific ones
- **Other nodes**: Use descriptive names like `$method`, `$property`, `$node`

### PHPDoc Comments

- **Public methods**: Always include `@implements Rule<Class_>` or similar on rule classes
- **Private methods**: Always add a brief description comment explaining the method's purpose
- **Return types**: Document complex return types with `@return` (e.g., `@return array{name: string, custom: bool}|null`)
- **Parameters**: Document array parameters with `@param string[]` or similar when applicable

### Error Messages

- **Modal verbs**: Use "must" for requirements (e.g., "Method must be public"), not "should"
- **Consistency**: Be consistent with message structure across similar rules
- **Tips**: Always provide actionable tips with `->tip()` to guide users toward solutions
- **Formatting**: Use double quotes for strings in sprintf, backticks in markdown for code

### Code Structure

- **Early returns**: Check for attribute presence first, return empty array if not found
- **Null checks**: Check for null/undefined values before proceeding
- **Error collection**: Use `$errors = []` array and collect all errors before returning
- **Method order**:
1. `getNodeType()`
2. `processNode()`
3. Private helper methods (alphabetically sorted if multiple)

### Validation Order in `processNode()`

1. Check if node has required attribute (return `[]` if not)
2. Check if `namespacedName` exists (when needed)
3. Initialize error array: `$errors = []`
4. Get reflection class if needed: `$reflClass = $this->reflectionProvider->getClass(...)`
5. Iterate over relevant elements (methods, properties, etc.)
6. Perform validations and collect errors
7. Return `$errors`

## Best Practices

1. **Naming**: Rules should have a descriptive name and end with `Rule`
2. **Identifiers**: Use the format `symfonyUX.twigComponent.descriptiveName` for error identifiers
3. **Clear messages**: Error messages should be explicit and include a `tip()` with a suggestion
2. **Identifiers**: Use the format `symfonyUX.twigComponent.descriptiveName` or `symfonyUX.liveComponent.descriptiveName` for error identifiers
3. **Clear messages**: Error messages must be explicit and include a `tip()` with a suggestion
4. **Complete tests**: Always test valid cases, invalid cases, and non-components
5. **Documentation**: Document each rule in the README with concrete examples
6. **Validation**: Always run `symfony composer qa-fix` before committing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public function processNode(Node $node, Scope $scope): array
$methodName = $method->name->toString();

$errors[] = RuleErrorBuilder::message(
sprintf('LiveAction method "%s()" should be public.', $methodName)
sprintf('LiveAction method "%s()" must be public.', $methodName)
)
->identifier('symfonyUX.liveComponent.liveActionMethodsShouldBePublic')
->line($method->getLine())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function processNode(Node $node, Scope $scope): array

if (! $method->isPublic()) {
$errors[] = RuleErrorBuilder::message(sprintf(
'LiveListener method "%s()" should be public.',
'LiveListener method "%s()" must be public.',
$method->name->toString()
))
->identifier('symfonyUX.liveComponent.liveListenerMethodShouldBePublic')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public function processNode(Node $node, Scope $scope): array
$classNameString = $className->toString();
if (str_ends_with($classNameString, 'Component')) {
return [
RuleErrorBuilder::message(sprintf('Twig component class "%s" should not end with "Component".', $classNameString))
RuleErrorBuilder::message(sprintf('Twig component class "%s" must not end with "Component".', $classNameString))
->identifier('symfonyUX.twigComponent.classNameShouldNotEndWithComponent')
->line($className->getLine())
->tip('Remove the "Component" suffix from the class name.')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

/**
* Extract the exposePublicProps value from the component attribute.
*/
private function getExposePublicPropsValue(Node\Attribute $attribute): ?bool
{
foreach ($attribute->args as $arg) {
Expand Down
2 changes: 2 additions & 0 deletions src/Rules/TwigComponent/ForbiddenAttributesPropertyRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ public function processNode(Node $node, Scope $scope): array
}

/**
* Extract the attributes variable name from the component attribute.
*
* @return array{name: string, custom: bool}|null
*/
private function getAttributesVarName(Node\Attribute $attribute): ?array
Expand Down
14 changes: 7 additions & 7 deletions src/Rules/TwigComponent/MethodsShouldBePublicOrPrivateRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ public function processNode(Node $node, Scope $scope): array
}

$errors = [];
$classReflection = $this->reflectionProvider->getClass($className);
$abstractTraitMethods = $this->getAbstractTraitMethods($classReflection);
$reflClass = $this->reflectionProvider->getClass($className);
$abstractTraitMethods = $this->getAbstractTraitMethods($reflClass);

foreach ($node->getMethods() as $method) {
if (! $method->isProtected()) {
Expand All @@ -62,7 +62,7 @@ public function processNode(Node $node, Scope $scope): array
}

$errors[] = RuleErrorBuilder::message(
sprintf('Method "%s()" in a Twig component should not be protected.', $methodName)
sprintf('Method "%s()" in a Twig component must not be protected.', $methodName)
)
->identifier('symfonyUX.twigComponent.methodsShouldBePublicOrPrivate')
->line($method->getLine())
Expand All @@ -77,7 +77,7 @@ public function processNode(Node $node, Scope $scope): array
}

$reportedTraitMethods = [];
foreach ($classReflection->getTraits() as $traitReflection) {
foreach ($reflClass->getTraits() as $traitReflection) {
// Use native reflection to get trait methods
foreach ($traitReflection->getNativeReflection()->getMethods() as $traitMethod) {
if (! $traitMethod->isProtected()) {
Expand Down Expand Up @@ -111,7 +111,7 @@ public function processNode(Node $node, Scope $scope): array
}

$errors[] = RuleErrorBuilder::message(
sprintf('Method "%s()" in a Twig component should not be protected.', $methodName)
sprintf('Method "%s()" in a Twig component must not be protected.', $methodName)
)
->identifier('symfonyUX.twigComponent.methodsShouldBePublicOrPrivate')
->line($lineNumber)
Expand All @@ -128,11 +128,11 @@ public function processNode(Node $node, Scope $scope): array
*
* @return array<string, true>
*/
private function getAbstractTraitMethods(ClassReflection $classReflection): array
private function getAbstractTraitMethods(ClassReflection $reflClass): array
{
$abstractTraitMethods = [];

foreach ($classReflection->getTraits() as $traitReflection) {
foreach ($reflClass->getTraits() as $traitReflection) {
foreach ($traitReflection->getNativeReflection()->getMethods() as $method) {
if ($method->isAbstract()) {
$abstractTraitMethods[$method->getName()] = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public function processNode(Node $node, Scope $scope): array

if (! $this->isCamelCase($propertyName)) {
$errors[] = RuleErrorBuilder::message(
sprintf('Public property "%s" in a Twig component should be in camelCase.', $propertyName)
sprintf('Public property "%s" in a Twig component must be in camelCase.', $propertyName)
)
->identifier('symfonyUX.twigComponent.publicPropertiesShouldBeCamelCase')
->line($property->getLine())
Expand All @@ -54,12 +54,18 @@ public function processNode(Node $node, Scope $scope): array
return $errors;
}

/**
* Check if a property name is in camelCase format.
*/
private function isCamelCase(string $name): bool
{
// Property should start with a lowercase letter and contain no underscores
return preg_match('/^[a-z][a-zA-Z0-9]*$/', $name) === 1;
}

/**
* Convert a string to camelCase format.
*/
private function toCamelCase(string $name): string
{
// Convert snake_case or PascalCase to camelCase
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public function testViolations(): void
[__DIR__ . '/Fixture/LiveComponentWithPrivateLiveAction.php'],
[
[
'LiveAction method "save()" should be public.',
'LiveAction method "save()" must be public.',
15,
'Methods annotated with #[LiveAction] must be public to be accessible as component actions.',
],
Expand All @@ -30,7 +30,7 @@ public function testViolations(): void
[__DIR__ . '/Fixture/LiveComponentWithProtectedLiveAction.php'],
[
[
'LiveAction method "delete()" should be public.',
'LiveAction method "delete()" must be public.',
15,
'Methods annotated with #[LiveAction] must be public to be accessible as component actions.',
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ public function testViolations(): void
[__DIR__ . '/Fixture/PrivateLiveListener.php'],
[
[
'LiveListener method "onAnotherEvent()" should be public.',
'LiveListener method "onAnotherEvent()" must be public.',
13,
'Change the method visibility to public.',
],
[
'LiveListener method "onSomeEvent()" should be public.',
'LiveListener method "onSomeEvent()" must be public.',
18,
'Change the method visibility to public.',
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public function testViolations(): void
[__DIR__ . '/Fixture/AlertComponent.php'],
[
[
'Twig component class "AlertComponent" should not end with "Component".',
'Twig component class "AlertComponent" must not end with "Component".',
10,
'Remove the "Component" suffix from the class name.',
],
Expand All @@ -30,7 +30,7 @@ public function testViolations(): void
[__DIR__ . '/Fixture/CounterComponent.php'],
[
[
'Twig component class "CounterComponent" should not end with "Component".',
'Twig component class "CounterComponent" must not end with "Component".',
10,
'Remove the "Component" suffix from the class name.',
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public function testViolations(): void
[__DIR__ . '/Fixture/ComponentWithProtectedMethod.php'],
[
[
'Method "protectedMethod()" in a Twig component should not be protected.',
'Method "protectedMethod()" in a Twig component must not be protected.',
14,
'Twig component methods should be either public or private, not protected.',
],
Expand All @@ -30,7 +30,7 @@ public function testViolations(): void
[__DIR__ . '/Fixture/LiveComponentWithProtectedMethod.php'],
[
[
'Method "protectedMethod()" in a Twig component should not be protected.',
'Method "protectedMethod()" in a Twig component must not be protected.',
14,
'Twig component methods should be either public or private, not protected.',
],
Expand All @@ -42,7 +42,7 @@ public function testViolations(): void
[__DIR__ . '/Fixture/ComponentWithProtectedConcreteMethodFromTrait.php'],
[
[
'Method "protectedConcreteTraitMethod()" in a Twig component should not be protected.',
'Method "protectedConcreteTraitMethod()" in a Twig component must not be protected.',
12,
'Twig component methods should be either public or private, not protected.',
],
Expand All @@ -53,7 +53,7 @@ public function testViolations(): void
[__DIR__ . '/Fixture/LiveComponentWithProtectedConcreteMethodFromTrait.php'],
[
[
'Method "protectedConcreteTraitMethod()" in a Twig component should not be protected.',
'Method "protectedConcreteTraitMethod()" in a Twig component must not be protected.',
12,
'Twig component methods should be either public or private, not protected.',
],
Expand All @@ -65,7 +65,7 @@ public function testViolations(): void
[__DIR__ . '/Fixture/ComponentWithDuplicateTraitMethods.php'],
[
[
'Method "duplicateConcreteMethod()" in a Twig component should not be protected.',
'Method "duplicateConcreteMethod()" in a Twig component must not be protected.',
12,
'Twig component methods should be either public or private, not protected.',
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ public function testViolations(): void
[__DIR__ . '/Fixture/ComponentWithSnakeCaseProperty.php'],
[
[
'Public property "user_name" in a Twig component should be in camelCase.',
'Public property "user_name" in a Twig component must be in camelCase.',
12,
'Consider renaming "user_name" to "userName".',
],
[
'Public property "is_active" in a Twig component should be in camelCase.',
'Public property "is_active" in a Twig component must be in camelCase.',
14,
'Consider renaming "is_active" to "isActive".',
],
Expand All @@ -35,7 +35,7 @@ public function testViolations(): void
[__DIR__ . '/Fixture/ComponentWithPascalCaseProperty.php'],
[
[
'Public property "UserName" in a Twig component should be in camelCase.',
'Public property "UserName" in a Twig component must be in camelCase.',
12,
'Consider renaming "UserName" to "userName".',
],
Expand All @@ -46,12 +46,12 @@ public function testViolations(): void
[__DIR__ . '/Fixture/LiveComponentWithSnakeCaseProperty.php'],
[
[
'Public property "user_name" in a Twig component should be in camelCase.',
'Public property "user_name" in a Twig component must be in camelCase.',
12,
'Consider renaming "user_name" to "userName".',
],
[
'Public property "is_active" in a Twig component should be in camelCase.',
'Public property "is_active" in a Twig component must be in camelCase.',
14,
'Consider renaming "is_active" to "isActive".',
],
Expand Down