Skip to content

[TypeDeclaration] Handle nullable MockObject on TypedPropertyFromCreateMockAssignRector#6179

Merged
samsonasik merged 3 commits intomainfrom
nullable-mock-object
Jul 23, 2024
Merged

[TypeDeclaration] Handle nullable MockObject on TypedPropertyFromCreateMockAssignRector#6179
samsonasik merged 3 commits intomainfrom
nullable-mock-object

Conversation

@samsonasik
Copy link
Copy Markdown
Member

@samsonasik
Copy link
Copy Markdown
Member Author

All checks have passed 🎉 @TomasVotruba I am merging it ;)

@samsonasik samsonasik merged commit 1a58300 into main Jul 23, 2024
@samsonasik samsonasik deleted the nullable-mock-object branch July 23, 2024 21:50
Comment on lines +109 to +111
if (! $classReflection instanceof ClassReflection) {
$classReflection = $this->reflectionResolver->resolveClassReflection($node);
}
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 should not be needed.

Copy link
Copy Markdown
Member Author

@samsonasik samsonasik Jul 23, 2024

Choose a reason for hiding this comment

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

it initialized as null before loop, I don't want to get class reflection when class doesn't have property, so early instanceof is used for tweak, then early skip when it got null

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 rule should be simple as possible. Please remove the $classReflection = $this->reflectionResolver->resolveClassReflection($node); and use simple check as before.

Also, wait for review before next merges. I need to review those first

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.

sure, see #6181

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants