Skip to content

Commit 0487b72

Browse files
micheleorsellimicheleorselli
committed
refactors ClassDescriptionBuilder in order to remove several null checks in FileVisitor
1 parent 33dca6f commit 0487b72

File tree

7 files changed

+86
-62
lines changed

7 files changed

+86
-62
lines changed

‎src/Analyzer/ClassDescription.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,10 @@ public function setFullPath(string $fullPath): void
6666

6767
public static function build(string $FQCN): ClassDescriptionBuilder
6868
{
69-
return ClassDescriptionBuilder::create($FQCN);
69+
$cb = ClassDescriptionBuilder::create();
70+
$cb->setClassName($FQCN);
71+
72+
return $cb;
7073
}
7174

7275
public function getName(): string

‎src/Analyzer/ClassDescriptionBuilder.php‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@
33

44
namespace Arkitect\Analyzer;
55

6+
use Webmozart\Assert\Assert;
7+
68
class ClassDescriptionBuilder
79
{
810
/** @var list<ClassDependency> */
911
private $classDependencies;
1012

11-
/** @var FullyQualifiedClassName */
13+
/** @var ?FullyQualifiedClassName */
1214
private $FQCN;
1315

1416
/** @var list<FullyQualifiedClassName> */
@@ -36,9 +38,10 @@ class ClassDescriptionBuilder
3638
* @param list<ClassDependency> $classDependencies
3739
* @param list<FullyQualifiedClassName> $interfaces
3840
* @param list<FullyQualifiedClassName> $attributes
41+
* @param ?FullyQualifiedClassName $FQCN
3942
*/
4043
private function __construct(
41-
FullyQualifiedClassName $FQCN,
44+
?FullyQualifiedClassName $FQCN,
4245
string $filePath,
4346
array $classDependencies,
4447
array $interfaces,
@@ -57,10 +60,10 @@ private function __construct(
5760
$this->attributes = $attributes;
5861
}
5962

60-
public static function create(string $FQCN): self
63+
public static function create(): self
6164
{
6265
return new self(
63-
FullyQualifiedClassName::fromString($FQCN),
66+
null,
6467
'',
6568
[],
6669
[],
@@ -71,6 +74,23 @@ public static function create(string $FQCN): self
7174
);
7275
}
7376

77+
public function setClassName(string $FQCN): void
78+
{
79+
$this->FQCN = FullyQualifiedClassName::fromString($FQCN);
80+
}
81+
82+
public function clear(): void
83+
{
84+
$this->FQCN = null;
85+
$this->filePath = '';
86+
$this->classDependencies = [];
87+
$this->interfaces = [];
88+
$this->final = false;
89+
$this->abstract = false;
90+
$this->docBlock = [];
91+
$this->attributes = [];
92+
}
93+
7494
public function setFilePath(string $filePath): self
7595
{
7696
$this->filePath = $filePath;
@@ -103,6 +123,8 @@ public function setExtends(string $FQCN, int $line): self
103123

104124
public function get(): ClassDescription
105125
{
126+
Assert::notNull($this->FQCN);
127+
106128
$cd = new ClassDescription(
107129
$this->FQCN,
108130
$this->classDependencies,

‎src/Analyzer/FileParserFactory.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ public static function createFileParser(TargetPhpVersion $targetPhpVersion): Fil
1313
{
1414
return new FileParser(
1515
new NodeTraverser(),
16-
new FileVisitor(),
16+
new FileVisitor(ClassDescriptionBuilder::create()),
1717
new NameResolver(),
1818
$targetPhpVersion
1919
);

‎src/Analyzer/FileVisitor.php‎

Lines changed: 31 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,22 @@
99

1010
class FileVisitor extends NodeVisitorAbstract
1111
{
12-
/** @var ?ClassDescriptionBuilder */
12+
/** @var ClassDescriptionBuilder */
1313
private $classDescriptionBuilder;
1414

1515
/** @var array */
1616
private $classDescriptions = [];
1717

18+
public function __construct(ClassDescriptionBuilder $classDescriptionBuilder)
19+
{
20+
$this->classDescriptionBuilder = $classDescriptionBuilder;
21+
}
22+
1823
public function enterNode(Node $node): void
1924
{
2025
if ($node instanceof Node\Stmt\Class_) {
2126
if (!$node->isAnonymous() && null !== $node->namespacedName) {
22-
/** @psalm-suppress UndefinedPropertyFetch */
23-
$this->classDescriptionBuilder = ClassDescriptionBuilder::create(
24-
$node->namespacedName->toCodeString()
25-
);
26-
}
27-
28-
if (null === $this->classDescriptionBuilder) {
29-
return;
27+
$this->classDescriptionBuilder->setClassName($node->namespacedName->toCodeString());
3028
}
3129

3230
foreach ($node->implements as $interface) {
@@ -55,15 +53,25 @@ public function enterNode(Node $node): void
5553
}
5654
}
5755

56+
if ($node instanceof Node\Stmt\Enum_ && null !== $node->namespacedName) {
57+
$this->classDescriptionBuilder->setClassName($node->namespacedName->toCodeString());
58+
59+
foreach ($node->attrGroups as $attributeGroup) {
60+
foreach ($attributeGroup->attrs as $attribute) {
61+
$this->classDescriptionBuilder
62+
->addAttribute($attribute->name->toString(), $attribute->getLine());
63+
}
64+
}
65+
}
66+
5867
/**
5968
* adding static classes as dependencies
6069
* $constantValue = StaticClass::constant;.
6170
*
6271
* @see FileVisitorTest::test_it_should_return_errors_for_const_outside_namespace
6372
*/
6473
if ($node instanceof Node\Expr\ClassConstFetch &&
65-
method_exists($node->class, 'toString') &&
66-
null !== $this->classDescriptionBuilder
74+
method_exists($node->class, 'toString')
6775
) {
6876
if ($this->isSelfOrStaticOrParent($node->class->toString())) {
6977
return;
@@ -80,8 +88,7 @@ public function enterNode(Node $node): void
8088
* @see FileVisitorTest::test_should_returns_all_dependencies
8189
*/
8290
if ($node instanceof Node\Expr\StaticCall &&
83-
method_exists($node->class, 'toString') &&
84-
null !== $this->classDescriptionBuilder
91+
method_exists($node->class, 'toString')
8592
) {
8693
if ($this->isSelfOrStaticOrParent($node->class->toString())) {
8794
return;
@@ -92,8 +99,7 @@ public function enterNode(Node $node): void
9299
}
93100

94101
if ($node instanceof Node\Expr\Instanceof_ &&
95-
method_exists($node->class, 'toString') &&
96-
null !== $this->classDescriptionBuilder
102+
method_exists($node->class, 'toString')
97103
) {
98104
if ($this->isSelfOrStaticOrParent($node->class->toString())) {
99105
return;
@@ -104,8 +110,7 @@ public function enterNode(Node $node): void
104110
}
105111

106112
if ($node instanceof Node\Expr\New_ &&
107-
!($node->class instanceof Node\Expr\Variable) &&
108-
null !== $this->classDescriptionBuilder
113+
!($node->class instanceof Node\Expr\Variable)
109114
) {
110115
if ((method_exists($node->class, 'isAnonymous') && $node->class->isAnonymous()) ||
111116
!method_exists($node->class, 'toString')) {
@@ -120,27 +125,13 @@ public function enterNode(Node $node): void
120125
->addDependency(new ClassDependency($node->class->toString(), $node->getLine()));
121126
}
122127

123-
if ($node instanceof Node\Stmt\Enum_ && null !== $node->namespacedName) {
124-
/** @psalm-suppress UndefinedPropertyFetch */
125-
$this->classDescriptionBuilder = ClassDescriptionBuilder::create(
126-
$node->namespacedName->toCodeString()
127-
);
128-
129-
foreach ($node->attrGroups as $attributeGroup) {
130-
foreach ($attributeGroup->attrs as $attribute) {
131-
$this->classDescriptionBuilder
132-
->addAttribute($attribute->name->toString(), $attribute->getLine());
133-
}
134-
}
135-
}
136-
137128
/**
138129
* matches parameters dependency in property definitions like
139130
* public NotBlank $foo;.
140131
*
141132
* @see FileVisitorTest::test_it_parse_typed_property
142133
*/
143-
if ($node instanceof Node\Stmt\Property && null !== $this->classDescriptionBuilder) {
134+
if ($node instanceof Node\Stmt\Property) {
144135
if (null === $node->type) {
145136
return;
146137
}
@@ -159,7 +150,7 @@ public function enterNode(Node $node): void
159150
}
160151
}
161152

162-
if (null !== $this->classDescriptionBuilder && null !== $node->getDocComment()) {
153+
if (null !== $node->getDocComment()) {
163154
/** @var Doc $docComment */
164155
$docComment = $node->getDocComment();
165156

@@ -172,7 +163,7 @@ public function enterNode(Node $node): void
172163
*
173164
* @see FileVisitorTest::test_should_returns_all_dependencies
174165
*/
175-
if ($node instanceof Node\Param && null !== $this->classDescriptionBuilder) {
166+
if ($node instanceof Node\Param) {
176167
$this->addParamDependency($node);
177168
}
178169
}
@@ -189,16 +180,14 @@ public function clearParsedClassDescriptions(): void
189180

190181
public function leaveNode(Node $node): void
191182
{
192-
if ($node instanceof Node\Stmt\Class_ && null !== $this->classDescriptionBuilder) {
193-
$classDescription = $this->classDescriptionBuilder->get();
194-
195-
$this->classDescriptions[] = $classDescription;
183+
if ($node instanceof Node\Stmt\Class_ && !$node->isAnonymous()) {
184+
$this->classDescriptions[] = $this->classDescriptionBuilder->get();
185+
$this->classDescriptionBuilder->clear();
196186
}
197187

198-
if ($node instanceof Node\Stmt\Enum_ && null !== $this->classDescriptionBuilder) {
199-
$classDescription = $this->classDescriptionBuilder->get();
200-
201-
$this->classDescriptions[] = $classDescription;
188+
if ($node instanceof Node\Stmt\Enum_) {
189+
$this->classDescriptions[] = $this->classDescriptionBuilder->get();
190+
$this->classDescriptionBuilder->clear();
202191
}
203192
}
204193

@@ -221,10 +210,6 @@ private function addParamDependency(Node\Param $node): void
221210
return;
222211
}
223212

224-
if (null === $this->classDescriptionBuilder) {
225-
return;
226-
}
227-
228213
$this->classDescriptionBuilder
229214
->addDependency(new ClassDependency($node->type->toString(), $node->getLine()));
230215
}

‎tests/Unit/Analyzer/ClassDescriptionBuilderTest.php‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ class ClassDescriptionBuilderTest extends TestCase
1515
public function test_it_should_create_builder_with_dependency_and_interface(): void
1616
{
1717
$FQCN = 'HappyIsland';
18-
$classDescriptionBuilder = ClassDescriptionBuilder::create($FQCN);
18+
$classDescriptionBuilder = ClassDescriptionBuilder::create();
19+
$classDescriptionBuilder->setClassName($FQCN);
1920

2021
$classDependency = new ClassDependency('DepClass', 10);
2122

@@ -33,7 +34,8 @@ public function test_it_should_create_builder_with_dependency_and_interface(): v
3334
public function test_it_should_create_final_class(): void
3435
{
3536
$FQCN = 'HappyIsland';
36-
$classDescriptionBuilder = ClassDescriptionBuilder::create($FQCN);
37+
$classDescriptionBuilder = ClassDescriptionBuilder::create();
38+
$classDescriptionBuilder->setClassName($FQCN);
3739
$classDescriptionBuilder->setFinal(true);
3840

3941
$classDescription = $classDescriptionBuilder->get();
@@ -46,7 +48,8 @@ public function test_it_should_create_final_class(): void
4648
public function test_it_should_create_not_final_class(): void
4749
{
4850
$FQCN = 'HappyIsland';
49-
$classDescriptionBuilder = ClassDescriptionBuilder::create($FQCN);
51+
$classDescriptionBuilder = ClassDescriptionBuilder::create();
52+
$classDescriptionBuilder->setClassName($FQCN);
5053
$classDescriptionBuilder->setFinal(false);
5154

5255
$classDescription = $classDescriptionBuilder->get();
@@ -59,7 +62,8 @@ public function test_it_should_create_not_final_class(): void
5962
public function test_it_should_create_abstract_class(): void
6063
{
6164
$FQCN = 'HappyIsland';
62-
$classDescriptionBuilder = ClassDescriptionBuilder::create($FQCN);
65+
$classDescriptionBuilder = ClassDescriptionBuilder::create();
66+
$classDescriptionBuilder->setClassName($FQCN);
6367
$classDescriptionBuilder->setAbstract(true);
6468

6569
$classDescription = $classDescriptionBuilder->get();
@@ -72,7 +76,8 @@ public function test_it_should_create_abstract_class(): void
7276
public function test_it_should_create_not_abstract_class(): void
7377
{
7478
$FQCN = 'HappyIsland';
75-
$classDescriptionBuilder = ClassDescriptionBuilder::create($FQCN);
79+
$classDescriptionBuilder = ClassDescriptionBuilder::create();
80+
$classDescriptionBuilder->setClassName($FQCN);
7681
$classDescriptionBuilder->setAbstract(false);
7782

7883
$classDescription = $classDescriptionBuilder->get();
@@ -85,7 +90,8 @@ public function test_it_should_create_not_abstract_class(): void
8590
public function test_it_should_create_annotated_class(): void
8691
{
8792
$FQCN = 'HappyIsland';
88-
$classDescriptionBuilder = ClassDescriptionBuilder::create($FQCN);
93+
$classDescriptionBuilder = ClassDescriptionBuilder::create();
94+
$classDescriptionBuilder->setClassName($FQCN);
8995
$classDescriptionBuilder->addDocBlock('/**
9096
* @psalm-immutable
9197
*/');
@@ -105,7 +111,8 @@ public function test_it_should_create_annotated_class(): void
105111
public function test_it_should_add_attributes(): void
106112
{
107113
$FQCN = 'HappyIsland';
108-
$classDescriptionBuilder = ClassDescriptionBuilder::create($FQCN);
114+
$classDescriptionBuilder = ClassDescriptionBuilder::create();
115+
$classDescriptionBuilder->setClassName($FQCN);
109116
$classDescriptionBuilder->addAttribute('AttrClass', 27);
110117

111118
$classDescription = $classDescriptionBuilder->get();

‎tests/Unit/Analyzer/FileVisitorTest.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,14 +88,15 @@ class Cat implements AnInterface
8888
$fp->parse($code, 'relativePathName');
8989
$cd = $fp->getClassDescriptions();
9090

91-
self::assertCount(3, $cd);
91+
self::assertCount(2, $cd);
9292
self::assertInstanceOf(ClassDescription::class, $cd[0]);
9393
self::assertInstanceOf(ClassDescription::class, $cd[1]);
9494

9595
$expectedInterfaces = [
9696
new ClassDependency('Root\Namespace1\AnInterface', 7),
9797
new ClassDependency('Root\Namespace1\InterfaceTwo', 7),
9898
new ClassDependency('Root\Namespace1\Another\ForbiddenInterface', 11),
99+
new ClassDependency('Root\Namespace1\Proj', 23),
99100
];
100101

101102
$this->assertEquals($expectedInterfaces, $cd[0]->getDependencies());

‎tests/Unit/Rules/ConstraintsTest.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,11 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
3434
$violations = new Violations();
3535
$because = 'we want to add this rule for our software';
3636

37+
$cb = ClassDescriptionBuilder::create();
38+
$cb->setClassName('Banana');
39+
3740
$expressionStore->checkAll(
38-
ClassDescriptionBuilder::create('Banana')->get(),
41+
$cb->get(),
3942
$violations,
4043
$because
4144
);
@@ -67,8 +70,11 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
6770
$violations = new Violations();
6871
$because = 'we want to add this rule for our software';
6972

73+
$cb = ClassDescriptionBuilder::create();
74+
$cb->setClassName('Banana');
75+
7076
$expressionStore->checkAll(
71-
ClassDescriptionBuilder::create('Banana')->get(),
77+
$cb->get(),
7278
$violations,
7379
$because
7480
);

0 commit comments

Comments
 (0)