Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump Psalm level to 5 #4094

Merged
merged 14 commits into from
Jun 20, 2020
Merged

Bump Psalm level to 5 #4094

merged 14 commits into from
Jun 20, 2020

Conversation

morozov
Copy link
Member

@morozov morozov commented Jun 20, 2020

Q A
Type improvement
BC Break no


if ($value === false) {
return '';
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we don't consider this a BC-break?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this should be suppressed. I'm not sure if the “BC break” is the right term here because the new value corresponds to the documented return type while the old one didn't.

@@ -95,3 +95,8 @@ parameters:
message: '~Method Doctrine\\DBAL\\Driver\\PDOSqlsrv\\Connection\:\:lastInsertId\(\) should return string but returns string\|false\|null\.~'
paths:
- %currentWorkingDirectory%/lib/Doctrine/DBAL/Driver/PDOSqlsrv/Connection.php

-
message: '~Method Doctrine\\DBAL\\Portability\\Connection::prepare\(\) should return Doctrine\\DBAL\\Statement but returns Doctrine\\DBAL\\Portability\\Statement\.~'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is caused by the issue earlier identified in #4045:

The portability layer is currently implemented on two levels of abstraction at the same time: most of its components implement driver-level interfaces but the Connection class extends the DBAL connection.

@morozov morozov closed this Jun 20, 2020
@morozov morozov reopened this Jun 20, 2020
@morozov morozov requested a review from greg0ire June 20, 2020 15:03
@greg0ire greg0ire closed this Jun 20, 2020
@greg0ire greg0ire reopened this Jun 20, 2020
@greg0ire greg0ire merged commit 3caae11 into doctrine:2.10.x Jun 20, 2020
@greg0ire
Copy link
Member

Thanks @morozov !

@greg0ire greg0ire self-assigned this Jun 20, 2020
@morozov
Copy link
Member Author

morozov commented Sep 3, 2020

The assertion was done to prove the method contract. It's expected to return a DriverConnection but the $_conn property is nullable. Thus we let the static analyzers know that by the end of the call to connect(), the property contains a connection.

Your use case violates the API. You can define your own implementation (in fact, a mock) of the DriverConnection interface and assign it.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants