Skip to content

Commit 7e134ad

Browse files
TomasVotrubaTomasVotruba
committed
[cleanup] AnnotationWithValueToAttributeRector to avoid using local node property
1 parent 795a773 commit 7e134ad

9 files changed

Lines changed: 69 additions & 55 deletions

File tree

‎phpstan.neon‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,4 @@ parameters:
4444
- '#Doing instanceof PHPStan\\Type\\.+ is error\-prone and deprecated#'
4545

4646
- identifier: instanceof.alwaysTrue
47-
- identifier: assign.propertyType
47+
#- identifier: assign.propertyType

rules-tests/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector/Fixture/some_class.php.inc renamed to rules-tests/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector/Fixture/some_test.php.inc

File renamed without changes.

‎rules/AnnotationsToAttributes/Rector/Class_/AnnotationWithValueToAttributeRector.php‎

Lines changed: 51 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ final class AnnotationWithValueToAttributeRector extends AbstractRector implemen
3434
*/
3535
private array $annotationWithValueToAttributes = [];
3636

37-
private ?Class_ $currentClass = null;
37+
private bool $hasChanged = false;
3838

3939
public function __construct(
4040
private readonly PhpDocTagRemover $phpDocTagRemover,
@@ -86,7 +86,7 @@ final class SomeTest extends TestCase
8686
*/
8787
public function getNodeTypes(): array
8888
{
89-
return [Class_::class, ClassMethod::class];
89+
return [Class_::class];
9090
}
9191

9292
public function provideMinPhpVersion(): int
@@ -95,24 +95,61 @@ public function provideMinPhpVersion(): int
9595
}
9696

9797
/**
98-
* @param Class_|ClassMethod $node
98+
* @param Class_ $node
9999
*/
100100
public function refactor(Node $node): ?Node
101101
{
102+
102103
if (! $this->testsNodeAnalyzer->isInTestClass($node)) {
103104
return null;
104105
}
105106

106-
if ($node instanceof Class_) {
107-
$this->currentClass = $node;
107+
$this->hasChanged = false;
108+
109+
// handle class level
110+
// $classPhpDocInfo = $this->phpDocInfoFactory->createFromNode($node);
111+
$this->refactorClassMethodOrClass($node, $node);
112+
113+
foreach ($node->getMethods() as $classMethod) {
114+
$this->refactorClassMethodOrClass($classMethod, $node);
108115
}
109116

110-
$phpDocInfo = $this->phpDocInfoFactory->createFromNode($node);
111-
if (! $phpDocInfo instanceof PhpDocInfo) {
117+
if (! $this->hasChanged) {
112118
return null;
113119
}
114120

115-
$hasChanged = false;
121+
return $node;
122+
}
123+
124+
/**
125+
* @param mixed[] $configuration
126+
*/
127+
public function configure(array $configuration): void
128+
{
129+
Assert::allIsInstanceOf($configuration, AnnotationWithValueToAttribute::class);
130+
$this->annotationWithValueToAttributes = $configuration;
131+
}
132+
133+
private function resolveAttributeValue(
134+
GenericTagValueNode $genericTagValueNode,
135+
AnnotationWithValueToAttribute $annotationWithValueToAttribute
136+
): mixed {
137+
$valueMap = $annotationWithValueToAttribute->getValueMap();
138+
if ($valueMap === []) {
139+
// no map? convert value as it is
140+
return $genericTagValueNode->value;
141+
}
142+
143+
$originalValue = strtolower($genericTagValueNode->value);
144+
return $valueMap[$originalValue];
145+
}
146+
147+
private function refactorClassMethodOrClass(ClassMethod|Class_ $classOrClass, Class_ $class): void
148+
{
149+
$phpDocInfo = $this->phpDocInfoFactory->createFromNode($classOrClass);
150+
if (! $phpDocInfo instanceof PhpDocInfo) {
151+
return;
152+
}
116153

117154
foreach ($this->annotationWithValueToAttributes as $annotationWithValueToAttribute) {
118155
/** @var PhpDocTagNode[] $desiredTagValueNodes */
@@ -133,47 +170,18 @@ public function refactor(Node $node): ?Node
133170
[$attributeValue]
134171
);
135172

136-
if ($node instanceof ClassMethod && $annotationWithValueToAttribute->getIsOnClassLevel() && $this->currentClass instanceof Class_) {
137-
Assert::isAOf($this->currentClass, Class_::class);
138-
$this->currentClass->attrGroups = array_merge($this->currentClass->attrGroups, [$attributeGroup]);
173+
if ($annotationWithValueToAttribute->getIsOnClassLevel()) {
174+
$class->attrGroups = array_merge($class->attrGroups, [$attributeGroup]);
139175
} else {
140-
$node->attrGroups = array_merge($node->attrGroups, [$attributeGroup]);
176+
$classOrClass->attrGroups = array_merge($classOrClass->attrGroups, [$attributeGroup]);
141177
}
142178

143179
// cleanup
144180
$this->phpDocTagRemover->removeTagValueFromNode($phpDocInfo, $desiredTagValueNode);
145-
$hasChanged = true;
146-
}
147-
}
148-
149-
if ($hasChanged) {
150-
$this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($node);
151-
return $node;
152-
}
153-
154-
return null;
155-
}
181+
$this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($classOrClass);
156182

157-
/**
158-
* @param mixed[] $configuration
159-
*/
160-
public function configure(array $configuration): void
161-
{
162-
Assert::allIsInstanceOf($configuration, AnnotationWithValueToAttribute::class);
163-
$this->annotationWithValueToAttributes = $configuration;
164-
}
165-
166-
private function resolveAttributeValue(
167-
GenericTagValueNode $genericTagValueNode,
168-
AnnotationWithValueToAttribute $annotationWithValueToAttribute
169-
): mixed {
170-
$valueMap = $annotationWithValueToAttribute->getValueMap();
171-
if ($valueMap === []) {
172-
// no map? convert value as it is
173-
return $genericTagValueNode->value;
183+
$this->hasChanged = true;
184+
}
174185
}
175-
176-
$originalValue = strtolower($genericTagValueNode->value);
177-
return $valueMap[$originalValue];
178186
}
179187
}

‎rules/AnnotationsToAttributes/Rector/Class_/RequiresAnnotationWithValueToAttributeRector.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ private function refactorClassMethod(ClassMethod $classMethod): ?ClassMethod
172172

173173
$this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($classMethod);
174174
$classMethod->attrGroups = array_merge($classMethod->attrGroups, $requiresAttributeGroups);
175+
175176
$this->removeMethodRequiresAnnotations($phpDocInfo);
176177

177178
return $classMethod;

‎rules/CodeQuality/Rector/ClassMethod/ReplaceTestFunctionPrefixWithAttributeRector.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use PhpParser\Node\Stmt\ClassMethod;
1010
use Rector\Php80\NodeAnalyzer\PhpAttributeAnalyzer;
1111
use Rector\PhpAttribute\NodeFactory\PhpAttributeGroupFactory;
12+
use Rector\PHPUnit\Enum\PHPUnitAttribute;
1213
use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer;
1314
use Rector\Rector\AbstractRector;
1415
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -75,7 +76,7 @@ public function refactor(Node $node): ?Node
7576
return null;
7677
}
7778

78-
if ($this->phpAttributeAnalyzer->hasPhpAttributes($node, ['PHPUnit\\Framework\\Attributes\\Test'])) {
79+
if ($this->phpAttributeAnalyzer->hasPhpAttributes($node, [PHPUnitAttribute::TEST])) {
7980
return null;
8081
}
8182

‎rules/CodeQuality/Rector/Class_/AddParamTypeFromDependsRector.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,16 +131,17 @@ public function refactor(Node $node): ?Node
131131
return $node;
132132
}
133133

134-
private function resolveReturnTypeOfDependsMethod(ClassMethod $classMethod, Class_ $class): ?Node
135-
{
134+
private function resolveReturnTypeOfDependsMethod(
135+
ClassMethod $classMethod,
136+
Class_ $class
137+
): Node\ComplexType|Node\Identifier|Node\Name|null {
136138
$dependsMethodName = $this->resolveDependsAnnotationOrAttributeMethod($classMethod);
137139

138140
if ($dependsMethodName === null || $dependsMethodName === '') {
139141
return null;
140142
}
141143

142144
$dependsClassMethod = $class->getMethod($dependsMethodName);
143-
144145
if (! $dependsClassMethod instanceof ClassMethod) {
145146
return null;
146147
}

‎src/Enum/PHPUnitAttribute.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,6 @@ final class PHPUnitAttribute
2121
public const REQUIRES_PHP_EXTENSION = 'PHPUnit\Framework\Attributes\RequiresPhpExtension';
2222

2323
public const REQUIRES_SETTING = 'PHPUnit\Framework\Attributes\RequiresSetting';
24+
25+
public const TEST = 'PHPUnit\Framework\Attributes\Test';
2426
}

‎src/Enum/PHPUnitClassName.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,9 @@ final class PHPUnitClassName
3030
* @var string
3131
*/
3232
public const TEST_LISTENER = 'PHPUnit\Framework\TestListener';
33+
34+
/**
35+
* @var string[]
36+
*/
37+
public const TEST_CLASSES = [self::TEST_CASE, self::TEST_CASE_LEGACY];
3338
}

‎src/NodeAnalyzer/TestsNodeAnalyzer.php‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,12 @@
1313
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
1414
use Rector\NodeNameResolver\NodeNameResolver;
1515
use Rector\NodeTypeResolver\NodeTypeResolver;
16+
use Rector\PHPUnit\Enum\PHPUnitAttribute;
1617
use Rector\PHPUnit\Enum\PHPUnitClassName;
1718
use Rector\Reflection\ReflectionResolver;
1819

1920
final readonly class TestsNodeAnalyzer
2021
{
21-
/**
22-
* @var string[]
23-
*/
24-
private const TEST_CASE_OBJECT_CLASSES = [PHPUnitClassName::TEST_CASE, PHPUnitClassName::TEST_CASE_LEGACY];
25-
2622
public function __construct(
2723
private NodeTypeResolver $nodeTypeResolver,
2824
private NodeNameResolver $nodeNameResolver,
@@ -38,7 +34,7 @@ public function isInTestClass(Node $node): bool
3834
return false;
3935
}
4036

41-
foreach (self::TEST_CASE_OBJECT_CLASSES as $testCaseObjectClass) {
37+
foreach (PHPUnitClassName::TEST_CLASSES as $testCaseObjectClass) {
4238
if ($classReflection->is($testCaseObjectClass)) {
4339
return true;
4440
}
@@ -59,7 +55,7 @@ public function isTestClassMethod(ClassMethod $classMethod): bool
5955

6056
foreach ($classMethod->getAttrGroups() as $attributeGroup) {
6157
foreach ($attributeGroup->attrs as $attribute) {
62-
if ($attribute->name->toString() === 'PHPUnit\\Framework\\Attributes\\Test') {
58+
if ($this->nodeNameResolver->isName($attribute->name, PHPUnitAttribute::TEST)) {
6359
return true;
6460
}
6561
}

0 commit comments

Comments
 (0)