Give password_get_info() the array shape it always returns - #6299
Open
SanderMuller wants to merge 1 commit into
Open
Give password_get_info() the array shape it always returns#6299SanderMuller wants to merge 1 commit into
SanderMuller wants to merge 1 commit into
Conversation
It returned a bare array, so $info['algo'] was mixed and a key typo went
unreported. The shape is fixed and documented: algo, algoName and options,
with algo changing from int to string|null in PHP 8.0.
Measured on the two targets, before and after:
phpVersion 70400 array -> array{algo: int, ...}
$info['algo'] mixed -> int
phpVersion 80300 array -> array{algo: string|null, ...}
$info['algo'] mixed -> string|null
The baseline entry carries the pre-8.0 signature and the 8.0 delta carries
the newer one, matching password_hash() directly above it, since the deltas
apply forward from the oldest map.
This removes a false negative rather than only sharpening a type. In real code
"if (!$info['algo'])" is a condition on mixed, which phpstan-strict-rules
passes through its implicit-mixed branch and never reports. With the shape it
is a condition on string|null and gets reported, which is what the rule is for.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
password_get_info()returns a barearray, so$info['algo']ismixedand a mistyped key is never reported. The shape is fixed and documented, andalgochanged frominttostring|nullin PHP 8.0.Measured on both targets, before and after:
array,$info['algo']ismixedarray{algo: int, algoName: string, options: array<string, mixed>},$info['algo']isintarray,$info['algo']ismixedarray{algo: string|null, algoName: string, options: array<string, mixed>},$info['algo']isstring|nullThe keys match the
#[ArrayShape]onpassword_get_info()in the vendored phpstorm-stubs, which annotatesalgoasintand has not been updated for the 8.0 change. Thestring|nullhalf is from running PHP 8: a bcrypt hash gives'2y'and plain text givesnull.The baseline map carries the pre-8.0 signature and the 8.0 delta carries the newer one, matching
password_hash()on the line below it, since the deltas apply forward from the oldest map.This also removes a false negative rather than only sharpening a type.
if (!$info['algo'])is currently a condition onmixed, which phpstan-strict-rules passes through its implicit-mixed branch and never reports. With the shape it is a condition onstring|null, which is what that rule exists to catch. I found it that way round: an analyser without this gap reported the condition and PHPStan did not.