Skip to content
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

Fix some issues of Psalm in level 6 #2327

Merged
merged 7 commits into from
Jun 21, 2021
Merged

Conversation

franmomu
Copy link
Contributor

Q A
Type improvement
BC Break no
Fixed issues

Summary

Right now we are using level 7 of psalm, I raised it locally to level 6 and this PR fixes some of the issues that I think they are easy to review and only in tests directory.

The first commits are not SA related, just some polishing.

For the remaning ones I'll open another PR.

$bucket->getFilesCollection()
->expects($this->any())
$filesCollection = $bucket->getFilesCollection();
assert($filesCollection instanceof Collection && $filesCollection instanceof MockObject);
Copy link
Contributor Author

@franmomu franmomu Jun 21, 2021

Choose a reason for hiding this comment

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

here, $bucket is a mock and getFilesCollection returns also a mock, not sure how that can be expressed other than this or we can just ignore it.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with this assert 👍

@@ -129,7 +129,10 @@ public function testClosureToPHP($input, $output)
return $return;
})($input);

$this->assertInstanceOf('DateTimeImmutable', $return);
// @phpstan-ignore-next-line
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to ignore assert below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because of the eval in the closure, phpstan says:

 ------ ------------------------------------------------------------------------------
  Line   tests/Doctrine/ODM/MongoDB/Tests/Types/DateImmutableTypeTest.php
 ------ ------------------------------------------------------------------------------
  131    Call to function assert() with false will always evaluate to false.
  131    Instanceof between null and DateTimeImmutable will always evaluate to false.
 ------ ------------------------------------------------------------------------------

I've been trying other ways, but none worked 😞

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, indeed the eval makes things worse... thanks for explaining!

@malarzm malarzm added the Task label Jun 21, 2021
@malarzm malarzm added this to the 2.3.0 milestone Jun 21, 2021
@malarzm malarzm merged commit d35b8d4 into doctrine:2.3.x Jun 21, 2021
@malarzm
Copy link
Member

malarzm commented Jun 21, 2021

Thanks @franmomu!

@franmomu franmomu deleted the issues_tests_sa branch June 22, 2021 09:29
@alcaeus alcaeus modified the milestones: 2.3.0-alpha1, 2.3.0 Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants