From a2e9ba259bede0d17e3f718f9065c1f2dbf66731 Mon Sep 17 00:00:00 2001 From: Simon Bigelmayr Date: Fri, 5 Dec 2025 16:07:52 +0100 Subject: [PATCH 01/11] feat(phpstan): add CanonicalCapitalizationRule for enforcing term spelling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add abstract CanonicalCapitalizationRule base class that checks: - Variable names - Parameter names - Method names - Class names - String literals Add LabIDCapitalizationRule to enforce "Lab ID" spelling over variants like LabID, labID, LABID, Labid, Lab-ID, Lab Id, etc. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../Rules/CanonicalCapitalizationRule.php | 109 ++++++++++++++++++ src/PHPStan/Rules/LabIDCapitalizationRule.php | 41 +++++++ tests/PHPStan/LabIDCapitalizationRuleTest.php | 58 ++++++++++ 3 files changed, 208 insertions(+) create mode 100644 src/PHPStan/Rules/CanonicalCapitalizationRule.php create mode 100644 src/PHPStan/Rules/LabIDCapitalizationRule.php create mode 100644 tests/PHPStan/LabIDCapitalizationRuleTest.php diff --git a/src/PHPStan/Rules/CanonicalCapitalizationRule.php b/src/PHPStan/Rules/CanonicalCapitalizationRule.php new file mode 100644 index 0000000..3acab2e --- /dev/null +++ b/src/PHPStan/Rules/CanonicalCapitalizationRule.php @@ -0,0 +1,109 @@ + */ +abstract class CanonicalCapitalizationRule implements Rule +{ + /** The correct canonical form, e.g. "Lab ID". */ + abstract protected function getCanonicalForm(): string; + + /** @return array Wrong variants to detect, e.g. ['LabID', 'labID', 'LABID']. */ + abstract protected function getWrongVariants(): array; + + abstract protected function getErrorIdentifier(): string; + + public function getNodeType(): string + { + return Node::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $nodeName = $this->extractName($node); + + if ($nodeName === null) { + return []; + } + + $wrongVariant = $this->findWrongVariant($nodeName); + + if ($wrongVariant === null) { + return []; + } + + $expectedName = $this->fixCapitalization($nodeName, $wrongVariant); + $displayName = $this->formatNameForMessage($node, $nodeName); + $displayExpectedName = $this->formatNameForMessage($node, $expectedName); + + return [ + RuleErrorBuilder::message(<<getType()} "{$displayName}" should use "{$this->getCanonicalForm()}" instead of "{$wrongVariant}", rename it to "{$displayExpectedName}". + TXT) + ->identifier($this->getErrorIdentifier()) + ->build(), + ]; + } + + protected function extractName(Node $node): ?string + { + if ($node instanceof Variable && is_string($node->name)) { + return $node->name; + } + + if ($node instanceof Param && $node->var instanceof Variable && is_string($node->var->name)) { + return $node->var->name; + } + + if ($node instanceof ClassMethod) { + return $node->name->name; + } + + if ($node instanceof Class_ && $node->name instanceof Identifier) { + return $node->name->name; + } + + if ($node instanceof String_) { + return $node->value; + } + + return null; + } + + protected function formatNameForMessage(Node $node, string $name): string + { + if ($node instanceof Variable || $node instanceof Param) { + return '$' . $name; + } + + return $name; + } + + protected function findWrongVariant(string $nodeName): ?string + { + foreach ($this->getWrongVariants() as $wrongVariant) { + if (Str::contains($nodeName, $wrongVariant)) { + return $wrongVariant; + } + } + + return null; + } + + protected function fixCapitalization(string $nodeName, string $wrongVariant): string + { + return str_replace($wrongVariant, $this->getCanonicalForm(), $nodeName); + } +} diff --git a/src/PHPStan/Rules/LabIDCapitalizationRule.php b/src/PHPStan/Rules/LabIDCapitalizationRule.php new file mode 100644 index 0000000..0f428c7 --- /dev/null +++ b/src/PHPStan/Rules/LabIDCapitalizationRule.php @@ -0,0 +1,41 @@ + */ + protected function getWrongVariants(): array + { + return [ + 'LabID', + 'labID', + 'LABID', + 'Labid', + 'labid', + 'Lab-ID', + 'lab-Id', + 'Lab Id', + 'lab id', + ]; + } + + protected function getErrorIdentifier(): string + { + return 'mll.labIDCapitalization'; + } +} diff --git a/tests/PHPStan/LabIDCapitalizationRuleTest.php b/tests/PHPStan/LabIDCapitalizationRuleTest.php new file mode 100644 index 0000000..3392f89 --- /dev/null +++ b/tests/PHPStan/LabIDCapitalizationRuleTest.php @@ -0,0 +1,58 @@ +rule = new LabIDCapitalizationRule(); + } + + /** @dataProvider wrongCapitalizations */ + #[DataProvider('wrongCapitalizations')] + public function testDetectsWrongCapitalizations(string $input, string $wrongVariant, string $expected): void + { + $findWrongVariant = new \ReflectionMethod($this->rule, 'findWrongVariant'); + $fixCapitalization = new \ReflectionMethod($this->rule, 'fixCapitalization'); + + self::assertSame($wrongVariant, $findWrongVariant->invoke($this->rule, $input)); + self::assertSame($expected, $fixCapitalization->invoke($this->rule, $input, $wrongVariant)); + } + + /** @return iterable */ + public static function wrongCapitalizations(): iterable + { + yield 'LabID in variable' => ['getLabID', 'LabID', 'getLab ID']; + yield 'labID in variable' => ['labIDValue', 'labID', 'Lab IDValue']; + yield 'LABID uppercase' => ['LABID_CONSTANT', 'LABID', 'Lab ID_CONSTANT']; + yield 'Labid mixed' => ['Labid', 'Labid', 'Lab ID']; + yield 'Lab-ID with hyphen' => ['Lab-ID', 'Lab-ID', 'Lab ID']; + yield 'Lab Id wrong case' => ['Lab Id', 'Lab Id', 'Lab ID']; + yield 'In string literal' => ['The LabID is wrong', 'LabID', 'The Lab ID is wrong']; + } + + /** @dataProvider correctCapitalizations */ + #[DataProvider('correctCapitalizations')] + public function testAllowsCorrectCapitalizations(string $input): void + { + $findWrongVariant = new \ReflectionMethod($this->rule, 'findWrongVariant'); + + self::assertNull($findWrongVariant->invoke($this->rule, $input)); + } + + /** @return iterable */ + public static function correctCapitalizations(): iterable + { + yield 'Correct Lab ID' => ['Lab ID']; + yield 'In sentence' => ['The Lab ID is correct']; + yield 'labID as variable (camelCase)' => ['labId']; // This is ID capitalization, not Lab ID + yield 'Unrelated' => ['Laboratory']; + } +} From 79f642a681f5efd8764aff056e726e7be34a05a0 Mon Sep 17 00:00:00 2001 From: Simon Bigelmayr Date: Fri, 5 Dec 2025 16:37:29 +0100 Subject: [PATCH 02/11] refactor(phpstan): limit CanonicalCapitalizationRule to string literals only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per coding guidelines: "Unless the circumstances demand otherwise (all lower, no spaces, ...)" - canonical forms like "Lab ID" with spaces only apply to user-facing text in string literals, not to PHP identifiers (variables, methods, classes) where spaces are invalid. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../Rules/CanonicalCapitalizationRule.php | 75 +++++-------------- tests/PHPStan/LabIDCapitalizationRuleTest.php | 17 ++--- 2 files changed, 27 insertions(+), 65 deletions(-) diff --git a/src/PHPStan/Rules/CanonicalCapitalizationRule.php b/src/PHPStan/Rules/CanonicalCapitalizationRule.php index 3acab2e..2cfe76d 100644 --- a/src/PHPStan/Rules/CanonicalCapitalizationRule.php +++ b/src/PHPStan/Rules/CanonicalCapitalizationRule.php @@ -4,17 +4,20 @@ use Illuminate\Support\Str; use PhpParser\Node; -use PhpParser\Node\Expr\Variable; -use PhpParser\Node\Identifier; -use PhpParser\Node\Param; use PhpParser\Node\Scalar\String_; -use PhpParser\Node\Stmt\Class_; -use PhpParser\Node\Stmt\ClassMethod; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -/** @implements Rule */ +/** + * Enforces canonical capitalization in string literals. + * + * Only checks string literals because canonical forms like "Lab ID" contain spaces, + * which are invalid in PHP identifiers (variables, methods, classes). + * Per coding guidelines: "Unless the circumstances demand otherwise (all lower, no spaces, ...)". + * + * @implements Rule + */ abstract class CanonicalCapitalizationRule implements Rule { /** The correct canonical form, e.g. "Lab ID". */ @@ -27,74 +30,34 @@ abstract protected function getErrorIdentifier(): string; public function getNodeType(): string { - return Node::class; + return String_::class; } + /** @param String_ $node */ public function processNode(Node $node, Scope $scope): array { - $nodeName = $this->extractName($node); - - if ($nodeName === null) { - return []; - } - - $wrongVariant = $this->findWrongVariant($nodeName); + $value = $node->value; + $wrongVariant = $this->findWrongVariant($value); if ($wrongVariant === null) { return []; } - $expectedName = $this->fixCapitalization($nodeName, $wrongVariant); - $displayName = $this->formatNameForMessage($node, $nodeName); - $displayExpectedName = $this->formatNameForMessage($node, $expectedName); + $expectedValue = $this->fixCapitalization($value, $wrongVariant); return [ RuleErrorBuilder::message(<<getType()} "{$displayName}" should use "{$this->getCanonicalForm()}" instead of "{$wrongVariant}", rename it to "{$displayExpectedName}". + String "{$value}" should use "{$this->getCanonicalForm()}" instead of "{$wrongVariant}", change it to "{$expectedValue}". TXT) ->identifier($this->getErrorIdentifier()) ->build(), ]; } - protected function extractName(Node $node): ?string - { - if ($node instanceof Variable && is_string($node->name)) { - return $node->name; - } - - if ($node instanceof Param && $node->var instanceof Variable && is_string($node->var->name)) { - return $node->var->name; - } - - if ($node instanceof ClassMethod) { - return $node->name->name; - } - - if ($node instanceof Class_ && $node->name instanceof Identifier) { - return $node->name->name; - } - - if ($node instanceof String_) { - return $node->value; - } - - return null; - } - - protected function formatNameForMessage(Node $node, string $name): string - { - if ($node instanceof Variable || $node instanceof Param) { - return '$' . $name; - } - - return $name; - } - - protected function findWrongVariant(string $nodeName): ?string + protected function findWrongVariant(string $value): ?string { foreach ($this->getWrongVariants() as $wrongVariant) { - if (Str::contains($nodeName, $wrongVariant)) { + if (Str::contains($value, $wrongVariant)) { return $wrongVariant; } } @@ -102,8 +65,8 @@ protected function findWrongVariant(string $nodeName): ?string return null; } - protected function fixCapitalization(string $nodeName, string $wrongVariant): string + protected function fixCapitalization(string $value, string $wrongVariant): string { - return str_replace($wrongVariant, $this->getCanonicalForm(), $nodeName); + return str_replace($wrongVariant, $this->getCanonicalForm(), $value); } } diff --git a/tests/PHPStan/LabIDCapitalizationRuleTest.php b/tests/PHPStan/LabIDCapitalizationRuleTest.php index 3392f89..73356c5 100644 --- a/tests/PHPStan/LabIDCapitalizationRuleTest.php +++ b/tests/PHPStan/LabIDCapitalizationRuleTest.php @@ -29,13 +29,12 @@ public function testDetectsWrongCapitalizations(string $input, string $wrongVari /** @return iterable */ public static function wrongCapitalizations(): iterable { - yield 'LabID in variable' => ['getLabID', 'LabID', 'getLab ID']; - yield 'labID in variable' => ['labIDValue', 'labID', 'Lab IDValue']; - yield 'LABID uppercase' => ['LABID_CONSTANT', 'LABID', 'Lab ID_CONSTANT']; - yield 'Labid mixed' => ['Labid', 'Labid', 'Lab ID']; - yield 'Lab-ID with hyphen' => ['Lab-ID', 'Lab-ID', 'Lab ID']; - yield 'Lab Id wrong case' => ['Lab Id', 'Lab Id', 'Lab ID']; - yield 'In string literal' => ['The LabID is wrong', 'LabID', 'The Lab ID is wrong']; + yield 'LabID' => ['The LabID is wrong', 'LabID', 'The Lab ID is wrong']; + yield 'labID' => ['Your labID was submitted', 'labID', 'Your Lab ID was submitted']; + yield 'LABID' => ['LABID not found', 'LABID', 'Lab ID not found']; + yield 'Labid' => ['Labid missing', 'Labid', 'Lab ID missing']; + yield 'Lab-ID with hyphen' => ['Enter Lab-ID here', 'Lab-ID', 'Enter Lab ID here']; + yield 'Lab Id wrong case' => ['Lab Id is required', 'Lab Id', 'Lab ID is required']; } /** @dataProvider correctCapitalizations */ @@ -52,7 +51,7 @@ public static function correctCapitalizations(): iterable { yield 'Correct Lab ID' => ['Lab ID']; yield 'In sentence' => ['The Lab ID is correct']; - yield 'labID as variable (camelCase)' => ['labId']; // This is ID capitalization, not Lab ID - yield 'Unrelated' => ['Laboratory']; + yield 'labId lowercase' => ['labId']; // This is ID capitalization, not Lab ID + yield 'Unrelated word' => ['Laboratory']; } } From 9f0874afcec01e054adcfdef94df0c219e4d0eae Mon Sep 17 00:00:00 2001 From: Simon Bigelmayr Date: Fri, 5 Dec 2025 16:42:46 +0100 Subject: [PATCH 03/11] refactor(phpstan): use @lang GraphQL annotation to skip GraphQL queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of checking for braces, detect the /* @lang GraphQL */ IDE annotation which explicitly marks GraphQL query strings. This is more precise and follows the existing convention in the codebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../Rules/CanonicalCapitalizationRule.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/PHPStan/Rules/CanonicalCapitalizationRule.php b/src/PHPStan/Rules/CanonicalCapitalizationRule.php index 2cfe76d..6a1db20 100644 --- a/src/PHPStan/Rules/CanonicalCapitalizationRule.php +++ b/src/PHPStan/Rules/CanonicalCapitalizationRule.php @@ -36,6 +36,10 @@ public function getNodeType(): string /** @param String_ $node */ public function processNode(Node $node, Scope $scope): array { + if ($this->hasGraphQLAnnotation($node)) { + return []; + } + $value = $node->value; $wrongVariant = $this->findWrongVariant($value); @@ -54,6 +58,18 @@ public function processNode(Node $node, Scope $scope): array ]; } + /** GraphQL queries are annotated with @lang GraphQL and use field names that can't contain spaces. */ + protected function hasGraphQLAnnotation(String_ $node): bool + { + foreach ($node->getComments() as $comment) { + if (Str::contains($comment->getText(), '@lang GraphQL')) { + return true; + } + } + + return false; + } + protected function findWrongVariant(string $value): ?string { foreach ($this->getWrongVariants() as $wrongVariant) { From 5601bf30b79f8e85ad6ad9899472f6ca78c9c985 Mon Sep 17 00:00:00 2001 From: Simon Bigelmayr Date: Fri, 5 Dec 2025 16:46:29 +0100 Subject: [PATCH 04/11] test(phpstan): add integration test for LabIDCapitalizationRule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests that: - Wrong capitalizations in strings are detected (LabID -> Lab ID) - Correct capitalizations are ignored - GraphQL queries with @lang annotation are skipped - GraphQL queries without annotation are still checked 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- phpstan.neon | 10 ++ ...LabIDCapitalizationRuleIntegrationTest.php | 98 +++++++++++++++++++ tests/PHPStan/data/lab-id-capitalization.php | 38 +++++++ tests/PHPStan/phpstan-test.neon | 5 + 4 files changed, 151 insertions(+) create mode 100644 tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php create mode 100644 tests/PHPStan/data/lab-id-capitalization.php create mode 100644 tests/PHPStan/phpstan-test.neon diff --git a/phpstan.neon b/phpstan.neon index 59a9b70..2340768 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -30,3 +30,13 @@ parameters: - message: '#Enumerations are only allowed since PHP 8\.1\.#' paths: - tests/Enum + + # Test fixtures intentionally contain Lab ID capitalization violations + - message: '#should use "Lab ID" instead of#' + paths: + - tests/PHPStan/data/ + + # PHPStan internal API usage is acceptable in tests + - message: '#is not covered by backward compatibility promise#' + paths: + - tests/PHPStan/ diff --git a/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php b/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php new file mode 100644 index 0000000..aaca69d --- /dev/null +++ b/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php @@ -0,0 +1,98 @@ +runAnalyse(__DIR__ . '/data/lab-id-capitalization.php'); + + $labIDErrors = array_filter( + $errors, + static fn (Error $error): bool => str_contains($error->getMessage(), 'should use "Lab ID"'), + ); + + self::assertNotEmpty($labIDErrors, 'Should detect wrong capitalization in strings'); + + $errorLines = array_map( + static fn (Error $error): int => $error->getLine() ?? 0, + $labIDErrors, + ); + + // Line 9: 'The LabID is wrong' - should be detected + self::assertContains(9, $errorLines, 'Should detect LabID on line 9'); + + // Line 30-35: GraphQL query WITHOUT @lang annotation - should be detected + self::assertTrue( + count(array_filter($errorLines, static fn (int $line): bool => $line >= 29 && $line <= 36)) > 0, + 'Should detect labID in GraphQL query without @lang annotation', + ); + } + + public function testIgnoresGraphQLQueryWithAnnotation(): void + { + $errors = $this->runAnalyse(__DIR__ . '/data/lab-id-capitalization.php'); + + $labIDErrors = array_filter( + $errors, + static fn (Error $error): bool => str_contains($error->getMessage(), 'should use "Lab ID"'), + ); + + $errorLines = array_map( + static fn (Error $error): int => $error->getLine() ?? 0, + $labIDErrors, + ); + + // Lines 18-25: GraphQL query WITH @lang annotation - should NOT be detected + $graphqlAnnotatedErrors = array_filter( + $errorLines, + static fn (int $line): bool => $line >= 18 && $line <= 25, + ); + + self::assertEmpty($graphqlAnnotatedErrors, 'Should ignore GraphQL query with @lang annotation'); + } + + public function testIgnoresCorrectCapitalization(): void + { + $errors = $this->runAnalyse(__DIR__ . '/data/lab-id-capitalization.php'); + + $labIDErrors = array_filter( + $errors, + static fn (Error $error): bool => str_contains($error->getMessage(), 'should use "Lab ID"'), + ); + + $errorLines = array_map( + static fn (Error $error): int => $error->getLine() ?? 0, + $labIDErrors, + ); + + // Line 14: 'The Lab ID is correct' - should NOT be detected + self::assertNotContains(14, $errorLines, 'Should not report correct Lab ID capitalization'); + } + + /** @return array */ + private function runAnalyse(string $file): array + { + $file = self::getFileHelper()->normalizePath($file); + + /** @var Analyser $analyser */ + $analyser = self::getContainer()->getByType(Analyser::class); + + $result = $analyser->analyse([$file]); + + return $result->getErrors(); + } + + /** @return array */ + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/phpstan-test.neon', + ]; + } +} diff --git a/tests/PHPStan/data/lab-id-capitalization.php b/tests/PHPStan/data/lab-id-capitalization.php new file mode 100644 index 0000000..fba1304 --- /dev/null +++ b/tests/PHPStan/data/lab-id-capitalization.php @@ -0,0 +1,38 @@ + Date: Fri, 5 Dec 2025 16:51:24 +0100 Subject: [PATCH 05/11] feat(phpstan): skip identifier-like strings in CanonicalCapitalizationRule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strings that look like valid identifiers (matching ^[a-zA-Z_][a-zA-Z0-9_]*$) are likely array keys, API field names, or database columns where spaces are not valid. Skip these to focus on user-facing text. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../Rules/CanonicalCapitalizationRule.php | 11 ++++++++++ ...LabIDCapitalizationRuleIntegrationTest.php | 22 +++++++++++++++++++ tests/PHPStan/data/lab-id-capitalization.php | 18 +++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/src/PHPStan/Rules/CanonicalCapitalizationRule.php b/src/PHPStan/Rules/CanonicalCapitalizationRule.php index 6a1db20..f019e58 100644 --- a/src/PHPStan/Rules/CanonicalCapitalizationRule.php +++ b/src/PHPStan/Rules/CanonicalCapitalizationRule.php @@ -41,6 +41,11 @@ public function processNode(Node $node, Scope $scope): array } $value = $node->value; + + if ($this->looksLikeIdentifier($value)) { + return []; + } + $wrongVariant = $this->findWrongVariant($value); if ($wrongVariant === null) { @@ -58,6 +63,12 @@ public function processNode(Node $node, Scope $scope): array ]; } + /** Identifiers (array keys, field names) can't contain spaces, so skip them. */ + protected function looksLikeIdentifier(string $value): bool + { + return \Safe\preg_match('/^[a-zA-Z_][a-zA-Z0-9_]*$/', $value) === 1; + } + /** GraphQL queries are annotated with @lang GraphQL and use field names that can't contain spaces. */ protected function hasGraphQLAnnotation(String_ $node): bool { diff --git a/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php b/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php index aaca69d..3fb35df 100644 --- a/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php +++ b/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php @@ -75,6 +75,28 @@ public function testIgnoresCorrectCapitalization(): void self::assertNotContains(14, $errorLines, 'Should not report correct Lab ID capitalization'); } + public function testIgnoresIdentifierStrings(): void + { + $errors = $this->runAnalyse(__DIR__ . '/data/lab-id-capitalization.php'); + + $labIDErrors = array_filter( + $errors, + static fn (Error $error): bool => str_contains($error->getMessage(), 'should use "Lab ID"'), + ); + + $errorLines = array_map( + static fn (Error $error): int => $error->getLine() ?? 0, + $labIDErrors, + ); + + // Lines 44-45: Array keys 'labID', 'LabID' - should NOT be detected (identifier-like) + self::assertNotContains(44, $errorLines, 'Should not report array key labID'); + self::assertNotContains(45, $errorLines, 'Should not report array key LabID'); + + // Line 52: Single identifier string 'labID' - should NOT be detected + self::assertNotContains(52, $errorLines, 'Should not report identifier string labID'); + } + /** @return array */ private function runAnalyse(string $file): array { diff --git a/tests/PHPStan/data/lab-id-capitalization.php b/tests/PHPStan/data/lab-id-capitalization.php index fba1304..7e63f5f 100644 --- a/tests/PHPStan/data/lab-id-capitalization.php +++ b/tests/PHPStan/data/lab-id-capitalization.php @@ -35,4 +35,22 @@ public function graphqlQueryWithoutAnnotationIsChecked(): string } '; } + + /** @return array */ + public function arrayKeysAreIgnored(): array + { + // Array keys that look like identifiers should be ignored + return [ + 'labID' => 'some value', + 'LabID' => 'another value', + ]; + } + + public function identifierStringsAreIgnored(): string + { + // Single identifier strings (no spaces) should be ignored + $key = 'labID'; + + return $key; + } } From 6b22bdd591b1e6e18c0529f5ab72f6169605ec83 Mon Sep 17 00:00:00 2001 From: Simon Bigelmayr Date: Fri, 5 Dec 2025 20:06:08 +0100 Subject: [PATCH 06/11] feat(phpstan): skip strings with @lang annotation in capitalization rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generalize GraphQL annotation check to skip any @lang annotated string (SQL, GraphQL, RegExp, etc.) since these contain identifiers that cannot have spaces. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../Rules/CanonicalCapitalizationRule.php | 12 +++-- ...LabIDCapitalizationRuleIntegrationTest.php | 44 +++++++++++++++++++ tests/PHPStan/data/lab-id-capitalization.php | 17 +++++++ 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/src/PHPStan/Rules/CanonicalCapitalizationRule.php b/src/PHPStan/Rules/CanonicalCapitalizationRule.php index f019e58..f6a071e 100644 --- a/src/PHPStan/Rules/CanonicalCapitalizationRule.php +++ b/src/PHPStan/Rules/CanonicalCapitalizationRule.php @@ -36,7 +36,7 @@ public function getNodeType(): string /** @param String_ $node */ public function processNode(Node $node, Scope $scope): array { - if ($this->hasGraphQLAnnotation($node)) { + if ($this->hasLanguageAnnotation($node)) { return []; } @@ -69,11 +69,15 @@ protected function looksLikeIdentifier(string $value): bool return \Safe\preg_match('/^[a-zA-Z_][a-zA-Z0-9_]*$/', $value) === 1; } - /** GraphQL queries are annotated with @lang GraphQL and use field names that can't contain spaces. */ - protected function hasGraphQLAnnotation(String_ $node): bool + /** + * Strings annotated with @lang (GraphQL, SQL, etc.) use field/column names that can't contain spaces. + * + * @see https://www.jetbrains.com/help/phpstorm/using-language-injections.html + */ + protected function hasLanguageAnnotation(String_ $node): bool { foreach ($node->getComments() as $comment) { - if (Str::contains($comment->getText(), '@lang GraphQL')) { + if (Str::contains($comment->getText(), '@lang')) { return true; } } diff --git a/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php b/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php index 3fb35df..e7325f0 100644 --- a/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php +++ b/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php @@ -97,6 +97,50 @@ public function testIgnoresIdentifierStrings(): void self::assertNotContains(52, $errorLines, 'Should not report identifier string labID'); } + public function testIgnoresSqlQueryWithAnnotation(): void + { + $errors = $this->runAnalyse(__DIR__ . '/data/lab-id-capitalization.php'); + + $labIDErrors = array_filter( + $errors, + static fn (Error $error): bool => str_contains($error->getMessage(), 'should use "Lab ID"'), + ); + + $errorLines = array_map( + static fn (Error $error): int => $error->getLine() ?? 0, + $labIDErrors, + ); + + // Lines 57-64: SQL query WITH @lang annotation - should NOT be detected + $sqlAnnotatedErrors = array_filter( + $errorLines, + static fn (int $line): bool => $line >= 57 && $line <= 64, + ); + + self::assertEmpty($sqlAnnotatedErrors, 'Should ignore SQL query with @lang annotation'); + } + + public function testDetectsSqlQueryWithoutAnnotation(): void + { + $errors = $this->runAnalyse(__DIR__ . '/data/lab-id-capitalization.php'); + + $labIDErrors = array_filter( + $errors, + static fn (Error $error): bool => str_contains($error->getMessage(), 'should use "Lab ID"'), + ); + + $errorLines = array_map( + static fn (Error $error): int => $error->getLine() ?? 0, + $labIDErrors, + ); + + // Lines 66-72: SQL query WITHOUT @lang annotation - should be detected + self::assertTrue( + count(array_filter($errorLines, static fn (int $line): bool => $line >= 66 && $line <= 72)) > 0, + 'Should detect labID in SQL query without @lang annotation', + ); + } + /** @return array */ private function runAnalyse(string $file): array { diff --git a/tests/PHPStan/data/lab-id-capitalization.php b/tests/PHPStan/data/lab-id-capitalization.php index 7e63f5f..d4404d1 100644 --- a/tests/PHPStan/data/lab-id-capitalization.php +++ b/tests/PHPStan/data/lab-id-capitalization.php @@ -53,4 +53,21 @@ public function identifierStringsAreIgnored(): string return $key; } + + public function sqlQueryIsIgnored(): string + { + return /* @lang SQL */ ' + SELECT exam_no AS labID + FROM examinations + WHERE labID > 1000 + '; + } + + public function sqlQueryWithoutAnnotationIsChecked(): string + { + return ' + SELECT exam_no AS labID + FROM examinations + '; + } } From f32c025aab7e096c641ca39c7413f4795e283440 Mon Sep 17 00:00:00 2001 From: Simon Bigelmayr Date: Fri, 5 Dec 2025 20:08:34 +0100 Subject: [PATCH 07/11] fix(tests): add setAccessible for PHP 7.4 compatibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ReflectionMethod requires setAccessible(true) to invoke protected methods in PHP < 8.1. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- tests/PHPStan/LabIDCapitalizationRuleTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/PHPStan/LabIDCapitalizationRuleTest.php b/tests/PHPStan/LabIDCapitalizationRuleTest.php index 73356c5..bbf755d 100644 --- a/tests/PHPStan/LabIDCapitalizationRuleTest.php +++ b/tests/PHPStan/LabIDCapitalizationRuleTest.php @@ -20,7 +20,9 @@ protected function setUp(): void public function testDetectsWrongCapitalizations(string $input, string $wrongVariant, string $expected): void { $findWrongVariant = new \ReflectionMethod($this->rule, 'findWrongVariant'); + $findWrongVariant->setAccessible(true); $fixCapitalization = new \ReflectionMethod($this->rule, 'fixCapitalization'); + $fixCapitalization->setAccessible(true); self::assertSame($wrongVariant, $findWrongVariant->invoke($this->rule, $input)); self::assertSame($expected, $fixCapitalization->invoke($this->rule, $input, $wrongVariant)); @@ -42,6 +44,7 @@ public static function wrongCapitalizations(): iterable public function testAllowsCorrectCapitalizations(string $input): void { $findWrongVariant = new \ReflectionMethod($this->rule, 'findWrongVariant'); + $findWrongVariant->setAccessible(true); self::assertNull($findWrongVariant->invoke($this->rule, $input)); } From 479f58efb2dec0203f979242389be3846527d370 Mon Sep 17 00:00:00 2001 From: Simon Bigelmayr Date: Fri, 5 Dec 2025 20:11:00 +0100 Subject: [PATCH 08/11] test: require PHP 8.3+ for PHPStan integration tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PHPStan internals (PHPStanTestCase) have compatibility issues with older PHP versions due to PHPStan/PHP-Parser version conflicts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php b/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php index e7325f0..4ad4a21 100644 --- a/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php +++ b/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php @@ -6,6 +6,9 @@ use PHPStan\Analyser\Error; use PHPStan\Testing\PHPStanTestCase; +/** + * @requires PHP >= 8.3 + */ final class LabIDCapitalizationRuleIntegrationTest extends PHPStanTestCase { public function testDetectsWrongCapitalizationInString(): void From 9d25b95f7ea4d31fe105709b1877a018c3719ec5 Mon Sep 17 00:00:00 2001 From: Simon Bigelmayr Date: Mon, 8 Dec 2025 15:44:15 +0100 Subject: [PATCH 09/11] feat: change visibility of findWrongVariant and fixCapitalization methods to public simplify tests --- .../Rules/CanonicalCapitalizationRule.php | 4 +- ...LabIDCapitalizationRuleIntegrationTest.php | 128 ++++++------------ tests/PHPStan/LabIDCapitalizationRuleTest.php | 14 +- 3 files changed, 50 insertions(+), 96 deletions(-) diff --git a/src/PHPStan/Rules/CanonicalCapitalizationRule.php b/src/PHPStan/Rules/CanonicalCapitalizationRule.php index f6a071e..175e9a9 100644 --- a/src/PHPStan/Rules/CanonicalCapitalizationRule.php +++ b/src/PHPStan/Rules/CanonicalCapitalizationRule.php @@ -85,7 +85,7 @@ protected function hasLanguageAnnotation(String_ $node): bool return false; } - protected function findWrongVariant(string $value): ?string + public function findWrongVariant(string $value): ?string { foreach ($this->getWrongVariants() as $wrongVariant) { if (Str::contains($value, $wrongVariant)) { @@ -96,7 +96,7 @@ protected function findWrongVariant(string $value): ?string return null; } - protected function fixCapitalization(string $value, string $wrongVariant): string + public function fixCapitalization(string $value, string $wrongVariant): string { return str_replace($wrongVariant, $this->getCanonicalForm(), $value); } diff --git a/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php b/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php index 06aff32..07c61c9 100644 --- a/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php +++ b/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php @@ -12,19 +12,40 @@ */ final class LabIDCapitalizationRuleIntegrationTest extends PHPStanTestCase { - /** @return iterable>}> */ + /** @return iterable>}> */ public static function dataIntegrationTests(): iterable { self::getContainer(); - yield 'lab-id-capitalization fixture' => [[ - // Line 9: 'The LabID is wrong' - wrong capitalization - 9 => ['String "The LabID is wrong" should use "Lab ID" instead of "LabID", change it to "The Lab ID is wrong".'], - // Lines 30-36: GraphQL query WITHOUT @lang annotation - wrong capitalization - 30 => ['String "' . "\n {\n patient {\n labID\n }\n }\n " . '" should use "Lab ID" instead of "labID", change it to "' . "\n {\n patient {\n Lab ID\n }\n }\n " . '".'], - // Lines 66-72: SQL query WITHOUT @lang annotation - wrong capitalization - 68 => ['String "' . "\n SELECT exam_no AS labID\n FROM examinations\n " . '" should use "Lab ID" instead of "labID", change it to "' . "\n SELECT exam_no AS Lab ID\n FROM examinations\n " . '".'], - ]]; + yield 'detects wrong capitalization' => [ + __DIR__ . '/data/lab-id-capitalization.php', + [ + // Line 9: 'The LabID is wrong' - wrong capitalization + 9 => ['String "The LabID is wrong" should use "Lab ID" instead of "LabID", change it to "The Lab ID is wrong".'], + // Line 30: GraphQL query WITHOUT @lang annotation - wrong capitalization (multiline string starts here) + 30 => ['String " + { + patient { + labID + } + } + " should use "Lab ID" instead of "labID", change it to " + { + patient { + Lab ID + } + } + ".'], + // Line 68: SQL query WITHOUT @lang annotation - wrong capitalization (multiline string starts here) + 68 => ['String " + SELECT exam_no AS labID + FROM examinations + " should use "Lab ID" instead of "labID", change it to " + SELECT exam_no AS Lab ID + FROM examinations + ".'], + ], + ]; } /** @@ -33,9 +54,9 @@ public static function dataIntegrationTests(): iterable * @dataProvider dataIntegrationTests */ #[DataProvider('dataIntegrationTests')] - public function testIntegration(array $expectedErrors): void + public function testIntegration(string $file, array $expectedErrors): void { - $errors = $this->runAnalyse(__DIR__ . '/data/lab-id-capitalization.php'); + $errors = $this->runAnalyse($file); $labIDErrors = array_filter( $errors, @@ -46,60 +67,10 @@ public function testIntegration(array $expectedErrors): void self::assertEmpty($labIDErrors, 'Should not report errors for correct capitalization'); } else { self::assertNotEmpty($labIDErrors, 'Should detect wrong Lab ID capitalization'); - $this->assertSameErrorMessages($expectedErrors, $labIDErrors); + $this->assertExpectedErrors($expectedErrors, $labIDErrors); } } - public function testIgnoresCorrectCapitalization(): void - { - $errors = $this->runAnalyse(__DIR__ . '/data/lab-id-capitalization.php'); - $errorLines = $this->getErrorLines($errors); - - // Line 14: 'The Lab ID is correct' - should NOT be detected - self::assertNotContains(14, $errorLines, 'Should not report correct Lab ID capitalization'); - } - - public function testIgnoresGraphQLQueryWithAnnotation(): void - { - $errors = $this->runAnalyse(__DIR__ . '/data/lab-id-capitalization.php'); - $errorLines = $this->getErrorLines($errors); - - // Lines 18-25: GraphQL query WITH @lang annotation - should NOT be detected - $graphqlAnnotatedErrors = array_filter( - $errorLines, - static fn (int $line): bool => $line >= 18 && $line <= 25, - ); - - self::assertEmpty($graphqlAnnotatedErrors, 'Should ignore GraphQL query with @lang annotation'); - } - - public function testIgnoresIdentifierStrings(): void - { - $errors = $this->runAnalyse(__DIR__ . '/data/lab-id-capitalization.php'); - $errorLines = $this->getErrorLines($errors); - - // Lines 44-45: Array keys 'labID', 'LabID' - should NOT be detected (identifier-like) - self::assertNotContains(44, $errorLines, 'Should not report array key labID'); - self::assertNotContains(45, $errorLines, 'Should not report array key LabID'); - - // Line 52: Single identifier string 'labID' - should NOT be detected - self::assertNotContains(52, $errorLines, 'Should not report identifier string labID'); - } - - public function testIgnoresSqlQueryWithAnnotation(): void - { - $errors = $this->runAnalyse(__DIR__ . '/data/lab-id-capitalization.php'); - $errorLines = $this->getErrorLines($errors); - - // Lines 57-64: SQL query WITH @lang annotation - should NOT be detected - $sqlAnnotatedErrors = array_filter( - $errorLines, - static fn (int $line): bool => $line >= 57 && $line <= 64, - ); - - self::assertEmpty($sqlAnnotatedErrors, 'Should ignore SQL query with @lang annotation'); - } - /** @return array */ private function runAnalyse(string $file): array { @@ -113,37 +84,28 @@ private function runAnalyse(string $file): array return $result->getErrors(); } - /** - * @param array $errors - * - * @return array - */ - private function getErrorLines(array $errors): array - { - $labIDErrors = array_filter( - $errors, - static fn (Error $error): bool => str_contains($error->getMessage(), 'should use "Lab ID"'), - ); - - return array_map( - static fn (Error $error): int => $error->getLine() ?? 0, - $labIDErrors, - ); - } - /** * @param array> $expectedErrors - * @param array $errors + * @param array $actualErrors */ - private function assertSameErrorMessages(array $expectedErrors, array $errors): void + private function assertExpectedErrors(array $expectedErrors, array $actualErrors): void { - foreach ($errors as $error) { + foreach ($actualErrors as $error) { $errorLine = $error->getLine() ?? 0; $errorMessage = $error->getMessage(); self::assertArrayHasKey($errorLine, $expectedErrors, "Unexpected error at line {$errorLine}: {$errorMessage}"); self::assertContains($errorMessage, $expectedErrors[$errorLine]); } + + // Verify we got all expected errors + $actualLines = array_map( + static fn (Error $error): int => $error->getLine() ?? 0, + $actualErrors, + ); + foreach (array_keys($expectedErrors) as $expectedLine) { + self::assertContains($expectedLine, $actualLines, "Expected error at line {$expectedLine} was not reported"); + } } /** @return array */ diff --git a/tests/PHPStan/LabIDCapitalizationRuleTest.php b/tests/PHPStan/LabIDCapitalizationRuleTest.php index bbf755d..692f562 100644 --- a/tests/PHPStan/LabIDCapitalizationRuleTest.php +++ b/tests/PHPStan/LabIDCapitalizationRuleTest.php @@ -19,13 +19,8 @@ protected function setUp(): void #[DataProvider('wrongCapitalizations')] public function testDetectsWrongCapitalizations(string $input, string $wrongVariant, string $expected): void { - $findWrongVariant = new \ReflectionMethod($this->rule, 'findWrongVariant'); - $findWrongVariant->setAccessible(true); - $fixCapitalization = new \ReflectionMethod($this->rule, 'fixCapitalization'); - $fixCapitalization->setAccessible(true); - - self::assertSame($wrongVariant, $findWrongVariant->invoke($this->rule, $input)); - self::assertSame($expected, $fixCapitalization->invoke($this->rule, $input, $wrongVariant)); + self::assertSame($wrongVariant, $this->rule->findWrongVariant($input)); + self::assertSame($expected, $this->rule->fixCapitalization($input, $wrongVariant)); } /** @return iterable */ @@ -43,10 +38,7 @@ public static function wrongCapitalizations(): iterable #[DataProvider('correctCapitalizations')] public function testAllowsCorrectCapitalizations(string $input): void { - $findWrongVariant = new \ReflectionMethod($this->rule, 'findWrongVariant'); - $findWrongVariant->setAccessible(true); - - self::assertNull($findWrongVariant->invoke($this->rule, $input)); + self::assertNull($this->rule->findWrongVariant($input)); } /** @return iterable */ From 158d3991ef41ef47f44f2ea2cfdd99c8b9542d92 Mon Sep 17 00:00:00 2001 From: Simon Bigelmayr Date: Mon, 8 Dec 2025 15:59:10 +0100 Subject: [PATCH 10/11] refactor: simplify capitalization rule logic and improve test structure --- .../Rules/CanonicalCapitalizationRule.php | 19 ++--- src/PHPStan/Rules/IdToIDRule.php | 1 - src/PHPStan/Rules/LabIDCapitalizationRule.php | 9 --- tests/PHPStan/IdToIDRuleIntegrationTest.php | 65 +++------------- ...LabIDCapitalizationRuleIntegrationTest.php | 66 ++-------------- tests/PHPStan/PHPStanIntegrationTestCase.php | 78 +++++++++++++++++++ 6 files changed, 101 insertions(+), 137 deletions(-) create mode 100644 tests/PHPStan/PHPStanIntegrationTestCase.php diff --git a/src/PHPStan/Rules/CanonicalCapitalizationRule.php b/src/PHPStan/Rules/CanonicalCapitalizationRule.php index 175e9a9..a6658cd 100644 --- a/src/PHPStan/Rules/CanonicalCapitalizationRule.php +++ b/src/PHPStan/Rules/CanonicalCapitalizationRule.php @@ -10,20 +10,16 @@ use PHPStan\Rules\RuleErrorBuilder; /** - * Enforces canonical capitalization in string literals. - * * Only checks string literals because canonical forms like "Lab ID" contain spaces, * which are invalid in PHP identifiers (variables, methods, classes). - * Per coding guidelines: "Unless the circumstances demand otherwise (all lower, no spaces, ...)". * * @implements Rule */ abstract class CanonicalCapitalizationRule implements Rule { - /** The correct canonical form, e.g. "Lab ID". */ abstract protected function getCanonicalForm(): string; - /** @return array Wrong variants to detect, e.g. ['LabID', 'labID', 'LABID']. */ + /** @return array */ abstract protected function getWrongVariants(): array; abstract protected function getErrorIdentifier(): string; @@ -42,7 +38,7 @@ public function processNode(Node $node, Scope $scope): array $value = $node->value; - if ($this->looksLikeIdentifier($value)) { + if ($this->cannotContainSpaces($value)) { return []; } @@ -63,17 +59,12 @@ public function processNode(Node $node, Scope $scope): array ]; } - /** Identifiers (array keys, field names) can't contain spaces, so skip them. */ - protected function looksLikeIdentifier(string $value): bool + protected function cannotContainSpaces(string $value): bool { - return \Safe\preg_match('/^[a-zA-Z_][a-zA-Z0-9_]*$/', $value) === 1; + return ! str_contains($value, ' '); } - /** - * Strings annotated with @lang (GraphQL, SQL, etc.) use field/column names that can't contain spaces. - * - * @see https://www.jetbrains.com/help/phpstorm/using-language-injections.html - */ + /** @see https://www.jetbrains.com/help/phpstorm/using-language-injections.html */ protected function hasLanguageAnnotation(String_ $node): bool { foreach ($node->getComments() as $comment) { diff --git a/src/PHPStan/Rules/IdToIDRule.php b/src/PHPStan/Rules/IdToIDRule.php index 951ed34..ce376cf 100644 --- a/src/PHPStan/Rules/IdToIDRule.php +++ b/src/PHPStan/Rules/IdToIDRule.php @@ -23,7 +23,6 @@ abstract protected function getErrorIdentifier(): string; abstract protected function extractName(Node $node): ?string; - /** Override for custom formatting (e.g., adding $ prefix for variables). */ protected function formatNameForMessage(string $name): string { return $name; diff --git a/src/PHPStan/Rules/LabIDCapitalizationRule.php b/src/PHPStan/Rules/LabIDCapitalizationRule.php index 0f428c7..bd0583d 100644 --- a/src/PHPStan/Rules/LabIDCapitalizationRule.php +++ b/src/PHPStan/Rules/LabIDCapitalizationRule.php @@ -2,15 +2,6 @@ namespace MLL\Utils\PHPStan\Rules; -/** - * Enforces correct capitalization of "Lab ID". - * - * Detects variants like: - * - LabID, labID, LABID (missing space) - * - Labid, labid (wrong case) - * - Lab-ID, lab-Id (wrong separator) - * - Lab Id (wrong case after space) - */ final class LabIDCapitalizationRule extends CanonicalCapitalizationRule { protected function getCanonicalForm(): string diff --git a/tests/PHPStan/IdToIDRuleIntegrationTest.php b/tests/PHPStan/IdToIDRuleIntegrationTest.php index 49849e6..8908363 100644 --- a/tests/PHPStan/IdToIDRuleIntegrationTest.php +++ b/tests/PHPStan/IdToIDRuleIntegrationTest.php @@ -2,19 +2,18 @@ namespace MLL\Utils\Tests\PHPStan; -use PHPStan\Analyser\Analyser; -use PHPStan\Analyser\Error; -use PHPStan\Testing\PHPStanTestCase; use PHPUnit\Framework\Attributes\DataProvider; -final class IdToIDRuleIntegrationTest extends PHPStanTestCase +final class IdToIDRuleIntegrationTest extends PHPStanIntegrationTestCase { + private const ERROR_PATTERN = 'should use "ID" instead of "Id"'; + /** @return iterable>}> */ public static function dataIntegrationTests(): iterable { self::getContainer(); - yield [__DIR__ . '/data/wrong-capitalization.php', [ + yield 'wrong capitalization' => [__DIR__ . '/data/wrong-capitalization.php', [ 5 => [ 'Name of Stmt_Class "LabIdProcessor" should use "ID" instead of "Id", rename it to "LabIDProcessor".', ], @@ -31,7 +30,7 @@ public static function dataIntegrationTests(): iterable ], ]]; - yield [__DIR__ . '/data/correct-capitalization.php', []]; + yield 'correct capitalization' => [__DIR__ . '/data/correct-capitalization.php', []]; } /** @@ -42,58 +41,14 @@ public static function dataIntegrationTests(): iterable #[DataProvider('dataIntegrationTests')] public function testIntegration(string $file, array $expectedErrors): void { - $errors = $this->runAnalyse($file); - - $ourErrors = array_filter( - $errors, - static function (Error $error): bool { - $message = $error->getMessage(); - - return str_contains($message, 'should use "ID" instead of "Id"'); - } - ); + $errors = $this->analyseFile($file); + $filteredErrors = $this->filterErrors($errors, self::ERROR_PATTERN); if ($expectedErrors === []) { - self::assertEmpty($ourErrors, 'Should not report errors for correct capitalization'); + self::assertEmpty($filteredErrors, 'Should not report errors for correct capitalization'); } else { - self::assertNotEmpty($ourErrors, 'Should detect wrong capitalization'); - $this->assertSameErrorMessages($expectedErrors, $ourErrors); - } - } - - /** @return array */ - private function runAnalyse(string $file): array - { - $file = self::getFileHelper()->normalizePath($file); - - /** @var Analyser $analyser */ - $analyser = self::getContainer()->getByType(Analyser::class); - - $result = $analyser->analyse([$file]); - - return $result->getErrors(); - } - - /** - * @param array> $expectedErrors - * @param array $errors - */ - private function assertSameErrorMessages(array $expectedErrors, array $errors): void - { - foreach ($errors as $error) { - $errorLine = $error->getLine() ?? 0; - $errorMessage = $error->getMessage(); - - self::assertArrayHasKey($errorLine, $expectedErrors, "Unexpected error at line {$errorLine}: {$errorMessage}"); - self::assertContains($errorMessage, $expectedErrors[$errorLine]); + self::assertNotEmpty($filteredErrors, 'Should detect wrong capitalization'); + $this->assertExpectedErrors($expectedErrors, $filteredErrors); } } - - /** @return array */ - public static function getAdditionalConfigFiles(): array - { - return [ - __DIR__ . '/phpstan-test.neon', - ]; - } } diff --git a/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php b/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php index 07c61c9..c29ec5b 100644 --- a/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php +++ b/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php @@ -2,16 +2,15 @@ namespace MLL\Utils\Tests\PHPStan; -use PHPStan\Analyser\Analyser; -use PHPStan\Analyser\Error; -use PHPStan\Testing\PHPStanTestCase; use PHPUnit\Framework\Attributes\DataProvider; /** * @requires PHP >= 8.3 */ -final class LabIDCapitalizationRuleIntegrationTest extends PHPStanTestCase +final class LabIDCapitalizationRuleIntegrationTest extends PHPStanIntegrationTestCase { + private const ERROR_PATTERN = 'should use "Lab ID"'; + /** @return iterable>}> */ public static function dataIntegrationTests(): iterable { @@ -56,63 +55,14 @@ public static function dataIntegrationTests(): iterable #[DataProvider('dataIntegrationTests')] public function testIntegration(string $file, array $expectedErrors): void { - $errors = $this->runAnalyse($file); - - $labIDErrors = array_filter( - $errors, - static fn (Error $error): bool => str_contains($error->getMessage(), 'should use "Lab ID"'), - ); + $errors = $this->analyseFile($file); + $filteredErrors = $this->filterErrors($errors, self::ERROR_PATTERN); if ($expectedErrors === []) { - self::assertEmpty($labIDErrors, 'Should not report errors for correct capitalization'); + self::assertEmpty($filteredErrors, 'Should not report errors for correct capitalization'); } else { - self::assertNotEmpty($labIDErrors, 'Should detect wrong Lab ID capitalization'); - $this->assertExpectedErrors($expectedErrors, $labIDErrors); - } - } - - /** @return array */ - private function runAnalyse(string $file): array - { - $file = self::getFileHelper()->normalizePath($file); - - /** @var Analyser $analyser */ - $analyser = self::getContainer()->getByType(Analyser::class); - - $result = $analyser->analyse([$file]); - - return $result->getErrors(); - } - - /** - * @param array> $expectedErrors - * @param array $actualErrors - */ - private function assertExpectedErrors(array $expectedErrors, array $actualErrors): void - { - foreach ($actualErrors as $error) { - $errorLine = $error->getLine() ?? 0; - $errorMessage = $error->getMessage(); - - self::assertArrayHasKey($errorLine, $expectedErrors, "Unexpected error at line {$errorLine}: {$errorMessage}"); - self::assertContains($errorMessage, $expectedErrors[$errorLine]); + self::assertNotEmpty($filteredErrors, 'Should detect wrong Lab ID capitalization'); + $this->assertExpectedErrors($expectedErrors, $filteredErrors); } - - // Verify we got all expected errors - $actualLines = array_map( - static fn (Error $error): int => $error->getLine() ?? 0, - $actualErrors, - ); - foreach (array_keys($expectedErrors) as $expectedLine) { - self::assertContains($expectedLine, $actualLines, "Expected error at line {$expectedLine} was not reported"); - } - } - - /** @return array */ - public static function getAdditionalConfigFiles(): array - { - return [ - __DIR__ . '/phpstan-test.neon', - ]; } } diff --git a/tests/PHPStan/PHPStanIntegrationTestCase.php b/tests/PHPStan/PHPStanIntegrationTestCase.php new file mode 100644 index 0000000..885dc1f --- /dev/null +++ b/tests/PHPStan/PHPStanIntegrationTestCase.php @@ -0,0 +1,78 @@ + */ + protected function analyseFile(string $file): array + { + $file = self::getFileHelper()->normalizePath($file); + + /** @var Analyser $analyser */ + $analyser = self::getContainer()->getByType(Analyser::class); + + $result = $analyser->analyse([$file]); + + return $result->getErrors(); + } + + /** + * Filter errors by message pattern. + * + * @param array $errors + * + * @return array + */ + protected function filterErrors(array $errors, string $messagePattern): array + { + return array_filter( + $errors, + static fn (Error $error): bool => str_contains($error->getMessage(), $messagePattern), + ); + } + + /** + * Assert that actual errors match expected errors by line number and message. + * + * @param array> $expectedErrors Map of line number to expected messages + * @param array $actualErrors + */ + protected function assertExpectedErrors(array $expectedErrors, array $actualErrors): void + { + // Check all actual errors are expected + foreach ($actualErrors as $error) { + $errorLine = $error->getLine() ?? 0; + $errorMessage = $error->getMessage(); + + self::assertArrayHasKey($errorLine, $expectedErrors, "Unexpected error at line {$errorLine}: {$errorMessage}"); + self::assertContains($errorMessage, $expectedErrors[$errorLine]); + } + + // Check all expected errors were reported + $actualLines = array_map( + static fn (Error $error): int => $error->getLine() ?? 0, + $actualErrors, + ); + foreach (array_keys($expectedErrors) as $expectedLine) { + self::assertContains($expectedLine, $actualLines, "Expected error at line {$expectedLine} was not reported"); + } + } + + /** @return array */ + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/phpstan-test.neon', + ]; + } +} From 433ad0f64d38c0c183ae590fd417d0192f02d5e4 Mon Sep 17 00:00:00 2001 From: Simon Bigelmayr Date: Mon, 8 Dec 2025 16:05:10 +0100 Subject: [PATCH 11/11] refactor(tests): remove redundant comments and simplify LabID capitalization tests --- .../LabIDCapitalizationRuleIntegrationTest.php | 5 +---- tests/PHPStan/LabIDCapitalizationRuleTest.php | 2 +- tests/PHPStan/PHPStanIntegrationTestCase.php | 13 +------------ tests/PHPStan/data/lab-id-capitalization.php | 2 -- 4 files changed, 3 insertions(+), 19 deletions(-) diff --git a/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php b/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php index c29ec5b..079f245 100644 --- a/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php +++ b/tests/PHPStan/LabIDCapitalizationRuleIntegrationTest.php @@ -19,9 +19,7 @@ public static function dataIntegrationTests(): iterable yield 'detects wrong capitalization' => [ __DIR__ . '/data/lab-id-capitalization.php', [ - // Line 9: 'The LabID is wrong' - wrong capitalization 9 => ['String "The LabID is wrong" should use "Lab ID" instead of "LabID", change it to "The Lab ID is wrong".'], - // Line 30: GraphQL query WITHOUT @lang annotation - wrong capitalization (multiline string starts here) 30 => ['String " { patient { @@ -35,8 +33,7 @@ public static function dataIntegrationTests(): iterable } } ".'], - // Line 68: SQL query WITHOUT @lang annotation - wrong capitalization (multiline string starts here) - 68 => ['String " + 66 => ['String " SELECT exam_no AS labID FROM examinations " should use "Lab ID" instead of "labID", change it to " diff --git a/tests/PHPStan/LabIDCapitalizationRuleTest.php b/tests/PHPStan/LabIDCapitalizationRuleTest.php index 692f562..f833d22 100644 --- a/tests/PHPStan/LabIDCapitalizationRuleTest.php +++ b/tests/PHPStan/LabIDCapitalizationRuleTest.php @@ -46,7 +46,7 @@ public static function correctCapitalizations(): iterable { yield 'Correct Lab ID' => ['Lab ID']; yield 'In sentence' => ['The Lab ID is correct']; - yield 'labId lowercase' => ['labId']; // This is ID capitalization, not Lab ID + yield 'labId lowercase' => ['labId']; yield 'Unrelated word' => ['Laboratory']; } } diff --git a/tests/PHPStan/PHPStanIntegrationTestCase.php b/tests/PHPStan/PHPStanIntegrationTestCase.php index 885dc1f..a750570 100644 --- a/tests/PHPStan/PHPStanIntegrationTestCase.php +++ b/tests/PHPStan/PHPStanIntegrationTestCase.php @@ -6,11 +6,6 @@ use PHPStan\Analyser\Error; use PHPStan\Testing\PHPStanTestCase; -/** - * Base class for PHPStan rule integration tests. - * - * Provides common functionality for analyzing files and asserting expected errors. - */ abstract class PHPStanIntegrationTestCase extends PHPStanTestCase { /** @return array */ @@ -27,8 +22,6 @@ protected function analyseFile(string $file): array } /** - * Filter errors by message pattern. - * * @param array $errors * * @return array @@ -42,14 +35,11 @@ protected function filterErrors(array $errors, string $messagePattern): array } /** - * Assert that actual errors match expected errors by line number and message. - * - * @param array> $expectedErrors Map of line number to expected messages + * @param array> $expectedErrors * @param array $actualErrors */ protected function assertExpectedErrors(array $expectedErrors, array $actualErrors): void { - // Check all actual errors are expected foreach ($actualErrors as $error) { $errorLine = $error->getLine() ?? 0; $errorMessage = $error->getMessage(); @@ -58,7 +48,6 @@ protected function assertExpectedErrors(array $expectedErrors, array $actualErro self::assertContains($errorMessage, $expectedErrors[$errorLine]); } - // Check all expected errors were reported $actualLines = array_map( static fn (Error $error): int => $error->getLine() ?? 0, $actualErrors, diff --git a/tests/PHPStan/data/lab-id-capitalization.php b/tests/PHPStan/data/lab-id-capitalization.php index d4404d1..767d399 100644 --- a/tests/PHPStan/data/lab-id-capitalization.php +++ b/tests/PHPStan/data/lab-id-capitalization.php @@ -39,7 +39,6 @@ public function graphqlQueryWithoutAnnotationIsChecked(): string /** @return array */ public function arrayKeysAreIgnored(): array { - // Array keys that look like identifiers should be ignored return [ 'labID' => 'some value', 'LabID' => 'another value', @@ -48,7 +47,6 @@ public function arrayKeysAreIgnored(): array public function identifierStringsAreIgnored(): string { - // Single identifier strings (no spaces) should be ignored $key = 'labID'; return $key;