Phpstan first batch of fixes (#1075)

Author: Dylan203

.travis.yml

@@ -68,8 +68,8 @@ install:
   # Download Drupal 8 core from the Github mirror because it is faster.
   - git clone --branch $DRUPAL_CORE --depth 1 https://github.com/drupal/drupal.git $DRUPAL_BUILD_DIR
 
-  # Reference the module in the build site.
-  - ln -s $TRAVIS_BUILD_DIR $DRUPAL_BUILD_DIR/modules/graphql
+  # Copy the module to the build site. This makes PHPStan find drupal reliably.
+  - cp -r $TRAVIS_BUILD_DIR $DRUPAL_BUILD_DIR/modules/graphql
 
   # Copy the customized phpunit configuration file to the core directory so
   # the relative paths are correct.
@@ -94,6 +94,12 @@ install:
   - 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" ]];
@@ -107,6 +113,9 @@ script:
   - if [[ "$WITH_PHPDBG_COVERAGE" != "true" ]];
       then $DRUPAL_BUILD_DIR/vendor/bin/phpunit --configuration $DRUPAL_BUILD_DIR/core/phpunit.xml $TRAVIS_BUILD_DIR;
     fi
+  
+  # Run Phpstan
+  - cd $DRUPAL_BUILD_DIR/modules/graphql && $DRUPAL_BUILD_DIR/vendor/bin/phpstan analyse
 
 after_success:
   - if [[ "$WITH_PHPDBG_COVERAGE" == "true" ]];

phpstan.neon

@@ -0,0 +1,17 @@
+includes:
+	- ../../vendor/phpstan/phpstan-deprecation-rules/rules.neon
+	- ../../vendor/mglaman/phpstan-drupal/extension.neon
+
+parameters:
+	level: 1
+	customRulesetUsed: true
+	paths:
+		- .
+	ignoreErrors:
+		# @todo Ignore phpstan-drupal extension's rules for now, activate later.
+		- '#\Drupal calls should be avoided in classes, use dependency injection instead#'
+		# new static() is a best practice in Drupal, so we cannot fix that.
+		- "#^Unsafe usage of new static\\(\\)\\.$#"
+		# Drupal allows object property access to custom fields, so we cannot fix
+		# that.
+		- "#^Access to an undefined property Drupal\\\\#"

src/Plugin/DataProducerPluginManager.php

@@ -69,7 +69,7 @@ public function __construct(
 
     $this->alterInfo('graphql_data_producer');
     $this->useCaches(empty($config['development']));
-    $this->setCacheBackend($definitionCacheBackend, 'producer', ['graphql']);
+    $this->setCacheBackend($definitionCacheBackend, 'graphql_data_producer', ['graphql_data_producer']);
 
     $this->requestStack = $requestStack;
     $this->contextsManager = $contextsManager;

src/Plugin/GraphQL/DataProducer/Entity/EntityLoad.php

@@ -130,7 +130,7 @@ public function __construct(
   /**
    * @param string $type
    * @param string $id
-   * @param string $language
+   * @param string|null $language
    * @param array|null $bundles
    * @param bool|null $access
    * @param \Drupal\Core\Session\AccountInterface|null $accessUser
@@ -139,7 +139,7 @@ public function __construct(
    *
    * @return \GraphQL\Deferred
    */
-  public function resolve($type, $id, $language, ?array $bundles, ?bool $access, ?AccountInterface $accessUser, ?string $accessOperation, FieldContext $context) {
+  public function resolve($type, $id, ?string $language, ?array $bundles, ?bool $access, ?AccountInterface $accessUser, ?string $accessOperation, FieldContext $context) {
     $resolver = $this->entityBuffer->add($type, $id);
 
     return new Deferred(function () use ($type, $language, $bundles, $resolver, $context, $access, $accessUser, $accessOperation) {

src/Plugin/GraphQL/DataProducer/Entity/EntityLoadByUuid.php

@@ -130,16 +130,16 @@ public function __construct(
   /**
    * @param string $type
    * @param string $uuid
-   * @param array|string $language
-   * @param array|string $bundles
+   * @param array|null $language
+   * @param array|null $bundles
    * @param bool|null $access
    * @param \Drupal\Core\Session\AccountInterface|null $accessUser
    * @param string|null $accessOperation
    * @param \Drupal\graphql\GraphQL\Execution\FieldContext $context
    *
    * @return \GraphQL\Deferred
    */
-  public function resolve($type, $uuid, $language, $bundles, ?bool $access, ?AccountInterface $accessUser, ?string $accessOperation, FieldContext $context) {
+  public function resolve($type, $uuid, ?array $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) {

src/Plugin/GraphQL/DataProducer/Entity/EntityLoadMultiple.php

@@ -130,7 +130,7 @@ public function __construct(
   /**
    * @param string $type
    * @param array $ids
-   * @param string $language
+   * @param string|null $language
    * @param array|null $bundles
    * @param bool $access
    * @param \Drupal\Core\Session\AccountInterface|null $accessUser
@@ -139,7 +139,7 @@ public function __construct(
    *
    * @return \GraphQL\Deferred
    */
-  public function resolve($type, array $ids, $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) {

src/Plugin/GraphQL/DataProducer/Entity/EntityRendered.php

@@ -9,7 +9,6 @@
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
 use Drupal\Core\Render\RenderContext;
 use Drupal\Core\Render\RendererInterface;
-use Drupal\graphql\GraphQL\Execution\ResolveContext;
 use Drupal\graphql\Plugin\GraphQL\DataProducer\DataProducerPluginBase;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
@@ -117,11 +116,4 @@ public function resolve(EntityInterface $entity, $mode, RefinableCacheableDepend
     return (string) $result;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  protected function shouldLookupEdgeCache(array $values, ResolveContext $context, ResolveInfo $info) {
-    return TRUE;
-  }
-
 }

src/Plugin/GraphQL/DataProducer/Field/EntityReference.php

@@ -9,6 +9,7 @@
 use Drupal\Core\Entity\TranslatableInterface;
 use Drupal\Core\Field\EntityReferenceFieldItemListInterface;
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
+use Drupal\Core\Session\AccountInterface;
 use Drupal\graphql\GraphQL\Buffers\EntityBuffer;
 use Drupal\graphql\GraphQL\Execution\FieldContext;
 use Drupal\graphql\Plugin\GraphQL\DataProducer\DataProducerPluginBase;
@@ -132,7 +133,7 @@ public function __construct(
   /**
    * @param \Drupal\Core\Entity\EntityInterface $entity
    * @param string $field
-   * @param string $language
+   * @param string|null $language
    * @param array|null $bundles
    * @param bool|null $access
    * @param \Drupal\Core\Session\AccountInterface|null $accessUser
@@ -141,7 +142,7 @@ public function __construct(
    *
    * @return \GraphQL\Deferred|null
    */
-  public function resolve(EntityInterface $entity, $field, $language, ?array $bundles, ?bool $access, ?AccountInterface $accessUser, ?string $accessOperation, FieldContext $context) {
+  public function resolve(EntityInterface $entity, $field, ?string $language, ?array $bundles, ?bool $access, ?AccountInterface $accessUser, ?string $accessOperation, FieldContext $context) {
     if (!$entity instanceof FieldableEntityInterface || !$entity->hasField($field)) {
       return NULL;
     }

src/Plugin/GraphQL/DataProducer/Routing/RouteLoad.php

@@ -6,6 +6,7 @@
 use Drupal\Core\Path\PathValidatorInterface;
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
 use Drupal\graphql\Plugin\GraphQL\DataProducer\DataProducerPluginBase;
+use Drupal\redirect\RedirectRepository;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -79,7 +80,7 @@ public function __construct(
     $pluginId,
     $pluginDefinition,
     PathValidatorInterface $pathValidator,
-    RedirectRepository $redirectRepository = NULL
+    ?RedirectRepository $redirectRepository = NULL
   ) {
     parent::__construct($configuration, $pluginId, $pluginDefinition);
     $this->pathValidator = $pathValidator;

src/Plugin/PersistedQueryPluginManager.php

@@ -45,7 +45,7 @@ public function __construct(
 
     $this->alterInfo('graphql_persisted_query');
     $this->useCaches(empty($config['development']));
-    $this->setCacheBackend($definitionCacheBackend, 'persisted_query', ['graphql']);
+    $this->setCacheBackend($definitionCacheBackend, 'graphql_persisted_query', ['graphql_persisted_query']);
   }
 
 }