PHPLIB-1141: Setup rector #1088

GromNaN · 2023-05-30T15:46:59Z

The library uses rector to refactor the code for new features.
To run automatic refactoring, use the rector command:

$ vendor/bin/rector

Job merged with psalm: https://github.com/mongodb/mongo-php-library/actions/runs/5134018025/jobs/9237409213?pr=1088

The rules need to be adjusted due to false positive like this:

The 3rd argument is correctly defined in ext-mongodb stub.

src/Operation/Delete.php:164

    ---------- begin diff ----------
@@ @@
         $bulk = new Bulk($this->createBulkWriteOptions());
         $bulk->delete($this->filter, $this->createDeleteOptions());

-        $writeResult = $server->executeBulkWrite($this->databaseName . '.' . $this->collectionName, $bulk, $this->createExecuteOptions());
+        $writeResult = $server->executeBulkWrite($this->databaseName . '.' . $this->collectionName, $bulk);

         return new DeleteResult($writeResult);
     }
    ----------- end diff -----------

The variable is always initialized when accessed.

tests/SpecTests/ClientSideEncryptionSpecTest.php:308

    ---------- begin diff ----------
@@ @@
      */
     public function testClientSideEncryption(stdClass $test, ?array $runOn, array $data, ?stdClass $encryptedFields = null, ?array $keyVaultData = null, ?stdClass $jsonSchema = null, ?string $databaseName = null, ?string $collectionName = null): void
     {
+        $context = null;
         if (isset(self::$incompleteTests[$this->dataDescription()])) {
             $this->markTestIncomplete(self::$incompleteTests[$this->dataDescription()]);
         }
@@ @@

     private function encryptCorpusValue(string $fieldName, stdClass $data, ClientEncryption $clientEncryption)
     {
+        $encrypted = null;
         $encryptionOptions = [
             'algorithm' => $data->algo === 'rand' ? ClientEncryption::AEAD_AES_256_CBC_HMAC_SHA_512_RANDOM : ClientEncryption::AEAD_AES_256_CBC_HMAC_SHA_512_DETERMINISTIC,
         ];
    ----------- end diff -----------

GromNaN · 2023-05-31T14:32:52Z

examples/aggregate.php

@@ -28,7 +28,7 @@ function toJSON(object $document): string
 $documents = [];

 for ($i = 0; $i < 100; $i++) {
-    $documents[] = ['randomValue' => rand(0, 1000)];
+    $documents[] = ['randomValue' => random_int(0, 1000)];


https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#randomfunctionrector

This seems a bit overzealous, as we don't actually need a cryptographically secure number here. But it's also just an example file so I don't feel strongly about it.

That's the new "best practice". Using it proves that the doc have been updated 😉

GromNaN · 2023-05-31T14:33:55Z

tests/GridFS/UnusableStream.php

@@ -20,7 +20,7 @@ public static function register($protocol = 'unusable'): void
            stream_wrapper_unregister($protocol);
        }

-        stream_wrapper_register($protocol, static::class, STREAM_IS_URL);
+        stream_wrapper_register($protocol, self::class, STREAM_IS_URL);


static is useless for a final class. self can be resolved at compile time.

GromNaN · 2023-05-31T14:34:40Z

tests/UnifiedSpecTests/Constraint/Matches.php

@@ -368,6 +368,7 @@ private function doToString()
        return 'matches ' . $this->exporter()->export($this->value);
    }

+    /** @psalm-return never-return */


Helps to detect code paths and avoid false-positive undefined variable access.

tests/GridFS/BucketFunctionalTest.php

jmikola · 2023-06-01T02:48:06Z

The variable is always initialized when accessed (in ClientSideEncryptionSpecTest)

Are those two examples due to the tooling not realizing that fail() and markTestIncomplete() throw?

jmikola · 2023-06-01T02:36:09Z

examples/aggregate.php

@@ -28,7 +28,7 @@ function toJSON(object $document): string
 $documents = [];

 for ($i = 0; $i < 100; $i++) {
-    $documents[] = ['randomValue' => rand(0, 1000)];
+    $documents[] = ['randomValue' => random_int(0, 1000)];


This seems a bit overzealous, as we don't actually need a cryptographically secure number here. But it's also just an example file so I don't feel strongly about it.

jmikola · 2023-06-01T02:36:46Z

rector.php

+        AddDefaultValueForUndefinedVariableRector::class => [
+            __DIR__ . '/tests/',
+        ],
+        // @see https://github.com/phpstan/phpstan-src/pull/2429


Would See: make more sense here since we're not in an actual doc block?

tests/GridFS/BucketFunctionalTest.php

jmikola · 2023-06-01T02:41:16Z

tests/Operation/WatchFunctionalTest.php

@@ -1107,8 +1107,7 @@ public function testResumeTokenNotFoundDoesNotAdvanceKey(): void
        try {
            $changeStream->next();
            $this->fail('Exception for missing resume token was not thrown');
-        } catch (ResumeTokenException $e) {
-        } catch (ServerException $e) {
+        } catch (ResumeTokenException | ServerException $e) {


Ah, I missed that this was available since 7.1 (long before general union types in 8.0): https://wiki.php.net/rfc/multiple-catch

jmikola · 2023-06-01T02:50:55Z

.github/workflows/static-analysis.yml

@@ -65,3 +65,6 @@ jobs:

      - name: "Run Psalm"
        run: "vendor/bin/psalm --show-info=false --stats --output-format=github --threads=$(nproc)"
+
+      - name: "Run Rector"
+        run: "vendor/bin/rector --ansi --dry-run"


Noted that --dry-run returns a non-zero exit code if changes are detected, which makes it suitable for CI.

tests/GridFS/BucketFunctionalTest.php

…lenameAndRevisionToExist

GromNaN · 2023-06-01T10:43:17Z

Are those two examples due to the tooling not realizing that fail() and markTestIncomplete() throw?

PHPUnit 9.6 had the annotation @psalm-return never-return that was then replaced by :never native return type.

People seems to have the opposite behavior: phpstan/phpstan-phpunit#52

GromNaN force-pushed the PHPLIB-1141 branch 2 times, most recently from 8789703 to 8d5f04c Compare May 30, 2023 15:54

GromNaN marked this pull request as draft May 30, 2023 16:02

GromNaN force-pushed the PHPLIB-1141 branch from 07fd33e to 3fc6d54 Compare May 31, 2023 08:14

GromNaN mentioned this pull request May 31, 2023

Fix MongoDB Manager stubs phpstan/phpstan-src#2429

Closed

GromNaN force-pushed the PHPLIB-1141 branch 3 times, most recently from 7905ad1 to 7da6c97 Compare May 31, 2023 14:46

GromNaN commented May 31, 2023

View reviewed changes

GromNaN requested a review from jmikola May 31, 2023 14:47

GromNaN marked this pull request as ready for review May 31, 2023 14:48

GromNaN requested a review from alcaeus May 31, 2023 14:48

GromNaN force-pushed the PHPLIB-1141 branch from 7da6c97 to db23ced Compare May 31, 2023 14:50

jmikola reviewed Jun 1, 2023

View reviewed changes

GromNaN added 5 commits June 1, 2023 12:20

Setup rector config

5b9420b

Setup rector in CI

0c29d03

Merge rector into static analysis job

d4905d7

Disable bugged rules

2688276

Fix BucketFunctionalTest::testOpenDownloadStreamByNameShouldRequireFi…

219b648

…lenameAndRevisionToExist

GromNaN force-pushed the PHPLIB-1141 branch from db23ced to 219b648 Compare June 1, 2023 10:26

GromNaN merged commit 9d4e7b8 into mongodb:master Jun 1, 2023

GromNaN deleted the PHPLIB-1141 branch June 1, 2023 10:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PHPLIB-1141: Setup rector #1088

PHPLIB-1141: Setup rector #1088

GromNaN commented May 30, 2023 •

edited

Loading

GromNaN May 31, 2023

jmikola Jun 1, 2023

GromNaN Jun 1, 2023

GromNaN May 31, 2023

GromNaN May 31, 2023

jmikola commented Jun 1, 2023

jmikola Jun 1, 2023

jmikola Jun 1, 2023

jmikola Jun 1, 2023

jmikola Jun 1, 2023

GromNaN commented Jun 1, 2023

PHPLIB-1141: Setup rector #1088

PHPLIB-1141: Setup rector #1088

Conversation

GromNaN commented May 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmikola commented Jun 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GromNaN commented Jun 1, 2023

GromNaN commented May 30, 2023 •

edited

Loading