Recent Scrutinizer Messages #3740
Conversation
Get rid of them where possible.
There was a problem hiding this comment.
On a more general note, I feel nowaday PHPStan is way better than Scrutinizer static analyzer and we probably should not spend too much time on accommodating Scrutinizer false positives.
While Scrutinizer is still able to catch things that PHPStan cannot, so it makes sense to keep it, I don't think we should riddle our code with @scrutinizer ignore-type or similar. Instead we can ignore whole range of errors directly from Scrutinizer UI. Typically $drawing->setImageResource(/** @scrutinizer ignore-type */ $gdImage); is a false positive that clutter our code for nothing.
PS: A quick search shows ~732 @scrutinizer comments against ~78 @phpstan. The illustrates my point better than I imagined.
| @@ -3442,7 +3442,7 @@ public function getHashCode() | |||
| * @param string $range Range to extract title from | |||
| * @param bool $returnRange Return range? (see example) | |||
| * | |||
| * @return ($range is non-empty-string ? ($returnRange is true ? array{0: string, 1: string} : string) : ($returnRange is true ? array{0: null, 1: null} : null)) | |||
| * @return ($range is non-empty-string ? ($returnRange is true ? array{0: string, 1: string} : string) : ($returnRange is true ? array{0: null, 1: null} : null)) Scrutinizer does not understand this but it helps Phpstan | |||
There was a problem hiding this comment.
| * @return ($range is non-empty-string ? ($returnRange is true ? array{0: string, 1: string} : string) : ($returnRange is true ? array{0: null, 1: null} : null)) Scrutinizer does not understand this but it helps Phpstan | |
| * @return ($range is non-empty-string ? ($returnRange is true ? array{0: string, 1: string} : string) : ($returnRange is true ? array{0: null, 1: null} : null)) |
Don't think we should have those kind of comment that end up in end-user documentation.
|
I gave you access to Scrutinizer:
|
|
@PowerKiKi converting to draft so that you can carry on with your work in #3743. As for the comments in the doc-block, I couldn't figure out how else to indicate that Scrutinizer cares but we consciously don't care (as opposed to we've never even looked at the message). I agree that Phpstan is way better than Scrutinizer in its analysis. |
Now you should be able to do that on https://scrutinizer-ci.com/g/PHPOffice/PhpSpreadsheet/issues/master |
|
Superseded by #3743. |
Get rid of them where possible.
This is:
Checklist:
Why this change is needed?
Provide an explanation of why this change is needed, with links to any Issues (if appropriate).
If this is a bugfix or a new feature, and there are no existing Issues, then please also create an issue that will make it easier to track progress with this PR.