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 3 #4348

Merged
merged 21 commits into from
Oct 16, 2020
Merged

Bump Psalm level to 3 #4348

merged 21 commits into from
Oct 16, 2020

Conversation

morozov
Copy link
Member

@morozov morozov commented Oct 16, 2020

Q A
Type improvement
BC Break no

@morozov morozov marked this pull request as ready for review October 16, 2020 06:16
@morozov morozov requested a review from greg0ire October 16, 2020 06:16
Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

Please check Slack, I'd like to talk about merges up.

@@ -51,7 +51,7 @@ protected function setUp(): void
}

/**
* @return Connection|MockObject
* @return Connection&MockObject
Copy link
Member

Choose a reason for hiding this comment

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

I have recently seen people at Sonata using something even better: MockObject<Connection>. Would that work for us?

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 doesn't seem to work with the latest version of dependencies:

$ composer show | grep psalm

psalm/plugin-phpunit                           0.10.1  Psalm plugin for PHPUnit
vimeo/psalm                                    3.17.2  A static analysis tool for finding...

$ psalm

...
ERROR: InvalidArgument - tests/Doctrine/Tests/DBAL/Types/VarDateTimeTest.php:67:71 - Argument 2 of Doctrine\DBAL\Types\VarDateTimeType::convertToPHPValue expects Doctrine\DBAL\Platforms\AbstractPlatform, PHPUnit\Framework\MockObject\MockObject<Doctrine\DBAL\Platforms\AbstractPlatform> provided (see https://psalm.dev/004)
        self::assertSame($date, $this->type->convertToPHPValue($date, $this->platform));


------------------------------
251 errors found
------------------------------

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I can't find the piece of code where I see that so nevermind…

Comment on lines 21 to 23
if ($error !== null) {
$message = $error['message'];
} else {
$message = 'Unknown error';
}
Copy link
Member

Choose a reason for hiding this comment

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

Let's avoid the negative logic and the else:

Suggested change
if ($error !== null) {
$message = $error['message'];
} else {
$message = 'Unknown error';
}
if ($error === null) {
return new self('Unknown error');
}
return new self($error['message']);

Comment on lines 683 to 686
$sql = 'SELECT test_int, test_string'
. ', ' . $platform->getBitOrComparisonExpression('test_int', 2) . ' AS bit_or'
. ', ' . $platform->getBitAndComparisonExpression('test_int', 2) . ' AS bit_and'
. ' FROM fetch_table';
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$sql = 'SELECT test_int, test_string'
. ', ' . $platform->getBitOrComparisonExpression('test_int', 2) . ' AS bit_or'
. ', ' . $platform->getBitAndComparisonExpression('test_int', 2) . ' AS bit_and'
. ' FROM fetch_table';
$sql = sprintf(
<<<'SQL'
SELECT
test_int,
test_string,
%s AS bit_or,
%s as bit_and
FROM fetch_table
SQL
,
$platform->getBitOrComparisonExpression('test_int', 2),
$platform->getBitAndComparisonExpression('test_int', 2)
);

modulo the cs issues of course

@morozov morozov merged commit 9d63c3b into doctrine:2.11.x Oct 16, 2020
@morozov morozov deleted the psalm-3 branch October 16, 2020 23:19
@morozov morozov self-assigned this Oct 16, 2020
@morozov morozov added this to the 2.11.2 milestone Oct 16, 2020
rgrellmann added a commit to Rossmann-IT/dbal that referenced this pull request Mar 7, 2021
Release [2.11.2](https://github.com/doctrine/dbal/milestone/81)

2.11.2
======

- Total issues resolved: **5**
- Total pull requests resolved: **16**
- Total contributors: **10**

Static Analysis
---------------

 - [4353: Update Psalm to 3.17.2 and lock the version used with GitHub Actions](doctrine#4353) thanks to @morozov
 - [4348: Bump Psalm level to 3](doctrine#4348) thanks to @morozov
 - [4332: Static analysis improvements](doctrine#4332) thanks to @morozov
 - [4319: Bump Psalm level to 4](doctrine#4319) thanks to @morozov

Code Style
----------

 - [4346: Minor CS improvement - use ::class for TestCase::expectException input](doctrine#4346) thanks to @mvorisek

 - [4344: Static analysis workflow](doctrine#4344) thanks to @greg0ire
 - [4340: Modernize existing ga](doctrine#4340) thanks to @greg0ire
 - [4309: Use cache action v2](doctrine#4309) thanks to @greg0ire
 - [4305: Move website config to default branch](doctrine#4305) thanks to @SenseException

Improvement,Prepared Statements
-------------------------------

 - [4341: Add Statement::fetchAllIndexedAssociative() and ::iterateIndexedAssociative()](doctrine#4341) thanks to @morozov and @ZaneCEO
 - [4338: Add Statement::fetchAllKeyValue() and ::iterateKeyValue()](doctrine#4338) thanks to @morozov

BC Fix,Query
------------

 - [4330: Fix regression in QueryBuilder::and|orWhere()](doctrine#4330) thanks to @BenMorel

Test Suite
----------

 - [4321: Update PHPUnit to 9.4](doctrine#4321) thanks to @morozov

Columns,SQL Server,Schema Managers
----------------------------------

 - [4315: Fix handling existing SQL Server column comment when other properties change](doctrine#4315) thanks to @trusek

CI
--

 - [4310: Migrate jobs away from Travis to Github Actions ](doctrine#4310) thanks to @greg0ire

BC Fix,Connections
------------------

 - [4308: doctrine#4295 Keep master, slaves, keepReplica params in MasterSlaveConnection](doctrine#4308) thanks to @kralos

New Feature,Prepared Statements
-------------------------------

 - [4289: Add a fetch mode methods for &quot;PDO::FETCH&doctrine#95;KEY&doctrine#95;PAIR&quot;](doctrine#4289) thanks to @tswcode

Bug,SQL Server,Schema
---------------------

 - [3400: Wrong column comment setting command in migrations of SQL Server](doctrine#3400) thanks to @msyfurukawa

# gpg: Signature made Mon Oct 19 04:18:17 2020
# gpg:                using DSA key 1BEDEE0A820BC30D858F9F0C2C3A645671828132
# gpg: Can't check signature: No public key
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 28, 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.

3 participants