Skip to content

Commit

Permalink
Moved illegalConstructorMethodCall rules from phpstan to phpstan-stri…
Browse files Browse the repository at this point in the history
…ct-rules
  • Loading branch information
ondrejmirtes committed Sep 24, 2024
1 parent ad53bd9 commit 63956f7
Show file tree
Hide file tree
Showing 8 changed files with 378 additions and 2 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ parameters:
switchConditionsMatchingType: false
noVariableVariables: false
strictArrayFilter: false
illegalConstructorMethodCall: false
```

Aside from introducing new custom rules, phpstan-strict-rules also [change the default values of some configuration parameters](https://github.com/phpstan/phpstan-strict-rules/blob/1.6.x/rules.neon#L1) that are present in PHPStan itself. These parameters are [documented on phpstan.org](https://phpstan.org/config-reference#stricter-analysis).
Expand Down
14 changes: 12 additions & 2 deletions rules.neon
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ parameters:
reportStaticMethodSignatures: true
reportMaybesInPropertyPhpDocTypes: true
reportWrongPhpDocTypeInVarTag: true
featureToggles:
illegalConstructorMethodCall: true
strictRules:
allRules: true
disallowedLooseComparison: %strictRules.allRules%
Expand All @@ -31,6 +29,7 @@ parameters:
switchConditionsMatchingType: %strictRules.allRules%
noVariableVariables: %strictRules.allRules%
strictArrayFilter: %strictRules.allRules%
illegalConstructorMethodCall: %strictRules.allRules%

parametersSchema:
strictRules: structure([
Expand All @@ -52,6 +51,7 @@ parametersSchema:
switchConditionsMatchingType: anyOf(bool(), arrayOf(bool()))
noVariableVariables: anyOf(bool(), arrayOf(bool()))
strictArrayFilter: anyOf(bool(), arrayOf(bool()))
illegalConstructorMethodCall: anyOf(bool(), arrayOf(bool()))
])

conditionalTags:
Expand Down Expand Up @@ -133,6 +133,10 @@ conditionalTags:
phpstan.rules.rule: %strictRules.noVariableVariables%
PHPStan\Rules\VariableVariables\VariablePropertyFetchRule:
phpstan.rules.rule: %strictRules.noVariableVariables%
PHPStan\Rules\Methods\IllegalConstructorMethodCallRule:
phpstan.rules.rule: %strictRules.illegalConstructorMethodCall%
PHPStan\Rules\Methods\IllegalConstructorStaticCallRule:
phpstan.rules.rule: %strictRules.illegalConstructorMethodCall%

services:
-
Expand Down Expand Up @@ -207,6 +211,12 @@ services:
-
class: PHPStan\Rules\Methods\WrongCaseOfInheritedMethodRule

-
class: PHPStan\Rules\Methods\IllegalConstructorMethodCallRule

-
class: PHPStan\Rules\Methods\IllegalConstructorStaticCallRule

-
class: PHPStan\Rules\Operators\OperandInArithmeticPostDecrementRule

Expand Down
34 changes: 34 additions & 0 deletions src/Rules/Methods/IllegalConstructorMethodCallRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Methods;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;

/**
* @implements Rule<Node\Expr\MethodCall>
*/
final 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->toLowerString() !== '__construct') {
return [];
}

return [
RuleErrorBuilder::message('Call to __construct() on an existing object is not allowed.')
->identifier('constructor.call')
->build(),
];
}

}
92 changes: 92 additions & 0 deletions src/Rules/Methods/IllegalConstructorStaticCallRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Methods;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use function array_key_exists;
use function array_map;
use function in_array;
use function sprintf;
use function strtolower;

/**
* @implements Rule<Node\Expr\StaticCall>
*/
final 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->toLowerString() !== '__construct') {
return [];
}

if ($this->isCollectCallingConstructor($node, $scope)) {
return [];
}

return [
RuleErrorBuilder::message('Static call to __construct() is only allowed on a parent class in the constructor.')
->identifier('constructor.call')
->build(),
];
}

private function isCollectCallingConstructor(Node\Expr\StaticCall $node, Scope $scope): bool
{
// __construct should be called from inside constructor
if ($scope->getFunction() === null) {
return false;
}

if ($scope->getFunction()->getName() !== '__construct') {
if (!$this->isInRenamedTraitConstructor($scope)) {
return false;
}
}

if (!$scope->isInClass()) {
return false;
}

if (!$node->class instanceof Node\Name) {
return false;
}

$parentClasses = array_map(static fn (string $name) => strtolower($name), $scope->getClassReflection()->getParentClassesNames());

return in_array(strtolower($scope->resolveName($node->class)), $parentClasses, true);
}

private function isInRenamedTraitConstructor(Scope $scope): bool
{
if (!$scope->isInClass()) {
return false;
}

if (!$scope->isInTrait()) {
return false;
}

if ($scope->getFunction() === null) {
return false;
}

$traitAliases = $scope->getClassReflection()->getNativeReflection()->getTraitAliases();
$functionName = $scope->getFunction()->getName();
if (!array_key_exists($functionName, $traitAliases)) {
return false;
}

return $traitAliases[$functionName] === sprintf('%s::%s', $scope->getTraitReflection()->getName(), '__construct');
}

}
37 changes: 37 additions & 0 deletions tests/Rules/Methods/IllegalConstructorMethodCallRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Methods;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;

/**
* @extends RuleTestCase<IllegalConstructorMethodCallRule>
*/
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'], [
[
'Call to __construct() on an existing object is not allowed.',
13,
],
[
'Call to __construct() on an existing object is not allowed.',
18,
],
[
'Call to __construct() on an existing object is not allowed.',
60,
],
]);
}

}
59 changes: 59 additions & 0 deletions tests/Rules/Methods/IllegalConstructorStaticCallRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Methods;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<IllegalConstructorStaticCallRule>
*/
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'], [
[
'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,
],
[
'Static call to __construct() is only allowed on a parent class in the constructor.',
50,
],
[
'Static call to __construct() is only allowed on a parent class in the constructor.',
100,
],
]);
}

public function testBug9577(): void
{
if (PHP_VERSION_ID < 80100) {
self::markTestSkipped('Test requires PHP 8.1.');
}

$this->analyse([__DIR__ . '/data/bug-9577.php'], []);
}

}
40 changes: 40 additions & 0 deletions tests/Rules/Methods/data/bug-9577.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php // lint >= 8.1

namespace Bug9577IllegalConstructorStaticCall;

trait StringableMessageTrait
{
public function __construct(
private readonly \Stringable $StringableMessage,
int $code = 0,
?\Throwable $previous = null,
) {
parent::__construct((string) $StringableMessage, $code, $previous);
}

public function getStringableMessage(): \Stringable
{
return $this->StringableMessage;
}
}

class SpecializedException extends \RuntimeException
{
use StringableMessageTrait {
StringableMessageTrait::__construct as __traitConstruct;
}

public function __construct(
private readonly object $aService,
\Stringable $StringableMessage,
int $code = 0,
?\Throwable $previous = null,
) {
$this->__traitConstruct($StringableMessage, $code, $previous);
}

public function getService(): object
{
return $this->aService;
}
}
Loading

0 comments on commit 63956f7

Please sign in to comment.