Fixed coding standard violations in the codebase #3306

morozov · 2018-10-02T05:25:24Z

Q	A
Type	improvement
BC Break	no

The coding standard violations have been fixed in bin/ and lib/. Besides the automated fixes via phpcbf, the following was changed:

Fixed the issues which phpcbf couldn't fix.
Removed leading underscore from private class memeber names.
Replaced class names represented by strings with the ::class syntax
Removed some remaining license headers
Replaced diff-sniffer with PHP_CodeSniffer for pull request validation, made the stage required.

Ocramius

🔥

svycka · 2018-11-16T08:43:25Z

@morozov this introduced bug #3349 and maybe more

morozov

Some more potential regressions were identified based on inspection of https://gist.github.com/morozov/66b9fc4de60eea4d0c9d2ab588e86c27.

morozov · 2018-11-24T00:34:53Z

lib/Doctrine/DBAL/Driver/IBMDB2/DB2Connection.php

     */
    public function __construct(array $params, $username, $password, $driverOptions = [])
    {
-        $isPersistent = (isset($params['persistent']) && $params['persistent'] == true);
+        $isPersistent = (isset($params['persistent']) && $params['persistent'] === true);


This can be considered a BC break. Could be replaced with if (!empty($params['persistent'])).

morozov · 2018-11-24T00:36:47Z

lib/Doctrine/DBAL/Driver/PDOMySql/Driver.php

     *
     * @return string The DSN.
     */
    protected function constructPdoDsn(array $params)
    {
        $dsn = 'mysql:';
-        if (isset($params['host']) && $params['host'] != '') {
+        if (isset($params['host']) && $params['host'] !== '') {


~~This can be considered a BC break. Could be replaced with ... && strcmp($params['host'], '') === 0.~~

Actually, it's fine. null woudn't pass the isset() check.

morozov · 2018-11-24T00:37:43Z

lib/Doctrine/DBAL/Driver/PDOPgSql/Driver.php

     *
     * @return string The DSN.
     */
    private function _constructPdoDsn(array $params)
    {
        $dsn = 'pgsql:';

-        if (isset($params['host']) && $params['host'] != '') {
+        if (isset($params['host']) && $params['host'] !== '') {


~~This can be considered a BC break. Could be replaced with ... && strcmp($params['host'], '') === 0.~~

Actually, it's fine. null woudn't pass the isset() check.

morozov · 2018-11-24T00:38:00Z

lib/Doctrine/DBAL/Driver/PDOPgSql/Driver.php

            $dsn .= 'host=' . $params['host'] . ';';
        }

-        if (isset($params['port']) && $params['port'] != '') {
+        if (isset($params['port']) && $params['port'] !== '') {


~~This can be considered a BC break. Could be replaced with ... && strcmp($params['port'], '') === 0.~~

Actually, it's fine. null woudn't pass the isset() check.

morozov · 2018-11-24T00:41:38Z

lib/Doctrine/DBAL/Platforms/DB2Platform.php

@@ -251,8 +234,8 @@ public function getDateDiffExpression($date1, $date2)
     */
    public function getDateTimeTypeDeclarationSQL(array $fieldDeclaration)
    {
-        if (isset($fieldDeclaration['version']) && $fieldDeclaration['version'] == true) {
-            return "TIMESTAMP(0) WITH DEFAULT";
+        if (isset($fieldDeclaration['version']) && $fieldDeclaration['version'] === true) {


This can be considered a BC break. Could be replaced with if (!empty($fieldDeclaration['version'])).

morozov · 2018-11-24T00:43:05Z

lib/Doctrine/DBAL/Platforms/DrizzlePlatform.php

@@ -468,7 +450,7 @@ protected function getDropPrimaryKeySQL($table)
     */
    public function getDateTimeTypeDeclarationSQL(array $fieldDeclaration)
    {
-        if (isset($fieldDeclaration['version']) && $fieldDeclaration['version'] == true) {
+        if (isset($fieldDeclaration['version']) && $fieldDeclaration['version'] === true) {


This can be considered a BC break. Could be replaced with if (!empty($fieldDeclaration['version'])).

morozov · 2018-11-24T00:45:31Z

lib/Doctrine/DBAL/Platforms/MySqlPlatform.php

@@ -291,7 +269,7 @@ public function getClobTypeDeclarationSQL(array $field)
     */
    public function getDateTimeTypeDeclarationSQL(array $fieldDeclaration)
    {
-        if (isset($fieldDeclaration['version']) && $fieldDeclaration['version'] == true) {
+        if (isset($fieldDeclaration['version']) && $fieldDeclaration['version'] === true) {


This can be considered a BC break. Could be replaced with if (!empty($fieldDeclaration['version'])).

morozov · 2018-11-24T00:53:11Z

lib/Doctrine/DBAL/Schema/Comparator.php

@@ -429,9 +420,11 @@ public function diffColumn(Column $column1, Column $column2)
        $changedProperties = [];

        foreach (['type', 'notnull', 'unsigned', 'autoincrement'] as $property) {
-            if ($properties1[$property] != $properties2[$property]) {
-                $changedProperties[] = $property;
+            if ($properties1[$property] === $properties2[$property]) {


This looks risky given the boolean types of the above may be not enforced. But I'd rather fix the root cause if the issue exists than the comparison itself.

morozov · 2018-11-24T00:53:52Z

lib/Doctrine/DBAL/Schema/Comparator.php

@@ -441,11 +434,11 @@ public function diffColumn(Column $column1, Column $column2)
            $changedProperties[] = 'comment';
        }

-        if ($properties1['default'] != $properties2['default'] ||
+        if ($properties1['default'] !== $properties2['default'] ||


This was reported in #3336.

@morozov

Release v2.9.0 This is a minor release of Doctrine DBAL that aggregates over 40 fixes and improvements developed by 18 contributors over the last 5 months. This release includes all changes of the 2.8.x series, as well as feature additions and improvements that couldn't land in patch releases. ## Backwards Compatibility Breaks This doesn't contain any intentional Backwards Compatibility (BC) breaks. ## Deprecations * The usage of `NULL` to specify the absence of an offset in `LIMIT`ed queries is deprecated. Use `0` instead. * It's not recommended to rely on the default length specified by implementations of `Type`. These values are not used by the library and will be removed. * It's not recommended to rely on the string representation of `Type` objects. * Regular-expression based asset filters are deprecated in favor of callback-based as more extensible. * Calling `Statement::fetchColumn()` with an invalid column index is deprecated. * The `dbal:import` CLI command is deprecated. Please use other database client applications for import. Please see details in the [UPGRADE.md](UPGRADE.md) documentation. ## New Features * Added support for MariaDB 10.3. * Added support for Windows authentication for SQL Server. * Added support for column length in index definitions on MySQL. ## Improvements and Fixes * Implemented handling BLOB objects represented as streams in the MySQL (`mysqli`) driver. * Implemented handling BLOB objects represented as streams in the IDM DB2 driver. * DBAL is now continuously tested with the PDO driver for Oracle. * Implemented handling of URLs in master-slave and pooling-shard connection configuration. * The codebase is now fully compatible with the Doctrine Coding Standard v5.0. Total issues resolved: **45** **Deprecations:** - [3244: Deprecated dbal:import CLI command](doctrine#3244) thanks to @morozov - [3253: Deprecated usage of the NULL offset in LIMITed queries](doctrine#3253) thanks to @morozov - [3256: Deprecate Doctrine\DBAL\Types\Type::getDefaultLength()](doctrine#3256) thanks to @Majkl578 - [3258: Deprecate Doctrine\DBAL\Types\Type::&doctrine#95;&doctrine#95;toString()](doctrine#3258) thanks to @Majkl578 - [3316: Deprecated regex-based asset filters](doctrine#3316) thanks to @morozov - [3359: Removed DataAccessTest::testFetchColumnNonExistingIndex() since it covers a bug in PDO](doctrine#3359) thanks to @morozov **New Features:** - [2412: Add mysql specific indexes with lengths](doctrine#2412) thanks to @bburnichon - [3278: Add support for MariaDB 10.3](doctrine#3278) thanks to @javiereguiluz - [3283: MariaDB improvements, support 10.3](doctrine#3283) thanks to @sidz - [3333: Allow windows (userless/passwordless) authentication for SQL Server](doctrine#3333) thanks to @odinsey **Bug Fixes:** - [3355: Implemented comparison of default values as strings regardless of their PHP types](doctrine#3355) thanks to @morozov and @Majkl578 **Improvements:** - [3201: Fix support for URL to account for master-slave and pooling-shard connections](doctrine#3201) thanks to @stof - [3217: Fix that MysqliStatement cannot handle streams](doctrine#3217) thanks to @mpdude - [3235: Use PSR-4 autoloader](doctrine#3235) thanks to @Majkl578 - [3254: Throw ConversionException when unserialization fail for array and object types](doctrine#3254) thanks to @seferov - [3259: Update export ignores](doctrine#3259) thanks to @Majkl578 - [3309: Implemented handling BLOBs represented as stream resources for IBM DB2](doctrine#3309) thanks to @morozov and @mpdude - [3331: Fetch all should use the driver statement's fetchAll method](doctrine#3331) thanks to @michaelcullum **Documentation Improvements:** - [3223: GitHub template grammar/spelling fixes](doctrine#3223) thanks to @GawainLynch - [3232: Removed NOW() from QueryBuilder usage examples](doctrine#3232) thanks to @morozov - [3239: 2.8 in README & branch alias to 2.9](doctrine#3239) thanks to @Majkl578 - [3269: Fixed type hints in DockBlocks](doctrine#3269) thanks to @marforon - [3275: Add .doctrine-project.json to root of the project.](doctrine#3275) thanks to @jwage - [3276: Update homepage](doctrine#3276) thanks to @Majkl578 - [3280: Use behaviuor instead of behavior](doctrine#3280) thanks to @BackEndTea - [3285: Remove old comment from MysqliStatement](doctrine#3285) thanks to @mpdude - [3318: Removed link to www.doctrine-project.org from doc blocks](doctrine#3318) thanks to @morozov - [3319: remove ClassLoader](doctrine#3319) thanks to @garak - [3337: Fix of links in documentation](doctrine#3337) thanks to @SenseException - [3350: Remove pdo&doctrine#95;sqlsrv from known vendor issues list](doctrine#3350) thanks to @ostrolucky - [3357: Fix typo](doctrine#3357) thanks to @BenMorel - [3370: Removed 2.7 from README](doctrine#3370) thanks to @morozov **Code Quality Improvements:** - [3252: Replaced call&doctrine#95;user&doctrine#95;func&doctrine#95;array() of a fixed method with the usage of variadic arguments](doctrine#3252) thanks to @morozov - [3306: Fixed coding standard violations in the codebase](doctrine#3306) thanks to @morozov - [3303: Updated doctrine/coding-standard to 5.0, ](doctrine#3303) thanks to @morozov - [3317: Implemented proper escaping of string literals in platforms and schema managers](doctrine#3317) thanks to @morozov - [3363: Remove redundant implements](doctrine#3363) thanks to @BenMorel **Continuous Integration Improvements:** - [3307: Test against the latest stable sqlsrv extension](doctrine#3307) thanks to @morozov - [3320: Trying to fix failing DB builds](doctrine#3320) thanks to @morozov - [3325: Updated PHPUnit to 7.4](doctrine#3325) thanks to @morozov - [3339: ContinuousPHP configuration for PDO Oracle driver](doctrine#3339) thanks to @morozov - [3365: Reorganize Travis build matrix](doctrine#3365) thanks to @BenMorel # gpg: Signature made Tue Dec 4 05:44:06 2018 # gpg: using RSA key 374EADAF543AE995 # gpg: Can't check signature: public key not found # Conflicts: # README.md # lib/Doctrine/DBAL/Driver/AbstractOracleDriver.php # lib/Doctrine/DBAL/Version.php

morozov added 3 commits October 1, 2018 22:21

Removed license headers

ba4c9a2

Automated coding style fixes using phpcbf

dca1465

Fixed failures introduced by strict comparison

9da1ea3

morozov changed the title ~~Doctrine coding standard 5 lib~~ Fixed coding standard violations in the codebase Oct 2, 2018

morozov added Improvement CI Code Style WIP labels Oct 2, 2018

morozov added 3 commits October 1, 2018 23:25

Manual coding style fixes

64939c3

Removed leading underscore from private class memeber names

1cbadb3

Replaced class names represented by strings with the ::class syntax

6350b9d

morozov force-pushed the doctrine-coding-standard-5-lib branch from a7780e4 to fc52194 Compare October 2, 2018 06:26

Validate pull requests by PHPCS, do not allow failure

4d50910

morozov force-pushed the doctrine-coding-standard-5-lib branch from fc52194 to 4d50910 Compare October 2, 2018 07:02

Ocramius added this to the 2.9.0 milestone Oct 2, 2018

Ocramius approved these changes Oct 2, 2018

View reviewed changes

Ocramius self-assigned this Oct 2, 2018

Ocramius merged commit b45ed5e into doctrine:master Oct 2, 2018

morozov deleted the doctrine-coding-standard-5-lib branch October 2, 2018 17:49

morozov mentioned this pull request Oct 2, 2018

Rename protected and private class members starting with an underscore #3004

Closed

svycka mentioned this pull request Nov 16, 2018

#3306 introduced at least one bug #3349

Closed

morozov commented Nov 24, 2018

View reviewed changes

morozov removed the WIP label Dec 3, 2018

This was referenced Dec 7, 2018

Enable format negotiation libero/content-api-bundle#15

Merged

Fix column name type hint #3396

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed coding standard violations in the codebase #3306

Fixed coding standard violations in the codebase #3306

morozov commented Oct 2, 2018

Ocramius left a comment

svycka commented Nov 16, 2018 •

edited

Loading

morozov left a comment

morozov Nov 24, 2018

morozov Nov 24, 2018 •

edited

Loading

morozov Nov 24, 2018 •

edited

Loading

morozov Nov 24, 2018 •

edited

Loading

morozov Nov 24, 2018

morozov Nov 24, 2018

morozov Nov 24, 2018

morozov Nov 24, 2018

cs278 Jan 10, 2019

morozov Nov 24, 2018

Fixed coding standard violations in the codebase #3306

Fixed coding standard violations in the codebase #3306

Conversation

morozov commented Oct 2, 2018

Ocramius left a comment

Choose a reason for hiding this comment

svycka commented Nov 16, 2018 • edited Loading

morozov left a comment

Choose a reason for hiding this comment

morozov Nov 24, 2018

Choose a reason for hiding this comment

morozov Nov 24, 2018 • edited Loading

Choose a reason for hiding this comment

morozov Nov 24, 2018 • edited Loading

Choose a reason for hiding this comment

morozov Nov 24, 2018 • edited Loading

Choose a reason for hiding this comment

morozov Nov 24, 2018

Choose a reason for hiding this comment

morozov Nov 24, 2018

Choose a reason for hiding this comment

morozov Nov 24, 2018

Choose a reason for hiding this comment

morozov Nov 24, 2018

Choose a reason for hiding this comment

cs278 Jan 10, 2019

Choose a reason for hiding this comment

morozov Nov 24, 2018

Choose a reason for hiding this comment

svycka commented Nov 16, 2018 •

edited

Loading

morozov Nov 24, 2018 •

edited

Loading

morozov Nov 24, 2018 •

edited

Loading

morozov Nov 24, 2018 •

edited

Loading