From b1d744768ae8fbb01e066b91380b6a3646968144 Mon Sep 17 00:00:00 2001
From: Ted Bowman <41201-tedbow@users.noreply.drupalcode.org>
Date: Thu, 23 Feb 2023 19:03:45 +0000
Subject: [PATCH] =?UTF-8?q?Issue=20#3342120=20by=20Wim=20Leers,=20yash.rod?=
=?UTF-8?q?e,=20tedbow,=20phenaproxima:=20Fix=20PHPStan=20violations=20(ha?=
=?UTF-8?q?ppened=20because=20PHPStan=20code=20checks=20stopped=20running?=
=?UTF-8?q?=E2=80=A6)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
---
.../src/BatchProcessor.php | 2 +-
.../src/Form/UpdaterForm.php | 2 +-
.../tests/src/Traits/FormTestTrait.php | 2 ++
.../src/FixtureManipulator.php | 2 +-
.../src/Kernel/PackageManagerKernelTestBase.php | 5 +----
.../tests/src/Kernel/StageOwnershipTest.php | 1 +
.../tests/src/Kernel/SymlinkValidatorTest.php | 2 ++
.../tests/src/Traits/ValidationTestTrait.php | 5 +++++
phpstan.neon.dist | 9 +++++++++
scripts/commit-code-check.sh | 7 +++----
src/BatchProcessor.php | 2 +-
src/Form/UpdaterForm.php | 2 +-
src/Validation/StatusChecker.php | 2 +-
src/Validator/StagedProjectsValidator.php | 15 +++++++--------
tests/src/Functional/ClickableHelpTest.php | 7 +++----
.../Kernel/StatusCheck/XdebugValidatorTest.php | 2 +-
16 files changed, 40 insertions(+), 27 deletions(-)
create mode 100644 phpstan.neon.dist
diff --git a/automatic_updates_extensions/src/BatchProcessor.php b/automatic_updates_extensions/src/BatchProcessor.php
index 9dbd00f0b5..bf2f48a37a 100644
--- a/automatic_updates_extensions/src/BatchProcessor.php
+++ b/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) {
diff --git a/automatic_updates_extensions/src/Form/UpdaterForm.php b/automatic_updates_extensions/src/Form/UpdaterForm.php
index 21e11646b2..dec4b3fc60 100644
--- a/automatic_updates_extensions/src/Form/UpdaterForm.php
+++ b/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);
diff --git a/automatic_updates_extensions/tests/src/Traits/FormTestTrait.php b/automatic_updates_extensions/tests/src/Traits/FormTestTrait.php
index af3ee30b6d..6835ddebe9 100644
--- a/automatic_updates_extensions/tests/src/Traits/FormTestTrait.php
+++ b/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);
}
diff --git a/package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php b/package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php
index 5392844db2..e85752cdc9 100644
--- a/package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php
+++ b/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)) {
diff --git a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
index 6635bf52c6..95fc412280 100644
--- a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
+++ b/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 = [
diff --git a/package_manager/tests/src/Kernel/StageOwnershipTest.php b/package_manager/tests/src/Kernel/StageOwnershipTest.php
index 27e5c310a6..a8de6438c7 100644
--- a/package_manager/tests/src/Kernel/StageOwnershipTest.php
+++ b/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;
}
/**
diff --git a/package_manager/tests/src/Kernel/SymlinkValidatorTest.php b/package_manager/tests/src/Kernel/SymlinkValidatorTest.php
index b753df107b..35011a8b53 100644
--- a/package_manager/tests/src/Kernel/SymlinkValidatorTest.php
+++ b/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());
diff --git a/package_manager/tests/src/Traits/ValidationTestTrait.php b/package_manager/tests/src/Traits/ValidationTestTrait.php
index 035c08f7e6..c3f27a1c67 100644
--- a/package_manager/tests/src/Traits/ValidationTestTrait.php
+++ b/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) {
diff --git a/phpstan.neon.dist b/phpstan.neon.dist
new file mode 100644
index 0000000000..fd16bd0054
--- /dev/null
+++ b/phpstan.neon.dist
@@ -0,0 +1,9 @@
+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
diff --git a/scripts/commit-code-check.sh b/scripts/commit-code-check.sh
index 3c77f47f28..91d0b7bc47 100755
--- a/scripts/commit-code-check.sh
+++ b/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 -
diff --git a/src/BatchProcessor.php b/src/BatchProcessor.php
index 657d69e25b..e991f25639 100644
--- a/src/BatchProcessor.php
+++ b/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) {
diff --git a/src/Form/UpdaterForm.php b/src/Form/UpdaterForm.php
index 4a569035ec..a6212210a0 100644
--- a/src/Form/UpdaterForm.php
+++ b/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();
diff --git a/src/Validation/StatusChecker.php b/src/Validation/StatusChecker.php
index a8b4c5f966..c98f561c20 100644
--- a/src/Validation/StatusChecker.php
+++ b/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',
diff --git a/src/Validator/StagedProjectsValidator.php b/src/Validator/StagedProjectsValidator.php
index e7863e0531..1fc86f7ab9 100644
--- a/src/Validator/StagedProjectsValidator.php
+++ b/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);
}
}
diff --git a/tests/src/Functional/ClickableHelpTest.php b/tests/src/Functional/ClickableHelpTest.php
index a87dde2e1f..685855f043 100644
--- a/tests/src/Functional/ClickableHelpTest.php
+++ b/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();
diff --git a/tests/src/Kernel/StatusCheck/XdebugValidatorTest.php b/tests/src/Kernel/StatusCheck/XdebugValidatorTest.php
index 0a53dc20e7..f7f87dacac 100644
--- a/tests/src/Kernel/StatusCheck/XdebugValidatorTest.php
+++ b/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);
--
GitLab