Expand Psalm coverage #1757
Expand Psalm coverage #1757
Conversation
| */ | ||
| public function withPassword($password = false) |
There was a problem hiding this comment.
Why is this change being made? Before, in PHPStorm, if you typed in $obj->withPassword you'd get a parameter hint that'd say withPassword(password). Now you get one that says withPassword(string)? In-so-far as parameter hints go this new one seems decidedly worse
There was a problem hiding this comment.
This change was because the arguments name in the interface is $string. I'll go ahead and update the interface to use $password instead.
| @@ -2382,6 +2382,9 @@ public function loadSPKAC($spkac) | |||
| $this->currentCert = false; | |||
| return false; | |||
| } | |||
| if (!is_array($spkac)) { | |||
| throw new \InvalidArgumentException('$spkac is not an array.'); | |||
| } | |||
There was a problem hiding this comment.
Right before this line there's this:
if (!isset($spkac) || $spkac === false) {
$this->currentCert = false;
return false;
}We should (1) reset $this->currentCert to false and (2) we should either throw exceptions for every error or return false for every error. For be it from me to preach the need for consistency since phpseclib uses a mix of camelCase and snake_case but we ought to have it when we can.
| @@ -3777,6 +3780,9 @@ private function formatSubjectPublicKey() | |||
|
|
|||
| $decoded = ASN1::decodeBER($publicKey); | |||
| $mapped = ASN1::asn1map($decoded[0], Maps\SubjectPublicKeyInfo::MAP); | |||
| if (!is_array($mapped)) { | |||
| throw new \InvalidArgumentException('$mapped is not an array.'); | |||
| } | |||
There was a problem hiding this comment.
Let's return false for this one as well. X509.php is getting a massive rewrite for 4.0 and I may replace a lot of the return false's with exceptions but, for the time being, let's stick with return false!
jack-worman
force-pushed
the
Expand_psalm_coverage
branch
from
55c3efe
to
ec9e203
Compare
|
I cherry-picked this from your branch into 3.0 and merged that into master: Thanks! |
Psalm coverage to everywhere except phpseclib/Crypt/, phpseclib/Math/ and tests/