Skip to content

Commit b6bee99

Browse files
committed
Add build rule rejecting optional parameters in non-@api methods and functions
Existing violations are recorded in build/baseline-optional-parameters.neon. Amp-Thread-ID: https://ampcode.com/threads/T-01a18404-a5f9-74cf-bb1b-815927dc7033
1 parent 034afd3 commit b6bee99

5 files changed

Lines changed: 3154 additions & 0 deletions

File tree

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Build;
4+
5+
use PhpParser\Node;
6+
use PHPStan\Analyser\Scope;
7+
use PHPStan\Reflection\ClassReflection;
8+
use PHPStan\Rules\Rule;
9+
use PHPStan\Rules\RuleErrorBuilder;
10+
use function count;
11+
use function is_string;
12+
use function preg_match;
13+
use function sprintf;
14+
use function strtolower;
15+
16+
/**
17+
* Optional parameters are allowed only in @api methods and functions, where
18+
* adding a parameter must not break existing callers. Internal code should
19+
* pass every argument explicitly.
20+
*
21+
* A class-level @api tag covers public methods (and protected methods of non-final classes),
22+
* but not the constructor, which is covered only by its own @api tag.
23+
*
24+
* @implements Rule<Node\FunctionLike>
25+
*/
26+
final class NoOptionalParameterRule implements Rule
27+
{
28+
29+
public function getNodeType(): string
30+
{
31+
return Node\FunctionLike::class;
32+
}
33+
34+
public function processNode(Node $node, Scope $scope): array
35+
{
36+
$positionsOptionalInAncestors = [];
37+
if ($node instanceof Node\Stmt\ClassMethod) {
38+
$methodName = $node->name->toString();
39+
$classReflection = $scope->getClassReflection();
40+
$declaringReflection = $scope->isInTrait() ? $scope->getTraitReflection() : $classReflection;
41+
if ($classReflection === null || $declaringReflection === null || $this->isApiMethod($node, $declaringReflection)) {
42+
return [];
43+
}
44+
$description = sprintf('method %s::%s()', $declaringReflection->getDisplayName(), $methodName);
45+
$positionsOptionalInAncestors = $this->getPositionsOptionalInAncestors($classReflection, $methodName);
46+
} elseif ($node instanceof Node\Stmt\Function_) {
47+
if ($this->hasApiTag($node)) {
48+
return [];
49+
}
50+
$description = sprintf('function %s()', $node->namespacedName !== null ? $node->namespacedName->toString() : $node->name->toString());
51+
} else {
52+
return [];
53+
}
54+
55+
$errors = [];
56+
foreach ($node->getParams() as $position => $param) {
57+
if ($param->default === null) {
58+
continue;
59+
}
60+
if (!$param->var instanceof Node\Expr\Variable || !is_string($param->var->name)) {
61+
continue;
62+
}
63+
if (isset($positionsOptionalInAncestors[$position])) {
64+
continue;
65+
}
66+
67+
$errors[] = RuleErrorBuilder::message(sprintf(
68+
'Parameter $%s of %s must not be optional. Optional parameters are allowed only in @api code.',
69+
$param->var->name,
70+
$description,
71+
))
72+
->identifier('phpstan.optionalParameter')
73+
->line($param->getStartLine())
74+
->build();
75+
}
76+
77+
return $errors;
78+
}
79+
80+
private function isApiMethod(Node\Stmt\ClassMethod $node, ClassReflection $classReflection): bool
81+
{
82+
if ($this->hasApiTag($node)) {
83+
return true;
84+
}
85+
86+
if (strtolower($node->name->toString()) === '__construct') {
87+
return false;
88+
}
89+
90+
if ($node->isPrivate()) {
91+
return false;
92+
}
93+
94+
if ($node->isProtected() && $classReflection->isFinal()) {
95+
return false;
96+
}
97+
98+
$resolvedPhpDoc = $classReflection->getResolvedPhpDoc();
99+
if ($resolvedPhpDoc === null) {
100+
return false;
101+
}
102+
103+
foreach ($resolvedPhpDoc->getPhpDocNodes() as $phpDocNode) {
104+
if (count($phpDocNode->getTagsByName('@api')) > 0) {
105+
return true;
106+
}
107+
}
108+
109+
return false;
110+
}
111+
112+
private function hasApiTag(Node $node): bool
113+
{
114+
$docComment = $node->getDocComment();
115+
if ($docComment === null) {
116+
return false;
117+
}
118+
119+
return preg_match('~@api(?![\w-])~', $docComment->getText()) === 1;
120+
}
121+
122+
/**
123+
* PHP requires an overriding method to keep these parameters optional.
124+
*
125+
* @return array<int, true>
126+
*/
127+
private function getPositionsOptionalInAncestors(ClassReflection $classReflection, string $methodName): array
128+
{
129+
$positions = [];
130+
foreach ($classReflection->getAncestors() as $ancestor) {
131+
if ($ancestor->getName() === $classReflection->getName() || $ancestor->isTrait() || !$ancestor->hasNativeMethod($methodName)) {
132+
continue;
133+
}
134+
135+
foreach ($ancestor->getNativeMethod($methodName)->getVariants() as $variant) {
136+
foreach ($variant->getParameters() as $position => $parameter) {
137+
if (!$parameter->isOptional()) {
138+
continue;
139+
}
140+
141+
$positions[$position] = true;
142+
}
143+
}
144+
}
145+
146+
return $positions;
147+
}
148+
149+
}

0 commit comments

Comments
 (0)