Issue #3342120 by Wim Leers, yash.rode, tedbow, phenaproxima: Fix PHPStan...

Issue #3342120 by Wim Leers, yash.rode, tedbow, phenaproxima: Fix PHPStan violations (happened because PHPStan code checks stopped running…)

automatic_updates_extensions/src/BatchProcessor.php

@@ -96,8 +96,8 @@ final class BatchProcessor {
    * @see \Drupal\automatic_updates\Updater::stage()
    */
   public static function stage(array &$context): void {
+    $stage_id = \Drupal::service('session')->get(static::STAGE_ID_SESSION_KEY);
     try {
-      $stage_id = \Drupal::service('session')->get(static::STAGE_ID_SESSION_KEY);
       static::getUpdater()->claim($stage_id)->stage();
     }
     catch (\Throwable $e) {

automatic_updates_extensions/src/Form/UpdaterForm.php

@@ -164,7 +164,7 @@ final class UpdaterForm extends UpdateFormBase {
       $results = [];
     }
     else {
-      $results = $this->runStatusCheck($this->extensionUpdater, $this->eventDispatcher, TRUE);
+      $results = $this->runStatusCheck($this->extensionUpdater, $this->eventDispatcher);
     }
     $this->displayResults($results, $this->renderer);
     $security_level = ValidationResult::getOverallSeverity($results);

automatic_updates_extensions/tests/src/Traits/FormTestTrait.php

@@ -5,6 +5,7 @@ declare(strict_types = 1);
 namespace Drupal\Tests\automatic_updates_extensions\Traits;
 
 use Behat\Mink\WebAssert;
+use Drupal\Tests\BrowserTestBase;
 
 /**
  * Common methods for testing the update form.
@@ -41,6 +42,7 @@ trait FormTestTrait {
    *   The no of rows in table.
    */
   protected function assertUpdatesCount(int $expected_update_count): void {
+    assert($this instanceof BrowserTestBase);
     $this->assertSession()->elementsCount('css', '.update-recommended tbody tr', $expected_update_count);
   }
 

package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php

@@ -214,7 +214,7 @@ class FixtureManipulator {
       $data['dev-package-names'] = array_values($data['dev-package-names']);
     }
     // Add the package back to the list, if we have data for it.
-    if (isset($package)) {
+    if (isset($install_json_package)) {
       $data['packages'][] = $install_json_package;
 
       if ($is_dev_requirement || !empty($is_existing_dev_package)) {

package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php

@@ -276,10 +276,7 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase {
 
     // This validator will persist through container rebuilds.
     // @see ::register()
-    $validator = new TestDiskSpaceValidator(
-      $path_locator,
-      $this->container->get('string_translation')
-    );
+    $validator = new TestDiskSpaceValidator($path_locator);
     // By default, the validator should report that the root, vendor, and
     // temporary directories have basically infinite free space.
     $validator->freeSpace = [

package_manager/tests/src/Kernel/StageOwnershipTest.php

@@ -267,6 +267,7 @@ class StageOwnershipTest extends PackageManagerKernelTestBase {
         // don't wan't to do that in this test, since we're specifically testing
         // what happens when we try to delete a stage directory with
         // write-protected files.
+        return TRUE;
       }
 
       /**

package_manager/tests/src/Kernel/SymlinkValidatorTest.php

@@ -17,6 +17,7 @@ use PhpTuf\ComposerStager\Domain\Value\Path\PathInterface;
 use PhpTuf\ComposerStager\Domain\Value\PathList\PathListInterface;
 use PHPUnit\Framework\Assert;
 use Prophecy\Argument;
+use Prophecy\Prophecy\ObjectProphecy;
 
 /**
  * @covers \Drupal\package_manager\Validator\SymlinkValidator
@@ -105,6 +106,7 @@ class SymlinkValidatorTest extends PackageManagerKernelTestBase {
       // Ensure that the Composer Stager's symlink precondition is invoked.
       $this->precondition->assertIsFulfilled(...$arguments)
         ->will(function (array $arguments) use ($symlinks_exist): void {
+          assert($this instanceof ObjectProphecy);
           // Ensure that 'ignore/me' is present in ignored paths.
           Assert::assertContains('ignore/me', $arguments[2]->getAll());
 

package_manager/tests/src/Traits/ValidationTestTrait.php

@@ -5,8 +5,10 @@ declare(strict_types = 1);
 namespace Drupal\Tests\package_manager\Traits;
 
 use Drupal\Core\StringTranslation\TranslatableMarkup;
+use Drupal\KernelTests\KernelTestBase;
 use Drupal\package_manager\PathLocator;
 use Drupal\package_manager\ValidationResult;
+use Drupal\Tests\BrowserTestBase;
 use Drupal\Tests\UnitTestCase;
 
 /**
@@ -64,6 +66,8 @@ trait ValidationTestTrait {
    */
   protected function resolvePlaceholdersInArrayValuesWithRealPaths(array $subject, ?PathLocator $path_locator = NULL, ?string $stage_dir = NULL): array {
     if (!$path_locator) {
+      // Only kernel and browser tests have $this->container.
+      assert($this instanceof KernelTestBase || $this instanceof BrowserTestBase);
       $path_locator = $this->container->get('package_manager.path_locator');
     }
     $subject = str_replace(
@@ -97,6 +101,7 @@ trait ValidationTestTrait {
   protected function getValidationResultsAsArray(array $results): array {
     $string_translation_stub = NULL;
     if (is_a(get_called_class(), UnitTestCase::class, TRUE)) {
+      assert($this instanceof UnitTestCase);
       $string_translation_stub = $this->getStringTranslationStub();
     }
     return array_values(array_map(static function (ValidationResult $result) use ($string_translation_stub) {

phpstan.neon.dist

+includes:
+	- %rootDir%/../../../core/phpstan.neon.dist
+
+parameters:
+  ignoreErrors:
+    # Drupal core's PHPStan config file ignores the non-anonymous variant of this.
+    - "#^Anonymous class extends @internal class#"
+  # Drupal core needs to ignore more things than we need to!
+  reportUnmatchedIgnoredErrors: false

scripts/commit-code-check.sh

@@ -95,10 +95,9 @@ print_title "[1/4] PHPCS"
 $CORE_DIRECTORY/vendor/bin/phpcs $MODULE_DIRECTORY -ps --standard="$CORE_DIRECTORY/core/phpcs.xml.dist"
 print_results $? "PHPCS"
 
-# @todo Uncomment in https://drupal.org/i/3342120
-# print_title "[2/4] PHPStan"
-# php -d apc.enabled=0 -d apc.enable_cli=0 $CORE_DIRECTORY/vendor/bin/phpstan analyze --no-progress --configuration="$CORE_DIRECTORY/core/phpstan.neon.dist" $MODULE_DIRECTORY
-# print_results $? "PHPStan"
+print_title "[2/4] PHPStan"
+php -d apc.enabled=0 -d apc.enable_cli=0 $CORE_DIRECTORY/vendor/bin/phpstan analyze --no-progress --configuration="$MODULE_DIRECTORY/phpstan.neon.dist" $MODULE_DIRECTORY
+print_results $? "PHPStan"
 
 print_title "[3/4] CSpell"
 cd $CORE_DIRECTORY/core && yarn run -s spellcheck --no-progress --root $MODULE_DIRECTORY -c .cspell.json "**" && cd -

src/BatchProcessor.php

@@ -106,8 +106,8 @@ final class BatchProcessor {
    * @see \Drupal\automatic_updates\Updater::stage()
    */
   public static function stage(array &$context): void {
+    $stage_id = \Drupal::service('session')->get(static::STAGE_ID_SESSION_KEY);
     try {
-      $stage_id = \Drupal::service('session')->get(static::STAGE_ID_SESSION_KEY);
       static::getUpdater()->claim($stage_id)->stage();
     }
     catch (\Throwable $e) {

src/Form/UpdaterForm.php

@@ -152,7 +152,7 @@ final class UpdaterForm extends UpdateFormBase {
       $results = [];
     }
     else {
-      $results = $this->runStatusCheck($this->updater, $this->eventDispatcher, TRUE);
+      $results = $this->runStatusCheck($this->updater, $this->eventDispatcher);
     }
     $this->displayResults($results, $this->renderer);
     $project = $project_info->getProjectInfo();

src/Validation/StatusChecker.php

@@ -69,7 +69,7 @@ final class StatusChecker implements EventSubscriberInterface {
     else {
       $stage = $this->cronUpdater;
     }
-    $results = $this->runStatusCheck($stage, $this->eventDispatcher, TRUE);
+    $results = $this->runStatusCheck($stage, $this->eventDispatcher);
 
     $this->keyValueExpirable->setWithExpire(
       'status_check_last_run',

src/Validator/StagedProjectsValidator.php

@@ -103,6 +103,7 @@ final class StagedProjectsValidator implements EventSubscriberInterface {
     if ($updated_packages = array_filter($updated_packages, $filter)) {
       $staged_packages = $stage->getInstalledPackages();
 
+      $version_change_messages = [];
       foreach ($updated_packages as $name => $updated_package) {
         $version_change_messages[] = $this->t(
           "@type '@name' from @active_version to @staged_version.",
@@ -114,14 +115,12 @@ final class StagedProjectsValidator implements EventSubscriberInterface {
           ]
         );
       }
-      if (!empty($version_change_messages)) {
-        $version_change_summary = $this->formatPlural(
-          count($version_change_messages),
-          'The update cannot proceed because the following Drupal project was unexpectedly updated. Only Drupal Core updates are currently supported.',
-          'The update cannot proceed because the following Drupal projects were unexpectedly updated. Only Drupal Core updates are currently supported.'
-        );
-        $event->addError($version_change_messages, $version_change_summary);
-      }
+      $version_change_summary = $this->formatPlural(
+        count($version_change_messages),
+        'The update cannot proceed because the following Drupal project was unexpectedly updated. Only Drupal Core updates are currently supported.',
+        'The update cannot proceed because the following Drupal projects were unexpectedly updated. Only Drupal Core updates are currently supported.'
+      );
+      $event->addError($version_change_messages, $version_change_summary);
     }
   }
 

tests/src/Functional/ClickableHelpTest.php

@@ -32,16 +32,15 @@ class ClickableHelpTest extends AutomaticUpdatesFunctionalTestBase {
     unset($this->disableValidators[array_search('package_manager.validator.composer_executable', $this->disableValidators)]);
     parent::setUp();
     $this->setReleaseMetadata(__DIR__ . '/../../../package_manager/tests/fixtures/release-history/drupal.9.8.1-security.xml');
-    $this->checkerRunnerUser = $this->createUser([
-      'administer site configuration',
-    ]);
   }
 
   /**
    * Tests if composer executable is not present then the help link clickable.
    */
   public function testHelpLinkClickable(): void {
-    $this->drupalLogin($this->checkerRunnerUser);
+    $this->drupalLogin($this->createUser([
+      'administer site configuration',
+    ]));
     $this->config('package_manager.settings')
       ->set('executables.composer', '/not/matching/path/to/composer')
       ->save();

tests/src/Kernel/StatusCheck/XdebugValidatorTest.php

@@ -50,7 +50,7 @@ class XdebugValidatorTest extends AutomaticUpdatesKernelTestBase {
     $stage->require(['drupal/random']);
     $this->assertUpdateStagedTimes(1);
     $event_dispatcher = \Drupal::service('event_dispatcher');
-    $result = $this->runStatusCheck($stage, $event_dispatcher, TRUE);
+    $result = $this->runStatusCheck($stage, $event_dispatcher);
     $this->assertSame($message->getUntranslatedString(), $result[0]->getMessages()[0]->getUntranslatedString());
     $stage->destroy(TRUE);