Skip to content

Commit 292d551

Browse files
Joseph Edmondsclaude
andcommitted
feat: Add 7 new PHPStan rules (4 default + 3 optional)
Default rules (auto-enabled for all projects): - ForbidNewDateTimeRule: Ban mutable DateTime in favour of DateTimeImmutable - ForbidEmptyLanguageConstructRule: Ban empty() language construct - ForbidLooseComparisonRule: Ban == and != loose comparisons - ForbidDeprecatedSerializableRule: Ban deprecated Serializable interface Optional rules (enable per-project in phpstan.neon): - ForbidInlinePhpstanIgnoreRule: Ban inline @PHPStan-Ignore annotations - ForbidSilentCatchRule: Catch blocks must use/log/rethrow exception - RequireReadonlyServiceRule: Service classes must be final readonly Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 7a7b8e4 commit 292d551

9 files changed

Lines changed: 751 additions & 0 deletions

rules-default.neon

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ rules:
33
- LTS\PHPQA\PHPStan\Rules\ForbidEmptyCatchBlockRule
44
- LTS\PHPQA\PHPStan\Rules\RequireDeclareStrictTypesRule
55
- LTS\PHPQA\PHPStan\Rules\ForbidAllowMockWithoutExpectationsRule
6+
- LTS\PHPQA\PHPStan\Rules\ForbidNewDateTimeRule
7+
- LTS\PHPQA\PHPStan\Rules\ForbidEmptyLanguageConstructRule
8+
- LTS\PHPQA\PHPStan\Rules\ForbidLooseComparisonRule
9+
- LTS\PHPQA\PHPStan\Rules\ForbidDeprecatedSerializableRule
610

711
services:
812
-

rules-optional.neon

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,21 @@
2222
# Note: RequireExplicitDIAttributeRule has no constructor params.
2323
#
2424
# - LTS\PHPQA\PHPStan\Rules\RequireExplicitDIAttributeRule
25+
#
26+
# STRICT EXCEPTION HANDLING (catch blocks must use the exception):
27+
# ForbidSilentCatchRule is stricter than ForbidEmptyCatchBlockRule (default).
28+
# It flags catch blocks that have statements but ignore the caught exception.
29+
#
30+
# - LTS\PHPQA\PHPStan\Rules\ForbidSilentCatchRule
31+
#
32+
# INLINE PHPSTAN SUPPRESSION BAN (enforce fixing over suppressing):
33+
# Bans @phpstan-ignore annotations. Forces type issues to be fixed properly
34+
# instead of suppressed inline. Irreducible cases go in phpstan.neon ignoreErrors.
35+
#
36+
# - LTS\PHPQA\PHPStan\Rules\ForbidInlinePhpstanIgnoreRule
37+
#
38+
# READONLY SERVICES (enforce immutable services):
39+
# Requires service classes to be "final readonly class". Skips entities,
40+
# controllers, commands, and framework classes that need mutability.
41+
#
42+
# - LTS\PHPQA\PHPStan\Rules\RequireReadonlyServiceRule
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace LTS\PHPQA\PHPStan\Rules;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Stmt\Class_;
9+
use PHPStan\Analyser\Scope;
10+
use PHPStan\Rules\Rule;
11+
use PHPStan\Rules\RuleErrorBuilder;
12+
13+
/**
14+
* Bans the deprecated Serializable interface.
15+
*
16+
* The Serializable interface was deprecated in PHP 8.1. Classes should use
17+
* the __serialize() and its counterpart magic methods instead, which provide
18+
* better type safety and performance.
19+
*
20+
* @implements Rule<Class_>
21+
*/
22+
final class ForbidDeprecatedSerializableRule implements Rule
23+
{
24+
public function getNodeType(): string
25+
{
26+
return Class_::class;
27+
}
28+
29+
/**
30+
* @return list<\PHPStan\Rules\IdentifierRuleError>
31+
*/
32+
public function processNode(Node $node, Scope $scope): array
33+
{
34+
$className = null !== $node->name ? $node->name->name : 'anonymous';
35+
36+
foreach ($node->implements as $interface) {
37+
$resolved = $scope->resolveName($interface);
38+
39+
if ('Serializable' === $resolved) {
40+
return [
41+
RuleErrorBuilder::message(
42+
\sprintf(
43+
'Class %s implements the deprecated Serializable interface (PHP 8.1). '
44+
. 'Use __serialize() and its counterpart magic methods instead. '
45+
. 'Remove "implements Serializable" and the legacy serialization methods.',
46+
$className,
47+
),
48+
)->identifier('phpqaci.deprecatedSerializable')->build(),
49+
];
50+
}
51+
}
52+
53+
return [];
54+
}
55+
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace LTS\PHPQA\PHPStan\Rules;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Expr\Empty_;
9+
use PHPStan\Analyser\Scope;
10+
use PHPStan\Rules\Rule;
11+
use PHPStan\Rules\RuleErrorBuilder;
12+
13+
/**
14+
* Bans the empty() language construct.
15+
*
16+
* empty() silently accepts undefined variables and performs loose type coercion:
17+
* empty(0), empty(''), empty('0'), empty([]), empty(null) all return true.
18+
* This hides type errors and makes code unpredictable.
19+
*
20+
* WRONG:
21+
* if (empty($array)) { ... }
22+
*
23+
* RIGHT:
24+
* if ([] === $array) { ... }
25+
* if ('' === $string) { ... }
26+
* if (null === $value) { ... }
27+
*
28+
* @implements Rule<Empty_>
29+
*/
30+
final class ForbidEmptyLanguageConstructRule implements Rule
31+
{
32+
public function getNodeType(): string
33+
{
34+
return Empty_::class;
35+
}
36+
37+
/**
38+
* @return list<\PHPStan\Rules\IdentifierRuleError>
39+
*/
40+
public function processNode(Node $node, Scope $scope): array
41+
{
42+
return [
43+
RuleErrorBuilder::message(
44+
'empty() is banned. Use strict comparison instead: '
45+
. '"[] === $var" for arrays, '
46+
. '"$var === \'\'" for strings, '
47+
. '"$var === null" for nullable. '
48+
. 'empty() hides type errors and silently accepts undefined variables.',
49+
)->identifier('phpqaci.emptyLanguageConstruct')->build(),
50+
];
51+
}
52+
}
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace LTS\PHPQA\PHPStan\Rules;
6+
7+
use PhpParser\Comment;
8+
use PhpParser\Node;
9+
use PhpParser\Node\Stmt;
10+
use PHPStan\Analyser\Scope;
11+
use PHPStan\Rules\Rule;
12+
use PHPStan\Rules\RuleErrorBuilder;
13+
14+
/**
15+
* Bans inline PHPStan suppression annotations in source code.
16+
*
17+
* Inline suppression annotations hide type errors instead of fixing them.
18+
* Fix the underlying type issue using type guards, array shapes, or Safe functions.
19+
*
20+
* If a suppression is truly irreducible, it must be managed in phpstan.neon
21+
* with a specific identifier and path instead of hidden inline.
22+
*
23+
* Skips test files and qaConfig files.
24+
*
25+
* @implements Rule<Stmt>
26+
*/
27+
final class ForbidInlinePhpstanIgnoreRule implements Rule
28+
{
29+
/** Matches the PHPStan inline suppression annotation pattern */
30+
private const string PATTERN = '/@phpstan\x2dignore/';
31+
32+
public function getNodeType(): string
33+
{
34+
return Stmt::class;
35+
}
36+
37+
/**
38+
* @return list<\PHPStan\Rules\IdentifierRuleError>
39+
*/
40+
public function processNode(Node $node, Scope $scope): array
41+
{
42+
if ($this->isTestContext($scope)) {
43+
return [];
44+
}
45+
46+
$namespace = $scope->getNamespace();
47+
if (null !== $namespace && str_starts_with($namespace, 'QaConfig')) {
48+
return [];
49+
}
50+
51+
$comments = $node->getComments();
52+
if ([] === $comments) {
53+
return [];
54+
}
55+
56+
$errors = [];
57+
58+
foreach ($comments as $comment) {
59+
if (!$this->containsSuppressionAnnotation($comment)) {
60+
continue;
61+
}
62+
63+
$errors[] = RuleErrorBuilder::message(
64+
'Inline PHPStan suppression annotations are forbidden. '
65+
. 'Fix the underlying type issue instead '
66+
. '(use type guards, array shapes, \\Safe\\ functions, etc.). '
67+
. 'If truly irreducible, manage via ignoreErrors in phpstan.neon '
68+
. 'with a specific identifier and path.',
69+
)->identifier('phpqaci.inlinePhpstanIgnore')->line($comment->getStartLine())->build();
70+
}
71+
72+
return $errors;
73+
}
74+
75+
private function containsSuppressionAnnotation(Comment $comment): bool
76+
{
77+
$matchCount = \preg_match(self::PATTERN, $comment->getText());
78+
79+
return \is_int($matchCount) && $matchCount > 0;
80+
}
81+
82+
private function isTestContext(Scope $scope): bool
83+
{
84+
$namespace = $scope->getNamespace();
85+
if (null !== $namespace && str_contains($namespace, 'Tests')) {
86+
return true;
87+
}
88+
89+
return str_contains($scope->getFile(), '/tests/');
90+
}
91+
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace LTS\PHPQA\PHPStan\Rules;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Expr\BinaryOp;
9+
use PhpParser\Node\Expr\BinaryOp\Equal;
10+
use PhpParser\Node\Expr\BinaryOp\NotEqual;
11+
use PHPStan\Analyser\Scope;
12+
use PHPStan\Rules\Rule;
13+
use PHPStan\Rules\RuleErrorBuilder;
14+
15+
/**
16+
* Bans loose comparison operators == and !=.
17+
*
18+
* Loose comparisons perform type coercion, leading to surprising results:
19+
* - "0" == false (true)
20+
* - "" == null (true)
21+
* - "1" == true (true)
22+
*
23+
* Strict comparisons (=== and !==) compare both value and type.
24+
*
25+
* WRONG:
26+
* if ($status == 'active') { ... }
27+
*
28+
* RIGHT:
29+
* if ($status === 'active') { ... }
30+
*
31+
* @implements Rule<BinaryOp>
32+
*/
33+
final class ForbidLooseComparisonRule implements Rule
34+
{
35+
public function getNodeType(): string
36+
{
37+
return BinaryOp::class;
38+
}
39+
40+
/**
41+
* @return list<\PHPStan\Rules\IdentifierRuleError>
42+
*/
43+
public function processNode(Node $node, Scope $scope): array
44+
{
45+
if ($node instanceof Equal) {
46+
return [
47+
RuleErrorBuilder::message(
48+
'Loose comparison (==) is banned. Use strict comparison (===) instead. '
49+
. 'Loose comparisons cause type coercion bugs.',
50+
)->identifier('phpqaci.looseComparison')->build(),
51+
];
52+
}
53+
54+
if ($node instanceof NotEqual) {
55+
return [
56+
RuleErrorBuilder::message(
57+
'Loose comparison (!=) is banned. Use strict comparison (!==) instead. '
58+
. 'Loose comparisons cause type coercion bugs.',
59+
)->identifier('phpqaci.looseComparison')->build(),
60+
];
61+
}
62+
63+
return [];
64+
}
65+
}
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace LTS\PHPQA\PHPStan\Rules;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Expr\New_;
9+
use PhpParser\Node\Name;
10+
use PHPStan\Analyser\Scope;
11+
use PHPStan\Rules\Rule;
12+
use PHPStan\Rules\RuleErrorBuilder;
13+
14+
/**
15+
* Bans instantiation of mutable DateTime in favour of DateTimeImmutable.
16+
*
17+
* Mutable DateTime objects cause subtle bugs when passed between services:
18+
* a callee can modify a date the caller still references. DateTimeImmutable
19+
* eliminates this entire bug class by returning new instances from mutation
20+
* methods.
21+
*
22+
* WRONG:
23+
* $now = new DateTime();
24+
*
25+
* RIGHT:
26+
* $now = new DateTimeImmutable();
27+
*
28+
* Skips test files since tests may legitimately create DateTime objects.
29+
*
30+
* @implements Rule<New_>
31+
*/
32+
final class ForbidNewDateTimeRule implements Rule
33+
{
34+
public function getNodeType(): string
35+
{
36+
return New_::class;
37+
}
38+
39+
/**
40+
* @return list<\PHPStan\Rules\IdentifierRuleError>
41+
*/
42+
public function processNode(Node $node, Scope $scope): array
43+
{
44+
if (!$node->class instanceof Name) {
45+
return [];
46+
}
47+
48+
if ($this->isTestContext($scope)) {
49+
return [];
50+
}
51+
52+
$resolvedName = $scope->resolveName($node->class);
53+
54+
if ('DateTime' !== $resolvedName) {
55+
return [];
56+
}
57+
58+
return [
59+
RuleErrorBuilder::message(
60+
'Use DateTimeImmutable instead of DateTime. Mutable date/time objects cause subtle bugs '
61+
. 'when shared between services. Replace "new DateTime()" with "new DateTimeImmutable()".',
62+
)->identifier('phpqaci.newDateTime')->build(),
63+
];
64+
}
65+
66+
private function isTestContext(Scope $scope): bool
67+
{
68+
$namespace = $scope->getNamespace();
69+
if (null !== $namespace && str_contains($namespace, 'Tests')) {
70+
return true;
71+
}
72+
73+
return str_contains($scope->getFile(), '/tests/');
74+
}
75+
}

0 commit comments

Comments
 (0)