Overriding method rule - check final

ondrejmirtes · ondrejmirtes · commit 9d87da817721 · 2020-06-04T23:07:58.000+02:00
diff --git a/conf/config.level0.neon b/conf/config.level0.neon
@@ -29,6 +29,7 @@ rules:
 	- PHPStan\Rules\Functions\InnerFunctionRule
 	- PHPStan\Rules\Functions\PrintfParametersRule
 	- PHPStan\Rules\Methods\ExistingClassesInTypehintsRule
+	- PHPStan\Rules\Methods\OverridingMethodRule
 	- PHPStan\Rules\Properties\AccessPropertiesInAssignRule
 	- PHPStan\Rules\Properties\AccessStaticPropertiesInAssignRule
 	- PHPStan\Rules\Variables\ThisVariableRule
diff --git a/src/Reflection/MethodPrototypeReflection.php b/src/Reflection/MethodPrototypeReflection.php
@@ -7,6 +7,8 @@ class MethodPrototypeReflection implements ClassMemberReflection
 
 	private \PHPStan\Reflection\ClassReflection $declaringClass;
 
+	private string $name;
+
 	private bool $isStatic;
 
 	private bool $isPrivate;
@@ -15,19 +17,30 @@ class MethodPrototypeReflection implements ClassMemberReflection
 
 	private bool $isAbstract;
 
+	private bool $isFinal;
+
 	public function __construct(
+		string $name,
 		ClassReflection $declaringClass,
 		bool $isStatic,
 		bool $isPrivate,
 		bool $isPublic,
-		bool $isAbstract
+		bool $isAbstract,
+		bool $isFinal
 	)
 	{
+		$this->name = $name;
 		$this->declaringClass = $declaringClass;
 		$this->isStatic = $isStatic;
 		$this->isPrivate = $isPrivate;
 		$this->isPublic = $isPublic;
 		$this->isAbstract = $isAbstract;
+		$this->isFinal = $isFinal;
+	}
+
+	public function getName(): string
+	{
+		return $this->name;
 	}
 
 	public function getDeclaringClass(): ClassReflection
@@ -55,6 +68,11 @@ public function isAbstract(): bool
 		return $this->isAbstract;
 	}
 
+	public function isFinal(): bool
+	{
+		return $this->isFinal;
+	}
+
 	public function getDocComment(): ?string
 	{
 		return null;
diff --git a/src/Reflection/Native/NativeMethodReflection.php b/src/Reflection/Native/NativeMethodReflection.php
@@ -84,11 +84,13 @@ public function getPrototype(): ClassMemberReflection
 			$prototypeDeclaringClass = $this->reflectionProvider->getClass($prototypeMethod->getDeclaringClass()->getName());
 
 			return new MethodPrototypeReflection(
+				$prototypeMethod->getName(),
 				$prototypeDeclaringClass,
 				$prototypeMethod->isStatic(),
 				$prototypeMethod->isPrivate(),
 				$prototypeMethod->isPublic(),
-				$prototypeMethod->isAbstract()
+				$prototypeMethod->isAbstract(),
+				$prototypeMethod->isFinal()
 			);
 		} catch (\ReflectionException $e) {
 			return $this;
@@ -125,7 +127,7 @@ public function isInternal(): TrinaryLogic
 
 	public function isFinal(): TrinaryLogic
 	{
-		return TrinaryLogic::createNo();
+		return TrinaryLogic::createFromBoolean($this->reflection->isFinal());
 	}
 
 	public function getThrowType(): ?Type
diff --git a/src/Reflection/Php/PhpFunctionFromParserNodeReflection.php b/src/Reflection/Php/PhpFunctionFromParserNodeReflection.php
@@ -210,7 +210,11 @@ public function isInternal(): TrinaryLogic
 
 	public function isFinal(): TrinaryLogic
 	{
-		return TrinaryLogic::createFromBoolean($this->isFinal);
+		$finalMethod = false;
+		if ($this->functionLike instanceof ClassMethod) {
+			$finalMethod = $this->functionLike->isFinal();
+		}
+		return TrinaryLogic::createFromBoolean($finalMethod || $this->isFinal);
 	}
 
 	public function getThrowType(): ?Type
diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php
@@ -158,11 +158,13 @@ public function getPrototype(): ClassMemberReflection
 			$prototypeDeclaringClass = $this->reflectionProvider->getClass($prototypeMethod->getDeclaringClass()->getName());
 
 			return new MethodPrototypeReflection(
+				$prototypeMethod->getName(),
 				$prototypeDeclaringClass,
 				$prototypeMethod->isStatic(),
 				$prototypeMethod->isPrivate(),
 				$prototypeMethod->isPublic(),
-				$prototypeMethod->isAbstract()
+				$prototypeMethod->isAbstract(),
+				$prototypeMethod->isFinal()
 			);
 		} catch (\ReflectionException $e) {
 			return $this;
diff --git a/src/Rules/Methods/OverridingMethodRule.php b/src/Rules/Methods/OverridingMethodRule.php
@@ -0,0 +1,52 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Methods;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Node\InClassMethodNode;
+use PHPStan\Reflection\MethodPrototypeReflection;
+use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection;
+use PHPStan\Rules\Rule;
+
+/**
+ * @implements Rule<InClassMethodNode>
+ */
+class OverridingMethodRule implements Rule
+{
+
+	public function getNodeType(): string
+	{
+		return InClassMethodNode::class;
+	}
+
+	public function processNode(Node $node, Scope $scope): array
+	{
+		$method = $scope->getFunction();
+		if (!$method instanceof PhpMethodFromParserNodeReflection) {
+			throw new \PHPStan\ShouldNotHappenException();
+		}
+
+		$prototype = $method->getPrototype();
+		if ($prototype->getDeclaringClass()->getName() === $method->getDeclaringClass()->getName()) {
+			return [];
+		}
+		if (!$prototype instanceof MethodPrototypeReflection) {
+			return [];
+		}
+
+		$messages = [];
+		if ($prototype->isFinal()) {
+			$messages[] = sprintf(
+				'Method %s::%s() overrides final method %s::%s().',
+				$method->getDeclaringClass()->getName(),
+				$method->getName(),
+				$prototype->getDeclaringClass()->getName(),
+				$prototype->getName()
+			);
+		}
+
+		return $messages;
+	}
+
+}
diff --git a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php
@@ -0,0 +1,33 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Methods;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+
+/**
+ * @extends RuleTestCase<OverridingMethodRule>
+ */
+class OverridingMethodRuleTest extends RuleTestCase
+{
+
+	protected function getRule(): Rule
+	{
+		return new OverridingMethodRule();
+	}
+
+	public function testOverridingFinalMethod(): void
+	{
+		if (!self::$useStaticReflectionProvider) {
+			$this->markTestSkipped('Test requires static reflection.');
+		}
+
+		$this->analyse([__DIR__ . '/data/overriding-final-method.php'], [
+			[
+				'Method OverridingFinalMethod\Bar::doFoo() overrides final method OverridingFinalMethod\Foo::doFoo().',
+				18,
+			],
+		]);
+	}
+
+}
diff --git a/tests/PHPStan/Rules/Methods/data/overriding-final-method.php b/tests/PHPStan/Rules/Methods/data/overriding-final-method.php
@@ -0,0 +1,23 @@
+<?php
+
+namespace OverridingFinalMethod;
+
+class Foo
+{
+
+	final public function doFoo()
+	{
+
+	}
+
+}
+
+class Bar extends Foo
+{
+
+	public function doFoo()
+	{
+
+	}
+
+}

Original file line number	Diff line number	Diff line change
`@@ -210,7 +210,11 @@ public function isInternal(): TrinaryLogic`
`210`	`210`
`211`	`211`	`public function isFinal(): TrinaryLogic`
`212`	`212`	`{`
`213`		`- return TrinaryLogic::createFromBoolean($this->isFinal);`
	`213`	`+ $finalMethod = false;`
	`214`	`+ if ($this->functionLike instanceof ClassMethod) {`
	`215`	`+ $finalMethod = $this->functionLike->isFinal();`
	`216`	`+ }`
	`217`	`+ return TrinaryLogic::createFromBoolean($finalMethod \|\| $this->isFinal);`
`214`	`218`	`}`
`215`	`219`
`216`	`220`	`public function getThrowType(): ?Type`