Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Rules/Methods/ConsistentConstructorRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE

return array_merge(
$this->methodParameterComparisonHelper->compare($parentConstructor, $parentConstructor->getDeclaringClass(), $method, $scope, true),
$this->methodVisibilityComparisonHelper->compare($parentConstructor, $parentConstructor->getDeclaringClass(), $method),
$this->methodVisibilityComparisonHelper->compare($parentConstructor, $parentConstructor->getDeclaringClass(), $method, $node->getOriginalNode()),
);
}

Expand Down
5 changes: 4 additions & 1 deletion src/Rules/Methods/MethodVisibilityComparisonHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace PHPStan\Rules\Methods;

use PhpParser\Node\Stmt\ClassMethod;
use PHPStan\DependencyInjection\AutowiredService;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Reflection\ExtendedMethodReflection;
Expand All @@ -15,7 +16,7 @@ final class MethodVisibilityComparisonHelper
{

/** @return list<IdentifierRuleError> */
public function compare(ExtendedMethodReflection $prototype, ClassReflection $prototypeDeclaringClass, PhpMethodFromParserNodeReflection $method): array
public function compare(ExtendedMethodReflection $prototype, ClassReflection $prototypeDeclaringClass, PhpMethodFromParserNodeReflection $method, ClassMethod $node): array
{
/** @var list<IdentifierRuleError> $messages */
$messages = [];
Expand All @@ -32,6 +33,7 @@ public function compare(ExtendedMethodReflection $prototype, ClassReflection $pr
))
->nonIgnorable()
->identifier('method.visibility')
->line($node->name->getStartLine())
->build();
}
} elseif ($method->isPrivate()) {
Expand All @@ -44,6 +46,7 @@ public function compare(ExtendedMethodReflection $prototype, ClassReflection $pr
))
->nonIgnorable()
->identifier('method.visibility')
->line($node->name->getStartLine())
->build();
}

Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Methods/OverridingMethodRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataE
}

if ($checkVisibility) {
$messages = array_merge($messages, $this->methodVisibilityComparisonHelper->compare($prototype, $prototypeDeclaringClass, $method));
$messages = array_merge($messages, $this->methodVisibilityComparisonHelper->compare($prototype, $prototypeDeclaringClass, $method, $node->getOriginalNode()));
}

$prototypeVariants = $prototype->getVariants();
Expand Down
12 changes: 12 additions & 0 deletions tests/PHPStan/Rules/Methods/ConsistentConstructorRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use PHPStan\Rules\Classes\ConsistentConstructorHelper;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use PHPUnit\Framework\Attributes\RequiresPhp;
use function sprintf;

/** @extends RuleTestCase<ConsistentConstructorRule> */
Expand Down Expand Up @@ -62,4 +63,15 @@ public function testBug12137(): void
]);
}

#[RequiresPhp('>= 8.0.0')]
public function testBug14398(): void
{
$this->analyse([__DIR__ . '/data/bug-14398-consistent-constructor.php'], [
[
'Protected method Bug14398ConsistentConstructor\ChildClass::__construct() overriding public method Bug14398ConsistentConstructor\ParentClass::__construct() should also be public.',
25,
],
]);
}

}
24 changes: 24 additions & 0 deletions tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -881,4 +881,28 @@ public function testBug14457(): void
]);
}

#[RequiresPhp('>= 8.0.0')]
public function testBug14398(): void
{
$this->phpVersionId = PHP_VERSION_ID;
$this->analyse([__DIR__ . '/data/bug-14398.php'], [
[
'Private method Bug14398\ChildClass::publicMethod() overriding public method Bug14398\ParentClass::publicMethod() should also be public.',
40,
],
[
'Protected method Bug14398\ChildClass::publicWithDocblock() overriding public method Bug14398\ParentClass::publicWithDocblock() should also be public.',
50,
],
[
'Private method Bug14398\ChildClass::protectedMethod() overriding protected method Bug14398\ParentClass::protectedMethod() should be protected or public.',
56,
],
[
'Private method Bug14398\ChildClass::withoutAttribute() overriding public method Bug14398\ParentClass::withoutAttribute() should also be public.',
60,
],
]);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php // lint >= 8.0

namespace Bug14398ConsistentConstructor;

#[\Attribute]
class Marker
{

}

/** @phpstan-consistent-constructor */
class ParentClass
{

public function __construct()
{
}

}

class ChildClass extends ParentClass
{

#[Marker]
protected function __construct()
{
}

}
64 changes: 64 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-14398.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php // lint >= 8.0

namespace Bug14398;

#[\Attribute]
class Marker
{

public function __construct(string $name = '')
{
}

}

class ParentClass
{

public function publicMethod(): void
{
}

public function publicWithDocblock(): void
{
}

protected function protectedMethod(): void
{
}

public function withoutAttribute(): void
{
}

}

class ChildClass extends ParentClass
{

#[\Override]
private function publicMethod(): void
{
}

/**
* Docblock above a multi-line attribute.
*/
#[Marker(
name: 'foo',
)]
protected function publicWithDocblock(): void
{
}

#[Marker]
#[\Override]
private function protectedMethod(): void
{
}

private function withoutAttribute(): void
{
}

}
Loading