[CodeQuality] Replace static with self for private constants #2332
Conversation
| $classReflection = $scope->getClassReflection(); | ||
| if (! $classReflection instanceof ClassReflection) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
This should only apply when it used inside final class, ref https://3v4l.org/jYOjL vs https://3v4l.org/LWrtJ which result is different.
You can verify inside class with final with:
| } | |
| } | |
| if (! $classReflection->isClass()) { | |
| return false; | |
| } | |
| if (! $classReflection->isFinal()) { | |
| return false; | |
| } |
There was a problem hiding this comment.
Also, you can get ClassReflection via ReflectionResolver->resolveClassReflection() ref
You can inject ReflectionResolver into constructor:
public function __construct(
private readonly \Rector\Core\Reflection\ReflectionResolver $reflectionResolver,
) {
}There was a problem hiding this comment.
see my comment #2332 (comment) for cover non-final class if it going to be included to avoid constant redefinition
| [ | ||
| new CodeSample( | ||
| <<<'CODE_SAMPLE' | ||
| class Foo { |
There was a problem hiding this comment.
| class Foo { | |
| final class Foo { |
| CODE_SAMPLE | ||
| , | ||
| <<<'CODE_SAMPLE' | ||
| class Foo { |
There was a problem hiding this comment.
| class Foo { | |
| final class Foo { |
| public function getRuleDefinition(): RuleDefinition | ||
| { | ||
| return new RuleDefinition( | ||
| 'Replaces static::* access to private constants with self::*', |
There was a problem hiding this comment.
| 'Replaces static::* access to private constants with self::*', | |
| 'Replaces static::* access to private constants with self::* on final class', |
|
For non-final class that extendable, it seems you may looking for child of classes, eg: loop and verify: foreach ($childrenClassReflections as $childrenClassReflection) {
if ($childrenClassReflection->hasConstant($constantName)) {
return false;
}
} |
|
please add test for skip extended class that has the same constant base on #2332 (comment) |
|
After some thinking, it can be dangerous that may cause a BC break if the class is not final as extendable can be anywere. This should be configurable if need to be applied, with:
|
|
I am closing it as it can cause BC break without various check like my comments before. Feel free to reopen PR when you have a chance to polish it. Thank you. |
I wasn't able to find a rule like this and we use this in our project, have a look at the problem here: