-
Notifications
You must be signed in to change notification settings - Fork 0
feat(phpstan): add CanonicalCapitalizationRule for enforcing term spelling #65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
a2e9ba2
79f642a
9f0874a
5601bf3
2d582a0
6b22bdd
f32c025
479f58e
26f14c9
9d25b95
158d399
433ad0f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,94 @@ | ||||||||||||||
| <?php declare(strict_types=1); | ||||||||||||||
|
|
||||||||||||||
| namespace MLL\Utils\PHPStan\Rules; | ||||||||||||||
|
|
||||||||||||||
| use Illuminate\Support\Str; | ||||||||||||||
| use PhpParser\Node; | ||||||||||||||
| use PhpParser\Node\Scalar\String_; | ||||||||||||||
| use PHPStan\Analyser\Scope; | ||||||||||||||
| use PHPStan\Rules\Rule; | ||||||||||||||
| use PHPStan\Rules\RuleErrorBuilder; | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Only checks string literals because canonical forms like "Lab ID" contain spaces, | ||||||||||||||
| * which are invalid in PHP identifiers (variables, methods, classes). | ||||||||||||||
| * | ||||||||||||||
| * @implements Rule<String_> | ||||||||||||||
| */ | ||||||||||||||
| abstract class CanonicalCapitalizationRule implements Rule | ||||||||||||||
| { | ||||||||||||||
| abstract protected function getCanonicalForm(): string; | ||||||||||||||
|
|
||||||||||||||
| /** @return array<int, string> */ | ||||||||||||||
| abstract protected function getWrongVariants(): array; | ||||||||||||||
|
|
||||||||||||||
| abstract protected function getErrorIdentifier(): string; | ||||||||||||||
|
|
||||||||||||||
| public function getNodeType(): string | ||||||||||||||
| { | ||||||||||||||
| return String_::class; | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fangen wir damit auch alle möglichen Varianten in PHP Strings zu definieren? |
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /** @param String_ $node */ | ||||||||||||||
| public function processNode(Node $node, Scope $scope): array | ||||||||||||||
| { | ||||||||||||||
| if ($this->hasLanguageAnnotation($node)) { | ||||||||||||||
| return []; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| $value = $node->value; | ||||||||||||||
|
|
||||||||||||||
| if ($this->cannotContainSpaces($value)) { | ||||||||||||||
| return []; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| $wrongVariant = $this->findWrongVariant($value); | ||||||||||||||
|
|
||||||||||||||
| if ($wrongVariant === null) { | ||||||||||||||
| return []; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| $expectedValue = $this->fixCapitalization($value, $wrongVariant); | ||||||||||||||
|
|
||||||||||||||
| return [ | ||||||||||||||
| RuleErrorBuilder::message(<<<TXT | ||||||||||||||
| String "{$value}" should use "{$this->getCanonicalForm()}" instead of "{$wrongVariant}", change it to "{$expectedValue}". | ||||||||||||||
| TXT) | ||||||||||||||
| ->identifier($this->getErrorIdentifier()) | ||||||||||||||
| ->build(), | ||||||||||||||
| ]; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| protected function cannotContainSpaces(string $value): bool | ||||||||||||||
| { | ||||||||||||||
| return ! str_contains($value, ' '); | ||||||||||||||
|
Comment on lines
+62
to
+64
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Diese Heuristik kann schon wild inakkurat sein. Wenn jemand fälschlicherweise |
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /** @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')) { | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Eventuell können wir das hier einschränken auf gewisse Languages. In |
||||||||||||||
| return true; | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| return false; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| public function findWrongVariant(string $value): ?string | ||||||||||||||
| { | ||||||||||||||
| foreach ($this->getWrongVariants() as $wrongVariant) { | ||||||||||||||
| if (Str::contains($value, $wrongVariant)) { | ||||||||||||||
| return $wrongVariant; | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| return null; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| public function fixCapitalization(string $value, string $wrongVariant): string | ||||||||||||||
| { | ||||||||||||||
| return str_replace($wrongVariant, $this->getCanonicalForm(), $value); | ||||||||||||||
|
Comment on lines
+90
to
+92
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| <?php declare(strict_types=1); | ||
|
|
||
| namespace MLL\Utils\PHPStan\Rules; | ||
|
|
||
| final class LabIDCapitalizationRule extends CanonicalCapitalizationRule | ||
| { | ||
| protected function getCanonicalForm(): string | ||
| { | ||
| return 'Lab ID'; | ||
| } | ||
|
|
||
| /** @return array<int, string> */ | ||
| 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'; | ||
| } | ||
| } |
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| <?php declare(strict_types=1); | ||
|
|
||
| namespace MLL\Utils\Tests\PHPStan; | ||
|
|
||
| use PHPUnit\Framework\Attributes\DataProvider; | ||
|
|
||
| final class IdToIDRuleIntegrationTest extends PHPStanIntegrationTestCase | ||
| { | ||
| private const ERROR_PATTERN = 'should use "ID" instead of "Id"'; | ||
|
|
||
| /** @return iterable<array{0: string, 1: array<int, array<int, string>>}> */ | ||
| public static function dataIntegrationTests(): iterable | ||
| { | ||
| self::getContainer(); | ||
|
|
||
| yield 'wrong capitalization' => [__DIR__ . '/data/wrong-capitalization.php', [ | ||
| 5 => [ | ||
| 'Name of Stmt_Class "LabIdProcessor" should use "ID" instead of "Id", rename it to "LabIDProcessor".', | ||
| ], | ||
| 7 => [ | ||
| 'Name of Stmt_ClassMethod "getLabId" should use "ID" instead of "Id", rename it to "getLabID".', | ||
| ], | ||
| 12 => [ | ||
| 'Name of Stmt_ClassMethod "processLabId" should use "ID" instead of "Id", rename it to "processLabID".', | ||
| 'Name of Param "$labId" should use "ID" instead of "Id", rename it to "$labID".', | ||
| ], | ||
| 14 => [ | ||
| 'Name of Expr_Variable "$sampleId" should use "ID" instead of "Id", rename it to "$sampleID".', | ||
| 'Name of Expr_Variable "$labId" should use "ID" instead of "Id", rename it to "$labID".', | ||
| ], | ||
| ]]; | ||
|
|
||
| yield 'correct capitalization' => [__DIR__ . '/data/correct-capitalization.php', []]; | ||
| } | ||
|
|
||
| /** | ||
| * @param array<int, array<int, string>> $expectedErrors | ||
| * | ||
| * @dataProvider dataIntegrationTests | ||
| */ | ||
| #[DataProvider('dataIntegrationTests')] | ||
| public function testIntegration(string $file, array $expectedErrors): void | ||
| { | ||
| $errors = $this->analyseFile($file); | ||
| $filteredErrors = $this->filterErrors($errors, self::ERROR_PATTERN); | ||
|
|
||
| if ($expectedErrors === []) { | ||
| self::assertEmpty($filteredErrors, 'Should not report errors for correct capitalization'); | ||
| } else { | ||
| self::assertNotEmpty($filteredErrors, 'Should detect wrong capitalization'); | ||
| $this->assertExpectedErrors($expectedErrors, $filteredErrors); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| <?php declare(strict_types=1); | ||
|
|
||
| namespace MLL\Utils\Tests\PHPStan; | ||
|
|
||
| use PHPUnit\Framework\Attributes\DataProvider; | ||
|
|
||
| /** | ||
| * @requires PHP >= 8.3 | ||
| */ | ||
| final class LabIDCapitalizationRuleIntegrationTest extends PHPStanIntegrationTestCase | ||
| { | ||
| private const ERROR_PATTERN = 'should use "Lab ID"'; | ||
|
|
||
| /** @return iterable<string, array{0: string, 1: array<int, array<int, string>>}> */ | ||
| public static function dataIntegrationTests(): iterable | ||
| { | ||
| self::getContainer(); | ||
|
|
||
| yield 'detects wrong capitalization' => [ | ||
| __DIR__ . '/data/lab-id-capitalization.php', | ||
| [ | ||
| 9 => ['String "The LabID is wrong" should use "Lab ID" instead of "LabID", change it to "The Lab ID is wrong".'], | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hier den ganzen String in der Nachricht zu zeigen kann sehr unübersichtlich werden, wenn es sich um lange Multiline-Werte handelt. |
||
| 30 => ['String " | ||
| { | ||
| patient { | ||
| labID | ||
| } | ||
| } | ||
| " should use "Lab ID" instead of "labID", change it to " | ||
| { | ||
| patient { | ||
| Lab ID | ||
| } | ||
| } | ||
| ".'], | ||
|
Comment on lines
+23
to
+35
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Negativbeispiel, in dem Fall sollte via |
||
| 66 => ['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 | ||
| ".'], | ||
|
Comment on lines
+36
to
+42
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Auch Negativbeispiel, |
||
| ], | ||
| ]; | ||
| } | ||
|
|
||
| /** | ||
| * @param array<int, array<int, string>> $expectedErrors | ||
| * | ||
| * @dataProvider dataIntegrationTests | ||
| */ | ||
| #[DataProvider('dataIntegrationTests')] | ||
| public function testIntegration(string $file, array $expectedErrors): void | ||
| { | ||
| $errors = $this->analyseFile($file); | ||
| $filteredErrors = $this->filterErrors($errors, self::ERROR_PATTERN); | ||
|
|
||
| if ($expectedErrors === []) { | ||
| self::assertEmpty($filteredErrors, 'Should not report errors for correct capitalization'); | ||
| } else { | ||
| self::assertNotEmpty($filteredErrors, 'Should detect wrong Lab ID capitalization'); | ||
| $this->assertExpectedErrors($expectedErrors, $filteredErrors); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich finde es nicht korrekt hier von
Capitalizationzu sprechen. Würde auch die Überschrift der Guideline abändern, wie wäre es mitCanonical ...: