Skip to content

Commit ce81771

Browse files
phpstan-botclaude
andcommitted
Change MethodVisibilityComparisonHelper::compare() to accept Node instead of optional line number
Address review feedback: replace optional `?int $line = null` parameter with a required `Node $node` parameter, calling `$node->getStartLine()` inside the helper. This simplifies callers and makes the API more explicit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent abae22b commit ce81771

File tree

3 files changed

+12
-15
lines changed

3 files changed

+12
-15
lines changed

src/Rules/Methods/ConsistentConstructorRule.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public function processNode(Node $node, Scope $scope): array
4848

4949
return array_merge(
5050
$this->methodParameterComparisonHelper->compare($parentConstructor, $parentConstructor->getDeclaringClass(), $method, true),
51-
$this->methodVisibilityComparisonHelper->compare($parentConstructor, $parentConstructor->getDeclaringClass(), $method, $node->getOriginalNode()->name->getStartLine()),
51+
$this->methodVisibilityComparisonHelper->compare($parentConstructor, $parentConstructor->getDeclaringClass(), $method, $node->getOriginalNode()->name),
5252
);
5353
}
5454

src/Rules/Methods/MethodVisibilityComparisonHelper.php

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PHPStan\Rules\Methods;
44

5+
use PhpParser\Node;
56
use PHPStan\DependencyInjection\AutowiredService;
67
use PHPStan\Reflection\ClassReflection;
78
use PHPStan\Reflection\ExtendedMethodReflection;
@@ -15,14 +16,14 @@ final class MethodVisibilityComparisonHelper
1516
{
1617

1718
/** @return list<IdentifierRuleError> */
18-
public function compare(ExtendedMethodReflection $prototype, ClassReflection $prototypeDeclaringClass, PhpMethodFromParserNodeReflection $method, ?int $line = null): array
19+
public function compare(ExtendedMethodReflection $prototype, ClassReflection $prototypeDeclaringClass, PhpMethodFromParserNodeReflection $method, Node $node): array
1920
{
2021
/** @var list<IdentifierRuleError> $messages */
2122
$messages = [];
2223

2324
if ($prototype->isPublic()) {
2425
if (!$method->isPublic()) {
25-
$errorBuilder = RuleErrorBuilder::message(sprintf(
26+
$messages[] = RuleErrorBuilder::message(sprintf(
2627
'%s method %s::%s() overriding public method %s::%s() should also be public.',
2728
$method->isPrivate() ? 'Private' : 'Protected',
2829
$method->getDeclaringClass()->getDisplayName(),
@@ -31,26 +32,22 @@ public function compare(ExtendedMethodReflection $prototype, ClassReflection $pr
3132
$prototype->getName(),
3233
))
3334
->nonIgnorable()
34-
->identifier('method.visibility');
35-
if ($line !== null) {
36-
$errorBuilder->line($line);
37-
}
38-
$messages[] = $errorBuilder->build();
35+
->identifier('method.visibility')
36+
->line($node->getStartLine())
37+
->build();
3938
}
4039
} elseif ($method->isPrivate()) {
41-
$errorBuilder = RuleErrorBuilder::message(sprintf(
40+
$messages[] = RuleErrorBuilder::message(sprintf(
4241
'Private method %s::%s() overriding protected method %s::%s() should be protected or public.',
4342
$method->getDeclaringClass()->getDisplayName(),
4443
$method->getName(),
4544
$prototypeDeclaringClass->getDisplayName(true),
4645
$prototype->getName(),
4746
))
4847
->nonIgnorable()
49-
->identifier('method.visibility');
50-
if ($line !== null) {
51-
$errorBuilder->line($line);
52-
}
53-
$messages[] = $errorBuilder->build();
48+
->identifier('method.visibility')
49+
->line($node->getStartLine())
50+
->build();
5451
}
5552

5653
return $messages;

src/Rules/Methods/OverridingMethodRule.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array
189189
}
190190

191191
if ($checkVisibility) {
192-
$messages = array_merge($messages, $this->methodVisibilityComparisonHelper->compare($prototype, $prototypeDeclaringClass, $method, $node->getOriginalNode()->name->getStartLine()));
192+
$messages = array_merge($messages, $this->methodVisibilityComparisonHelper->compare($prototype, $prototypeDeclaringClass, $method, $node->getOriginalNode()->name));
193193
}
194194

195195
$prototypeVariants = $prototype->getVariants();

0 commit comments

Comments
 (0)