From b6f818a7c8ed98561e064c4bdcc7c6af789b774d Mon Sep 17 00:00:00 2001 From: Dorian Villet Date: Thu, 3 Oct 2019 00:30:43 +0200 Subject: [PATCH 1/6] Add a failing test for MakeInheritedMethodVisibilitySameAsParentRector : 'return new self();' works, 'return new static();' does not. --- .../Fixture/skip_static_ctor.php.inc | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/skip_static_ctor.php.inc b/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/skip_static_ctor.php.inc index 6977c1894eb4..af9c41b520bc 100644 --- a/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/skip_static_ctor.php.inc +++ b/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/skip_static_ctor.php.inc @@ -2,10 +2,11 @@ namespace Rector\CodingStyle\Tests\Rector\ClassMethod\MakeInheritedMethodVisibilitySameAsParentRector\Fixture; -class SkipStaticCtor extends ParentWithPublicConstructor +final class SkipSelfCtor extends ParentWithPublicConstructor { - protected function __construct() + private function __construct() { + // do something } public static function create() @@ -14,6 +15,27 @@ class SkipStaticCtor extends ParentWithPublicConstructor } } +abstract class SkipStaticCtor extends ParentWithPublicConstructor +{ + protected function __construct() + { + // do something basic + } + + public static function create() + { + return new static(); + } +} + +class InheritedSkipCtor extends SkipStaticCtor +{ + protected function __construct() + { + // do something more + } +} + class ParentWithPublicConstructor { public function __construct() From 3c4f4315f8eb57b3ae31e6474471af8a741c7858 Mon Sep 17 00:00:00 2001 From: Dorian Villet Date: Thu, 3 Oct 2019 00:41:11 +0200 Subject: [PATCH 2/6] Restructure the tests files. --- .../Fixture/ParentWithPublicConstructor.php | 12 ++++++++++ .../Fixture/skip_self_ctor.php.inc | 15 +++++++++++++ .../Fixture/skip_static_ctor.php.inc | 22 ++----------------- ...SkipParentConstructOverrideInPHP72Test.php | 1 + 4 files changed, 30 insertions(+), 20 deletions(-) create mode 100644 packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/ParentWithPublicConstructor.php create mode 100644 packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/skip_self_ctor.php.inc diff --git a/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/ParentWithPublicConstructor.php b/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/ParentWithPublicConstructor.php new file mode 100644 index 000000000000..e6013192c666 --- /dev/null +++ b/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/ParentWithPublicConstructor.php @@ -0,0 +1,12 @@ + Date: Thu, 3 Oct 2019 00:50:40 +0200 Subject: [PATCH 3/6] Fix the implementation for 'return new static();'. --- .../MakeInheritedMethodVisibilitySameAsParentRector.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/CodingStyle/src/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector.php b/packages/CodingStyle/src/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector.php index 5ec20206a59d..c18b60f05328 100644 --- a/packages/CodingStyle/src/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector.php +++ b/packages/CodingStyle/src/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector.php @@ -177,7 +177,7 @@ private function isConstructorWithStaticFactory(ClassMethod $classMethod, string continue; } - $isStaticSelfFactory = $this->isStaticSelfFactory($iteratedClassMethod); + $isStaticSelfFactory = $this->isStaticNamedConstructor($iteratedClassMethod); if ($isStaticSelfFactory === false) { continue; @@ -193,7 +193,7 @@ private function isConstructorWithStaticFactory(ClassMethod $classMethod, string * Looks for: * public static someMethod() { return new self(); } */ - private function isStaticSelfFactory(ClassMethod $classMethod): bool + private function isStaticNamedConstructor(ClassMethod $classMethod): bool { if (! $classMethod->isPublic()) { return false; @@ -212,7 +212,7 @@ private function isStaticSelfFactory(ClassMethod $classMethod): bool return false; } - return $this->isName($node->expr->class, 'self'); + return $this->isName($node->expr->class, 'self') || $this->isName($node->expr->class, 'static'); }); } } From 3d5d1fbb3dab2d919b23c6b39428df071bda6961 Mon Sep 17 00:00:00 2001 From: Dorian Villet Date: Thu, 3 Oct 2019 00:52:09 +0200 Subject: [PATCH 4/6] Update a DocBlock. --- .../MakeInheritedMethodVisibilitySameAsParentRector.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/CodingStyle/src/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector.php b/packages/CodingStyle/src/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector.php index c18b60f05328..69e03ddb69d3 100644 --- a/packages/CodingStyle/src/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector.php +++ b/packages/CodingStyle/src/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector.php @@ -192,6 +192,8 @@ private function isConstructorWithStaticFactory(ClassMethod $classMethod, string /** * Looks for: * public static someMethod() { return new self(); } + * or + * public static someMethod() { return new static(); } */ private function isStaticNamedConstructor(ClassMethod $classMethod): bool { From 8d8630473ebc75834afcb1056450d5bee490fa67 Mon Sep 17 00:00:00 2001 From: Dorian Villet Date: Thu, 3 Oct 2019 23:47:54 +0200 Subject: [PATCH 5/6] Use early return for readability and consistency --- ...MakeInheritedMethodVisibilitySameAsParentRector.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/CodingStyle/src/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector.php b/packages/CodingStyle/src/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector.php index 69e03ddb69d3..6a6a776cf5cc 100644 --- a/packages/CodingStyle/src/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector.php +++ b/packages/CodingStyle/src/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector.php @@ -214,7 +214,15 @@ private function isStaticNamedConstructor(ClassMethod $classMethod): bool return false; } - return $this->isName($node->expr->class, 'self') || $this->isName($node->expr->class, 'static'); + if ($this->isName($node->expr->class, 'self')) { + return true; + } + + if ($this->isName($node->expr->class, 'static')) { + return true; + } + + return false; }); } } From 109d0b8ca9c30e6220b3ecb01e914ccd2a0eea7a Mon Sep 17 00:00:00 2001 From: Dorian Villet Date: Thu, 3 Oct 2019 23:50:53 +0200 Subject: [PATCH 6/6] Move ParentWithPublicConstructor from Fixture to Source. --- .../Fixture/skip_self_ctor.php.inc | 2 ++ .../Fixture/skip_static_ctor.php.inc | 2 ++ .../{Fixture => Source}/ParentWithPublicConstructor.php | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) rename packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/{Fixture => Source}/ParentWithPublicConstructor.php (79%) diff --git a/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/skip_self_ctor.php.inc b/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/skip_self_ctor.php.inc index 30faddd16d39..8940466459b4 100644 --- a/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/skip_self_ctor.php.inc +++ b/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/skip_self_ctor.php.inc @@ -2,6 +2,8 @@ namespace Rector\CodingStyle\Tests\Rector\ClassMethod\MakeInheritedMethodVisibilitySameAsParentRector\Fixture; +use Rector\CodingStyle\Tests\Rector\ClassMethod\MakeInheritedMethodVisibilitySameAsParentRector\Source\ParentWithPublicConstructor; + final class SkipSelfCtor extends ParentWithPublicConstructor { private function __construct() diff --git a/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/skip_static_ctor.php.inc b/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/skip_static_ctor.php.inc index 9ad0fd163ed1..ef8e95c96bd0 100644 --- a/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/skip_static_ctor.php.inc +++ b/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/skip_static_ctor.php.inc @@ -2,6 +2,8 @@ namespace Rector\CodingStyle\Tests\Rector\ClassMethod\MakeInheritedMethodVisibilitySameAsParentRector\Fixture; +use Rector\CodingStyle\Tests\Rector\ClassMethod\MakeInheritedMethodVisibilitySameAsParentRector\Source\ParentWithPublicConstructor; + abstract class SkipStaticCtor extends ParentWithPublicConstructor { protected function __construct() diff --git a/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/ParentWithPublicConstructor.php b/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Source/ParentWithPublicConstructor.php similarity index 79% rename from packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/ParentWithPublicConstructor.php rename to packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Source/ParentWithPublicConstructor.php index e6013192c666..49151e332b0d 100644 --- a/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Fixture/ParentWithPublicConstructor.php +++ b/packages/CodingStyle/tests/Rector/ClassMethod/MakeInheritedMethodVisibilitySameAsParentRector/Source/ParentWithPublicConstructor.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Rector\CodingStyle\Tests\Rector\ClassMethod\MakeInheritedMethodVisibilitySameAsParentRector\Fixture; +namespace Rector\CodingStyle\Tests\Rector\ClassMethod\MakeInheritedMethodVisibilitySameAsParentRector\Source; class ParentWithPublicConstructor {