Skip to content

Commit 408d5f2

Browse files
fain182claude
fain182
and
claude
committed
Switch ForClasses expressions to pure reflection, no static fallback
Remove the static fallback from Implement and NotImplement (and the previously updated Extend, NotExtend, HaveTrait, NotHaveTrait) so that all ForClasses expressions use ReflectionClass exclusively. When a class is not autoloadable the catch block now returns silently instead of falling back to the parsed AST data. To support this, all test fixtures that previously relied on vfsStream or inline PHP strings with fake namespaces have been replaced with real autoloadable classes: - ExtendsThrowableTest: real fixtures under tests/Integration/Fixtures/ - CheckClassHaveTraitTest: moved to tests/Integration/PHPUnit/ with real fixture classes and global-namespace traits registered in classmap - CanParseEnumsTest: reads from tests/Unit/Analyzer/FileParser/Fixtures/ - RuleBuilderTest: uses tests/Unit/Rules/Fixtures/MyClass composer.json autoload-dev extended with App\ PSR-4 mapping for E2E mvc fixtures and classmap entries for ContainerAwareInterface and the three global-namespace traits. phpunit.xml gains <exclude> entries for all fixture subdirectories to prevent PHPUnit from warning about non-TestCase classes named *Test. Rename claude.md -> CLAUDE.md and add a section documenting the reflection contract and fixture conventions. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent a8ff510 commit 408d5f2

32 files changed

+772
-414
lines changed

claude.md renamed to CLAUDE.md

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ bin/
147147
- **`README.md`** - User-facing documentation, installation, usage, available rules
148148
- **`CONTRIBUTING.md`** - How to contribute (code, tests, docs)
149149
- **`CONTRIBUTORS.md`** - List of project contributors
150-
- **`claude.md`** - This file (Claude AI context)
150+
- **`CLAUDE.md`** - This file (Claude AI context)
151151

152152
### Configuration Files
153153
- **`phparkitect.php`** - Example configuration file for users
@@ -288,10 +288,62 @@ make test # Runs tests on configured PHP version
288288
- Mixed scenarios that could break
289289
5. **Test coverage:** Always add unit tests for new functionality
290290

291+
## Reflection-based expression rules (Option B — mandatory autoloading)
292+
293+
All `src/Expression/ForClasses/` expressions (`Implement`, `NotImplement`, `Extend`,
294+
`NotExtend`, `HaveTrait`, `NotHaveTrait`) use **pure reflection** with no static fallback:
295+
296+
```php
297+
try {
298+
$reflection = new \ReflectionClass($theClass->getFQCN());
299+
// ... use reflection ...
300+
} catch (\ReflectionException $e) {
301+
return; // class not autoloadable → skip silently
302+
}
303+
```
304+
305+
**Consequence:** every fixture class used in tests must be autoloadable. No vfsStream,
306+
no inline PHP strings with fake namespaces.
307+
308+
### Test fixture conventions
309+
310+
- Real fixture files live in `tests/*/Fixtures/` subdirectories
311+
- PSR-4 namespaced fixtures → covered by `"Arkitect\\Tests\\": "tests/"` in autoload-dev
312+
- Global-namespace traits (e.g. `DatabaseTransactions`) → listed in `classmap` in `composer.json`
313+
- E2E mvc fixtures (`App\Controller\*`, etc.) → `"App\\": "tests/E2E/_fixtures/mvc/"`
314+
- `ContainerAwareInterface` (global namespace) → listed in `classmap`
315+
- After adding fixtures: run `composer dump-autoload`
316+
317+
### composer.json autoload-dev (current state)
318+
319+
```json
320+
"autoload-dev": {
321+
"psr-4": {
322+
"Arkitect\\Tests\\": "tests/",
323+
"App\\": "tests/E2E/_fixtures/mvc/"
324+
},
325+
"classmap": [
326+
"tests/E2E/_fixtures/mvc/ContainerAwareInterface.php",
327+
"tests/Integration/PHPUnit/Fixtures/DatabaseTransactions.php",
328+
"tests/Integration/PHPUnit/Fixtures/RefreshDatabase.php",
329+
"tests/Integration/PHPUnit/Fixtures/HasUuid.php"
330+
]
331+
}
332+
```
333+
334+
### phpunit.xml — fixture directories excluded from test discovery
335+
336+
```xml
337+
<exclude>tests/Integration/PHPUnit/Fixtures</exclude>
338+
<exclude>tests/Integration/Fixtures</exclude>
339+
<exclude>tests/Unit/Analyzer/FileParser/Fixtures</exclude>
340+
<exclude>tests/Unit/Rules/Fixtures</exclude>
341+
```
342+
291343
## Testing Guidelines
292344

293345
- Unit tests go in `tests/` mirroring the `src/` structure
294-
- Use PHPUnit (7.5+, 9.0+, or 10.0+)
346+
- Use PHPUnit (9.6+, 10.0+, 11.0+)
295347
- Use prophecy for mocking
296348
- Aim for good coverage of critical paths
297349
- Test both success and edge cases

‎composer.json‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,15 @@
4848
},
4949
"autoload-dev": {
5050
"psr-4": {
51-
"Arkitect\\Tests\\": "tests/"
52-
}
51+
"Arkitect\\Tests\\": "tests/",
52+
"App\\": "tests/E2E/_fixtures/mvc/"
53+
},
54+
"classmap": [
55+
"tests/E2E/_fixtures/mvc/ContainerAwareInterface.php",
56+
"tests/Integration/PHPUnit/Fixtures/DatabaseTransactions.php",
57+
"tests/Integration/PHPUnit/Fixtures/RefreshDatabase.php",
58+
"tests/Integration/PHPUnit/Fixtures/HasUuid.php"
59+
]
5360
},
5461
"config": {
5562
"bin-dir": "bin",

‎phpunit.xml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@
1515
<testsuites>
1616
<testsuite name="default">
1717
<directory>tests</directory>
18+
<exclude>tests/Integration/PHPUnit/Fixtures</exclude>
19+
<exclude>tests/Integration/Fixtures</exclude>
20+
<exclude>tests/Unit/Analyzer/FileParser/Fixtures</exclude>
21+
<exclude>tests/Unit/Rules/Fixtures</exclude>
1822
</testsuite>
1923
</testsuites>
2024

‎src/Expression/ForClasses/Extend.php‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace Arkitect\Expression\ForClasses;
66

77
use Arkitect\Analyzer\ClassDescription;
8+
use Arkitect\Analyzer\FullyQualifiedClassName;
89
use Arkitect\Expression\Description;
910
use Arkitect\Expression\Expression;
1011
use Arkitect\Rules\Violation;
@@ -30,23 +31,30 @@ public function describe(ClassDescription $theClass, string $because): Descripti
3031

3132
public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
3233
{
33-
$extends = $theClass->getExtends();
34+
try {
35+
$reflection = new \ReflectionClass($theClass->getFQCN());
36+
$parents = [];
37+
$parent = $reflection->getParentClass();
38+
while ($parent) {
39+
$parents[] = $parent->getName();
40+
$parent = $parent->getParentClass();
41+
}
42+
} catch (\ReflectionException $e) {
43+
return;
44+
}
3445

35-
/** @var string $className */
3646
foreach ($this->classNames as $className) {
37-
foreach ($extends as $extend) {
38-
if ($extend->matches($className)) {
47+
foreach ($parents as $parentName) {
48+
if (FullyQualifiedClassName::fromString($parentName)->matches($className)) {
3949
return;
4050
}
4151
}
4252
}
4353

44-
$violation = Violation::create(
54+
$violations->add(Violation::create(
4555
$theClass->getFQCN(),
4656
ViolationMessage::selfExplanatory($this->describe($theClass, $because)),
4757
$theClass->getFilePath()
48-
);
49-
50-
$violations->add($violation);
58+
));
5159
}
5260
}

‎src/Expression/ForClasses/HaveTrait.php‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
namespace Arkitect\Expression\ForClasses;
55

66
use Arkitect\Analyzer\ClassDescription;
7+
use Arkitect\Analyzer\FullyQualifiedClassName;
78
use Arkitect\Expression\Description;
89
use Arkitect\Expression\Expression;
910
use Arkitect\Rules\Violation;
@@ -36,16 +37,36 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
3637
return;
3738
}
3839

39-
if ($theClass->hasTrait($this->trait)) {
40+
$trait = $this->trait;
41+
42+
try {
43+
$reflection = new \ReflectionClass($theClass->getFQCN());
44+
$allTraits = [];
45+
$class = $reflection;
46+
while ($class) {
47+
foreach ($class->getTraitNames() as $traitName) {
48+
$allTraits[] = $traitName;
49+
}
50+
$class = $class->getParentClass() ?: null;
51+
}
52+
} catch (\ReflectionException $e) {
4053
return;
4154
}
4255

43-
$violations->add(
44-
Violation::create(
45-
$theClass->getFQCN(),
46-
ViolationMessage::selfExplanatory($this->describe($theClass, $because)),
47-
$theClass->getFilePath()
48-
)
56+
$found = array_reduce(
57+
$allTraits,
58+
static fn (bool $carry, string $traitName): bool => $carry || FullyQualifiedClassName::fromString($traitName)->matches($trait),
59+
false
4960
);
61+
62+
if (!$found) {
63+
$violations->add(
64+
Violation::create(
65+
$theClass->getFQCN(),
66+
ViolationMessage::selfExplanatory($this->describe($theClass, $because)),
67+
$theClass->getFilePath()
68+
)
69+
);
70+
}
5071
}
5172
}

‎src/Expression/ForClasses/Implement.php‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,25 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
3939
}
4040

4141
$interface = $this->interface;
42-
$interfaces = $theClass->getInterfaces();
43-
$implements = static fn (FullyQualifiedClassName $FQCN): bool => $FQCN->matches($interface);
42+
$found = false;
43+
44+
try {
45+
$reflection = new \ReflectionClass($theClass->getFQCN());
46+
$allInterfaces = $reflection->getInterfaceNames();
47+
$found = \count(array_filter(
48+
$allInterfaces,
49+
static fn (string $name): bool => FullyQualifiedClassName::fromString($name)->matches($interface)
50+
)) > 0;
51+
} catch (\ReflectionException $e) {
52+
return;
53+
}
4454

45-
if (0 === \count(array_filter($interfaces, $implements))) {
46-
$violation = Violation::create(
55+
if (!$found) {
56+
$violations->add(Violation::create(
4757
$theClass->getFQCN(),
4858
ViolationMessage::selfExplanatory($this->describe($theClass, $because)),
4959
$theClass->getFilePath()
50-
);
51-
$violations->add($violation);
60+
));
5261
}
5362
}
5463
}

‎src/Expression/ForClasses/NotExtend.php‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace Arkitect\Expression\ForClasses;
66

77
use Arkitect\Analyzer\ClassDescription;
8+
use Arkitect\Analyzer\FullyQualifiedClassName;
89
use Arkitect\Expression\Description;
910
use Arkitect\Expression\Expression;
1011
use Arkitect\Rules\Violation;
@@ -30,19 +31,27 @@ public function describe(ClassDescription $theClass, string $because): Descripti
3031

3132
public function evaluate(ClassDescription $theClass, Violations $violations, string $because): void
3233
{
33-
$extends = $theClass->getExtends();
34+
try {
35+
$reflection = new \ReflectionClass($theClass->getFQCN());
36+
$parents = [];
37+
$parent = $reflection->getParentClass();
38+
while ($parent) {
39+
$parents[] = $parent->getName();
40+
$parent = $parent->getParentClass();
41+
}
42+
} catch (\ReflectionException $e) {
43+
return;
44+
}
3445

35-
/** @var string $className */
3646
foreach ($this->classNames as $className) {
37-
foreach ($extends as $extend) {
38-
if ($extend->matches($className)) {
39-
$violation = Violation::create(
47+
foreach ($parents as $parentName) {
48+
if (FullyQualifiedClassName::fromString($parentName)->matches($className)) {
49+
$violations->add(Violation::create(
4050
$theClass->getFQCN(),
4151
ViolationMessage::selfExplanatory($this->describe($theClass, $because)),
4252
$theClass->getFilePath()
43-
);
44-
45-
$violations->add($violation);
53+
));
54+
break;
4655
}
4756
}
4857
}

‎src/Expression/ForClasses/NotHaveTrait.php‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,35 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
3939
}
4040

4141
$trait = $this->trait;
42-
$traits = $theClass->getTraits();
43-
$usesTrait = static fn (FullyQualifiedClassName $FQCN): bool => $FQCN->matches($trait);
44-
45-
if (\count(array_filter($traits, $usesTrait)) > 0) {
46-
$violation = Violation::create(
47-
$theClass->getFQCN(),
48-
ViolationMessage::selfExplanatory($this->describe($theClass, $because)),
49-
$theClass->getFilePath()
42+
43+
try {
44+
$reflection = new \ReflectionClass($theClass->getFQCN());
45+
$allTraits = [];
46+
$class = $reflection;
47+
while ($class) {
48+
foreach ($class->getTraitNames() as $traitName) {
49+
$allTraits[] = $traitName;
50+
}
51+
$class = $class->getParentClass() ?: null;
52+
}
53+
} catch (\ReflectionException $e) {
54+
return;
55+
}
56+
57+
$found = array_reduce(
58+
$allTraits,
59+
static fn (bool $carry, string $traitName): bool => $carry || FullyQualifiedClassName::fromString($traitName)->matches($trait),
60+
false
61+
);
62+
63+
if ($found) {
64+
$violations->add(
65+
Violation::create(
66+
$theClass->getFQCN(),
67+
ViolationMessage::selfExplanatory($this->describe($theClass, $because)),
68+
$theClass->getFilePath()
69+
)
5070
);
51-
$violations->add($violation);
5271
}
5372
}
5473
}

‎src/Expression/ForClasses/NotImplement.php‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,25 @@ public function evaluate(ClassDescription $theClass, Violations $violations, str
3939
}
4040

4141
$interface = $this->interface;
42-
$interfaces = $theClass->getInterfaces();
43-
$implements = static fn (FullyQualifiedClassName $FQCN): bool => $FQCN->matches($interface);
42+
$found = false;
43+
44+
try {
45+
$reflection = new \ReflectionClass($theClass->getFQCN());
46+
$allInterfaces = $reflection->getInterfaceNames();
47+
$found = \count(array_filter(
48+
$allInterfaces,
49+
static fn (string $name): bool => FullyQualifiedClassName::fromString($name)->matches($interface)
50+
)) > 0;
51+
} catch (\ReflectionException $e) {
52+
return;
53+
}
4454

45-
if (\count(array_filter($interfaces, $implements)) > 0) {
46-
$violation = Violation::create(
55+
if ($found) {
56+
$violations->add(Violation::create(
4757
$theClass->getFQCN(),
4858
ViolationMessage::selfExplanatory($this->describe($theClass, $because)),
4959
$theClass->getFilePath()
50-
);
51-
$violations->add($violation);
60+
));
5261
}
5362
}
5463
}

0 commit comments

Comments
 (0)