Consistency for empty reconciliations #7663
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
orklah
added
the
release:fix
kkmuffme
added a commit
to kkmuffme/psalm
that referenced
this pull request
Nov 18, 2023
revert vimeo#7663 including previous from_docblock Mixed assignments, as the tests required 2 suppressions and created an escape hatch via mixed on higher psalm error levels, where mixed isn't reported, thus hiding potentially fatal bugs. It's still possible to run the validation of docblock docs though: now with 1 @psalm-suppress (instead of 2) and a @var declaration that contains both possible types, to ensure later code won't escape any checks This is also a required preparation to fix some isset issues of vimeo#9759
kkmuffme
added a commit
to kkmuffme/psalm
that referenced
this pull request
Nov 18, 2023
revert vimeo#7663 including previous from_docblock Mixed assignments, as the tests required 2 suppressions and created an escape hatch via mixed on higher psalm error levels, where mixed isn't reported, thus hiding potentially fatal bugs. It's still possible to run the validation of docblock docs though: a @var declaration that contains both possible types, to ensure later code won't escape any checks (and no @psalm-suppress needed at all) This is also a required preparation to fix some isset issues of vimeo#9759
kkmuffme
added a commit
to kkmuffme/psalm
that referenced
this pull request
Nov 18, 2023
revert vimeo#7663 including previous from_docblock Mixed assignments, as the tests required 2 suppressions and created an escape hatch via mixed on higher psalm error levels, where mixed isn't reported, thus hiding potentially fatal bugs. It's still possible to run the validation of docblock docs though: a @var declaration that contains both possible types, to ensure later code won't escape any checks (and no @psalm-suppress needed at all) This is also a required preparation to fix some isset issues of vimeo#9759
kkmuffme
added a commit
to kkmuffme/psalm
that referenced
this pull request
Nov 21, 2023
revert vimeo#7663 including previous from_docblock Mixed assignments, as the tests required 2 suppressions and created an escape hatch via mixed on higher psalm error levels, where mixed isn't reported, thus hiding potentially fatal bugs. It's still possible to run the validation of docblock docs though: a @var declaration that contains both possible types, to ensure later code won't escape any checks (and no @psalm-suppress needed at all) This is also a required preparation to fix some isset issues of vimeo#9759
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When Psalm tries to reconcile an assertion with a type, there are cases where the result is empty. (meaning every possible known value has been excluded by the assertion).
There was inconsistencies between returning never, returning mixed and a conditional that checks the from_docblock to return mixed or never.
I didn't see the goal of checking the from_docblock, but when I removed it, I realized the goal is to allow checking an expected type from a docblock (There is a Validator test for that). I changed every case into the conditional for consistency