From 861024e40c9361e2c193db5f01d04152938d5112 Mon Sep 17 00:00:00 2001 From: Mathieu Santostefano Date: Tue, 23 Jan 2024 17:06:40 +0100 Subject: [PATCH 01/10] Add ShouldInclude and ShouldNotInclude rules --- docs/documentation/assertions.md | 5 ++ extension.neon | 12 +++++ .../Assertion/Relation/RelationAssertion.php | 5 +- .../ShouldInclude/IncludedTraitsRule.php | 15 ++++++ .../Relation/ShouldInclude/ShouldInclude.php | 50 +++++++++++++++++++ .../ShouldNotInclude/IncludedTraitsRule.php | 15 ++++++ .../ShouldNotInclude/ShouldNotInclude.php | 50 +++++++++++++++++++ .../Extractor/Relation/AllTraitsExtractor.php | 28 +++++++++++ src/Selector/ClassIncludes.php | 48 ++++++++++++++++++ src/Selector/SelectorPrimitive.php | 8 +++ src/Test/Builder/AssertionStep.php | 16 ++++++ tests/fixtures/Simple/SimpleTraitTwo.php | 5 ++ .../ShouldInclude/IncludedTraitsTest.php | 49 ++++++++++++++++++ .../ShowRuleNameIncludedTraitsTest.php | 49 ++++++++++++++++++ .../ShouldNotInclude/IncludedTraitsTest.php | 49 ++++++++++++++++++ .../ShowRuleNameIncludedTraitsTest.php | 49 ++++++++++++++++++ 16 files changed, 451 insertions(+), 2 deletions(-) create mode 100644 src/Rule/Assertion/Relation/ShouldInclude/IncludedTraitsRule.php create mode 100644 src/Rule/Assertion/Relation/ShouldInclude/ShouldInclude.php create mode 100644 src/Rule/Assertion/Relation/ShouldNotInclude/IncludedTraitsRule.php create mode 100644 src/Rule/Assertion/Relation/ShouldNotInclude/ShouldNotInclude.php create mode 100644 src/Rule/Extractor/Relation/AllTraitsExtractor.php create mode 100644 src/Selector/ClassIncludes.php create mode 100644 tests/fixtures/Simple/SimpleTraitTwo.php create mode 100644 tests/unit/rules/ShouldInclude/IncludedTraitsTest.php create mode 100644 tests/unit/rules/ShouldInclude/ShowRuleNameIncludedTraitsTest.php create mode 100644 tests/unit/rules/ShouldNotInclude/IncludedTraitsTest.php create mode 100644 tests/unit/rules/ShouldNotInclude/ShowRuleNameIncludedTraitsTest.php diff --git a/docs/documentation/assertions.md b/docs/documentation/assertions.md index bfb679a2..369405ed 100644 --- a/docs/documentation/assertions.md +++ b/docs/documentation/assertions.md @@ -34,6 +34,11 @@ It asserts that the selected classes **implement** the target interfaces. Also available: `shouldNotImplement()` +## shouldInclude() +It asserts that the selected classes **include** the target traits. + +Also available: `shouldNotInclude()` + ## shouldNotDependOn() It asserts that the selected classes **do not depend** on the target classes. diff --git a/extension.neon b/extension.neon index e6a5f63c..92cbbdfd 100644 --- a/extension.neon +++ b/extension.neon @@ -80,6 +80,12 @@ services: tags: - phpstan.rules.rule + # ShouldInclude rules + - + class: PHPat\Rule\Assertion\Relation\ShouldInclude\IncludedTraitsRule + tags: + - phpstan.rules.rule + # ShouldNotDepend rules - class: PHPat\Rule\Assertion\Relation\ShouldNotDepend\DirectInterfacesRule @@ -230,6 +236,12 @@ services: tags: - phpstan.rules.rule + # ShouldNotInclude rules + - + class: PHPat\Rule\Assertion\Relation\ShouldNotInclude\IncludedTraitsRule + tags: + - phpstan.rules.rule + # ShouldApplyAttribute rules - class: PHPat\Rule\Assertion\Relation\ShouldApplyAttribute\ClassAttributeRule diff --git a/src/Rule/Assertion/Relation/RelationAssertion.php b/src/Rule/Assertion/Relation/RelationAssertion.php index b4d9075c..4cedf506 100644 --- a/src/Rule/Assertion/Relation/RelationAssertion.php +++ b/src/Rule/Assertion/Relation/RelationAssertion.php @@ -7,6 +7,7 @@ use PHPat\Rule\Assertion\Relation\ShouldApplyAttribute\ShouldApplyAttribute; use PHPat\Rule\Assertion\Relation\ShouldExtend\ShouldExtend; use PHPat\Rule\Assertion\Relation\ShouldImplement\ShouldImplement; +use PHPat\Rule\Assertion\Relation\ShouldInclude\ShouldInclude; use PHPat\Selector\Classname; use PHPat\Selector\SelectorInterface; use PHPat\ShouldNotHappenException; @@ -98,8 +99,8 @@ protected function ruleApplies(Scope $scope, array $nodes): bool return false; } - // Can not skip if the rule is a ShouldExtend, ShouldImplement or ShouldApplyAttribute rule - if (is_a($this, ShouldExtend::class) || is_a($this, ShouldImplement::class) || is_a($this, ShouldApplyAttribute::class)) { + // Can not skip if the rule is a ShouldExtend, ShouldImplement, ShouldInclude or ShouldApplyAttribute rule + if (is_a($this, ShouldExtend::class) || is_a($this, ShouldImplement::class) || is_a($this, ShouldInclude::class) || is_a($this, ShouldApplyAttribute::class)) { return true; } diff --git a/src/Rule/Assertion/Relation/ShouldInclude/IncludedTraitsRule.php b/src/Rule/Assertion/Relation/ShouldInclude/IncludedTraitsRule.php new file mode 100644 index 00000000..3be5e46f --- /dev/null +++ b/src/Rule/Assertion/Relation/ShouldInclude/IncludedTraitsRule.php @@ -0,0 +1,15 @@ + + */ +final class IncludedTraitsRule extends ShouldInclude implements Rule +{ + use AllTraitsExtractor; +} diff --git a/src/Rule/Assertion/Relation/ShouldInclude/ShouldInclude.php b/src/Rule/Assertion/Relation/ShouldInclude/ShouldInclude.php new file mode 100644 index 00000000..774d540f --- /dev/null +++ b/src/Rule/Assertion/Relation/ShouldInclude/ShouldInclude.php @@ -0,0 +1,50 @@ +applyShould($ruleName, $subject, $targets, $targetExcludes, $nodes, $tips); + } + + protected function getMessage(string $ruleName, string $subject, string $target): string + { + return $this->prepareMessage( + $ruleName, + sprintf('%s should include %s', $subject, $target), + ); + } +} diff --git a/src/Rule/Assertion/Relation/ShouldNotInclude/IncludedTraitsRule.php b/src/Rule/Assertion/Relation/ShouldNotInclude/IncludedTraitsRule.php new file mode 100644 index 00000000..15c6ceee --- /dev/null +++ b/src/Rule/Assertion/Relation/ShouldNotInclude/IncludedTraitsRule.php @@ -0,0 +1,15 @@ + + */ +final class IncludedTraitsRule extends ShouldNotInclude implements Rule +{ + use AllTraitsExtractor; +} diff --git a/src/Rule/Assertion/Relation/ShouldNotInclude/ShouldNotInclude.php b/src/Rule/Assertion/Relation/ShouldNotInclude/ShouldNotInclude.php new file mode 100644 index 00000000..687c6a01 --- /dev/null +++ b/src/Rule/Assertion/Relation/ShouldNotInclude/ShouldNotInclude.php @@ -0,0 +1,50 @@ +applyShouldNot($ruleName, $subject, $targets, $targetExcludes, $nodes, $tips); + } + + protected function getMessage(string $ruleName, string $subject, string $target): string + { + return $this->prepareMessage( + $ruleName, + sprintf('%s should not include %s', $subject, $target), + ); + } +} diff --git a/src/Rule/Extractor/Relation/AllTraitsExtractor.php b/src/Rule/Extractor/Relation/AllTraitsExtractor.php new file mode 100644 index 00000000..7a2c9e78 --- /dev/null +++ b/src/Rule/Extractor/Relation/AllTraitsExtractor.php @@ -0,0 +1,28 @@ + + */ + protected function extractNodeClassNames(Node $node, Scope $scope): array + { + return array_map( + static fn (ClassReflection $c) => $c->getName(), + $node->getClassReflection()->getTraits() + ); + } +} diff --git a/src/Selector/ClassIncludes.php b/src/Selector/ClassIncludes.php new file mode 100644 index 00000000..40a20e64 --- /dev/null +++ b/src/Selector/ClassIncludes.php @@ -0,0 +1,48 @@ +traitname = $traitname; + $this->isRegex = $isRegex; + } + + public function getName(): string + { + return $this->traitname; + } + + public function matches(ClassReflection $classReflection): bool + { + if ($this->isRegex) { + return $this->matchesRegex($classReflection->getTraits()); + } + + return $classReflection->hasTraitUse($this->traitname); + } + + /** + * @param array $traits + */ + private function matchesRegex(array $traits): bool + { + foreach ($traits as $trait) { + if (preg_match($this->traitname, $trait->getName()) === 1) { + return true; + } + } + + return false; + } +} diff --git a/src/Selector/SelectorPrimitive.php b/src/Selector/SelectorPrimitive.php index 1b2ab078..62099ec3 100644 --- a/src/Selector/SelectorPrimitive.php +++ b/src/Selector/SelectorPrimitive.php @@ -172,4 +172,12 @@ public static function appliesAttribute(string $fqcn, bool $regex = false): Appl { return new AppliesAttribute($fqcn, $regex); } + + /** + * @param class-string|non-empty-string $fqcn + */ + public static function includes(string $fqcn, bool $regex = false): ClassIncludes + { + return new ClassIncludes($fqcn, $regex); + } } diff --git a/src/Test/Builder/AssertionStep.php b/src/Test/Builder/AssertionStep.php index 152ea826..2f8cda0b 100644 --- a/src/Test/Builder/AssertionStep.php +++ b/src/Test/Builder/AssertionStep.php @@ -14,10 +14,12 @@ use PHPat\Rule\Assertion\Relation\ShouldApplyAttribute\ShouldApplyAttribute; use PHPat\Rule\Assertion\Relation\ShouldExtend\ShouldExtend; use PHPat\Rule\Assertion\Relation\ShouldImplement\ShouldImplement; +use PHPat\Rule\Assertion\Relation\ShouldInclude\ShouldInclude; use PHPat\Rule\Assertion\Relation\ShouldNotConstruct\ShouldNotConstruct; use PHPat\Rule\Assertion\Relation\ShouldNotDepend\ShouldNotDepend; use PHPat\Rule\Assertion\Relation\ShouldNotExtend\ShouldNotExtend; use PHPat\Rule\Assertion\Relation\ShouldNotImplement\ShouldNotImplement; +use PHPat\Rule\Assertion\Relation\ShouldNotInclude\ShouldNotInclude; class AssertionStep extends AbstractStep { @@ -106,6 +108,20 @@ public function shouldImplement(): TargetStep return new TargetStep($this->rule); } + public function shouldNotInclude(): TargetStep + { + $this->rule->assertion = ShouldNotInclude::class; + + return new TargetStep($this->rule); + } + + public function shouldInclude(): TargetStep + { + $this->rule->assertion = ShouldInclude::class; + + return new TargetStep($this->rule); + } + public function shouldExtend(): TargetStep { $this->rule->assertion = ShouldExtend::class; diff --git a/tests/fixtures/Simple/SimpleTraitTwo.php b/tests/fixtures/Simple/SimpleTraitTwo.php new file mode 100644 index 00000000..1cfe0ec8 --- /dev/null +++ b/tests/fixtures/Simple/SimpleTraitTwo.php @@ -0,0 +1,5 @@ + + * @internal + * @coversNothing + */ +class IncludedTraitsTest extends RuleTestCase +{ + public const RULE_NAME = 'test_FixtureClassShouldIncludeSimpleTraitTwo'; + + public function testRule(): void + { + $this->analyse(['tests/fixtures/FixtureClass.php'], [ + [sprintf('%s should include %s', FixtureClass::class, SimpleTraitTwo::class), 29], + ]); + } + + protected function getRule(): Rule + { + $testParser = FakeTestParser::create( + self::RULE_NAME, + ShouldInclude::class, + [new Classname(FixtureClass::class, false)], + [new Classname(SimpleTraitTwo::class, false)] + ); + + return new IncludedTraitsRule( + new StatementBuilderFactory($testParser), + new Configuration(false, true, false), + $this->createReflectionProvider(), + self::getContainer()->getByType(FileTypeMapper::class) + ); + } +} diff --git a/tests/unit/rules/ShouldInclude/ShowRuleNameIncludedTraitsTest.php b/tests/unit/rules/ShouldInclude/ShowRuleNameIncludedTraitsTest.php new file mode 100644 index 00000000..5be06dbd --- /dev/null +++ b/tests/unit/rules/ShouldInclude/ShowRuleNameIncludedTraitsTest.php @@ -0,0 +1,49 @@ + + * @internal + * @coversNothing + */ +class ShowRuleNameIncludedTraitsTest extends RuleTestCase +{ + public const RULE_NAME = 'test_FixtureClassShouldIncludeSimpleTraitTwo'; + + public function testRule(): void + { + $this->analyse(['tests/fixtures/FixtureClass.php'], [ + [sprintf('%s: %s should include %s', self::RULE_NAME, FixtureClass::class, SimpleTraitTwo::class), 29], + ]); + } + + protected function getRule(): Rule + { + $testParser = FakeTestParser::create( + self::RULE_NAME, + ShouldInclude::class, + [new Classname(FixtureClass::class, false)], + [new Classname(SimpleTraitTwo::class, false)] + ); + + return new IncludedTraitsRule( + new StatementBuilderFactory($testParser), + new Configuration(false, true, true), + $this->createReflectionProvider(), + self::getContainer()->getByType(FileTypeMapper::class) + ); + } +} diff --git a/tests/unit/rules/ShouldNotInclude/IncludedTraitsTest.php b/tests/unit/rules/ShouldNotInclude/IncludedTraitsTest.php new file mode 100644 index 00000000..f20cee7a --- /dev/null +++ b/tests/unit/rules/ShouldNotInclude/IncludedTraitsTest.php @@ -0,0 +1,49 @@ + + * @internal + * @coversNothing + */ +class IncludedTraitsTest extends RuleTestCase +{ + public const RULE_NAME = 'test_FixtureClassShouldNotIncludeSimpleTrait'; + + public function testRule(): void + { + $this->analyse(['tests/fixtures/FixtureClass.php'], [ + [sprintf('%s should not include %s', FixtureClass::class, SimpleTrait::class), 29], + ]); + } + + protected function getRule(): Rule + { + $testParser = FakeTestParser::create( + self::RULE_NAME, + ShouldNotInclude::class, + [new Classname(FixtureClass::class, false)], + [new Classname(SimpleTrait::class, false)] + ); + + return new IncludedTraitsRule( + new StatementBuilderFactory($testParser), + new Configuration(false, true, false), + $this->createReflectionProvider(), + self::getContainer()->getByType(FileTypeMapper::class) + ); + } +} diff --git a/tests/unit/rules/ShouldNotInclude/ShowRuleNameIncludedTraitsTest.php b/tests/unit/rules/ShouldNotInclude/ShowRuleNameIncludedTraitsTest.php new file mode 100644 index 00000000..5f4b7ce5 --- /dev/null +++ b/tests/unit/rules/ShouldNotInclude/ShowRuleNameIncludedTraitsTest.php @@ -0,0 +1,49 @@ + + * @internal + * @coversNothing + */ +class ShowRuleNameIncludedTraitsTest extends RuleTestCase +{ + public const RULE_NAME = 'test_FixtureClassShouldNotIncludeSimpleTrait'; + + public function testRule(): void + { + $this->analyse(['tests/fixtures/FixtureClass.php'], [ + [sprintf('%s: %s should not include %s', self::RULE_NAME, FixtureClass::class, SimpleTrait::class), 29], + ]); + } + + protected function getRule(): Rule + { + $testParser = FakeTestParser::create( + self::RULE_NAME, + ShouldNotInclude::class, + [new Classname(FixtureClass::class, false)], + [new Classname(SimpleTrait::class, false)] + ); + + return new IncludedTraitsRule( + new StatementBuilderFactory($testParser), + new Configuration(false, true, true), + $this->createReflectionProvider(), + self::getContainer()->getByType(FileTypeMapper::class) + ); + } +} From 18a4b3644d5c32af635743941fd9645feadda4cc Mon Sep 17 00:00:00 2001 From: Geert Broekmans Date: Wed, 21 Feb 2024 11:21:43 +0100 Subject: [PATCH 02/10] Fix shouldBeNamed assertion not functioning on second run --- .../Declaration/DeclarationAssertion.php | 75 ++++++++----------- 1 file changed, 30 insertions(+), 45 deletions(-) diff --git a/src/Rule/Assertion/Declaration/DeclarationAssertion.php b/src/Rule/Assertion/Declaration/DeclarationAssertion.php index 94e90a77..1e16e528 100644 --- a/src/Rule/Assertion/Declaration/DeclarationAssertion.php +++ b/src/Rule/Assertion/Declaration/DeclarationAssertion.php @@ -43,19 +43,41 @@ public function __construct( */ public function processNode(Node $node, Scope $scope): array { - if (!$this->ruleApplies($scope)) { + $subject = $scope->getClassReflection(); + if ($subject === null) { return []; } - $meetsDeclaration = $this->meetsDeclaration($node, $scope, $this->getParams()); + $applicableStatements = array_filter( + $this->statements, + static function (array $statement) use ($subject): bool { + [$ruleName, $selector, $subjectExcludes, $tips, $params] = $statement; - return $this->validateGetErrors($scope, $meetsDeclaration); - } + if ($subject->isBuiltin() || !$selector->matches($subject)) { + return false; + } + foreach ($subjectExcludes as $exclude) { + if ($exclude->matches($subject)) { + return false; + } + } - // TODO: This is a temporary hack, the 'statement' concept needs to be reworked - public function getParams(): array - { - return $this->statements[0][4] ?? []; + return true; + } + ); + + return array_reduce( + $applicableStatements, + function (array $errors, array $statement) use ($node, $scope, $subject): array { + [$ruleName, $selector, $subjectExcludes, $tips, $params] = $statement; + + $meetsDeclaration = $this->meetsDeclaration($node, $scope, $statement[4]); + array_push($errors, ...$this->applyValidation($ruleName, $subject, $meetsDeclaration, $tips, $params)); + + return $errors; + }, + [] + ); } public function prepareMessage(string $ruleName, string $message): string @@ -77,41 +99,4 @@ abstract protected function getMessage(string $ruleName, string $subject, array * @return array */ abstract protected function applyValidation(string $ruleName, ClassReflection $subject, bool $meetsDeclaration, array $tips, array $params = []): array; - - protected function ruleApplies(Scope $scope): bool - { - if (!$scope->isInClass()) { - return false; - } - - return $scope->getClassReflection() !== null; - } - - /** - * @return array - * @throws ShouldNotHappenException - */ - protected function validateGetErrors(Scope $scope, bool $meetsDeclaration): array - { - $errors = []; - $subject = $scope->getClassReflection(); - if ($subject === null) { - throw new ShouldNotHappenException(); - } - - foreach ($this->statements as [$ruleName, $selector, $subjectExcludes, $tips, $params]) { - if ($subject->isBuiltin() || !$selector->matches($subject)) { - continue; - } - foreach ($subjectExcludes as $exclude) { - if ($exclude->matches($subject)) { - continue 2; - } - } - - array_push($errors, ...$this->applyValidation($ruleName, $subject, $meetsDeclaration, $tips, $params)); - } - - return $errors; - } } From b6efadd67e4ee14a1ead352824ecc977a43b027c Mon Sep 17 00:00:00 2001 From: Jakob Warkotsch Date: Wed, 21 Feb 2024 11:28:58 +0100 Subject: [PATCH 03/10] Detect dependencies in catch blocks --- docs/CHANGELOG.md | 3 ++ extension.neon | 8 +++ .../Relation/CanOnlyDepend/CatchBlockRule.php | 15 ++++++ .../ShouldNotDepend/CatchBlockRule.php | 15 ++++++ .../Relation/CatchBlockExtractor.php | 24 +++++++++ tests/fixtures/FixtureClass.php | 7 +++ .../rules/CanOnlyDepend/CatchBlockTest.php | 49 +++++++++++++++++++ .../rules/ShouldNotDepend/CatchBlockTest.php | 49 +++++++++++++++++++ 8 files changed, 170 insertions(+) create mode 100644 src/Rule/Assertion/Relation/CanOnlyDepend/CatchBlockRule.php create mode 100644 src/Rule/Assertion/Relation/ShouldNotDepend/CatchBlockRule.php create mode 100644 src/Rule/Extractor/Relation/CatchBlockExtractor.php create mode 100644 tests/unit/rules/CanOnlyDepend/CatchBlockTest.php create mode 100644 tests/unit/rules/ShouldNotDepend/CatchBlockTest.php diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 77309b8c..f38e546e 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,6 +1,9 @@ # Changelog All notable changes to this project will be documented in this file. +## 0.10.14 +* Add dependency detection for catch blocks. + ## 0.10.13 * Fix namespace selector matching similar namespaces diff --git a/extension.neon b/extension.neon index 92cbbdfd..a884acff 100644 --- a/extension.neon +++ b/extension.neon @@ -151,6 +151,10 @@ services: class: PHPat\Rule\Assertion\Relation\ShouldNotDepend\StaticMethodRule tags: - phpstan.rules.rule + - + class: PHPat\Rule\Assertion\Relation\ShouldNotDepend\CatchBlockRule + tags: + - phpstan.rules.rule # CanOnlyDepend rules - @@ -217,6 +221,10 @@ services: class: PHPat\Rule\Assertion\Relation\CanOnlyDepend\StaticMethodRule tags: - phpstan.rules.rule + - + class: PHPat\Rule\Assertion\Relation\CanOnlyDepend\CatchBlockRule + tags: + - phpstan.rules.rule # ShouldNotConstruct rules - diff --git a/src/Rule/Assertion/Relation/CanOnlyDepend/CatchBlockRule.php b/src/Rule/Assertion/Relation/CanOnlyDepend/CatchBlockRule.php new file mode 100644 index 00000000..1ccc4f41 --- /dev/null +++ b/src/Rule/Assertion/Relation/CanOnlyDepend/CatchBlockRule.php @@ -0,0 +1,15 @@ + + */ +final class CatchBlockRule extends CanOnlyDepend implements Rule +{ + use CatchBlockExtractor; +} diff --git a/src/Rule/Assertion/Relation/ShouldNotDepend/CatchBlockRule.php b/src/Rule/Assertion/Relation/ShouldNotDepend/CatchBlockRule.php new file mode 100644 index 00000000..e645c269 --- /dev/null +++ b/src/Rule/Assertion/Relation/ShouldNotDepend/CatchBlockRule.php @@ -0,0 +1,15 @@ + + */ +final class CatchBlockRule extends ShouldNotDepend implements Rule +{ + use CatchBlockExtractor; +} diff --git a/src/Rule/Extractor/Relation/CatchBlockExtractor.php b/src/Rule/Extractor/Relation/CatchBlockExtractor.php new file mode 100644 index 00000000..30f5589a --- /dev/null +++ b/src/Rule/Extractor/Relation/CatchBlockExtractor.php @@ -0,0 +1,24 @@ + + */ + protected function extractNodeClassNames(Node $node, Scope $scope): array + { + return namesToClassStrings($node->types); + } +} diff --git a/tests/fixtures/FixtureClass.php b/tests/fixtures/FixtureClass.php index fc2efdb8..b629e2ac 100644 --- a/tests/fixtures/FixtureClass.php +++ b/tests/fixtures/FixtureClass.php @@ -90,4 +90,11 @@ public function doSomething(string $modelClass) { return new $modelClass(); } + + public function catchException(): void + { + try { + } catch (SimpleException $e) { + } + } } diff --git a/tests/unit/rules/CanOnlyDepend/CatchBlockTest.php b/tests/unit/rules/CanOnlyDepend/CatchBlockTest.php new file mode 100644 index 00000000..84fedcc4 --- /dev/null +++ b/tests/unit/rules/CanOnlyDepend/CatchBlockTest.php @@ -0,0 +1,49 @@ + + * @internal + * @coversNothing + */ +class CatchBlockTest extends RuleTestCase +{ + public const RULE_NAME = 'test_FixtureClassCanOnlyDependSimpleException'; + + public function testRule(): void + { + $this->analyse(['tests/fixtures/FixtureClass.php'], [ + [sprintf('%s should not depend on %s', FixtureClass::class, SimpleException::class), 97], + ]); + } + + protected function getRule(): Rule + { + $testParser = FakeTestParser::create( + self::RULE_NAME, + CanOnlyDepend::class, + [new Classname(FixtureClass::class, false)], + [new Classname(\Exception::class, false)] + ); + + return new CatchBlockRule( + new StatementBuilderFactory($testParser), + new Configuration(false, true, false), + $this->createReflectionProvider(), + self::getContainer()->getByType(FileTypeMapper::class) + ); + } +} diff --git a/tests/unit/rules/ShouldNotDepend/CatchBlockTest.php b/tests/unit/rules/ShouldNotDepend/CatchBlockTest.php new file mode 100644 index 00000000..aa5c9339 --- /dev/null +++ b/tests/unit/rules/ShouldNotDepend/CatchBlockTest.php @@ -0,0 +1,49 @@ + + * @internal + * @coversNothing + */ +class CatchBlockTest extends RuleTestCase +{ + public const RULE_NAME = 'test_FixtureClassShouldNotDependSimpleException'; + + public function testRule(): void + { + $this->analyse(['tests/fixtures/FixtureClass.php'], [ + [sprintf('%s should not depend on %s', FixtureClass::class, SimpleException::class), 97], + ]); + } + + protected function getRule(): Rule + { + $testParser = FakeTestParser::create( + self::RULE_NAME, + ShouldNotDepend::class, + [new Classname(FixtureClass::class, false)], + [new Classname(SimpleException::class, false)] + ); + + return new CatchBlockRule( + new StatementBuilderFactory($testParser), + new Configuration(false, true, false), + $this->createReflectionProvider(), + self::getContainer()->getByType(FileTypeMapper::class) + ); + } +} From 68ed6b6bad551f222c816a773f2828d548bbf23e Mon Sep 17 00:00:00 2001 From: Carlos Alandete Sastre Date: Wed, 21 Feb 2024 11:33:50 +0100 Subject: [PATCH 04/10] Update CHANGELOG --- docs/CHANGELOG.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index f38e546e..4d0d1d29 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -2,10 +2,12 @@ All notable changes to this project will be documented in this file. ## 0.10.14 -* Add dependency detection for catch blocks. +* Add `shouldInclude()` and `shouldNotInclude` assertions. +* Detect catch blocks in dependency assertions. +* Fix shouldBeNamed assertion not functioning on second run. ## 0.10.13 -* Fix namespace selector matching similar namespaces +* Fix namespace selector matching similar namespaces. ## 0.10.12 * Add `shouldBeNamed()` assertion. From 209ae1db3beadd94e8ebd7f33f8e59c263961908 Mon Sep 17 00:00:00 2001 From: Geert Broekmans Date: Sun, 3 Mar 2024 18:39:24 +0100 Subject: [PATCH 05/10] Add ShouldNotBeReadonly assertion --- .github/workflows/integrate.yaml | 4 ++ docs/CHANGELOG.md | 3 ++ docs/documentation/assertions.md | 2 + extension.neon | 6 +++ .../ShouldNotBeReadonly/IsReadonlyRule.php | 15 ++++++ .../ShouldNotBeReadonly.php | 49 +++++++++++++++++++ src/Test/Builder/AssertionStep.php | 8 +++ 7 files changed, 87 insertions(+) create mode 100644 src/Rule/Assertion/Declaration/ShouldNotBeReadonly/IsReadonlyRule.php create mode 100644 src/Rule/Assertion/Declaration/ShouldNotBeReadonly/ShouldNotBeReadonly.php diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index dcc5b536..a77f1b04 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -23,6 +23,7 @@ jobs: - "7.4" - "8.0" - "8.1" + - "8.2" dependencies: - "highest" steps: @@ -60,6 +61,7 @@ jobs: - "7.4" - "8.0" - "8.1" + - "8.2" dependencies: - "highest" steps: @@ -95,6 +97,7 @@ jobs: - "7.4" - "8.0" - "8.1" + - "8.2" dependencies: - "highest" steps: @@ -132,6 +135,7 @@ jobs: - "7.4" - "8.0" - "8.1" + - "8.2" dependencies: - "highest" steps: diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 4d0d1d29..6c4ce424 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,6 +1,9 @@ # Changelog All notable changes to this project will be documented in this file. +## 0.10.15 +* Add `shouldNotBeReadonly` assertion. + ## 0.10.14 * Add `shouldInclude()` and `shouldNotInclude` assertions. * Detect catch blocks in dependency assertions. diff --git a/docs/documentation/assertions.md b/docs/documentation/assertions.md index 369405ed..35868f93 100644 --- a/docs/documentation/assertions.md +++ b/docs/documentation/assertions.md @@ -21,6 +21,8 @@ It asserts that the selected classes are **interfaces**. ## shouldBeReadonly() It asserts that the selected classes are declared as **readonly**. +Also available: `shouldNotBeReadonly()` + ## shouldHaveOnlyOnePublicMethod() It asserts that the selected classes only have **one public method** (besides constructor). diff --git a/extension.neon b/extension.neon index a884acff..c634651e 100644 --- a/extension.neon +++ b/extension.neon @@ -30,6 +30,12 @@ services: tags: - phpstan.rules.rule + # ShouldNotBeReadonly rules + - + class: PHPat\Rule\Assertion\Declaration\ShouldNotBeReadonly\IsReadonlyRule + tags: + - phpstan.rules.rule + # ShouldBeAbstract rules - class: PHPat\Rule\Assertion\Declaration\ShouldBeAbstract\AbstractRule diff --git a/src/Rule/Assertion/Declaration/ShouldNotBeReadonly/IsReadonlyRule.php b/src/Rule/Assertion/Declaration/ShouldNotBeReadonly/IsReadonlyRule.php new file mode 100644 index 00000000..a20fd796 --- /dev/null +++ b/src/Rule/Assertion/Declaration/ShouldNotBeReadonly/IsReadonlyRule.php @@ -0,0 +1,15 @@ + + */ +final class IsReadonlyRule extends ShouldNotBeReadonly implements Rule +{ + use ReadonlyExtractor; +} diff --git a/src/Rule/Assertion/Declaration/ShouldNotBeReadonly/ShouldNotBeReadonly.php b/src/Rule/Assertion/Declaration/ShouldNotBeReadonly/ShouldNotBeReadonly.php new file mode 100644 index 00000000..5507d177 --- /dev/null +++ b/src/Rule/Assertion/Declaration/ShouldNotBeReadonly/ShouldNotBeReadonly.php @@ -0,0 +1,49 @@ + $tips + * @return array + */ + protected function applyValidation(string $ruleName, ClassReflection $subject, bool $meetsDeclaration, array $tips, array $params = []): array + { + return $this->applyShouldNot($ruleName, $subject, $meetsDeclaration, $tips, $params); + } + + protected function getMessage(string $ruleName, string $subject, array $params = []): string + { + return $this->prepareMessage( + $ruleName, + sprintf('%s should not be readonly', $subject) + ); + } +} diff --git a/src/Test/Builder/AssertionStep.php b/src/Test/Builder/AssertionStep.php index 2f8cda0b..b6a2da99 100644 --- a/src/Test/Builder/AssertionStep.php +++ b/src/Test/Builder/AssertionStep.php @@ -10,6 +10,7 @@ use PHPat\Rule\Assertion\Declaration\ShouldHaveOnlyOnePublicMethod\ShouldHaveOnlyOnePublicMethod; use PHPat\Rule\Assertion\Declaration\ShouldNotBeAbstract\ShouldNotBeAbstract; use PHPat\Rule\Assertion\Declaration\ShouldNotBeFinal\ShouldNotBeFinal; +use PHPat\Rule\Assertion\Declaration\ShouldNotBeReadonly\ShouldNotBeReadonly; use PHPat\Rule\Assertion\Relation\CanOnlyDepend\CanOnlyDepend; use PHPat\Rule\Assertion\Relation\ShouldApplyAttribute\ShouldApplyAttribute; use PHPat\Rule\Assertion\Relation\ShouldExtend\ShouldExtend; @@ -52,6 +53,13 @@ public function shouldBeReadonly(): Rule return new BuildStep($this->rule); } + public function shouldNotBeReadonly(): Rule + { + $this->rule->assertion = ShouldNotBeReadonly::class; + + return new BuildStep($this->rule); + } + public function shouldBeFinal(): Rule { $this->rule->assertion = ShouldBeFinal::class; From f051ea63bcd4242f83d79be737d1557b2c86b4f3 Mon Sep 17 00:00:00 2001 From: Carlos A Sastre Date: Sun, 3 Mar 2024 18:44:53 +0100 Subject: [PATCH 06/10] Fix builder declaration tips return type --- src/Test/Builder/AssertionStep.php | 36 +++++++++++++++--------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/Test/Builder/AssertionStep.php b/src/Test/Builder/AssertionStep.php index b6a2da99..1d04cbf0 100644 --- a/src/Test/Builder/AssertionStep.php +++ b/src/Test/Builder/AssertionStep.php @@ -24,54 +24,54 @@ class AssertionStep extends AbstractStep { - public function shouldBeNamed(string $classname, bool $regex = false): Rule + public function shouldBeNamed(string $classname, bool $regex = false): TipOrBuildStep { $this->rule->assertion = ShouldBeNamed::class; $this->rule->params = ['isRegex' => $regex, 'classname' => $classname]; - return new BuildStep($this->rule); + return new TipOrBuildStep($this->rule); } - public function shouldBeAbstract(): Rule + public function shouldBeAbstract(): TipOrBuildStep { $this->rule->assertion = ShouldBeAbstract::class; - return new BuildStep($this->rule); + return new TipOrBuildStep($this->rule); } - public function shouldNotBeAbstract(): Rule + public function shouldNotBeAbstract(): TipOrBuildStep { $this->rule->assertion = ShouldNotBeAbstract::class; - return new BuildStep($this->rule); + return new TipOrBuildStep($this->rule); } - public function shouldBeReadonly(): Rule + public function shouldBeReadonly(): TipOrBuildStep { $this->rule->assertion = ShouldBeReadonly::class; - return new BuildStep($this->rule); + return new TipOrBuildStep($this->rule); } - public function shouldNotBeReadonly(): Rule + public function shouldNotBeReadonly(): TipOrBuildStep { $this->rule->assertion = ShouldNotBeReadonly::class; - return new BuildStep($this->rule); + return new TipOrBuildStep($this->rule); } - public function shouldBeFinal(): Rule + public function shouldBeFinal(): TipOrBuildStep { $this->rule->assertion = ShouldBeFinal::class; - return new BuildStep($this->rule); + return new TipOrBuildStep($this->rule); } - public function shouldNotBeFinal(): Rule + public function shouldNotBeFinal(): TipOrBuildStep { $this->rule->assertion = ShouldNotBeFinal::class; - return new BuildStep($this->rule); + return new TipOrBuildStep($this->rule); } public function shouldNotDependOn(): TargetStep @@ -137,11 +137,11 @@ public function shouldExtend(): TargetStep return new TargetStep($this->rule); } - public function shouldHaveOnlyOnePublicMethod(): Rule + public function shouldHaveOnlyOnePublicMethod(): TipOrBuildStep { $this->rule->assertion = ShouldHaveOnlyOnePublicMethod::class; - return new BuildStep($this->rule); + return new TipOrBuildStep($this->rule); } public function shouldApplyAttribute(): TargetStep @@ -151,10 +151,10 @@ public function shouldApplyAttribute(): TargetStep return new TargetStep($this->rule); } - public function shouldBeInterface(): Rule + public function shouldBeInterface(): TipOrBuildStep { $this->rule->assertion = ShouldBeInterface::class; - return new BuildStep($this->rule); + return new TipOrBuildStep($this->rule); } } From 1bd0e36fca427abdc238c3b6c3eee6dca54bce85 Mon Sep 17 00:00:00 2001 From: Carlos Alandete Sastre Date: Sun, 3 Mar 2024 18:48:43 +0100 Subject: [PATCH 07/10] Update CHANGELOG --- docs/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 6c4ce424..b3eec966 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -3,6 +3,7 @@ All notable changes to this project will be documented in this file. ## 0.10.15 * Add `shouldNotBeReadonly` assertion. +* Fix return types when building declaration rules with tips. ## 0.10.14 * Add `shouldInclude()` and `shouldNotInclude` assertions. From 027eb6a0d27e1828794f3a938bf6242abec5b19e Mon Sep 17 00:00:00 2001 From: Carlos Alandete Sastre Date: Sun, 3 Mar 2024 18:50:34 +0100 Subject: [PATCH 08/10] Run CI on PHP83 --- .github/workflows/integrate.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index a77f1b04..1d02e66e 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -20,10 +20,7 @@ jobs: strategy: matrix: php-version: - - "7.4" - - "8.0" - - "8.1" - - "8.2" + - "8.3" dependencies: - "highest" steps: @@ -62,6 +59,7 @@ jobs: - "8.0" - "8.1" - "8.2" + - "8.3" dependencies: - "highest" steps: @@ -98,6 +96,7 @@ jobs: - "8.0" - "8.1" - "8.2" + - "8.3" dependencies: - "highest" steps: @@ -136,6 +135,7 @@ jobs: - "8.0" - "8.1" - "8.2" + - "8.3" dependencies: - "highest" steps: From 95cd1d1f413d22a783786f128c42f097ef1a340b Mon Sep 17 00:00:00 2001 From: Carlos Alandete Sastre Date: Sun, 3 Mar 2024 18:56:42 +0100 Subject: [PATCH 09/10] Merge PHPStan configurations --- .github/workflows/integrate.yaml | 2 +- ci/phpstan-phpat.neon | 7 ++++++- ci/phpstan.neon | 9 --------- docs/contributing.md | 1 - 4 files changed, 7 insertions(+), 12 deletions(-) delete mode 100644 ci/phpstan.neon diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index 1d02e66e..c7620deb 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -87,7 +87,7 @@ jobs: run: "vendor/bin/phpstan analyse -c ci/phpstan-phpat.neon" static-code-analysis: - name: "Static Code Analysis" + name: "Static Code and Architecture analysis" runs-on: "ubuntu-latest" strategy: matrix: diff --git a/ci/phpstan-phpat.neon b/ci/phpstan-phpat.neon index c758fa56..dcd5f6ac 100644 --- a/ci/phpstan-phpat.neon +++ b/ci/phpstan-phpat.neon @@ -2,10 +2,15 @@ includes: - ../extension.neon parameters: - level: 0 + level: 8 paths: - ../src + - ../tests/unit/rules - ../tests/architecture + ignoreErrors: + - + message: "#no value type specified in iterable type array\\.$#" + path: * phpat: ignore_built_in_classes: false show_rule_names: true diff --git a/ci/phpstan.neon b/ci/phpstan.neon deleted file mode 100644 index b174785c..00000000 --- a/ci/phpstan.neon +++ /dev/null @@ -1,9 +0,0 @@ -parameters: - level: 8 - paths: - - ../src - - ../tests/unit/rules - ignoreErrors: - - - message: "#no value type specified in iterable type array\\.$#" - path: * diff --git a/docs/contributing.md b/docs/contributing.md index 251ac99d..32acd338 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -19,7 +19,6 @@ There are several ways to help out: ```bash composer validate --strict vendor/bin/php-cs-fixer fix --config ./ci/php-cs-fixer.php -vendor/bin/phpstan analyse -c ci/phpstan.neon vendor/bin/phpstan analyse -c ci/phpstan-phpat.neon vendor/bin/psalm -c ci/psalm.xml vendor/bin/phpunit tests/unit/ From f1ecc120a566ad6144add3b55fe01e16d8ab28bd Mon Sep 17 00:00:00 2001 From: Carlos Alandete Sastre Date: Sun, 3 Mar 2024 18:59:17 +0100 Subject: [PATCH 10/10] Merge PHPStan configurations --- .github/workflows/integrate.yaml | 43 +++----------------------------- 1 file changed, 3 insertions(+), 40 deletions(-) diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index c7620deb..ad43ef1d 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -49,43 +49,6 @@ jobs: - name: "Run friendsofphp/php-cs-fixer" run: "vendor/bin/php-cs-fixer check --config ./ci/php-cs-fixer.php" - architecture-analysis: - name: "Architecture analysis" - runs-on: "ubuntu-latest" - strategy: - matrix: - php-version: - - "7.4" - - "8.0" - - "8.1" - - "8.2" - - "8.3" - dependencies: - - "highest" - steps: - - name: "Checkout" - uses: "actions/checkout@v2" - - name: "Install PHP with extensions" - uses: "shivammathur/setup-php@v2" - with: - coverage: "none" - extensions: "${{ env.PHP_EXTENSIONS }}" - php-version: "${{ matrix.php-version }}" - - name: "Determine composer cache directory" - uses: "./.github/actions/composer/composer/determine-cache-directory" - - name: "Cache dependencies installed with composer" - uses: "actions/cache@v2" - with: - path: "${{ env.COMPOSER_CACHE_DIR }}" - key: "php-${{ matrix.php-version }}-composer-${{ matrix.dependencies }}-${{ hashFiles('composer.lock') }}" - restore-keys: "php-${{ matrix.php-version }}-composer-${{ matrix.dependencies }}-" - - name: "Install ${{ matrix.dependencies }} dependencies with composer" - uses: "./.github/actions/composer/composer/install" - with: - dependencies: "${{ matrix.dependencies }}" - - name: "Run phpstan+phpat for architecture tests" - run: "vendor/bin/phpstan analyse -c ci/phpstan-phpat.neon" - static-code-analysis: name: "Static Code and Architecture analysis" runs-on: "ubuntu-latest" @@ -120,9 +83,9 @@ jobs: uses: "./.github/actions/composer/composer/install" with: dependencies: "${{ matrix.dependencies }}" - - name: "Run phpstan/phpstan" - run: "vendor/bin/phpstan analyse -c ci/phpstan.neon" - - name: "Run vimeo/psalm" + - name: "Run phpstan+phpat" + run: "vendor/bin/phpstan analyse -c ci/phpstan-phpat.neon" + - name: "Run psalm" run: "vendor/bin/psalm -c ci/psalm.xml" tests: