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

Ignore throws in function mocks #245

Merged
merged 5 commits into from
Jun 28, 2024

Conversation

unfulvio
Copy link
Collaborator

@unfulvio unfulvio commented Jun 24, 2024

Summary

I have noticed that in some circumstances the mocks contained in API/function-mocks.php could trigger false positives in static analyzers like PhpStan or just the IDE if using WP_Mock within a project. I think by removing the @throws solves this, although PhpStan will complain about it in this project. I suppose since these are just internal mocks we can safely ignore.

I did try using @noinspection tags but PhpStorm wasn't happy (whether specific to unhandled exceptions or not).

As for the strict type notations, same reason, externally it will reflect WP behavior (the alternative was to leave non-strict and rely only on phpdoc -- like WP does -- but then PhpStan would complain again... could swap that for a phpstan-ignore again, up to you)

Closes #248

Contributor checklist

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly
  • I have added tests to cover changes introduced by this pull request
  • All new and existing tests pass

Testing

Reviewer checklist

  • Code changes review
  • Documentation changes review
  • Unit tests pass
  • Static analysis passes

@coveralls
Copy link

Coverage Status

coverage: 66.445%. remained the same
when pulling 10dcee6 on ignore-throws-in-function-mocks
into 48b7f22 on trunk.

@coveralls
Copy link

Coverage Status

coverage: 66.445%. remained the same
when pulling 93465a8 on ignore-throws-in-function-mocks
into 48b7f22 on trunk.

@unfulvio unfulvio self-assigned this Jun 24, 2024
@coveralls
Copy link

Coverage Status

coverage: 66.312% (-0.1%) from 66.445%
when pulling d96aa5f on ignore-throws-in-function-mocks
into 48b7f22 on trunk.

Copy link
Contributor

@agibson-godaddy agibson-godaddy left a comment

Choose a reason for hiding this comment

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

Thanks for this! I too had noticed this and it was irksome. I appreciate a fix!

Just want to test it out quickly before merging, but it looks good.

Copy link
Contributor

@agibson-godaddy agibson-godaddy left a comment

Choose a reason for hiding this comment

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

Confirmed this fixes the issue!

@agibson-godaddy agibson-godaddy merged commit 73814e2 into trunk Jun 28, 2024
7 of 8 checks passed
@agibson-godaddy agibson-godaddy deleted the ignore-throws-in-function-mocks branch June 28, 2024 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants