Skip to content

Commit 76a5ecd

Browse files
Add 12 Tier 2 PHPStan rules for deeper convention enforcement
Adds method-body analysis, constructor inspection, scope checking, and cache usage rules. Total rule count is now 29. All rules pass PHPStan level 8 self-analysis and produce no false positives against phpnomad/cli. New rules: AdaptersMustImplement, CanSetContainerMustUseTrait, InitializerUseHasInterfaces, ListenerNoBroadcastInHandle, DatastoreTypeHintInterface, DatabaseScope, ControllerMustSetStatus, TaskDispatchViaStrategy, NoCacheStrategyInBusiness, CacheGetMustCatchException, NoHardcodedTtl, CacheKeyViaInterface. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5689f96 commit 76a5ecd

69 files changed

Lines changed: 2268 additions & 1 deletion

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

README.md

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
Custom [PHPStan](https://phpstan.org/) rules that enforce [PHPNomad](https://github.com/phpnomad) framework conventions.
44

5-
These rules catch architectural anti-patterns at static analysis time, including models with inline serialization, service locator usage, raw SQL, singleton abuse, and missing interface implementations.
5+
These rules catch architectural anti-patterns at static analysis time, including models with inline serialization, service locator usage, raw SQL, singleton abuse, missing interface implementations, database scope violations, cache misuse, and more.
66

77
## Installation
88

@@ -29,6 +29,12 @@ includes:
2929
| `phpnomad.model.setter` | Models must not have setter methods (`set*`). Models are immutable. |
3030
| `phpnomad.model.arrayMethod` | Models must not have `toArray()` or `fromArray()`. Use a separate adapter class. |
3131

32+
### Adapters
33+
34+
| Identifier | Description |
35+
|---|---|
36+
| `phpnomad.adapter.notImplementing` | Classes named `*Adapter` or in `Adapters` namespaces must implement `ModelAdapter`. Exempts `MutationAdapter` implementations. |
37+
3238
### Events
3339

3440
| Identifier | Description |
@@ -37,12 +43,20 @@ includes:
3743
| `phpnomad.event.notFinal` | Event classes should be declared `final`. |
3844
| `phpnomad.event.notReadonly` | Event properties and promoted constructor parameters should be `readonly`. |
3945
| `phpnomad.listener.notImplementing` | Classes in `Listeners` namespaces or named `*Listener` must implement `CanHandle`. |
46+
| `phpnomad.listener.broadcastInHandle` | Listeners must not call `EventStrategy::broadcast()` from within their `handle()` method. Prevents infinite loops. |
4047

4148
### DI / Container
4249

4350
| Identifier | Description |
4451
|---|---|
4552
| `phpnomad.di.serviceLocator` | `InstanceProvider::get()` must not be called inside business classes. Use constructor injection. Calls inside initializers and bootstrappers are allowed. |
53+
| `phpnomad.di.missingTrait` | Classes implementing `CanSetContainer` must use the `HasSettableContainer` trait. |
54+
55+
### Initializers
56+
57+
| Identifier | Description |
58+
|---|---|
59+
| `phpnomad.initializer.useInterface` | Initializers should use `Has*` interfaces (e.g., `HasListeners`, `HasTaskHandlers`) instead of manually calling registration methods like `EventStrategy::attach()`. |
4660

4761
### Facades
4862

@@ -56,6 +70,14 @@ includes:
5670
| Identifier | Description |
5771
|---|---|
5872
| `phpnomad.database.rawSql` | Raw SQL strings (`SELECT ... FROM`, `INSERT INTO`, etc.) must not appear in code. Use PHPNomad datastore patterns. |
73+
| `phpnomad.database.scope` | `QueryBuilder`, `ClauseBuilder`, and `QueryStrategy` must only be used inside Datastore classes. |
74+
| `phpnomad.database.concreteTableHint` | Constructor parameters should type-hint the `Datastore` interface, not concrete `Table` subclasses. |
75+
76+
### Controllers
77+
78+
| Identifier | Description |
79+
|---|---|
80+
| `phpnomad.controller.noStatus` | Controller `getResponse()` must explicitly set an HTTP status code via `setStatus()` or `setError()`. |
5981

6082
### Console
6183

@@ -70,6 +92,16 @@ includes:
7092
|---|---|
7193
| `phpnomad.task.notImplementing` | Classes in `Tasks` namespaces or named `*Task` must implement `PHPNomad\Tasks\Interfaces\Task`. |
7294
| `phpnomad.taskHandler.notImplementing` | Task handler classes must implement `PHPNomad\Tasks\Interfaces\CanHandleTask`. |
95+
| `phpnomad.task.directHandle` | Task handlers must be dispatched via `TaskStrategy::dispatch()`, not by calling `handle()` directly. |
96+
97+
### Cache
98+
99+
| Identifier | Description |
100+
|---|---|
101+
| `phpnomad.cache.directStrategy` | Business classes should not inject `CacheStrategy` directly. Use `CacheableService` or a Facade. |
102+
| `phpnomad.cache.uncaughtException` | `CacheStrategy::get()` throws `CachedItemNotFoundException`. Ensure the call is wrapped in a try/catch. |
103+
| `phpnomad.cache.hardcodedTtl` | Do not hardcode TTL values as integer literals. Use a `CachePolicy` or configuration constant. |
104+
| `phpnomad.cache.stringConcatKey` | Use the `HasCacheKey` interface for cache key generation instead of string concatenation. |
73105

74106
### General
75107

extension.neon

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,15 @@ rules:
2020
- PHPNomad\PhpstanRules\Rules\General\NoGlobalKeywordRule
2121
- PHPNomad\PhpstanRules\Rules\Tasks\TasksMustImplementRule
2222
- PHPNomad\PhpstanRules\Rules\Tasks\TaskHandlersMustImplementRule
23+
- PHPNomad\PhpstanRules\Rules\Adapters\AdaptersMustImplementRule
24+
- PHPNomad\PhpstanRules\Rules\Di\CanSetContainerMustUseTraitRule
25+
- PHPNomad\PhpstanRules\Rules\Initializers\InitializerUseHasInterfacesRule
26+
- PHPNomad\PhpstanRules\Rules\Events\ListenerNoBroadcastInHandleRule
27+
- PHPNomad\PhpstanRules\Rules\Database\DatastoreTypeHintInterfaceRule
28+
- PHPNomad\PhpstanRules\Rules\Database\DatabaseScopeRule
29+
- PHPNomad\PhpstanRules\Rules\Controllers\ControllerMustSetStatusRule
30+
- PHPNomad\PhpstanRules\Rules\Tasks\TaskDispatchViaStrategyRule
31+
- PHPNomad\PhpstanRules\Rules\Cache\NoCacheStrategyInBusinessRule
32+
- PHPNomad\PhpstanRules\Rules\Cache\CacheGetMustCatchExceptionRule
33+
- PHPNomad\PhpstanRules\Rules\Cache\NoHardcodedTtlRule
34+
- PHPNomad\PhpstanRules\Rules\Cache\CacheKeyViaInterfaceRule
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
<?php
2+
3+
namespace PHPNomad\PhpstanRules\Rules\Adapters;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Stmt\Class_;
7+
use PHPStan\Analyser\Scope;
8+
use PHPStan\Node\InClassNode;
9+
use PHPStan\Rules\Rule;
10+
use PHPStan\Rules\RuleErrorBuilder;
11+
12+
/**
13+
* @implements Rule<InClassNode>
14+
*/
15+
class AdaptersMustImplementRule implements Rule
16+
{
17+
private const MODEL_ADAPTER_INTERFACE = 'PHPNomad\Datastore\Interfaces\ModelAdapter';
18+
private const MUTATION_ADAPTER_INTERFACE = 'PHPNomad\Mutator\Interfaces\MutationAdapter';
19+
20+
public function getNodeType(): string
21+
{
22+
return InClassNode::class;
23+
}
24+
25+
public function processNode(Node $node, Scope $scope): array
26+
{
27+
$classReflection = $node->getClassReflection();
28+
$originalNode = $node->getOriginalNode();
29+
if (!$originalNode instanceof Class_) {
30+
return [];
31+
}
32+
33+
if ($classReflection->isAbstract()) {
34+
return [];
35+
}
36+
37+
$className = $classReflection->getName();
38+
39+
if (!$this->looksLikeAdapter($className, $originalNode)) {
40+
return [];
41+
}
42+
43+
if ($classReflection->implementsInterface(self::MUTATION_ADAPTER_INTERFACE)) {
44+
return [];
45+
}
46+
47+
if ($classReflection->implementsInterface(self::MODEL_ADAPTER_INTERFACE)) {
48+
return [];
49+
}
50+
51+
return [
52+
RuleErrorBuilder::message(
53+
'Adapter classes must implement PHPNomad\Datastore\Interfaces\ModelAdapter.'
54+
)
55+
->identifier('phpnomad.adapter.notImplementing')
56+
->build(),
57+
];
58+
}
59+
60+
private function looksLikeAdapter(string $className, Class_ $node): bool
61+
{
62+
$shortName = substr($className, strrpos($className, '\\') + 1);
63+
64+
if (str_ends_with($shortName, 'Adapter')) {
65+
return true;
66+
}
67+
68+
if ($node->namespacedName !== null) {
69+
$namespace = $node->namespacedName->toString();
70+
if (str_contains($namespace, '\Adapters\\')) {
71+
return true;
72+
}
73+
}
74+
75+
return false;
76+
}
77+
}
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<?php
2+
3+
namespace PHPNomad\PhpstanRules\Rules\Cache;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr\MethodCall;
7+
use PhpParser\Node\Identifier;
8+
use PHPStan\Analyser\Scope;
9+
use PHPStan\Rules\Rule;
10+
use PHPStan\Rules\RuleErrorBuilder;
11+
use PHPStan\Type\ObjectType;
12+
13+
/**
14+
* @implements Rule<MethodCall>
15+
*/
16+
class CacheGetMustCatchExceptionRule implements Rule
17+
{
18+
private const CACHE_STRATEGY = 'PHPNomad\Cache\Interfaces\CacheStrategy';
19+
private const FACADE = 'PHPNomad\Facade\Abstracts\Facade';
20+
21+
public function getNodeType(): string
22+
{
23+
return MethodCall::class;
24+
}
25+
26+
public function processNode(Node $node, Scope $scope): array
27+
{
28+
if (!$node->name instanceof Identifier) {
29+
return [];
30+
}
31+
32+
if ($node->name->toString() !== 'get') {
33+
return [];
34+
}
35+
36+
$callerType = $scope->getType($node->var);
37+
$cacheType = new ObjectType(self::CACHE_STRATEGY);
38+
39+
if (!$cacheType->isSuperTypeOf($callerType)->yes()) {
40+
return [];
41+
}
42+
43+
$classReflection = $scope->getClassReflection();
44+
45+
if ($classReflection === null) {
46+
return [];
47+
}
48+
49+
if ($classReflection->implementsInterface(self::CACHE_STRATEGY)) {
50+
return [];
51+
}
52+
53+
$parent = $classReflection->getParentClass();
54+
while ($parent !== null) {
55+
if ((string) $parent->getName() === self::FACADE) {
56+
return [];
57+
}
58+
$parent = $parent->getParentClass();
59+
}
60+
61+
return [
62+
RuleErrorBuilder::message(
63+
'CacheStrategy::get() throws CachedItemNotFoundException. Ensure this call is wrapped in a try/catch.'
64+
)
65+
->identifier('phpnomad.cache.uncaughtException')
66+
->tip('Catch CachedItemNotFoundException or use the Cache facade\'s load() method instead.')
67+
->build(),
68+
];
69+
}
70+
}
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php
2+
3+
namespace PHPNomad\PhpstanRules\Rules\Cache;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr\MethodCall;
7+
use PhpParser\Node\Identifier;
8+
use PHPStan\Analyser\Scope;
9+
use PHPStan\Rules\Rule;
10+
use PHPStan\Rules\RuleErrorBuilder;
11+
use PHPStan\Type\ObjectType;
12+
13+
/**
14+
* @implements Rule<MethodCall>
15+
*/
16+
class CacheKeyViaInterfaceRule implements Rule
17+
{
18+
private const CACHE_STRATEGY = 'PHPNomad\Cache\Interfaces\CacheStrategy';
19+
20+
private const KEY_METHODS = ['get', 'set', 'delete', 'exists'];
21+
22+
public function getNodeType(): string
23+
{
24+
return MethodCall::class;
25+
}
26+
27+
public function processNode(Node $node, Scope $scope): array
28+
{
29+
if (!$node->name instanceof Identifier) {
30+
return [];
31+
}
32+
33+
if (!in_array($node->name->toString(), self::KEY_METHODS, true)) {
34+
return [];
35+
}
36+
37+
$callerType = $scope->getType($node->var);
38+
$cacheType = new ObjectType(self::CACHE_STRATEGY);
39+
40+
if (!$cacheType->isSuperTypeOf($callerType)->yes()) {
41+
return [];
42+
}
43+
44+
if (count($node->getArgs()) < 1) {
45+
return [];
46+
}
47+
48+
$keyArg = $node->getArgs()[0]->value;
49+
50+
if ($keyArg instanceof \PhpParser\Node\Expr\BinaryOp\Concat
51+
|| $keyArg instanceof \PhpParser\Node\Scalar\InterpolatedString) {
52+
return [
53+
RuleErrorBuilder::message(
54+
'Use HasCacheKey interface for cache key generation instead of string concatenation.'
55+
)
56+
->identifier('phpnomad.cache.stringConcatKey')
57+
->build(),
58+
];
59+
}
60+
61+
return [];
62+
}
63+
}

0 commit comments

Comments
 (0)