From f0579f0e755f88d20ea17d9c9ddaffd3cefdfb9c Mon Sep 17 00:00:00 2001 From: n-ogawa Date: Tue, 12 Apr 2022 23:12:09 +0900 Subject: [PATCH 1/9] Add Rule to check method call constructor --- .../IllegalConstructorMethodCallRule.php | 44 ++++++++++++++++ .../IllegalConstructorStaticCallRule.php | 45 ++++++++++++++++ .../IllegalConstructorMethodCallRuleTest.php | 33 ++++++++++++ .../IllegalConstructorStaticCallRuleTest.php | 33 ++++++++++++ .../illegal-constructor-call-rule-test.php | 51 +++++++++++++++++++ 5 files changed, 206 insertions(+) create mode 100644 src/Rules/Methods/IllegalConstructorMethodCallRule.php create mode 100644 src/Rules/Methods/IllegalConstructorStaticCallRule.php create mode 100644 tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php create mode 100644 tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php create mode 100644 tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php diff --git a/src/Rules/Methods/IllegalConstructorMethodCallRule.php b/src/Rules/Methods/IllegalConstructorMethodCallRule.php new file mode 100644 index 0000000000..e67e65c000 --- /dev/null +++ b/src/Rules/Methods/IllegalConstructorMethodCallRule.php @@ -0,0 +1,44 @@ + + */ +class IllegalConstructorMethodCallRule implements Rule +{ + + public function getNodeType(): string + { + return Node\Expr\MethodCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$node->name instanceof Node\Identifier || $node->name->name !== '__construct') { + return []; + } + + return $this->isCollectCallingConstructor($node, $scope) + ? [] + : ['__construct() should not be called outside constructor.']; + } + + private function isCollectCallingConstructor(Node $node, Scope $scope): bool + { + if (!$node instanceof Node\Expr\MethodCall) { + return true; + } + // __construct should be called from inside constructor + if ($scope->getFunction() !== null && $scope->getFunction()->getName() !== '__construct') { + return false; + } + // In constructor, method call is allowed with 'this'; + return $node->var instanceof Node\Expr\Variable && $node->var->name === 'this'; + } + +} diff --git a/src/Rules/Methods/IllegalConstructorStaticCallRule.php b/src/Rules/Methods/IllegalConstructorStaticCallRule.php new file mode 100644 index 0000000000..8774d6f8cf --- /dev/null +++ b/src/Rules/Methods/IllegalConstructorStaticCallRule.php @@ -0,0 +1,45 @@ + + */ +class IllegalConstructorStaticCallRule implements Rule +{ + + public function getNodeType(): string + { + return Node\Expr\StaticCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$node->name instanceof Node\Identifier || $node->name->name !== '__construct') { + return []; + } + + return $this->isCollectCallingConstructor($node, $scope) + ? [] + : ['__construct() should not be called outside constructor.']; + } + + private function isCollectCallingConstructor(Node $node, Scope $scope): bool + { + if (!$node instanceof Node\Expr\StaticCall) { + return true; + } + // __construct should be called from inside constructor + if ($scope->getFunction() !== null && $scope->getFunction()->getName() !== '__construct') { + return false; + } + // In constructor, static call is allowed with 'self' or 'parent; + return $node->class instanceof Node\Name && in_array($node->class->getFirst(), ['self', 'parent'], true); + } + +} diff --git a/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php b/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php new file mode 100644 index 0000000000..22e6348d2d --- /dev/null +++ b/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php @@ -0,0 +1,33 @@ + + */ +class IllegalConstructorMethodCallRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new IllegalConstructorMethodCallRule(); + } + + public function testMethods(): void + { + $this->analyse([__DIR__ . '/data/illegal-constructor-call-rule-test.php'], [ + [ + '__construct() should not be called outside constructor.', + 16, + ], + [ + '__construct() should not be called outside constructor.', + 51, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php b/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php new file mode 100644 index 0000000000..a9876a20fb --- /dev/null +++ b/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php @@ -0,0 +1,33 @@ + + */ +class IllegalConstructorStaticCallRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new IllegalConstructorStaticCallRule(); + } + + public function testMethods(): void + { + $this->analyse([__DIR__ . '/data/illegal-constructor-call-rule-test.php'], [ + [ + '__construct() should not be called outside constructor.', + 29, + ], + [ + '__construct() should not be called outside constructor.', + 46, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php b/tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php new file mode 100644 index 0000000000..e4ff440581 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php @@ -0,0 +1,51 @@ + 1) { + return; + } + $this->__construct($datetime, $timezone); + } + + public function mutate(string $datetime = "now", ?DateTimeZone $timezone = null): void + { + $this->__construct($datetime, $timezone); + } +} + +class ExtendedDateTimeWithParentCall extends DateTimeImmutable +{ + public function __construct(string $datetime = "now", ?DateTimeZone $timezone = null) + { + parent::__construct($datetime, $timezone); + } + + public function mutate(string $datetime = "now", ?DateTimeZone $timezone = null): void + { + parent::__construct($datetime, $timezone); + } +} + +class ExtendedDateTimeWithSelfCall extends DateTimeImmutable +{ + public function __construct(string $datetime = "now", ?DateTimeZone $timezone = null) + { + // Avoid infinite loop + if (count(debug_backtrace()) > 1) { + return; + } + self::__construct($datetime, $timezone); + } + + public function mutate(string $datetime = "now", ?DateTimeZone $timezone = null): void + { + self::__construct($datetime, $timezone); + } +} + +$extendedDateTime = new ExtendedDateTimeWithMethodCall('2022/04/12'); +$extendedDateTime->__construct('2022/04/13'); From 2ceb04c057478ffab1279ee6d91a5b79b273521f Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 17 Apr 2022 12:46:55 +0200 Subject: [PATCH 2/9] Register the rules on level 2 so that they're executed (but only with bleedingEdge) --- conf/bleedingEdge.neon | 1 + conf/config.level2.neon | 10 ++++++++++ conf/config.neon | 2 ++ 3 files changed, 13 insertions(+) diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 61d5429b49..5495a500ed 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -8,3 +8,4 @@ parameters: arrayUnpacking: true nodeConnectingVisitorCompatibility: false disableRuntimeReflectionProvider: true + illegalConstructorMethodCall: true diff --git a/conf/config.level2.neon b/conf/config.level2.neon index becc7b5b02..8beb42e5f0 100644 --- a/conf/config.level2.neon +++ b/conf/config.level2.neon @@ -40,6 +40,12 @@ rules: - PHPStan\Rules\PhpDoc\WrongVariableNameInVarTagRule - PHPStan\Rules\Properties\AccessPrivatePropertyThroughStaticRule +conditionalTags: + PHPStan\Rules\Methods\IllegalConstructorMethodCallRule: + phpstan.rules.rule: %featureToggles.illegalConstructorMethodCall% + PHPStan\Rules\Methods\IllegalConstructorStaticCallRule: + phpstan.rules.rule: %featureToggles.illegalConstructorMethodCall% + services: - class: PHPStan\Rules\Classes\MixinRule @@ -53,6 +59,10 @@ services: reportMaybes: %reportMaybes% tags: - phpstan.rules.rule + - + class: PHPStan\Rules\Methods\IllegalConstructorMethodCallRule + - + class: PHPStan\Rules\Methods\IllegalConstructorStaticCallRule - class: PHPStan\Rules\PhpDoc\InvalidPhpDocVarTagTypeRule arguments: diff --git a/conf/config.neon b/conf/config.neon index 1210d69efc..144dbf1210 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -32,6 +32,7 @@ parameters: arrayFilter: false arrayUnpacking: false nodeConnectingVisitorCompatibility: true + illegalConstructorMethodCall: false fileExtensions: - php checkAdvancedIsset: false @@ -219,6 +220,7 @@ parametersSchema: arrayFilter: bool(), arrayUnpacking: bool(), nodeConnectingVisitorCompatibility: bool(), + illegalConstructorMethodCall: bool(), ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() From 1458d225056d6955a383f37fb6157983ea5feaef Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 17 Apr 2022 12:48:33 +0200 Subject: [PATCH 3/9] Use RuleErrorBuilder --- .../Methods/IllegalConstructorMethodCallRule.php | 12 +++++++++--- .../Methods/IllegalConstructorStaticCallRule.php | 12 +++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/Rules/Methods/IllegalConstructorMethodCallRule.php b/src/Rules/Methods/IllegalConstructorMethodCallRule.php index e67e65c000..870b0fb6d7 100644 --- a/src/Rules/Methods/IllegalConstructorMethodCallRule.php +++ b/src/Rules/Methods/IllegalConstructorMethodCallRule.php @@ -5,6 +5,7 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; +use PHPStan\Rules\RuleErrorBuilder; /** * @implements Rule @@ -23,9 +24,14 @@ public function processNode(Node $node, Scope $scope): array return []; } - return $this->isCollectCallingConstructor($node, $scope) - ? [] - : ['__construct() should not be called outside constructor.']; + if ($this->isCollectCallingConstructor($node, $scope)) { + return []; + } + + return [ + RuleErrorBuilder::message('__construct() should not be called outside constructor.') + ->build(), + ]; } private function isCollectCallingConstructor(Node $node, Scope $scope): bool diff --git a/src/Rules/Methods/IllegalConstructorStaticCallRule.php b/src/Rules/Methods/IllegalConstructorStaticCallRule.php index 8774d6f8cf..b31c9b57f7 100644 --- a/src/Rules/Methods/IllegalConstructorStaticCallRule.php +++ b/src/Rules/Methods/IllegalConstructorStaticCallRule.php @@ -5,6 +5,7 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; +use PHPStan\Rules\RuleErrorBuilder; use function in_array; /** @@ -24,9 +25,14 @@ public function processNode(Node $node, Scope $scope): array return []; } - return $this->isCollectCallingConstructor($node, $scope) - ? [] - : ['__construct() should not be called outside constructor.']; + if ($this->isCollectCallingConstructor($node, $scope)) { + return []; + } + + return [ + RuleErrorBuilder::message('__construct() should not be called outside constructor.') + ->build(), + ]; } private function isCollectCallingConstructor(Node $node, Scope $scope): bool From 17b0800be82be0a79b2cfca600236c0c179e5a3d Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 17 Apr 2022 12:52:28 +0200 Subject: [PATCH 4/9] Improve error messages --- src/Rules/Methods/IllegalConstructorMethodCallRule.php | 2 +- src/Rules/Methods/IllegalConstructorStaticCallRule.php | 2 +- .../Rules/Methods/IllegalConstructorMethodCallRuleTest.php | 4 ++-- .../Rules/Methods/IllegalConstructorStaticCallRuleTest.php | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Rules/Methods/IllegalConstructorMethodCallRule.php b/src/Rules/Methods/IllegalConstructorMethodCallRule.php index 870b0fb6d7..ebd4d94f6e 100644 --- a/src/Rules/Methods/IllegalConstructorMethodCallRule.php +++ b/src/Rules/Methods/IllegalConstructorMethodCallRule.php @@ -29,7 +29,7 @@ public function processNode(Node $node, Scope $scope): array } return [ - RuleErrorBuilder::message('__construct() should not be called outside constructor.') + RuleErrorBuilder::message('Call to __construct() on an existing object is not allowed.') ->build(), ]; } diff --git a/src/Rules/Methods/IllegalConstructorStaticCallRule.php b/src/Rules/Methods/IllegalConstructorStaticCallRule.php index b31c9b57f7..6e620b21f5 100644 --- a/src/Rules/Methods/IllegalConstructorStaticCallRule.php +++ b/src/Rules/Methods/IllegalConstructorStaticCallRule.php @@ -30,7 +30,7 @@ public function processNode(Node $node, Scope $scope): array } return [ - RuleErrorBuilder::message('__construct() should not be called outside constructor.') + RuleErrorBuilder::message('Static call to __construct() is only allowed on a parent class in the constructor.') ->build(), ]; } diff --git a/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php b/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php index 22e6348d2d..98d5aee61c 100644 --- a/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php @@ -20,11 +20,11 @@ public function testMethods(): void { $this->analyse([__DIR__ . '/data/illegal-constructor-call-rule-test.php'], [ [ - '__construct() should not be called outside constructor.', + 'Call to __construct() on an existing object is not allowed.', 16, ], [ - '__construct() should not be called outside constructor.', + 'Call to __construct() on an existing object is not allowed.', 51, ], ]); diff --git a/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php b/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php index a9876a20fb..70a1f0fa77 100644 --- a/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php +++ b/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php @@ -20,11 +20,11 @@ public function testMethods(): void { $this->analyse([__DIR__ . '/data/illegal-constructor-call-rule-test.php'], [ [ - '__construct() should not be called outside constructor.', + 'Static call to __construct() is only allowed on a parent class in the constructor.', 29, ], [ - '__construct() should not be called outside constructor.', + 'Static call to __construct() is only allowed on a parent class in the constructor.', 46, ], ]); From 71a664064ba9679137347facb6aab986a8731c65 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 17 Apr 2022 12:54:02 +0200 Subject: [PATCH 5/9] Be case-insensitive --- src/Rules/Methods/IllegalConstructorMethodCallRule.php | 2 +- src/Rules/Methods/IllegalConstructorStaticCallRule.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Rules/Methods/IllegalConstructorMethodCallRule.php b/src/Rules/Methods/IllegalConstructorMethodCallRule.php index ebd4d94f6e..c99e5814ba 100644 --- a/src/Rules/Methods/IllegalConstructorMethodCallRule.php +++ b/src/Rules/Methods/IllegalConstructorMethodCallRule.php @@ -20,7 +20,7 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { - if (!$node->name instanceof Node\Identifier || $node->name->name !== '__construct') { + if (!$node->name instanceof Node\Identifier || $node->name->toLowerString() !== '__construct') { return []; } diff --git a/src/Rules/Methods/IllegalConstructorStaticCallRule.php b/src/Rules/Methods/IllegalConstructorStaticCallRule.php index 6e620b21f5..8ff692bb5f 100644 --- a/src/Rules/Methods/IllegalConstructorStaticCallRule.php +++ b/src/Rules/Methods/IllegalConstructorStaticCallRule.php @@ -21,7 +21,7 @@ public function getNodeType(): string public function processNode(Node $node, Scope $scope): array { - if (!$node->name instanceof Node\Identifier || $node->name->name !== '__construct') { + if (!$node->name instanceof Node\Identifier || $node->name->toLowerString() !== '__construct') { return []; } From 72a3a3dedda743a0425c92cd99591cd05cb59e01 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 17 Apr 2022 12:55:27 +0200 Subject: [PATCH 6/9] Adjust test --- .../Methods/IllegalConstructorMethodCallRuleTest.php | 2 +- .../data/illegal-constructor-call-rule-test.php | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php b/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php index 98d5aee61c..d78bf3d1cb 100644 --- a/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php @@ -25,7 +25,7 @@ public function testMethods(): void ], [ 'Call to __construct() on an existing object is not allowed.', - 51, + 56, ], ]); } diff --git a/tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php b/tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php index e4ff440581..e902002e31 100644 --- a/tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php +++ b/tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php @@ -47,5 +47,13 @@ public function mutate(string $datetime = "now", ?DateTimeZone $timezone = null) } } -$extendedDateTime = new ExtendedDateTimeWithMethodCall('2022/04/12'); -$extendedDateTime->__construct('2022/04/13'); +class Foo +{ + + public function doFoo() + { + $extendedDateTime = new ExtendedDateTimeWithMethodCall('2022/04/12'); + $extendedDateTime->__construct('2022/04/13'); + } + +} From 4daa27c0189a3adb0775e0213ebab6f1dec4c846 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 17 Apr 2022 13:00:08 +0200 Subject: [PATCH 7/9] Behave the same way with "self" like with the actual class name --- .../IllegalConstructorStaticCallRule.php | 19 ++++++++++++++++--- .../IllegalConstructorMethodCallRuleTest.php | 2 +- .../IllegalConstructorStaticCallRuleTest.php | 6 +++++- .../illegal-constructor-call-rule-test.php | 2 ++ 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/Rules/Methods/IllegalConstructorStaticCallRule.php b/src/Rules/Methods/IllegalConstructorStaticCallRule.php index 8ff692bb5f..f1fe067c99 100644 --- a/src/Rules/Methods/IllegalConstructorStaticCallRule.php +++ b/src/Rules/Methods/IllegalConstructorStaticCallRule.php @@ -6,7 +6,6 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use function in_array; /** * @implements Rule @@ -44,8 +43,22 @@ private function isCollectCallingConstructor(Node $node, Scope $scope): bool if ($scope->getFunction() !== null && $scope->getFunction()->getName() !== '__construct') { return false; } - // In constructor, static call is allowed with 'self' or 'parent; - return $node->class instanceof Node\Name && in_array($node->class->getFirst(), ['self', 'parent'], true); + + if (!$scope->isInClass()) { + return false; + } + + if (!$node->class instanceof Node\Name) { + return false; + } + + if ($node->class->toLowerString() === 'parent') { + return true; + } + + $className = $scope->resolveName($node->class); + + return $className === $scope->getClassReflection()->getName(); } } diff --git a/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php b/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php index d78bf3d1cb..a609162a70 100644 --- a/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php @@ -25,7 +25,7 @@ public function testMethods(): void ], [ 'Call to __construct() on an existing object is not allowed.', - 56, + 58, ], ]); } diff --git a/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php b/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php index 70a1f0fa77..6ec761d7a6 100644 --- a/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php +++ b/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php @@ -25,7 +25,11 @@ public function testMethods(): void ], [ 'Static call to __construct() is only allowed on a parent class in the constructor.', - 46, + 47, + ], + [ + 'Static call to __construct() is only allowed on a parent class in the constructor.', + 48, ], ]); } diff --git a/tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php b/tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php index e902002e31..8df51b725f 100644 --- a/tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php +++ b/tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php @@ -39,11 +39,13 @@ public function __construct(string $datetime = "now", ?DateTimeZone $timezone = return; } self::__construct($datetime, $timezone); + ExtendedDateTimeWithSelfCall::__construct($datetime, $timezone); } public function mutate(string $datetime = "now", ?DateTimeZone $timezone = null): void { self::__construct($datetime, $timezone); + ExtendedDateTimeWithSelfCall::__construct($datetime, $timezone); } } From f7cd97a2f1e734f6812f2744c4f72eb0c14ecd80 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 17 Apr 2022 13:05:40 +0200 Subject: [PATCH 8/9] Test files must have a namespace --- .../IllegalConstructorMethodCallRuleTest.php | 4 ++-- .../IllegalConstructorStaticCallRuleTest.php | 6 +++--- .../illegal-constructor-call-rule-test.php | 20 ++++++++++--------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php b/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php index a609162a70..0fc10a5e81 100644 --- a/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php @@ -21,11 +21,11 @@ public function testMethods(): void $this->analyse([__DIR__ . '/data/illegal-constructor-call-rule-test.php'], [ [ 'Call to __construct() on an existing object is not allowed.', - 16, + 18, ], [ 'Call to __construct() on an existing object is not allowed.', - 58, + 60, ], ]); } diff --git a/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php b/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php index 6ec761d7a6..eb1232cb9a 100644 --- a/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php +++ b/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php @@ -21,15 +21,15 @@ public function testMethods(): void $this->analyse([__DIR__ . '/data/illegal-constructor-call-rule-test.php'], [ [ 'Static call to __construct() is only allowed on a parent class in the constructor.', - 29, + 31, ], [ 'Static call to __construct() is only allowed on a parent class in the constructor.', - 47, + 49, ], [ 'Static call to __construct() is only allowed on a parent class in the constructor.', - 48, + 50, ], ]); } diff --git a/tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php b/tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php index 8df51b725f..f59fbde165 100644 --- a/tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php +++ b/tests/PHPStan/Rules/Methods/data/illegal-constructor-call-rule-test.php @@ -1,8 +1,10 @@ 1) { @@ -11,28 +13,28 @@ public function __construct(string $datetime = "now", ?DateTimeZone $timezone = $this->__construct($datetime, $timezone); } - public function mutate(string $datetime = "now", ?DateTimeZone $timezone = null): void + public function mutate(string $datetime = "now", ?\DateTimeZone $timezone = null): void { $this->__construct($datetime, $timezone); } } -class ExtendedDateTimeWithParentCall extends DateTimeImmutable +class ExtendedDateTimeWithParentCall extends \DateTimeImmutable { - public function __construct(string $datetime = "now", ?DateTimeZone $timezone = null) + public function __construct(string $datetime = "now", ?\DateTimeZone $timezone = null) { parent::__construct($datetime, $timezone); } - public function mutate(string $datetime = "now", ?DateTimeZone $timezone = null): void + public function mutate(string $datetime = "now", ?\DateTimeZone $timezone = null): void { parent::__construct($datetime, $timezone); } } -class ExtendedDateTimeWithSelfCall extends DateTimeImmutable +class ExtendedDateTimeWithSelfCall extends \DateTimeImmutable { - public function __construct(string $datetime = "now", ?DateTimeZone $timezone = null) + public function __construct(string $datetime = "now", ?\DateTimeZone $timezone = null) { // Avoid infinite loop if (count(debug_backtrace()) > 1) { @@ -42,7 +44,7 @@ public function __construct(string $datetime = "now", ?DateTimeZone $timezone = ExtendedDateTimeWithSelfCall::__construct($datetime, $timezone); } - public function mutate(string $datetime = "now", ?DateTimeZone $timezone = null): void + public function mutate(string $datetime = "now", ?\DateTimeZone $timezone = null): void { self::__construct($datetime, $timezone); ExtendedDateTimeWithSelfCall::__construct($datetime, $timezone); From 18930cca0e2eb932733a78d3e22b703bad4f33da Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 17 Apr 2022 13:06:58 +0200 Subject: [PATCH 9/9] All of these should be a violation --- .../IllegalConstructorMethodCallRule.php | 17 ----------------- .../IllegalConstructorStaticCallRule.php | 8 +------- .../IllegalConstructorMethodCallRuleTest.php | 4 ++++ .../IllegalConstructorStaticCallRuleTest.php | 8 ++++++++ 4 files changed, 13 insertions(+), 24 deletions(-) diff --git a/src/Rules/Methods/IllegalConstructorMethodCallRule.php b/src/Rules/Methods/IllegalConstructorMethodCallRule.php index c99e5814ba..94eb726ab3 100644 --- a/src/Rules/Methods/IllegalConstructorMethodCallRule.php +++ b/src/Rules/Methods/IllegalConstructorMethodCallRule.php @@ -24,27 +24,10 @@ public function processNode(Node $node, Scope $scope): array return []; } - if ($this->isCollectCallingConstructor($node, $scope)) { - return []; - } - return [ RuleErrorBuilder::message('Call to __construct() on an existing object is not allowed.') ->build(), ]; } - private function isCollectCallingConstructor(Node $node, Scope $scope): bool - { - if (!$node instanceof Node\Expr\MethodCall) { - return true; - } - // __construct should be called from inside constructor - if ($scope->getFunction() !== null && $scope->getFunction()->getName() !== '__construct') { - return false; - } - // In constructor, method call is allowed with 'this'; - return $node->var instanceof Node\Expr\Variable && $node->var->name === 'this'; - } - } diff --git a/src/Rules/Methods/IllegalConstructorStaticCallRule.php b/src/Rules/Methods/IllegalConstructorStaticCallRule.php index f1fe067c99..42647f1de8 100644 --- a/src/Rules/Methods/IllegalConstructorStaticCallRule.php +++ b/src/Rules/Methods/IllegalConstructorStaticCallRule.php @@ -52,13 +52,7 @@ private function isCollectCallingConstructor(Node $node, Scope $scope): bool return false; } - if ($node->class->toLowerString() === 'parent') { - return true; - } - - $className = $scope->resolveName($node->class); - - return $className === $scope->getClassReflection()->getName(); + return $node->class->toLowerString() === 'parent'; } } diff --git a/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php b/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php index 0fc10a5e81..8b0f957e85 100644 --- a/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php +++ b/tests/PHPStan/Rules/Methods/IllegalConstructorMethodCallRuleTest.php @@ -19,6 +19,10 @@ protected function getRule(): Rule public function testMethods(): void { $this->analyse([__DIR__ . '/data/illegal-constructor-call-rule-test.php'], [ + [ + 'Call to __construct() on an existing object is not allowed.', + 13, + ], [ 'Call to __construct() on an existing object is not allowed.', 18, diff --git a/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php b/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php index eb1232cb9a..922c497fa0 100644 --- a/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php +++ b/tests/PHPStan/Rules/Methods/IllegalConstructorStaticCallRuleTest.php @@ -23,6 +23,14 @@ public function testMethods(): void 'Static call to __construct() is only allowed on a parent class in the constructor.', 31, ], + [ + 'Static call to __construct() is only allowed on a parent class in the constructor.', + 43, + ], + [ + 'Static call to __construct() is only allowed on a parent class in the constructor.', + 44, + ], [ 'Static call to __construct() is only allowed on a parent class in the constructor.', 49,