Increase PHPStan rule level to 7 #733
Conversation
theimbender
force-pushed
the
feat-phpstan-level-7
branch
3 times, most recently
from
5a84df7 to
d758edc
Compare
src/Builder/ArchiveBuilder.php
Outdated
| $hasedPath = $includeArchiveChecksum ? hash_file('sha1', $path) : false; | ||
| $package->setDistSha1Checksum(false !== $hasedPath ? $hasedPath : null); |
There was a problem hiding this comment.
hash_file will never return a falsy string ('' or '0')
| $hasedPath = $includeArchiveChecksum ? hash_file('sha1', $path) : false; | |
| $package->setDistSha1Checksum(false !== $hasedPath ? $hasedPath : null); | |
| $package->setDistSha1Checksum($includeArchiveChecksum ? (hash_file('sha1', $path) ?: null) : null); |
There was a problem hiding this comment.
The return type for hash_file is string|false and this rule level disallows short ternary operators, I'll tweak this a little though.
| $baseUrlLength = strlen($baseUrl); | ||
|
|
||
| foreach ($rootConfig['providers'] as $package => $provider) { | ||
| $file = str_replace(['%package%', '%hash%'], [$package, $provider['sha256']], $rootConfig['providers-url']); | ||
| $file = (string) str_replace(['%package%', '%hash%'], [$package, $provider['sha256']], $rootConfig['providers-url']); |
There was a problem hiding this comment.
If you enforce the type of the input, the type of str_replace output should be correct.
| $file = (string) str_replace(['%package%', '%hash%'], [$package, $provider['sha256']], $rootConfig['providers-url']); | |
| $file = str_replace(['%package%', '%hash%'], [$package, $provider['sha256']], (string) $rootConfig['providers-url']); |
| if ($repository instanceof RepositoryInterface) { | ||
| $repositorySet->addRepository($repository); | ||
| } |
There was a problem hiding this comment.
This looks like an actual bugfix. But I don't think this is possible.
| @@ -577,15 +584,17 @@ private function matchAddr($addr1, $addr2, $len = 0, $chunklen = 32): bool | |||
| } | |||
|
|
|||
| /** | |||
| * @param RepositoryInterface[] $repositories | |||
| * @param array<RepositoryInterface|ConfigurableRepositoryInterface> $repositories | |||
There was a problem hiding this comment.
It should be a union type. I don't see any class that implement ConfigurableRepositoryInterface without RepositoryInterface.
| * @param array<RepositoryInterface|ConfigurableRepositoryInterface> $repositories | |
| * @param array<RepositoryInterface&ConfigurableRepositoryInterface> $repositories |
There was a problem hiding this comment.
I don't know if we want to change this from union to intersection types, when I do that there are several branches that would always evaluate to true or become unreachable, and some methods that use one of those for a parameter type start throwing errors as well.
There was a problem hiding this comment.
Ok then, feel free to mark as resolved.
| } | ||
| ); | ||
|
|
||
| return array_values($links); | ||
| } | ||
|
|
||
| /** | ||
| * @param RepositoryInterface[] $repositories | ||
| * @param array<RepositoryInterface|ConfigurableRepositoryInterface> $repositories |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
theimbender
force-pushed
the
feat-phpstan-level-7
branch
from
d758edc to
b826bd8
Compare
| $hasedPath = hash_file('sha1', $path); | ||
| $package->setDistSha1Checksum($includeArchiveChecksum ? (is_string($hasedPath) ? $hasedPath : null) : null); |
There was a problem hiding this comment.
Nitpicking, but I think "hased" is a typo?
Co-authored-by: Jérôme Tamarelle <[email protected]>
alcohol
force-pushed
the
feat-phpstan-level-7
branch
from
96e16f5 to
b4bc345
Compare
Resolves #659 by increasing PHPStan rule level to 7 to account for possible issues with union types. I think I've followed the logic correctly, but could definitely use feedback on whether there should be additional exceptions or if there are better ways of handling these.