Psalm 6

[greg0ire]

Q	A
Type	improvement
BC Break	no
Fixed issues	n/a

Summary

Pushing although there are still 9 errors left, because they are either blocked, or I need help.
See:

• Allow non-string keys vimeo/psalm#3221
• str_replace does not allow a null replacement vimeo/psalm#3224
• Psalm doesn't take into account method in child abstract class vimeo/psalm#3217
• Mark all methods that never return as such sebastianbergmann/phpunit#4187
• Document fluent methods as such sebastianbergmann/phpunit#4184
• Rebase on top of Bump cs libs #3983 , use array shapes directly in @param
• Figure out what to do about Psalm 6 #3977 (comment)
• Figure out what to do about Psalm 6 #3977 (comment)
• Check if things can be improved with PHPStan following undocumented key in array shape causes error phpstan/phpstan#3228 => can't reproduce now, not sure why
• Wait for a stable release of Psalm
• Wait for a stable release of PHPUnit
• squash what makes sense squashing
• do the merge locally
• prepare merge up for 2.11.x
• prepare merge up for 3.0.x
• prepare merge up for master

I will of course squash many of the commits together in the end.

[greg0ire]

In the remaining issues, there's one I don't know what to do about:

ERROR: NullArgument - tests/Doctrine/Tests/DBAL/Functional/Driver/SQLSrv/StatementTest.php:30:68 - Argument 1 of Doctrine\DBAL\Driver\Connection::prepare cannot be null, null value provided to parameter with type string (see https://psalm.dev/057)
        $stmt = $this->connection->getWrappedConnection()->prepare(null);

dbal/tests/Doctrine/Tests/DBAL/Functional/Driver/SQLSrv/StatementTest.php

Lines 10 to 37 in 155d028

	class StatementTest extends DbalFunctionalTestCase
	{
	protected function setUp() : void
	{
	if (! extension_loaded('sqlsrv')) {
	self::markTestSkipped('sqlsrv is not installed.');
	}
	parent::setUp();
	if ($this->connection->getDriver() instanceof Driver) {
	return;
	}
	self::markTestSkipped('sqlsrv only test');
	}
	public function testFailureToPrepareResultsInException() : void
	{
	// use the driver connection directly to avoid having exception wrapped
	$stmt = $this->connection->getWrappedConnection()->prepare(null);
	// it's impossible to prepare the statement without bound variables for SQL Server,
	// so the preparation happens before the first execution when variables are already in place
	$this->expectException(SQLSrvException::class);
	$stmt->execute();
	}
	}

Please help.

[greg0ire]

@morozov the failure in Travis is caused by a warning about using an array of interfaces in calls to createMock. It is fixed on 2.11 because VersionAwarePlatformDriver extends Driver on that branch. Should I create a short-lived test interface that extends both and use it in the test? I would remove it when merging up. Tell me if you have another idea (maybe I should target 2.11, for instance).

[morozov]

Should I create a short-lived test interface that extends both and use it in the test?

That should be fine.

[greg0ire]

@morozov phpcs fails unless I use @psalm-param instead of @param when using array shapes. An alternative is to get slevomat/coding-standard to 6.3.3, which can be done by upgrading phpstan/phpdoc-parser, doctrine/coding-standard and squizlabs/php_codesniffer. Should I go ahead and create another PR that upgrades these packages? It might make other things easier. I'm asking because 2.10.x is for bugfixes, and this will probably imply changing code in src, although not in a significant way.

[morozov]

@greg0ire let's make the needed upgrades and see if we can simplify the annotations. The ones in the driver manager are really concerning.

[greg0ire]

Down to 4 errors.

[greg0ire]

Down to 2 errors, with which I need help please @morozov 🙏 see the todo list in the original post for details.

[morozov]

dbal/tests/Doctrine/Tests/DBAL/Functional/Driver/SQLSrv/StatementTest.php

Lines 29 to 30 in 155d028

	// use the driver connection directly to avoid having exception wrapped
	$stmt = $this->connection->getWrappedConnection()->prepare(null);

IIRC, it just needs to be an invalid SQL that will cause failure being prepared. An empty string instead of null should work.

UPD: look at 8adf7f9#diff-c47062aaec500b4bf4ab79095668542cR30.

[greg0ire]

[pkruithof]

Sorry to comment on an older PR, but this commit causes phpstan to report an error: https://phpstan.org/r/e462e0f9-abc4-44bc-8a9a-80aca2717980

I've asked a support question there, and the author suggested a solution here: phpstan/phpstan#3849 (comment)

Would you accept a PR with the second solution? I don't think a third-party library should trigger phpstan errors in a project.

[morozov]

@pkruithof please go ahead and submit a PR. @greg0ire could you work with PHPStan on finding the root cause as suggested in the first option?

[pkruithof]

I just tried the suggestion but it did not help unfortunately, and I'm kind of out of my depth here. 😞

[morozov]

@pkruithof let's start with filing a new issue that would reproduce the problem.

[pkruithof]

Done 👉 #4264