-
-
Notifications
You must be signed in to change notification settings - Fork 359
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
Conversation
implemented 🎉 |
@TomasVotruba the phpstan notice seems due to new
|
@TomasVotruba I created PR to symplify/phpstan-extensions to revert realpath: |
|
||
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 comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need another test for a static
and/or parent
typed param?
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.
static already error on the first place :)
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.
multiple return already skipped at
rector-src/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromReturnNewRector.php
Lines 185 to 186 in c956f86
$returnType = $this->returnTypeInferer->inferFunctionLike($node); | |
if ($returnType instanceof UnionType) { |
I updated to use |
All checks have passed 🎉 @TomasVotruba I think it is ready. |
packages/Config/RectorConfig.php
Outdated
// not directory | ||
&& ! is_dir($skipRule) | ||
// not file | ||
&& ! is_file($skipRule) | ||
// a Rector end | ||
&& str_ends_with($skipRule, 'Rector') |
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
All checks have passed 🎉 @TomasVotruba I am merging it to have faster feedback to test ;) |
/** @var string|false $setRealpath */ | ||
$setRealpath = realpath($setFile); | ||
$setRealpath = (string) $setRealpath; | ||
$relativeFilePath = Strings::after($setRealpath, getcwd() . '/'); |
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.
This will be always string
, as we use existing set paths:
$setRealpath = realpath($setFile);
$relativeFilePath = Strings::after($setRealpath, getcwd() . '/');
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.
sure, I updated at :
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.
Thanks 👏
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.
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 comment
The reason will be displayed to describe this comment to others. Learn more.
@staabm that's what I thought, but since tools/
directory is not shipped to rector/rector
, I guess it safe for user, the concern is probably when directory/set removed in the future and not detected.
The better solution is probably use:
Assert::string($setRealpath);
so it will be detected early when path removed/renamed.
Thanks 🙏 |
@staabm this is to handle fallback from param on return new object: