tests(phpstan): Enable PHPStan level 2 (#1083)

klausi · web-flow · commit 69ccbbd78d0c · 2020-10-17T16:01:56.000+02:00
diff --git a/.travis.yml b/.travis.yml
@@ -88,21 +88,26 @@ install:
   # We don't care about any dirty changes in the vendor directory, throw them
   # all away.
   - composer --working-dir=$DRUPAL_BUILD_DIR config discard-changes true
-  # Bring in the module dependencies without requiring a merge plugin. The
-  # require also triggers a full 'composer install'.
-  - composer --no-interaction --working-dir=$DRUPAL_BUILD_DIR require webonyx/graphql-php:^0.13.1 drupal/typed_data:^1.0
+  - composer --no-interaction --working-dir=$DRUPAL_BUILD_DIR install
+  # Make sure to work with the latest PHPUnit version in Drupal 8.
   - composer --no-interaction --working-dir=$DRUPAL_BUILD_DIR run-script drupal-phpunit-upgrade
+  # Bring in the module dependencies without requiring a merge plugin.
+  # Require redirect module to prevent PHPStan error on an optional dependency.
+  # Install PHPStan to check for Drupal standards.
+  - composer --no-interaction --working-dir=$DRUPAL_BUILD_DIR require \
+    webonyx/graphql-php:^0.13.1 \
+    drupal/typed_data:^1.0 \
+    drupal/redirect:^1.6 \
+    phpstan/phpstan:^0.12.50 \
+    mglaman/phpstan-drupal:^0.12.3 \
+    phpstan/phpstan-deprecation-rules:^0.12.2 \
+    jangregor/phpstan-prophecy:^0.8 \
+    phpstan/phpstan-phpunit:^0.12
 
   # Install PHPCS to check for Drupal coding standards.
   - travis_retry composer global require drupal/coder:8.3.10
   - $HOME/.composer/vendor/bin/phpcs --config-set installed_paths $HOME/.composer/vendor/drupal/coder/coder_sniffer
 
-  # Install Phpstan to check for Drupal standards.
-  - composer --no-interaction --working-dir=$DRUPAL_BUILD_DIR require mglaman/phpstan-drupal ^0.12.3 phpstan/phpstan-deprecation-rules ^0.12.2
-  # Require redirect module to prevent PHPStan error on optional dependency
-  # injection of RedirectRepository in RouteLoad data producer.
-  - composer --no-interaction --working-dir=$DRUPAL_BUILD_DIR require drupal/redirect
-
 script:
   # Run the unit tests using phpdbg if the environment variable is 'true'.
   - if [[ "$WITH_PHPDBG_COVERAGE" == "true" ]];
diff --git a/phpcs.xml.dist b/phpcs.xml.dist
@@ -8,6 +8,14 @@
   <rule ref="Drupal"></rule>
   <rule ref="DrupalPractice"></rule>
 
+  <!-- TODO: Bug in Coder with variadic parameters -->
+  <rule ref="Drupal.Commenting.FunctionComment.ParamTypeSpaces">
+    <exclude-pattern>src/GraphQL/ResolverBuilder.php</exclude-pattern>
+  </rule>
+  <rule ref="Drupal.Commenting.FunctionComment.SpacingAfterParamType">
+    <exclude-pattern>src/GraphQL/ResolverBuilder.php</exclude-pattern>
+  </rule>
+
   <!-- TODO: those rules are disabled for now until we fix the coding standards for them. -->
   <rule ref="Drupal.Commenting.ClassComment.Missing">
     <exclude-pattern>src/PermissionProvider.php</exclude-pattern>
diff --git a/phpstan.neon b/phpstan.neon
@@ -1,12 +1,14 @@
 includes:
 	- ../../vendor/phpstan/phpstan-deprecation-rules/rules.neon
 	- ../../vendor/mglaman/phpstan-drupal/extension.neon
+	- ../../vendor/jangregor/phpstan-prophecy/extension.neon
+	- ../../vendor/phpstan/phpstan-phpunit/extension.neon
 
 parameters:
 	# PHPStan cannot find files in this test directory automatically.
 	scanDirectories:
 		- ../../core/tests/Drupal/Tests
-	level: 1
+	level: 2
 	customRulesetUsed: true
 	paths:
 		- .
@@ -26,3 +28,4 @@ parameters:
 		# Symfony 4 deprecations in Drupal 9 that we don't care about.
 		- "#deprecated class Symfony\\\\Component\\\\EventDispatcher\\\\Event#"
 		- "#deprecated class Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\GetResponseEvent#"
+		- "#^Method Symfony\\\\Contracts\\\\EventDispatcher\\\\EventDispatcherInterface\\:\\:dispatch\\(\\) invoked with 2 parameters, 1 required\\.$#"
diff --git a/src/Access/QueryAccessCheck.php b/src/Access/QueryAccessCheck.php
@@ -51,7 +51,8 @@ public function access(AccountInterface $account, ServerInterface $graphql_serve
 
     $request = $this->requestStack->getCurrentRequest();
     /** @var \GraphQL\Server\OperationParams[] $operations */
-    if (!$operations = $request->attributes->get('operations', [])) {
+    $operations = $request->attributes->get('operations', []);
+    if (!$operations) {
       return AccessResult::forbidden();
     }
 
diff --git a/src/Form/PersistedQueriesForm.php b/src/Form/PersistedQueriesForm.php
@@ -17,6 +17,13 @@ class PersistedQueriesForm extends EntityForm {
    */
   protected $persistedQueryPluginManager;
 
+  /**
+   * The entity being used by this form.
+   *
+   * @var \Drupal\graphql\Entity\Server
+   */
+  protected $entity;
+
   /**
    * PersistedQueriesForm constructor.
    *
@@ -48,7 +55,7 @@ public function getFormId() {
   public function form(array $form, FormStateInterface $form_state) {
     $form = parent::form($form, $form_state);
 
-    /** @var PersistedQueryPluginInterface[] $plugins */
+    /** @var \Drupal\graphql\Plugin\PersistedQueryPluginInterface[] $plugins */
     $plugins = $this->entity->getPersistedQueryInstances();
     $all_plugins = $this->getAllPersistedQueryPlugins();
     $form['#tree'] = TRUE;
diff --git a/src/Form/ServerForm.php b/src/Form/ServerForm.php
@@ -218,8 +218,8 @@ public function validateForm(array &$form, FormStateInterface $formState) {
       $formState->setErrorByName('endpoint', 'The endpoint path contains invalid characters.');
     }
 
-    /** @var \Drupal\graphql\Plugin\SchemaPluginInterface $instance */
     $schema = $formState->getValue('schema');
+    /** @var \Drupal\graphql\Plugin\SchemaPluginInterface $instance */
     $instance = $this->schemaManager->createInstance($schema);
     if (!empty($form['schema_configuration'][$schema]) && $instance instanceof PluginFormInterface && $instance instanceof ConfigurableInterface) {
       $state = SubformState::createForSubform($form['schema_configuration'][$schema], $form, $formState);
@@ -233,8 +233,8 @@ public function validateForm(array &$form, FormStateInterface $formState) {
   public function submitForm(array &$form, FormStateInterface $formState) {
     parent::submitForm($form, $formState);
 
-    /** @var \Drupal\graphql\Plugin\SchemaPluginInterface $instance */
     $schema = $formState->getValue('schema');
+    /** @var \Drupal\graphql\Plugin\SchemaPluginInterface $instance */
     $instance = $this->schemaManager->createInstance($schema);
     if ($instance instanceof PluginFormInterface && $instance instanceof ConfigurableInterface) {
       $state = SubformState::createForSubform($form['schema_configuration'][$schema], $form, $formState);
@@ -248,13 +248,14 @@ public function submitForm(array &$form, FormStateInterface $formState) {
    * @throws \Drupal\Component\Plugin\Exception\PluginException
    */
   public function save(array $form, FormStateInterface $formState) {
-    parent::save($form, $formState);
+    $save_result = parent::save($form, $formState);
 
     $this->messenger()->addMessage($this->t('Saved the %label server.', [
       '%label' => $this->entity->label(),
     ]));
 
     $formState->setRedirect('entity.graphql_server.collection');
+    return $save_result;
   }
 
 }
diff --git a/src/GraphQL/Buffers/EntityRevisionBuffer.php b/src/GraphQL/Buffers/EntityRevisionBuffer.php
@@ -66,9 +66,9 @@ public function resolveBufferArray(array $buffer) {
     $vids = array_values(array_unique($vids));
 
     // Load the buffered entities.
-    $entities = $this->entityTypeManager
-      ->getStorage($type)
-      ->loadMultipleRevisions($vids);
+    /** @var \Drupal\Core\Entity\RevisionableStorageInterface $storage */
+    $storage = $this->entityTypeManager->getStorage($type);
+    $entities = $storage->loadMultipleRevisions($vids);
 
     return array_map(function ($item) use ($entities) {
       if (is_array($item['vid'])) {
diff --git a/src/GraphQL/ResolverBuilder.php b/src/GraphQL/ResolverBuilder.php
@@ -34,7 +34,7 @@ public function produce($id, array $config = []) {
   }
 
   /**
-   * @param \Drupal\graphql\GraphQL\Resolver\ResolverInterface[]|array $resolvers
+   * @param \Drupal\graphql\GraphQL\Resolver\ResolverInterface ...$resolvers
    *
    * @return \Drupal\graphql\GraphQL\Resolver\Composite
    */
diff --git a/src/PermissionProvider.php b/src/PermissionProvider.php
@@ -11,7 +11,7 @@ class PermissionProvider {
   /**
    * The entity type manager service.
    *
-   * @var \Drupal\Core\Authentication\AuthenticationCollectorInterface
+   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
    */
   protected $entityTypeManager;
 
diff --git a/src/Plugin/GraphQL/DataProducer/Entity/EntityLoad.php b/src/Plugin/GraphQL/DataProducer/Entity/EntityLoad.php
@@ -30,7 +30,7 @@
  *       required = FALSE
  *     ),
  *     "language" = @ContextDefinition("string",
- *       label = @Translation("Entity languages"),
+ *       label = @Translation("Entity language"),
  *       required = FALSE
  *     ),
  *     "bundles" = @ContextDefinition("string",
diff --git a/src/Plugin/GraphQL/DataProducer/Entity/EntityLoadByUuid.php b/src/Plugin/GraphQL/DataProducer/Entity/EntityLoadByUuid.php
@@ -29,8 +29,7 @@
  *       label = @Translation("Unique identifier")
  *     ),
  *     "language" = @ContextDefinition("string",
- *       label = @Translation("Entity languages(s)"),
- *       multiple = TRUE,
+ *       label = @Translation("Entity language"),
  *       required = FALSE
  *     ),
  *     "bundles" = @ContextDefinition("string",
@@ -130,7 +129,7 @@ public function __construct(
   /**
    * @param string $type
    * @param string $uuid
-   * @param array|null $language
+   * @param string|null $language
    * @param array|null $bundles
    * @param bool|null $access
    * @param \Drupal\Core\Session\AccountInterface|null $accessUser
@@ -139,7 +138,7 @@ public function __construct(
    *
    * @return \GraphQL\Deferred
    */
-  public function resolve($type, $uuid, ?array $language, ?array $bundles, ?bool $access, ?AccountInterface $accessUser, ?string $accessOperation, FieldContext $context) {
+  public function resolve($type, $uuid, ?string $language, ?array $bundles, ?bool $access, ?AccountInterface $accessUser, ?string $accessOperation, FieldContext $context) {
     $resolver = $this->entityBuffer->add($type, $uuid);
 
     return new Deferred(function () use ($type, $language, $bundles, $resolver, $context, $access, $accessUser, $accessOperation) {
diff --git a/src/Plugin/GraphQL/DataProducer/Entity/EntityLoadMultiple.php b/src/Plugin/GraphQL/DataProducer/Entity/EntityLoadMultiple.php
@@ -139,12 +139,13 @@ public function __construct(
    *
    * @return \GraphQL\Deferred
    */
-  public function resolve($type, array $ids, ?string $language, ?array $bundles, bool $access, ?AccountInterface $accessUser, string $accessOperation, FieldContext $context) {
+  public function resolve($type, array $ids, ?string $language, ?array $bundles, bool $access, ?AccountInterface $accessUser, ?string $accessOperation, FieldContext $context) {
     $resolver = $this->entityBuffer->add($type, $ids);
 
     return new Deferred(function () use ($type, $language, $bundles, $resolver, $context, $access, $accessUser, $accessOperation) {
       /** @var \Drupal\Core\Entity\EntityInterface[] $entities */
-      if (!$entities = $resolver()) {
+      $entities = $resolver();
+      if (!$entities) {
         // If there is no entity with this id, add the list cache tags so that
         // the cache entry is purged whenever a new entity of this type is
         // saved.
diff --git a/src/Plugin/GraphQL/DataProducer/Entity/EntityRendered.php b/src/Plugin/GraphQL/DataProducer/Entity/EntityRendered.php
@@ -104,7 +104,6 @@ public function resolve(EntityInterface $entity, $mode, RefinableCacheableDepend
     $view = $builder->view($entity, $mode, $entity->language()->getId());
 
     $context = new RenderContext();
-    /** @var \GraphQL\Executor\ExecutionResult|\GraphQL\Executor\ExecutionResult[] $result */
     $result = $this->renderer->executeInRenderContext($context, function () use ($view) {
       return $this->renderer->render($view);
     });
diff --git a/src/Plugin/GraphQL/DataProducer/Entity/EntityTranslation.php b/src/Plugin/GraphQL/DataProducer/Entity/EntityTranslation.php
@@ -91,10 +91,10 @@ public function __construct(array $configuration, $pluginId, $pluginDefinition,
    * @param \Drupal\Core\Entity\EntityInterface $entity
    * @param string $language
    * @param bool|null $access
-   * @param \Drupal\graphql\Plugin\GraphQL\DataProducer\Entity\AccountInterface|null $accessUser
+   * @param \Drupal\Core\Session\AccountInterface|null $accessUser
    * @param string|null $accessOperation
    *
-   * @return |null
+   * @return \Drupal\Core\Entity\EntityInterface|null
    */
   public function resolve(EntityInterface $entity, $language, ?bool $access, ?AccountInterface $accessUser, ?string $accessOperation) {
     if ($entity instanceof TranslatableInterface && $entity->isTranslatable()) {
diff --git a/src/Plugin/GraphQL/DataProducer/EntityDefinition/Fields.php b/src/Plugin/GraphQL/DataProducer/EntityDefinition/Fields.php
@@ -122,7 +122,6 @@ public function resolve(
     FieldContext $field_context
   ): \Iterator {
     $entity_definition->getBundleEntityType();
-    /** @var \Drupal\Core\Entity\ContentEntityType $value */
     if ($entity_definition instanceof ContentEntityType) {
       if ($bundle_context) {
         $key = $bundle_context['key'];
diff --git a/src/Plugin/GraphQL/DataProducer/EntityDefinition/Fields/DefaultValue.php b/src/Plugin/GraphQL/DataProducer/EntityDefinition/Fields/DefaultValue.php
@@ -49,6 +49,7 @@ public function resolve(FieldDefinitionInterface $entity_definition_field) {
     else {
       return $default_value;
     }
+    return NULL;
   }
 
 }
diff --git a/src/Plugin/GraphQL/DataProducer/EntityDefinition/Fields/Settings.php b/src/Plugin/GraphQL/DataProducer/EntityDefinition/Fields/Settings.php
@@ -36,7 +36,7 @@ class Settings extends DataProducerPluginBase {
    * @param array|null $entity_form_display_context
    *   Entity form display context.
    *
-   * @return array
+   * @return \Iterator
    *   Field settings.
    */
   public function resolve(
diff --git a/src/Plugin/GraphQL/DataProducer/Taxonomy/TaxonomyLoadTree.php b/src/Plugin/GraphQL/DataProducer/Taxonomy/TaxonomyLoadTree.php
@@ -159,7 +159,8 @@ public function resolve(string $vid, int $parent, ?int $max_depth, ?string $lang
 
     return new Deferred(function () use ($language, $resolver, $context, $access, $accessUser, $accessOperation) {
       /** @var \Drupal\Core\Entity\EntityInterface[] $entities */
-      if (!$entities = $resolver()) {
+      $entities = $resolver();
+      if (!$entities) {
         // If there is no entity with this id, add the list cache tags so that
         // the cache entry is purged whenever a new entity of this type is
         // saved.
diff --git a/src/Plugin/GraphQL/DataProducer/User/PasswordReset.php b/src/Plugin/GraphQL/DataProducer/User/PasswordReset.php
@@ -135,7 +135,7 @@ public function resolve(string $email): ResponseInterface {
     // Show general error message also in case of unexpected response. Log to
     // watchdog for potential further investigation.
     if ($controller_response->getStatusCode() !== 200) {
-      $this->logger->warning("Unexpected response code @code during password reset.", ['@code' => $response->getStatusCode()]);
+      $this->logger->warning("Unexpected response code @code during password reset.", ['@code' => $controller_response->getStatusCode()]);
       $response->addViolation($this->t('Unable to reset password, please try again later.'));
     }
 
diff --git a/tests/src/Traits/MockingTrait.php b/tests/src/Traits/MockingTrait.php
@@ -46,7 +46,7 @@ trait MockingTrait {
    * @param mixed $value
    *   The return value. Can also be a value callback.
    *
-   * @return \PHPUnit_Framework_MockObject_Stub_ReturnCallback
+   * @return \PHPUnit\Framework\MockObject\Stub\ReturnCallback
    *   The return callback promise.
    */
   protected function toPromise($value) {
@@ -63,7 +63,7 @@ protected function toPromise($value) {
    * @param mixed $scope
    *   The resolver's bound object and class scope.
    *
-   * @return \PHPUnit_Framework_MockObject_Stub_ReturnCallback
+   * @return \PHPUnit\Framework\MockObject\Stub\ReturnCallback
    *   The return callback promise.
    */
   protected function toBoundPromise($value, $scope) {

Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,8 @@ public function access(AccountInterface $account, ServerInterface $graphql_serve`
`51`	`51`
`52`	`52`	`$request = $this->requestStack->getCurrentRequest();`
`53`	`53`	`/** @var \GraphQL\Server\OperationParams[] $operations */`
`54`		`- if (!$operations = $request->attributes->get('operations', [])) {`
	`54`	`+ $operations = $request->attributes->get('operations', []);`
	`55`	`+ if (!$operations) {`
`55`	`56`	`return AccessResult::forbidden();`
`56`	`57`	`}`
`57`	`58`
Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,7 @@ public function produce($id, array $config = []) {`
`34`	`34`	`}`
`35`	`35`
`36`	`36`	`/**`
`37`		`- * @param \Drupal\graphql\GraphQL\Resolver\ResolverInterface[]\|array $resolvers`
	`37`	`+ * @param \Drupal\graphql\GraphQL\Resolver\ResolverInterface ...$resolvers`
`38`	`38`	`*`
`39`	`39`	`* @return \Drupal\graphql\GraphQL\Resolver\Composite`
`40`	`40`	`*/`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ class PermissionProvider {`
`11`	`11`	`/**`
`12`	`12`	`* The entity type manager service.`
`13`	`13`	`*`
`14`		`- * @var \Drupal\Core\Authentication\AuthenticationCollectorInterface`
	`14`	`+ * @var \Drupal\Core\Entity\EntityTypeManagerInterface`
`15`	`15`	`*/`
`16`	`16`	`protected $entityTypeManager;`
`17`	`17`