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

Upgrade to PHPStan v0.12 #3799

Merged
merged 12 commits into from
Mar 30, 2020
Merged

Upgrade to PHPStan v0.12 #3799

merged 12 commits into from
Mar 30, 2020

Conversation

lcobucci
Copy link
Member

@lcobucci lcobucci commented Dec 30, 2019

Q A
Type dev improvement
BC Break no

Summary

Allowing us to use more advanced checks and have full generic support.

This also adds a baseline file, since we have new violations being reported but don't want to address them right now.

Adding to 2.10.x because it doesn't influence production code and help us on #3762

@morozov
Copy link
Member

morozov commented Dec 30, 2019

[…] but don't want to address them right now.

What's the point in using advanced checks and not taking them into account? How exactly is it related to #3762?

@lcobucci
Copy link
Member Author

lcobucci commented Dec 30, 2019

What's the point in using advanced checks and not taking them into account?

@morozov we're taking them into account, just not solving every little thing now - which is the use case of the baseline.

How exactly is it related to #3762?

v0.12 adds generics support, #3762 has a failure related to it (incompatibility of array<Doctrine\DBAL\Schema\Table> and array<object>).

@morozov
Copy link
Member

morozov commented Dec 30, 2019

Then can we just report/address this issue by using the new tool locally without an update?

@lcobucci
Copy link
Member Author

Then can we just report/address this issue by using the new tool locally without an update?

I don't follow... not updating won't allow us to use the features.

@morozov
Copy link
Member

morozov commented Dec 30, 2019

I don't follow either. By just baselining all reported errors we're not using it either. It's fine that the new version found an issue in an open PR but it doesn't mean we need to upgrade immediately and ignore all existing issues.

We should fix the ones fixing which won't cause a BC break as we have been always doing.

@lcobucci
Copy link
Member Author

The point of ignoring current errors that were discovered by the new version is to allow us to use the generics to unblock #3762 while fixing the issues that won't cause a BC-break in a separate PR (and we'd also prevent new issues from being merged).

We could wait for the fix, sure. I just won't have the time to them today and think that there's no point in waiting for getting help from a smarter static analysis.

@morozov
Copy link
Member

morozov commented Dec 30, 2019

I just won't have the time to them today […]

As far as I can tell, #3762 isn't that close to being done to consider the error reported by PHPStan 0.11 a blocker. It can be added to the whitelist and get the future work unblocked. It'd be a much smaller change.

[…] and think that there's no point in waiting for getting help from a smarter static analysis.

As I said, this way we're not getting help from the new PHPStan version by ignoring its input.

@lcobucci
Copy link
Member Author

As I said, this way we're not getting help from the new PHPStan version by ignoring its input.

We would, new PRs would be validated using the new version and new issues would be prevented...

@morozov
Copy link
Member

morozov commented Dec 30, 2019

We would, new PRs would be validated using the new version and new issues would be prevented...

Yeah. But it's not that urgent that we need to rush and ignore all existing issues.

beberlei
beberlei previously approved these changes Dec 31, 2019
phpstan-baseline.neon Outdated Show resolved Hide resolved
@lcobucci
Copy link
Member Author

lcobucci commented Jan 2, 2020

@morozov I'm (slowly) addressing the issues to solve your concerns, new commits are coming (eventually 😅).

lib/Doctrine/DBAL/Driver/IBMDB2/DB2Connection.php Outdated Show resolved Hide resolved
lib/Doctrine/DBAL/Driver/PDOStatement.php Show resolved Hide resolved
lib/Doctrine/DBAL/Platforms/DrizzlePlatform.php Outdated Show resolved Hide resolved
lib/Doctrine/DBAL/Platforms/OraclePlatform.php Outdated Show resolved Hide resolved
@morozov
Copy link
Member

morozov commented Mar 18, 2020

@greg0ire could you please reopen this PR? It'll probably take creating 2.10 from 2.10.x, pushing it upstream, reopening the PR, retargeting it to 2.10.x and deleting 2.10 back. IIRC, you did something like this for other accidentally closed ones.

@morozov
Copy link
Member

morozov commented Mar 18, 2020

FWIW, it is important to upgrade PHPStan sooner than later. The reason is that phpstan/phpstan 0.11 requires phpstan/phpdoc-parser ^0.3.5 while slevomat/coding-standard 6.2 (currently, dev-master) will require phpstan/phpdoc-parser ^0.4, so we won't be able to upgrade due to the dependency conflict.

@morozov morozov added this to the 2.10.2 milestone Mar 18, 2020
@morozov morozov force-pushed the upgrade-phpstan branch 2 times, most recently from 05912fe to 9215c9d Compare March 29, 2020 22:55
@morozov
Copy link
Member

morozov commented Mar 29, 2020

I've addressed all remaining issues and filed a couple of PHPStan bugs. Going to address the issues in @lcobucci's commits that I pointed out to earlier.

@morozov morozov requested a review from greg0ire March 30, 2020 00:56
@morozov morozov merged commit 86e563a into doctrine:2.10.x Mar 30, 2020
@morozov
Copy link
Member

morozov commented Mar 30, 2020

Thanks @lcobucci.

@lcobucci lcobucci deleted the upgrade-phpstan branch March 31, 2020 19:54
@lcobucci
Copy link
Member Author

No, thank YOU @morozov ❤️
You're doing a fantastic job and I'm sorry for not being able to help you more nowadays. Keep on rockin' 🤘

rgrellmann added a commit to Rossmann-IT/dbal that referenced this pull request Apr 22, 2020
Release [2.10.2](https://github.com/doctrine/dbal/milestone/75)

2.10.2
======

- Total issues resolved: **4**
- Total pull requests resolved: **19**
- Total contributors: **10**

Improvement,Static Analysis
---------------------------

 - [3964: Mark every exception as immutable](doctrine#3964) thanks to @greg0ire

CI,Improvement,Static Analysis
------------------------------

 - [3961: Stop relying on the master version of Psalm](doctrine#3961) thanks to @greg0ire
 - [3951: Setup static analysis with Psalm](doctrine#3951) thanks to @greg0ire
 - [3799: Upgrade to PHPStan v0.12](doctrine#3799) thanks to @lcobucci

Improvement,Logging,Test Suite
------------------------------

 - [3957: Reworked LoggingTest to be able to test Statement::executeUpdate()](doctrine#3957) thanks to @morozov

CI,Code Style,Improvement,Strict Typing
---------------------------------------

 - [3955: Remove baseline](doctrine#3955) thanks to @greg0ire

Bug,SQLite,Schema Introspection,Schema Managers
-----------------------------------------------

 - [3937: Column comment incorrectly introspected on SQLite](doctrine#3937) thanks to @morozov

Bug,Documentation,Prepared Statements,Query
-------------------------------------------

 - [3896: Updated documentation for QueryBuilder::execute() return value type](doctrine#3896) thanks to @morozov

Bug,Prepared Statements
-----------------------

 - [3894: Make sure that the $types array has the same keys $params](doctrine#3894) thanks to @morozov
 - [3893: Ensure the constructor arguments are passed to custom classes](doctrine#3893) thanks to @duncan3dc
 - [3843: Fix unquoted stmt fragments backslash escaping](doctrine#3843) thanks to @morozov

Documentation,Improvement
-------------------------

 - [3886: Update readme](doctrine#3886) thanks to @greg0ire
 - [3834: Fix docblock typos in DriverManager docs](doctrine#3834) thanks to @CHItA

CI,Improvement,MariaDB,MySQL
----------------------------

 - [3884: Use Docker consistently](doctrine#3884) thanks to @greg0ire
 - [3478: Improve readiness probe stability for containerized databases on CI](doctrine#3478) thanks to @morozov

 - [3883: Fix broken build](doctrine#3883) thanks to @greg0ire

Bug,Documentation,Query,Query Limit/Offset Modification
-------------------------------------------------------

 - [3842: Fixed the QueryBuilder::setMaxResults() signature to accept NULL](doctrine#3842) thanks to @morozov

Bug,Query
---------

 - [3832: Fix JOIN with no condition bug](doctrine#3832) thanks to @BenMorel

Bug,PostgreSQL,Schema Introspection
-----------------------------------

 - [3821: &doctrine#91;pg&doctrine#93; fix getting table information if search&doctrine#95;path contains escaped schema name](doctrine#3821) thanks to @linniksa

Documentation,Improvement,Logging
---------------------------------

 - [3812: Fix DebugStack#queries docblock type](doctrine#3812) thanks to @ostrolucky

Bug,Regression,Schema
---------------------

 - [3790: fixed unqualified table name of fk constraints when using schemas](doctrine#3790) thanks to @stlrnz and @Alarich

# gpg: Signature made Mon Apr 20 19:59:36 2020
# gpg:                using DSA key 2C3A645671828132
# gpg: Can't check signature: public key not found

# Conflicts:
#	README.md
@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.

6 participants