new static() is safe only with final constructor or in final class (level 0)

ondrejmirtes · ondrejmirtes · commit 7f5582e79c2c · 2019-08-17T21:23:06.000+02:00
diff --git a/conf/config.level0.neon b/conf/config.level0.neon
@@ -19,6 +19,7 @@ rules:
 	- PHPStan\Rules\Classes\ExistingClassInClassExtendsRule
 	- PHPStan\Rules\Classes\ExistingClassInTraitUseRule
 	- PHPStan\Rules\Classes\InstantiationRule
+	- PHPStan\Rules\Classes\NewStaticRule
 	- PHPStan\Rules\Functions\CallToFunctionParametersRule
 	- PHPStan\Rules\Functions\ExistingClassesInArrowFunctionTypehintsRule
 	- PHPStan\Rules\Functions\ExistingClassesInClosureTypehintsRule
diff --git a/src/Rules/Classes/NewStaticRule.php b/src/Rules/Classes/NewStaticRule.php
@@ -0,0 +1,62 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Classes;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Reflection\Php\PhpMethodReflection;
+use PHPStan\Rules\Rule;
+
+class NewStaticRule implements Rule
+{
+
+	public function getNodeType(): string
+	{
+		return Node\Expr\New_::class;
+	}
+
+	/**
+	 * @param \PhpParser\Node\Expr\New_ $node
+	 * @param \PHPStan\Analyser\Scope $scope
+	 * @return string[]
+	 */
+	public function processNode(Node $node, Scope $scope): array
+	{
+		if (!$node->class instanceof Node\Name) {
+			return [];
+		}
+
+		if (!$scope->isInClass()) {
+			return [];
+		}
+
+		if (strtolower($node->class->toString()) !== 'static') {
+			return [];
+		}
+
+		$classReflection = $scope->getClassReflection();
+		if ($classReflection->isFinal()) {
+			return [];
+		}
+
+		$messages = [
+			"Unsafe usage of new static().\n💡 Consider making the class or the constructor final.",
+		];
+		if (!$classReflection->hasConstructor()) {
+			return $messages;
+		}
+
+		$constructor = $classReflection->getConstructor();
+		if ($constructor->getPrototype()->getDeclaringClass()->isInterface()) {
+			return [];
+		}
+
+		$isFinal = $constructor instanceof PhpMethodReflection && $constructor->isFinal()->yes();
+		if (!$isFinal) {
+			return $messages;
+		}
+
+		return [];
+	}
+
+}
diff --git a/src/Type/ArrayType.php b/src/Type/ArrayType.php
@@ -300,7 +300,7 @@ public function traverse(callable $cb): Type
 		$itemType = $cb($this->itemType);
 
 		if ($keyType !== $this->keyType || $itemType !== $this->itemType) {
-			return new static($keyType, $itemType);
+			return new self($keyType, $itemType);
 		}
 
 		return $this;
diff --git a/src/Type/BooleanType.php b/src/Type/BooleanType.php
@@ -92,7 +92,7 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType): Type
 	 */
 	public static function __set_state(array $properties): Type
 	{
-		return new static();
+		return new self();
 	}
 
 }
diff --git a/src/Type/CallableType.php b/src/Type/CallableType.php
@@ -263,7 +263,7 @@ public function traverse(callable $cb): Type
 			);
 		}, $this->getParameters());
 
-		return new static(
+		return new self(
 			$parameters,
 			$cb($this->getReturnType()),
 			$this->isVariadic()
diff --git a/src/Type/ClosureType.php b/src/Type/ClosureType.php
@@ -294,7 +294,7 @@ public function getReturnType(): Type
 
 	public function traverse(callable $cb): Type
 	{
-		return new static(
+		return new self(
 			array_map(static function (NativeParameterReflection $param) use ($cb): NativeParameterReflection {
 				$defaultValue = $param->getDefaultValue();
 				return new NativeParameterReflection(
diff --git a/src/Type/Generic/TemplateMixedType.php b/src/Type/Generic/TemplateMixedType.php
@@ -152,7 +152,7 @@ public function subtract(Type $type): Type
 			$type = TypeCombinator::union($this->getSubtractedType(), $type);
 		}
 
-		return new static(
+		return new self(
 			$this->scope,
 			$this->strategy,
 			$this->name,
diff --git a/src/Type/IterableType.php b/src/Type/IterableType.php
@@ -200,7 +200,7 @@ public function traverse(callable $cb): Type
 		$itemType = $cb($this->itemType);
 
 		if ($keyType !== $this->keyType || $itemType !== $this->itemType) {
-			return new static($keyType, $itemType);
+			return new self($keyType, $itemType);
 		}
 
 		return $this;
diff --git a/src/Type/MixedType.php b/src/Type/MixedType.php
@@ -257,7 +257,7 @@ public function subtract(Type $type): Type
 			$type = TypeCombinator::union($this->subtractedType, $type);
 		}
 
-		return new static($this->isExplicitMixed, $type);
+		return new self($this->isExplicitMixed, $type);
 	}
 
 	public function getTypeWithoutSubtractedType(): Type
diff --git a/src/Type/ObjectType.php b/src/Type/ObjectType.php
@@ -719,7 +719,7 @@ public function traverse(callable $cb): Type
 		$subtractedType = $this->subtractedType !== null ? $cb($this->subtractedType) : null;
 
 		if ($subtractedType !== $this->subtractedType) {
-			return new static(
+			return new self(
 				$this->className,
 				$subtractedType
 			);
diff --git a/src/Type/ObjectWithoutClassType.php b/src/Type/ObjectWithoutClassType.php
@@ -146,7 +146,7 @@ public function traverse(callable $cb): Type
 		$subtractedType = $this->subtractedType !== null ? $cb($this->subtractedType) : null;
 
 		if ($subtractedType !== $this->subtractedType) {
-			return new static($subtractedType);
+			return new self($subtractedType);
 		}
 
 		return $this;
diff --git a/src/Type/StaticType.php b/src/Type/StaticType.php
@@ -22,7 +22,7 @@ class StaticType implements TypeWithClassName
 	/** @var \PHPStan\Type\ObjectType */
 	private $staticObjectType;
 
-	public function __construct(string $baseClass)
+	final public function __construct(string $baseClass)
 	{
 		$this->baseClass = $baseClass;
 		$this->staticObjectType = new ObjectType($baseClass);
diff --git a/tests/PHPStan/Rules/Classes/NewStaticRuleTest.php b/tests/PHPStan/Rules/Classes/NewStaticRuleTest.php
@@ -0,0 +1,30 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Classes;
+
+use PHPStan\Rules\Rule;
+
+class NewStaticRuleTest extends \PHPStan\Testing\RuleTestCase
+{
+
+	protected function getRule(): Rule
+	{
+		return new NewStaticRule();
+	}
+
+	public function testRule(): void
+	{
+		$error = "Unsafe usage of new static().\n💡 Consider making the class or the constructor final.";
+		$this->analyse([__DIR__ . '/data/new-static.php'], [
+			[
+				$error,
+				10,
+			],
+			[
+				$error,
+				25,
+			],
+		]);
+	}
+
+}
diff --git a/tests/PHPStan/Rules/Classes/data/new-static.php b/tests/PHPStan/Rules/Classes/data/new-static.php
@@ -0,0 +1,90 @@
+<?php
+
+namespace NewStatic;
+
+class NoConstructor
+{
+
+	public function doFoo()
+	{
+		$foo = new static();
+	}
+
+}
+
+class NonFinalConstructor
+{
+
+	public function __construct()
+	{
+
+	}
+
+	public function doFoo()
+	{
+		$foo = new static();
+	}
+
+}
+
+final class NoConstructorInFinalClass
+{
+
+	public function doFoo()
+	{
+		$foo = new static();
+	}
+
+}
+
+final class NonFinalConstructorInFinalClass
+{
+
+	public function __construct()
+	{
+
+	}
+
+	public function doFoo()
+	{
+		$foo = new static();
+	}
+
+}
+
+class FinalConstructorInNonFinalClass
+{
+
+	final public function __construct()
+	{
+
+	}
+
+	public function doFoo()
+	{
+		$foo = new static();
+	}
+
+}
+
+interface InterfaceWithConstructor
+{
+
+	public function __construct(int $i);
+
+}
+
+class ConstructorComingFromAnInterface implements InterfaceWithConstructor
+{
+
+	public function __construct(int $i)
+	{
+
+	}
+
+	public function doFoo()
+	{
+		$foo = new static(1);
+	}
+
+}

Original file line number	Diff line number	Diff line change
`@@ -300,7 +300,7 @@ public function traverse(callable $cb): Type`
`300`	`300`	`$itemType = $cb($this->itemType);`
`301`	`301`
`302`	`302`	`if ($keyType !== $this->keyType \|\| $itemType !== $this->itemType) {`
`303`		`- return new static($keyType, $itemType);`
	`303`	`+ return new self($keyType, $itemType);`
`304`	`304`	`}`
`305`	`305`
`306`	`306`	`return $this;`
Original file line number	Diff line number	Diff line change
`@@ -92,7 +92,7 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType): Type`
`92`	`92`	`*/`
`93`	`93`	`public static function __set_state(array $properties): Type`
`94`	`94`	`{`
`95`		`- return new static();`
	`95`	`+ return new self();`
`96`	`96`	`}`
`97`	`97`
`98`	`98`	`}`
Original file line number	Diff line number	Diff line change
`@@ -294,7 +294,7 @@ public function getReturnType(): Type`
`294`	`294`
`295`	`295`	`public function traverse(callable $cb): Type`
`296`	`296`	`{`
`297`		`- return new static(`
	`297`	`+ return new self(`
`298`	`298`	`array_map(static function (NativeParameterReflection $param) use ($cb): NativeParameterReflection {`
`299`	`299`	`$defaultValue = $param->getDefaultValue();`
`300`	`300`	`return new NativeParameterReflection(`
Original file line number	Diff line number	Diff line change
`@@ -152,7 +152,7 @@ public function subtract(Type $type): Type`
`152`	`152`	`$type = TypeCombinator::union($this->getSubtractedType(), $type);`
`153`	`153`	`}`
`154`	`154`
`155`		`- return new static(`
	`155`	`+ return new self(`
`156`	`156`	`$this->scope,`
`157`	`157`	`$this->strategy,`
`158`	`158`	`$this->name,`
Original file line number	Diff line number	Diff line change
`@@ -200,7 +200,7 @@ public function traverse(callable $cb): Type`
`200`	`200`	`$itemType = $cb($this->itemType);`
`201`	`201`
`202`	`202`	`if ($keyType !== $this->keyType \|\| $itemType !== $this->itemType) {`
`203`		`- return new static($keyType, $itemType);`
	`203`	`+ return new self($keyType, $itemType);`
`204`	`204`	`}`
`205`	`205`
`206`	`206`	`return $this;`
Original file line number	Diff line number	Diff line change
`@@ -257,7 +257,7 @@ public function subtract(Type $type): Type`
`257`	`257`	`$type = TypeCombinator::union($this->subtractedType, $type);`
`258`	`258`	`}`
`259`	`259`
`260`		`- return new static($this->isExplicitMixed, $type);`
	`260`	`+ return new self($this->isExplicitMixed, $type);`
`261`	`261`	`}`
`262`	`262`
`263`	`263`	`public function getTypeWithoutSubtractedType(): Type`