-
-
Notifications
You must be signed in to change notification settings - Fork 355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TypeDeclaration] Handle fallback from param same type object on ReturnTypeFromReturnNewRector #5039
[TypeDeclaration] Handle fallback from param same type object on ReturnTypeFromReturnNewRector #5039
Changes from 9 commits
be8b2eb
7594001
2a154c6
e69bc6e
f93af9d
ed8860f
11c285c
402d4ab
3bb67a3
8fae378
a5c966e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector\Fixture; | ||
|
||
use Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector\Source\SomeResponse; | ||
|
||
final class FallbackFromParam | ||
{ | ||
public function action(SomeResponse $someResponse) | ||
{ | ||
if (rand(0, 1)) { | ||
return new SomeResponse(); | ||
} | ||
|
||
return $someResponse; | ||
} | ||
} | ||
|
||
?> | ||
----- | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector\Fixture; | ||
|
||
use Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector\Source\SomeResponse; | ||
|
||
final class FallbackFromParam | ||
{ | ||
public function action(SomeResponse $someResponse): \Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector\Source\SomeResponse | ||
{ | ||
if (rand(0, 1)) { | ||
return new SomeResponse(); | ||
} | ||
|
||
return $someResponse; | ||
} | ||
} | ||
|
||
?> |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,39 @@ | ||||||
<?php | ||||||
|
||||||
declare(strict_types=1); | ||||||
|
||||||
namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector\Fixture; | ||||||
|
||||||
final class FallbackFromParamSelf | ||||||
{ | ||||||
public function action(self $obj) | ||||||
{ | ||||||
if (rand(0, 1)) { | ||||||
return new FallbackFromParamSelf(); | ||||||
} | ||||||
|
||||||
return $obj; | ||||||
} | ||||||
} | ||||||
|
||||||
?> | ||||||
----- | ||||||
<?php | ||||||
|
||||||
declare(strict_types=1); | ||||||
|
||||||
namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector\Fixture; | ||||||
|
||||||
final class FallbackFromParamSelf | ||||||
{ | ||||||
public function action(self $obj): \Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector\Fixture\FallbackFromParamSelf | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need another test for a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. static already error on the first place :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. multiple return already skipped at rector-src/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromReturnNewRector.php Lines 185 to 186 in c956f86
|
||||||
{ | ||||||
if (rand(0, 1)) { | ||||||
return new FallbackFromParamSelf(); | ||||||
} | ||||||
|
||||||
return $obj; | ||||||
} | ||||||
} | ||||||
|
||||||
?> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -141,7 +141,9 @@ static function (string $rectorClass): bool { | |
$hasError = true; | ||
$this->symfonyStyle->title('We could not find there rules in configs'); | ||
|
||
$setRealpath = (string) realpath($setFile); | ||
/** @var string|false $setRealpath */ | ||
$setRealpath = realpath($setFile); | ||
$setRealpath = (string) $setRealpath; | ||
$relativeFilePath = Strings::after($setRealpath, getcwd() . '/'); | ||
Comment on lines
+144
to
147
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will be always $setRealpath = realpath($setFile);
$relativeFilePath = Strings::after($setRealpath, getcwd() . '/'); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure, I updated at : There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks 👏 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the php process might still not be able to read this file (e.g. because of filesystem persmissions) and return false? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @staabm that's what I thought, but since The better solution is probably use: Assert::string($setRealpath); so it will be detected early when path removed/renamed. |
||
|
||
$this->symfonyStyle->writeln(' * ' . $relativeFilePath); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should do this cheap chrck earlier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated 8fae378