Skip to content

Commit 46e58e2

Browse files
committed
Remove redundant comments
Follow clean code principles: remove comments that explain 'what' or 'how' (obvious from the code) and keep only comments that explain 'why' when needed. Removed comments like: - 'Filter out PHP core classes' (method name is clear) - 'skip classes in the same namespace' (obvious from condition) - 'PHP core - filtered' in tests (evident from assertion) The code is self-documenting through clear method and variable names.
1 parent 15c9a35 commit 46e58e2

File tree

5 files changed

+10
-33
lines changed

5 files changed

+10
-33
lines changed

src/Analyzer/ClassDescriptionBuilder.php

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ public function addInterface(string $FQCN, int $line): self
7878

7979
public function addDependency(ClassDependency $cd): self
8080
{
81-
// Filter out PHP core classes
8281
if ($this->isPhpCoreClass($cd)) {
8382
return $this;
8483
}
@@ -175,11 +174,6 @@ public function build(): ClassDescription
175174
);
176175
}
177176

178-
/**
179-
* Checks if a dependency is a PHP core/internal class.
180-
* Uses Reflection to detect PHP built-in classes from core and extensions
181-
* (e.g., DateTime, Exception, MongoDB\Driver\Manager, Swoole\Server).
182-
*/
183177
private function isPhpCoreClass(ClassDependency $dependency): bool
184178
{
185179
$fqcn = $dependency->getFQCN();
@@ -191,8 +185,6 @@ private function isPhpCoreClass(ClassDependency $dependency): bool
191185

192186
return $reflection->isInternal();
193187
} catch (\ReflectionException $e) {
194-
// Class doesn't exist in the current environment
195-
// It's likely a user-defined class in the project being analyzed
196188
return false;
197189
}
198190
}

src/Expression/ForClasses/DependsOnlyOnTheseNamespaces.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,11 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
4040
/** @var ClassDependency $dependency */
4141
foreach ($dependencies as $dependency) {
4242
if ($theClass->namespaceMatches($dependency->getFQCN()->namespace())) {
43-
continue; // skip classes in the same namespace
43+
continue;
4444
}
4545

4646
if ($dependency->matchesOneOf(...$this->exclude)) {
47-
continue; // skip excluded namespaces
47+
continue;
4848
}
4949

5050
if (!$dependency->matchesOneOf(...$this->namespaces)) {

src/Expression/ForClasses/NotDependsOnTheseNamespaces.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
4040
/** @var ClassDependency $dependency */
4141
foreach ($dependencies as $dependency) {
4242
if ($dependency->matchesOneOf(...$this->exclude)) {
43-
continue; // skip excluded namespaces
43+
continue;
4444
}
4545

4646
if ($dependency->matchesOneOf(...$this->namespaces)) {

tests/Unit/Analyzer/ClassDescriptionBuilderTest.php

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,6 @@ public function test_it_should_filter_out_php_core_classes(): void
194194
->build();
195195

196196
self::assertInstanceOf(ClassDescription::class, $classDescription);
197-
198-
// PHP core classes should be filtered out
199197
self::assertCount(0, $classDescription->getDependencies());
200198
}
201199

@@ -210,8 +208,6 @@ public function test_it_should_not_filter_user_defined_classes_in_root_namespace
210208
->build();
211209

212210
self::assertInstanceOf(ClassDescription::class, $classDescription);
213-
214-
// User-defined classes in root namespace should NOT be filtered
215211
self::assertCount(1, $classDescription->getDependencies());
216212
self::assertEquals('NonExistentUserClass', $classDescription->getDependencies()[0]->getFQCN()->toString());
217213
}
@@ -228,8 +224,6 @@ public function test_it_should_not_filter_user_defined_classes_with_namespace():
228224
->build();
229225

230226
self::assertInstanceOf(ClassDescription::class, $classDescription);
231-
232-
// User-defined classes with namespaces should not be filtered
233227
self::assertCount(2, $classDescription->getDependencies());
234228
}
235229

@@ -240,16 +234,14 @@ public function test_it_should_filter_mixed_dependencies_correctly(): void
240234
$classDescription = (new ClassDescriptionBuilder())
241235
->setFilePath('src/Foo.php')
242236
->setClassName($FQCN)
243-
->addDependency(new ClassDependency('DateTime', 10)) // PHP core - filtered
244-
->addDependency(new ClassDependency('Vendor\Package\SomeClass', 15)) // Namespaced - kept
245-
->addDependency(new ClassDependency('Exception', 20)) // PHP core - filtered
246-
->addDependency(new ClassDependency('NonExistentUserClass', 25)) // User root class - kept
247-
->addDependency(new ClassDependency('PDO', 30)) // PHP core - filtered
237+
->addDependency(new ClassDependency('DateTime', 10))
238+
->addDependency(new ClassDependency('Vendor\Package\SomeClass', 15))
239+
->addDependency(new ClassDependency('Exception', 20))
240+
->addDependency(new ClassDependency('NonExistentUserClass', 25))
241+
->addDependency(new ClassDependency('PDO', 30))
248242
->build();
249243

250244
self::assertInstanceOf(ClassDescription::class, $classDescription);
251-
252-
// Should keep only the 2 non-PHP-core dependencies
253245
self::assertCount(2, $classDescription->getDependencies());
254246

255247
$dependencies = $classDescription->getDependencies();
@@ -261,19 +253,14 @@ public function test_it_should_filter_internal_classes_with_namespaces(): void
261253
{
262254
$FQCN = 'MyClass';
263255

264-
// ReflectionClass is a PHP internal class in the root namespace
265-
// If other internal namespaced classes exist (e.g., MongoDB\Driver\Manager),
266-
// they should also be filtered. We test with ReflectionClass which is always available.
267256
$classDescription = (new ClassDescriptionBuilder())
268257
->setFilePath('src/Foo.php')
269258
->setClassName($FQCN)
270-
->addDependency(new ClassDependency('ReflectionClass', 10)) // Internal root - filtered
271-
->addDependency(new ClassDependency('App\MyClass', 15)) // User namespaced - kept
259+
->addDependency(new ClassDependency('ReflectionClass', 10))
260+
->addDependency(new ClassDependency('App\MyClass', 15))
272261
->build();
273262

274263
self::assertInstanceOf(ClassDescription::class, $classDescription);
275-
276-
// ReflectionClass should be filtered, only App\MyClass should remain
277264
self::assertCount(1, $classDescription->getDependencies());
278265
self::assertEquals('App\MyClass', $classDescription->getDependencies()[0]->getFQCN()->toString());
279266
}

tests/Unit/Expressions/ForClasses/NotHaveDependencyOutsideNamespaceTest.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,6 @@ public function test_it_should_automatically_exclude_php_core_classes(): void
9797
$violations = new Violations();
9898
$notHaveDependencyOutsideNamespace->evaluate($classDescription, $violations, $because);
9999

100-
// PHP core classes are automatically filtered at the ClassDescriptionBuilder level
101-
// So only 'another\class' should be reported as a violation
102100
self::assertEquals(1, $violations->count());
103101
}
104102
}

0 commit comments

Comments
 (0)