PHPLIB-1106: Upgrade doctrine/coding-standard to 11.1.0 #1058
Conversation
jmikola
changed the title
phpcs.xml.dist
Outdated
| @@ -22,11 +22,17 @@ | |||
| <!-- Exclude sniffs that require newer PHP versions --> | |||
| <!-- ********************************************** --> | |||
|
|
|||
| <!-- Requires PHP 7.3 --> | |||
| <exclude name="SlevomatCodingStandard.Functions.RequireTrailingCommaInCall.MissingTrailingComma" /> | |||
There was a problem hiding this comment.
Out of curiosity, how does doctrine/coding-standard utilize these sniffs if it claims compatibility with PHP 7.2+ (see: composer.json). Does this have something to do with the version-specific patch files?
There was a problem hiding this comment.
I'd have to run some tests to figure this one out, but it is possible that specifying a PHP version in the phpcs configuration (like <config name="php_version" value="70200"/>) would fix this. IIRC it depends on the sniff actively checking for it though, so it may not work for every sniff. That said, I'm fine excluding such sniffs manually until we upgrade to the required PHP version.
There was a problem hiding this comment.
Ah, that makes sense. I do see a PHP version check in RequireTrailingCommaInCallSniff.
Per our earlier Slack conversation where you said:
I can only imagine it’s an issue due to us running phpcs on PHP 7.4 (e.g. an older version of phpdoc-parser that doesn’t support object shapes). You can try adding
<config name="php_version" value="70400"/>to phpcs.xml.dist and then running phpcs on a newer PHP version to see if that fixes the issue
I misunderstood this and thought we were running phpcs with syntax rules for 7.4, which isn't the case. I now see you were just referring to the GitHub Action running phpcs through PHP 7.4.
I'll play around with this PR some more. I think adding <config name="php_version" value="70200"/> (and keeping that in sync with our minimum supported version) would be best if that allows us to remove the "Exclude sniffs that require newer PHP versions" block here.
There was a problem hiding this comment.
Yes, I believe I confused this with psalm where the phpVersion config does something else. Adding the php_version configuration and keeping it in sync with our minimum required version sounds sensible to me.
| /** | ||
| * @see https://php.net/iterator.valid | ||
| */ | ||
| /** @see https://php.net/iterator.valid */ |
There was a problem hiding this comment.
These fixes came from SlevomatCodingStandard.Commenting.RequireOneLineDocComment.
I don't mind them applying to src/. I think they make the most sense in tests/ where we had a lot of @dataProvider annotations taking up multiple lines.
| @@ -199,6 +197,7 @@ public function next(): void | |||
|
|
|||
| try { | |||
| parent::next(); | |||
|
|
|||
There was a problem hiding this comment.
From SlevomatCodingStandard.Classes.ParentCallSpacing.
I don't feel strongly about this and it was a small change affecting only this file.
Also bumps phpcs to ^3.7
This ensures consistent reporting regardless of the PHP version used to execute phpcs. It also allows removing excludes for sniffs that apply to newer syntax.
PHPCBF RESULT SUMMARY ---------------------------------------------------------------------- FILE FIXED REMAINING ---------------------------------------------------------------------- src/Model/ChangeStreamIterator.php 2 0 ---------------------------------------------------------------------- A TOTAL OF 2 ERRORS WERE FIXED IN 1 FILE ---------------------------------------------------------------------- Time: 7.86 secs; Memory: 10MB
PHPCBF RESULT SUMMARY ------------------------------------------------------------------------- FILE FIXED REMAINING ------------------------------------------------------------------------- tests/Operation/DropCollectionTest.php 1 0 tests/Operation/ListIndexesTest.php 1 0 tests/Operation/EstimatedDocumentCountTest.php 1 0 tests/Operation/RenameCollectionTest.php 1 0 tests/Command/ListDatabasesTest.php 1 0 tests/Operation/ModifyCollectionTest.php 1 0 tests/Operation/ExplainTest.php 1 0 tests/Operation/InsertOneTest.php 2 0 tests/Operation/UpdateManyTest.php 3 0 tests/Operation/CountTest.php 2 0 tests/Operation/UpdateOneTest.php 3 0 tests/Operation/CreateIndexesTest.php 2 0 tests/UnifiedSpecTests/FailPointObserver.php 3 0 tests/Operation/DeleteTest.php 4 0 tests/Operation/ReplaceOneTest.php 3 0 src/Model/CallbackIterator.php 3 0 tests/Command/ListCollectionsTest.php 1 0 src/InsertOneResult.php 1 0 tests/PedantryTest.php 1 0 tests/Operation/DropIndexesTest.php 1 0 tests/Operation/WatchTest.php 1 0 tests/GridFS/WritableStreamFunctionalTest.php 3 0 src/Model/DatabaseInfo.php 1 0 tests/Operation/UpdateTest.php 3 0 tests/Operation/FindOneFunctionalTest.php 1 0 tests/Operation/InsertManyTest.php 2 0 tests/Operation/DropEncryptedCollectionTest.php 1 0 tests/Operation/CreateEncryptedCollectionTest.php 1 0 tests/Operation/FindOneAndUpdateTest.php 4 0 tests/Operation/FindAndModifyTest.php 1 0 src/Model/CachingIterator.php 4 0 tests/Model/IndexInputTest.php 2 0 tests/Operation/FindOneAndReplaceTest.php 4 0 tests/Operation/DropDatabaseTest.php 1 0 src/ChangeStream.php 1 0 tests/Operation/InsertOneFunctionalTest.php 3 0 tests/Operation/InsertManyFunctionalTest.php 2 0 src/Operation/WithTransaction.php 1 0 tests/Model/BSONIteratorTest.php 1 0 tests/Operation/DistinctTest.php 2 0 tests/Operation/AggregateTest.php 1 0 tests/Operation/DropDatabaseFunctionalTest.php 1 0 src/Model/ChangeStreamIterator.php 3 0 tests/UnifiedSpecTests/EventCollector.php 3 0 src/Model/IndexInfo.php 1 0 tests/SpecTests/ReadWriteConcernSpecTest.php 1 0 tests/Operation/DropCollectionFunctionalTest.php 1 0 src/Model/BSONIterator.php 1 0 tests/Operation/CreateCollectionTest.php 1 0 tests/UnifiedSpecTests/EntityMap.php 3 0 tests/Model/ChangeStreamIteratorTest.php 3 0 tests/Operation/FindAndModifyFunctionalTest.php 2 0 tests/UnifiedSpecTests/Constraint/IsBsonTypeTest.php 2 0 tests/Operation/DeleteFunctionalTest.php 1 0 tests/GridFS/BucketFunctionalTest.php 15 0 tests/FunctionsTest.php 9 0 tests/Exception/InvalidArgumentExceptionTest.php 1 0 tests/Operation/FindTest.php 3 0 tests/Operation/UpdateFunctionalTest.php 4 0 tests/UnifiedSpecTests/UnifiedSpecTest.php 10 0 src/Model/CollectionInfo.php 1 0 tests/Operation/DatabaseCommandTest.php 2 0 tests/SpecTests/DocumentsMatchConstraintTest.php 2 0 tests/SpecTests/TransactionsSpecTest.php 1 0 tests/Operation/BulkWriteTest.php 20 0 tests/GridFS/ReadableStreamFunctionalTest.php 6 0 tests/Database/DatabaseFunctionalTest.php 3 0 tests/Operation/AggregateFunctionalTest.php 1 0 src/GridFS/WritableStream.php 1 0 tests/Operation/MapReduceFunctionalTest.php 2 0 tests/Operation/DistinctFunctionalTest.php 1 0 tests/UnifiedSpecTests/EventObserver.php 3 0 tests/Collection/CrudSpecFunctionalTest.php 1 0 tests/Operation/FindFunctionalTest.php 1 0 tests/Operation/FindOneAndDeleteTest.php 2 0 src/Operation/MapReduce.php 1 0 tests/Operation/CountDocumentsTest.php 2 0 tests/Operation/ExplainFunctionalTest.php 15 0 tests/Operation/MapReduceTest.php 2 0 tests/SpecTests/ErrorExpectation.php 2 0 tests/UnifiedSpecTests/Constraint/MatchesTest.php 2 0 tests/SpecTests/Operation.php 4 0 tests/Operation/BulkWriteFunctionalTest.php 6 0 tests/SpecTests/PrimaryStepDownSpecTest.php 6 0 tests/ClientTest.php 2 0 tests/Collection/CollectionFunctionalTest.php 8 0 tests/DocumentationExamplesTest.php 3 0 tests/Operation/WatchFunctionalTest.php 1 0 ------------------------------------------------------------------------- A TOTAL OF 242 ERRORS WERE FIXED IN 88 FILES ------------------------------------------------------------------------- Time: 13.38 secs; Memory: 10MB
https://jira.mongodb.org/browse/PHPLIB-1106
This PR is based on the process from 903e101 (previous upgrade).