Skip to content

[TypeDeclaration] Skip ReturnTypeDeclarationRector when class parent extends is in 'vendor'#5308

Merged
TomasVotruba merged 14 commits intomasterfrom
close-5210
Jan 26, 2021
Merged

[TypeDeclaration] Skip ReturnTypeDeclarationRector when class parent extends is in 'vendor'#5308
TomasVotruba merged 14 commits intomasterfrom
close-5210

Conversation

@samsonasik
Copy link
Copy Markdown
Member

@samsonasik samsonasik commented Jan 25, 2021

Closes #5210 Fixes #5205

@samsonasik samsonasik mentioned this pull request Jan 25, 2021
@samsonasik samsonasik force-pushed the close-5210 branch 2 times, most recently from 7cead9b to bc1d6d6 Compare January 25, 2021 14:44
@samsonasik samsonasik changed the title [TypeDeclaration] Skip ReturnTypeDeclarationRector when parent extends is in 'vendor' [TypeDeclaration] Skip ReturnTypeDeclarationRector when class parent extends is in 'vendor' Jan 25, 2021
@samsonasik
Copy link
Copy Markdown
Member Author

All checks have passed 🎉 @TomasVotruba it is ready for review.

Comment on lines +259 to +278
private function isParentInVendor(ClassMethod $classMethod): bool
{
$parent = $classMethod->getAttribute(AttributeKey::PARENT_NODE);
if (! $parent instanceof Class_) {
return false;
}

if (! $parent->extends instanceof FullyQualified) {
return false;
}

$parentName = $parent->extends->toString();
$reflectionClass = new ReflectionClass($parentName);
if ($reflectionClass->isInternal()) {
return true;
}

$fileName = $reflectionClass->getFileName();
return Strings::contains((string) $fileName, 'vendor');
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic should be extracted to packages/vendor-locker

There might be some useful class like Rector\VendorLocker\NodeVendorLocker\ClassMethodVendorLockResolver

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implemented with use VendorLockResolver->isReturnChangeVendorLockedIn()

Comment thread phpstan.neon Outdated
Comment on lines +581 to +583
-
message: '#Class cognitive complexity is 32, keep it under 30#'
path: rules/type-declaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crossing cognitive complexity treshold should not be ignored.
Instead, should be solved by a new service extraction.

It works well with duplicated method PHPStan rule. That way we keep small consistent services with specific work and without duplication.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

@samsonasik
Copy link
Copy Markdown
Member Author

All checks have passed 🎉 @TomasVotruba it is ready for review.

@TomasVotruba
Copy link
Copy Markdown
Member

Thank you 👍

@TomasVotruba TomasVotruba merged commit 6ff43c3 into master Jan 26, 2021
@TomasVotruba TomasVotruba deleted the close-5210 branch January 26, 2021 09:04
@TomasVotruba
Copy link
Copy Markdown
Member

@samsonasik There is one pull-request that is related to this issue.

Could you verify it was fixed or handle it?
#5209

@samsonasik
Copy link
Copy Markdown
Member Author

I will try.

TomasVotruba added a commit that referenced this pull request Dec 2, 2023
rectorphp/rector-src@5c8fa12 [Doc] Typo fix on sample code documentation on SimplifyForeachToCoalescingRector (#5308)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 8, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect behavior of ReturnTypeDeclarationRector

3 participants