Skip to content

Commit

Permalink
ConflictingTraitConstantsRule
Browse files Browse the repository at this point in the history
  • Loading branch information
@ondrejmirtes
ondrejmirtes committed Nov 13, 2023
1 parent 64ffdd6 commit f94a3c3
Show file tree
Hide file tree
Showing 6 changed files with 437 additions and 0 deletions.
1 change: 1 addition & 0 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ lint:
--exclude tests/PHPStan/Rules/Methods/data/bug-9014.php \
--exclude tests/PHPStan/Rules/Methods/data/bug-10101.php \
--exclude tests/PHPStan/Rules/Methods/data/final-method-by-phpdoc.php \
--exclude tests/PHPStan/Rules/Traits/data/conflicting-trait-constants-types.php \
src tests

cs:
Expand Down
1 change: 1 addition & 0 deletions conf/config.level0.neon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ rules:
- PHPStan\Rules\Properties\PropertiesInInterfaceRule
- PHPStan\Rules\Properties\PropertyAttributesRule
- PHPStan\Rules\Properties\ReadOnlyPropertyRule
- PHPStan\Rules\Traits\ConflictingTraitConstantsRule
- PHPStan\Rules\Traits\ConstantsInTraitsRule
- PHPStan\Rules\Variables\UnsetRule
- PHPStan\Rules\Whitespace\FileWhitespaceRule
Expand Down
212 changes: 212 additions & 0 deletions src/Rules/Traits/ConflictingTraitConstantsRule.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,212 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Traits;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionClassConstant;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Reflection\InitializerExprContext;
use PHPStan\Reflection\InitializerExprTypeResolver;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\ParserNodeTypeToPHPStanType;
use PHPStan\Type\TypehintHelper;
use PHPStan\Type\VerbosityLevel;
use function sprintf;

/**
* @implements Rule<Node\Stmt\ClassConst>
*/
class ConflictingTraitConstantsRule implements Rule
{

public function __construct(private InitializerExprTypeResolver $initializerExprTypeResolver)
{
}

public function getNodeType(): string
{
return Node\Stmt\ClassConst::class;
}

public function processNode(Node $node, Scope $scope): array
{
if (!$scope->isInClass()) {
return [];
}

$classReflection = $scope->getClassReflection();
$traitConstants = [];
foreach ($classReflection->getTraits(true) as $trait) {
foreach ($trait->getNativeReflection()->getReflectionConstants() as $constant) {
$traitConstants[] = $constant;
}
}

$errors = [];
foreach ($node->consts as $const) {
foreach ($traitConstants as $traitConstant) {
if ($traitConstant->getName() !== $const->name->toString()) {
continue;
}

foreach ($this->processSingleConstant($classReflection, $traitConstant, $node, $const->value) as $error) {
$errors[] = $error;
}
}
}

return $errors;
}

/**
* @return list<RuleError>
*/
private function processSingleConstant(ClassReflection $classReflection, ReflectionClassConstant $traitConstant, Node\Stmt\ClassConst $classConst, Node\Expr $valueExpr): array
{
$errors = [];
if ($traitConstant->isPublic()) {
if ($classConst->isProtected()) {
$errors[] = RuleErrorBuilder::message(sprintf(
'Protected constant %s::%s overriding public constant %s::%s should also be public.',
$classReflection->getDisplayName(),
$traitConstant->getName(),
$traitConstant->getDeclaringClass()->getName(),
$traitConstant->getName(),
))->nonIgnorable()->build();
} elseif ($classConst->isPrivate()) {
$errors[] = RuleErrorBuilder::message(sprintf(
'Private constant %s::%s overriding public constant %s::%s should also be public.',
$classReflection->getDisplayName(),
$traitConstant->getName(),
$traitConstant->getDeclaringClass()->getName(),
$traitConstant->getName(),
))->nonIgnorable()->build();
}
} elseif ($traitConstant->isProtected()) {
if ($classConst->isPublic()) {
$errors[] = RuleErrorBuilder::message(sprintf(
'Public constant %s::%s overriding protected constant %s::%s should also be protected.',
$classReflection->getDisplayName(),
$traitConstant->getName(),
$traitConstant->getDeclaringClass()->getName(),
$traitConstant->getName(),
))->nonIgnorable()->build();
} elseif ($classConst->isPrivate()) {
$errors[] = RuleErrorBuilder::message(sprintf(
'Private constant %s::%s overriding protected constant %s::%s should also be protected.',
$classReflection->getDisplayName(),
$traitConstant->getName(),
$traitConstant->getDeclaringClass()->getName(),
$traitConstant->getName(),
))->nonIgnorable()->build();
}
} elseif ($traitConstant->isPrivate()) {
if ($classConst->isPublic()) {
$errors[] = RuleErrorBuilder::message(sprintf(
'Public constant %s::%s overriding private constant %s::%s should also be private.',
$classReflection->getDisplayName(),
$traitConstant->getName(),
$traitConstant->getDeclaringClass()->getName(),
$traitConstant->getName(),
))->nonIgnorable()->build();
} elseif ($classConst->isProtected()) {
$errors[] = RuleErrorBuilder::message(sprintf(
'Protected constant %s::%s overriding private constant %s::%s should also be private.',
$classReflection->getDisplayName(),
$traitConstant->getName(),
$traitConstant->getDeclaringClass()->getName(),
$traitConstant->getName(),
))->nonIgnorable()->build();
}
}

if ($traitConstant->isFinal()) {
if (!$classConst->isFinal()) {
$errors[] = RuleErrorBuilder::message(sprintf(
'Non-final constant %s::%s overriding final constant %s::%s should also be final.',
$classReflection->getDisplayName(),
$traitConstant->getName(),
$traitConstant->getDeclaringClass()->getName(),
$traitConstant->getName(),
))->nonIgnorable()->build();
}
} elseif ($classConst->isFinal()) {
$errors[] = RuleErrorBuilder::message(sprintf(
'Final constant %s::%s overriding non-final constant %s::%s should also be non-final.',
$classReflection->getDisplayName(),
$traitConstant->getName(),
$traitConstant->getDeclaringClass()->getName(),
$traitConstant->getName(),
))->nonIgnorable()->build();
}

$traitNativeType = $traitConstant->getType();
$constantNativeType = $classConst->type;
$traitDeclaringClass = $traitConstant->getDeclaringClass();
if ($traitNativeType === null) {
if ($constantNativeType !== null) {
$constantNativeTypeType = ParserNodeTypeToPHPStanType::resolve($constantNativeType, $classReflection);
$errors[] = RuleErrorBuilder::message(sprintf(
'Constant %s::%s (%s) overriding constant %s::%s should not have a native type.',
$classReflection->getDisplayName(),
$traitConstant->getName(),
$constantNativeTypeType->describe(VerbosityLevel::typeOnly()),
$traitConstant->getDeclaringClass()->getName(),
$traitConstant->getName(),
))->nonIgnorable()->build();
}
} elseif ($constantNativeType === null) {
$traitNativeTypeType = TypehintHelper::decideTypeFromReflection($traitNativeType, null, $traitDeclaringClass->getName());
$errors[] = RuleErrorBuilder::message(sprintf(
'Constant %s::%s overriding constant %s::%s (%s) should also have native type %s.',
$classReflection->getDisplayName(),
$traitConstant->getName(),
$traitConstant->getDeclaringClass()->getName(),
$traitConstant->getName(),
$traitNativeTypeType->describe(VerbosityLevel::typeOnly()),
$traitNativeTypeType->describe(VerbosityLevel::typeOnly()),
))->nonIgnorable()->build();
} else {
$traitNativeTypeType = TypehintHelper::decideTypeFromReflection($traitNativeType, null, $traitDeclaringClass->getName());
$constantNativeTypeType = ParserNodeTypeToPHPStanType::resolve($constantNativeType, $classReflection);
if (!$traitNativeTypeType->equals($constantNativeTypeType)) {
$errors[] = RuleErrorBuilder::message(sprintf(
'Constant %s::%s (%s) overriding constant %s::%s (%s) should have the same native type %s.',
$classReflection->getDisplayName(),
$traitConstant->getName(),
$constantNativeTypeType->describe(VerbosityLevel::typeOnly()),
$traitConstant->getDeclaringClass()->getName(),
$traitConstant->getName(),
$traitNativeTypeType->describe(VerbosityLevel::typeOnly()),
$traitNativeTypeType->describe(VerbosityLevel::typeOnly()),
))->nonIgnorable()->build();
}
}

$classConstantValueType = $this->initializerExprTypeResolver->getType($valueExpr, InitializerExprContext::fromClassReflection($classReflection));
$traitConstantValueType = $this->initializerExprTypeResolver->getType(
$traitConstant->getValueExpression(),
InitializerExprContext::fromClass(
$traitDeclaringClass->getName(),
$traitDeclaringClass->getFileName() !== false ? $traitDeclaringClass->getFileName() : null,
),
);
if (!$classConstantValueType->equals($traitConstantValueType)) {
$errors[] = RuleErrorBuilder::message(sprintf(
'Constant %s::%s with value %s overriding constant %s::%s with different value %s should have the same value.',
$classReflection->getDisplayName(),
$traitConstant->getName(),
$classConstantValueType->describe(VerbosityLevel::value()),
$traitConstant->getDeclaringClass()->getName(),
$traitConstant->getName(),
$traitConstantValueType->describe(VerbosityLevel::value()),
))->nonIgnorable()->build();
}

return $errors;
}

}
84 changes: 84 additions & 0 deletions tests/PHPStan/Rules/Traits/ConflictingTraitConstantsRuleTest.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Traits;

use PHPStan\Reflection\InitializerExprTypeResolver;
use PHPStan\Rules\Rule as TRule;
use PHPStan\Testing\RuleTestCase;

/**
* @extends RuleTestCase<ConflictingTraitConstantsRule>
*/
class ConflictingTraitConstantsRuleTest extends RuleTestCase
{

protected function getRule(): TRule
{
return new ConflictingTraitConstantsRule(self::getContainer()->getByType(InitializerExprTypeResolver::class));
}

public function testRule(): void
{
$this->analyse([__DIR__ . '/data/conflicting-trait-constants.php'], [
[
'Protected constant ConflictingTraitConstants\Bar::PUBLIC_CONSTANT overriding public constant ConflictingTraitConstants\Foo::PUBLIC_CONSTANT should also be public.',
23,
],
[
'Private constant ConflictingTraitConstants\Bar2::PUBLIC_CONSTANT overriding public constant ConflictingTraitConstants\Foo::PUBLIC_CONSTANT should also be public.',
32,
],
[
'Public constant ConflictingTraitConstants\Bar3::PROTECTED_CONSTANT overriding protected constant ConflictingTraitConstants\Foo::PROTECTED_CONSTANT should also be protected.',
41,
],
[
'Private constant ConflictingTraitConstants\Bar4::PROTECTED_CONSTANT overriding protected constant ConflictingTraitConstants\Foo::PROTECTED_CONSTANT should also be protected.',
50,
],
[
'Protected constant ConflictingTraitConstants\Bar5::PRIVATE_CONSTANT overriding private constant ConflictingTraitConstants\Foo::PRIVATE_CONSTANT should also be private.',
59,
],
[
'Public constant ConflictingTraitConstants\Bar5::PRIVATE_CONSTANT overriding private constant ConflictingTraitConstants\Foo::PRIVATE_CONSTANT should also be private.',
68,
],
[
'Non-final constant ConflictingTraitConstants\Bar6::PUBLIC_FINAL_CONSTANT overriding final constant ConflictingTraitConstants\Foo::PUBLIC_FINAL_CONSTANT should also be final.',
77,
],
[
'Final constant ConflictingTraitConstants\Bar7::PUBLIC_CONSTANT overriding non-final constant ConflictingTraitConstants\Foo::PUBLIC_CONSTANT should also be non-final.',
86,
],
[
'Constant ConflictingTraitConstants\Bar8::PUBLIC_CONSTANT with value 2 overriding constant ConflictingTraitConstants\Foo::PUBLIC_CONSTANT with different value 1 should have the same value.',
96,
],
]);
}

public function testNativeTypes(): void
{
if (PHP_VERSION_ID < 80300) {
$this->markTestSkipped('Test requires PHP 8.3.');
}

$this->analyse([__DIR__ . '/data/conflicting-trait-constants-types.php'], [
[
'Constant ConflictingTraitConstantsTypes\Baz::FOO_CONST (int) overriding constant ConflictingTraitConstantsTypes\Foo::FOO_CONST (int|string) should have the same native type int|string.',
28,
],
[
'Constant ConflictingTraitConstantsTypes\Baz::BAR_CONST (int) overriding constant ConflictingTraitConstantsTypes\Foo::BAR_CONST should not have a native type.',
30,
],
[
'Constant ConflictingTraitConstantsTypes\Lorem::FOO_CONST overriding constant ConflictingTraitConstantsTypes\Foo::FOO_CONST (int|string) should also have native type int|string.',
39,
],
]);
}

}
41 changes: 41 additions & 0 deletions tests/PHPStan/Rules/Traits/data/conflicting-trait-constants-types.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

namespace ConflictingTraitConstantsTypes;

trait Foo
{

public const int|string FOO_CONST = 1;

public const BAR_CONST = 1;

}

class Bar
{

use Foo;

public const int|string FOO_CONST = 1;

}

class Baz
{

use Foo;

public const int FOO_CONST = 1;

public const int BAR_CONST = 1;

}

class Lorem
{

use Foo;

public const FOO_CONST = 1;

}
Loading

0 comments on commit f94a3c3

Please sign in to comment.