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

Fix some whitelisted PHPStan errors #3822

Merged
merged 5 commits into from
Jan 16, 2020
Merged

Conversation

morozov
Copy link
Member

@morozov morozov commented Jan 16, 2020

Q A
Type improvement
BC Break yes
Fixed issues #3603, partially
  1. Some errors are no longer relevant due to the code changes and issues fixed in PHPStan.
  2. The RunSqlCommandTest is reworked to comply with its type constraints.
  3. Type tests were reworked to test their respective classes w/o using the type registry (less coupling). Additionally, the platform mocks were reworked to mock the expected behavior explicitly instead of relying on the default platform logic (less coupling).
  4. The absence of the primary key is indicated by Table::$_primaryKey = null instead of false (the actual BC break).
  5. Removed the preg_replace() error from the whitelist and added an assertion. Unless it's known under which circumstances a NULL may be returned (as pointed out in PHPStan is too strict about preg_replace() phpstan/phpstan#1901), we cannot test it. Hence, it's an assertion, not a condition with an exception.

@morozov morozov requested a review from Ocramius January 16, 2020 01:58
@morozov morozov added this to the 4.0.0 milestone Jan 16, 2020
@morozov morozov self-assigned this Jan 16, 2020
@morozov morozov merged commit 51fa945 into doctrine:master Jan 16, 2020
@morozov morozov deleted the issues/3603 branch January 16, 2020 17:39
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 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.

2 participants