Skip to content

Commit abae22b

Browse files
VincentLangletphpstan-bot
authored andcommitted
Fix phpstan/phpstan#14398: Incorrect line number for method visibility errors
- Added optional $line parameter to MethodVisibilityComparisonHelper::compare() - Pass method name start line from OverridingMethodRule and ConsistentConstructorRule - This ensures visibility errors point to the function declaration line, not the first attribute line - New regression test in tests/PHPStan/Rules/Methods/data/bug-14398.php
1 parent ba4c9e4 commit abae22b

File tree

5 files changed

+48
-9
lines changed

5 files changed

+48
-9
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),
51+
$this->methodVisibilityComparisonHelper->compare($parentConstructor, $parentConstructor->getDeclaringClass(), $method, $node->getOriginalNode()->name->getStartLine()),
5252
);
5353
}
5454

src/Rules/Methods/MethodVisibilityComparisonHelper.php

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ final class MethodVisibilityComparisonHelper
1515
{
1616

1717
/** @return list<IdentifierRuleError> */
18-
public function compare(ExtendedMethodReflection $prototype, ClassReflection $prototypeDeclaringClass, PhpMethodFromParserNodeReflection $method): array
18+
public function compare(ExtendedMethodReflection $prototype, ClassReflection $prototypeDeclaringClass, PhpMethodFromParserNodeReflection $method, ?int $line = null): array
1919
{
2020
/** @var list<IdentifierRuleError> $messages */
2121
$messages = [];
2222

2323
if ($prototype->isPublic()) {
2424
if (!$method->isPublic()) {
25-
$messages[] = RuleErrorBuilder::message(sprintf(
25+
$errorBuilder = RuleErrorBuilder::message(sprintf(
2626
'%s method %s::%s() overriding public method %s::%s() should also be public.',
2727
$method->isPrivate() ? 'Private' : 'Protected',
2828
$method->getDeclaringClass()->getDisplayName(),
@@ -31,20 +31,26 @@ public function compare(ExtendedMethodReflection $prototype, ClassReflection $pr
3131
$prototype->getName(),
3232
))
3333
->nonIgnorable()
34-
->identifier('method.visibility')
35-
->build();
34+
->identifier('method.visibility');
35+
if ($line !== null) {
36+
$errorBuilder->line($line);
37+
}
38+
$messages[] = $errorBuilder->build();
3639
}
3740
} elseif ($method->isPrivate()) {
38-
$messages[] = RuleErrorBuilder::message(sprintf(
41+
$errorBuilder = RuleErrorBuilder::message(sprintf(
3942
'Private method %s::%s() overriding protected method %s::%s() should be protected or public.',
4043
$method->getDeclaringClass()->getDisplayName(),
4144
$method->getName(),
4245
$prototypeDeclaringClass->getDisplayName(true),
4346
$prototype->getName(),
4447
))
4548
->nonIgnorable()
46-
->identifier('method.visibility')
47-
->build();
49+
->identifier('method.visibility');
50+
if ($line !== null) {
51+
$errorBuilder->line($line);
52+
}
53+
$messages[] = $errorBuilder->build();
4854
}
4955

5056
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));
192+
$messages = array_merge($messages, $this->methodVisibilityComparisonHelper->compare($prototype, $prototypeDeclaringClass, $method, $node->getOriginalNode()->name->getStartLine()));
193193
}
194194

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

tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -853,4 +853,16 @@ public function testFixWithTabs(): void
853853
$this->fix(__DIR__ . '/data/fix-with-tabs.php', __DIR__ . '/data/fix-with-tabs.php.fixed');
854854
}
855855

856+
#[RequiresPhp('>= 8.0')]
857+
public function testBug14398(): void
858+
{
859+
$this->phpVersionId = PHP_VERSION_ID;
860+
$this->analyse([__DIR__ . '/data/bug-14398.php'], [
861+
[
862+
'Private method Bug14398\ChildPrivateOverridesProtected::calculate() overriding protected method Bug14398\BaseProtectedMethod::calculate() should be protected or public.',
863+
17,
864+
],
865+
]);
866+
}
867+
856868
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php // lint >= 8.0
2+
3+
namespace Bug14398;
4+
5+
class BaseProtectedMethod
6+
{
7+
protected function calculate(): int
8+
{
9+
return 42;
10+
}
11+
}
12+
13+
class ChildPrivateOverridesProtected extends BaseProtectedMethod
14+
{
15+
// PHPStan: Private method … overriding protected method … should be protected or public.
16+
#[Override]
17+
private function calculate(): int
18+
{
19+
return 99;
20+
}
21+
}

0 commit comments

Comments
 (0)