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

Static analysis improvements #4332

Merged
merged 4 commits into from
Oct 10, 2020

Conversation

morozov
Copy link
Member

@morozov morozov commented Oct 10, 2020

Q A
Type improvement
BC Break no

During merging up #4319 to master (currently only prepared locally), some new Psalm and PHPStan failures occurred. Instead of just fixing them in the merge commit, I'd like to fix them in the earliest possible version.

  1. Psalm flags (array) casting in ReservedWordsCommand as a type error because getOption() may return a boolean which will be cast to [false] or [true] neither of which is a valid list of platforms:
    $keywordLists = (array) $input->getOption('list');
  2. SchemaManagerFunctionalTestCase has really creepy magic that requires an assertion for PHPStan but Psalm considers this assertion redundant:
    protected function getPlatformName(): string
    {
    $class = static::class;
    $e = explode('\\', $class);
    $testClass = end($e);
    assert(is_string($testClass));
    return strtolower(str_replace('SchemaManagerTest', '', $testClass));
    }

    Due to this magic, the test must be named exactly like the platform it's supposed to test or otherwise, it will be skipped. It's better to implement the logic of which test tests which platform explicitly.
  3. The type of the prepared statement parameter type argument is specified inconsistently across the codebase. The valid type of the type is int|string|Type|null which is one of the ParameterType constants, DBAL type name, DBAL type object, or NULL respectively.
  4. The $identifier argument in Connection::update() and ::delete() methods have been renamed to $criteria which it actually is. Otherwise, an "identifier" implies that a single record will be identified.

@morozov morozov added this to the 2.11.2 milestone Oct 10, 2020
@morozov morozov requested a review from greg0ire October 10, 2020 18:21
@morozov morozov force-pushed the static-analysis-improvements branch from 15f2a6f to 89e1f72 Compare October 10, 2020 19:03
@morozov morozov merged commit 4e3d016 into doctrine:2.11.x Oct 10, 2020
@morozov morozov deleted the static-analysis-improvements branch October 10, 2020 19:51
@morozov morozov self-assigned this Oct 10, 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 "PDO::FETCH&doctrine#95;KEY&doctrine#95;PAIR"](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.

2 participants