Fix phpstan errors

[BenMorel]

Q	A
Type	improvement
BC Break	no
Fixed issues	none

Summary

Fixes more phpstan errors, as suggested in #3801.

[BenMorel]

@morozov I don't know how to best handle the last 3 errors:

• 2 tests forcibly use a wrong type to ensure that an exception is thrown, but of course this makes phpstan unhappy:

dbal/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php

Line 624 in e0a92b0

$this->platform->getDropIndexSQL(['index'], 'table');

• Here array_keys() is called on an array that only has string keys, but phpstan doesn't understand this, and assumes that the result is an array containing int|string:

dbal/lib/Doctrine/DBAL/Query/QueryBuilder.php

Line 1319 in e0a92b0

throw NonUniqueAlias::new($join['joinAlias'], array_keys($knownAliases));

Any advice?

[morozov]

2 tests forcibly use a wrong type to ensure that an exception is thrown, but of course this makes phpstan unhappy

Drop them. This is what PHPStan is for :-)

Here array_keys() is called on an array that only has string keys, but phpstan doesn't understand this, and assumes that the result is an array containing int|string

It should have been fixed as per phpstan/phpstan#1847 but it seems it isn't. It looks like out of the two related suppressions only one is needed (the other is already removed in master, please rebase). Please leave the remaining one for now, file a PHPStan issue that would reproduce our case in their sandbox (same as it was done in phpstan/phpstan#1847) and update the reference to point to the newly created issue in our PHPStan configuration.

[BenMorel]

Drop them. This is what PHPStan is for :-)

Done!

please rebase

Done.

It should have been fixed as per phpstan/phpstan#1847 but it seems it isn't.

Strangely, it doesn't happen in their sandbox:

https://phpstan.org/r/eea2b886-3942-425d-8ecd-b50489599ff5

I've double checked, the fix should be included in the phpstan version we're using, too, so I don't understand. Any idea?

[morozov]

I've double checked, the fix should be included in the phpstan version we're using, too, so I don't understand. Any idea?

The difference may be in the config. Please put together the simplest possible example that gets flagged with our version and config and then see if it depends on the config. Then report whatever reproducible issue. IIRC, not all issues are reproducible in the sandbox (e.g. phpstan/phpstan#2510).

[BenMorel]

Finally reproduced it, and reported as phpstan/phpstan#2857.

I forgot to include the exception class in the sandbox, obviously. But for some reason phpstan didn't complain about the missing class at all.

[BenMorel]

@morozov All good now! 👍

[morozov]

@BenMorel, looks good! Please squash.

[morozov]

To get rid of the remaining PHPStan issue, we may replace the associative array representing a join in the Query Builder with an object similar to DependencyOrderNode used in #3762. This way, we will not only help static analyzers understand which properties and of what types a join has but also may help PHP manage memory more efficiently:

1. https://steemit.com/php/@crell/php-use-associative-arrays-basically-never
2. https://youtu.be/JBWgvUrb-q8?t=943

[BenMorel]

Please squash.

Just for my information, what's wrong with the "Squash & Merge" button on GitHub?

To get rid of the remaining PHPStan issue, we may replace the associative array representing a join in the Query Builder with an object similar to DependencyOrderNode used in #3762.

I'd love to replace associative arrays with objects. I may try to play with this next.

[morozov]

Just for my information, what's wrong with the "Squash & Merge" button on GitHub?

1. There won't be a merge commit, the changes will be squashed and just committed to the target branch (ref). We want to have merge commits in the repo for better attribution.
2. If a contributor sings their commits, they will also sign the squashed one. Otherwise, if squashed by GitHub, the resulting commit will be signed by GitHub, not by the contributor.

[BenMorel]

@morozov Thanks. Squashed!

[morozov]

Thanks, @BenMorel!