Apply round of code quality improvements from Rector (#4038)

modules/common/src/Controller/AuthCleanupHelper.php

@@ -21,8 +21,7 @@ class AuthCleanupHelper {
   public static function makePublicSpec(OpenApiSpec $spec) {
     $filteredSpec = static::removeAuthenticatedEndpoints($spec);
     $filteredSpec = static::cleanUpParameters($filteredSpec);
-    $filteredSpec = static::cleanUpSchemas($filteredSpec);
-    return $filteredSpec;
+    return static::cleanUpSchemas($filteredSpec);
   }
 
   /**

modules/common/src/Controller/OpenApiController.php

@@ -107,11 +107,11 @@ public function getComplete($format = "json") {
    * @param int $code
    *   HTTP response code.
    *
-   * @return \Symfony\Component\HttpFoundation\JsonResponse
+   * @return \Symfony\Component\HttpFoundation\Response
    *   OpenAPI spec response.
    */
   private function getYamlResponse($spec, $code = 200) {
-    $response = new Response(Yaml::encode($spec), 200, ['Content-type' => 'application/vnd.oai.openapi']);
+    $response = new Response(Yaml::encode($spec), $code, ['Content-type' => 'application/vnd.oai.openapi']);
     return $this->addCacheHeaders($response);
   }
 

modules/common/src/DataResource.php

@@ -385,7 +385,7 @@ public static function getIdentifierAndVersion($string) {
       $parts = self::parseUniqueIdentifier($string);
       return [$parts['identifier'], $parts['version']];
     }
-    catch (\Exception $e) {
+    catch (\Exception) {
     }
 
     // Partial identifier.
@@ -416,7 +416,7 @@ public static function getIdentifierAndVersion($string) {
    * @return object
    *   JSON-decoded object.
    */
-  private static function getDistribution($identifier) {
+  private static function getDistribution(mixed $identifier) {
     /** @var \Drupal\metastore\Storage\DataFactory $factory */
     $factory = \Drupal::service('dkan.metastore.storage');
     $storage = $factory->getInstance('distribution');

modules/common/src/DatasetInfo.php

@@ -229,7 +229,7 @@ protected function getStorage(string $identifier, string $version) {
     try {
       $storage = $this->datastore->getStorage($identifier, $version);
     }
-    catch (\Exception $e) {
+    catch (\Exception) {
       $storage = NULL;
     }
     return $storage;

modules/common/src/EventDispatcherTrait.php

@@ -26,10 +26,9 @@ trait EventDispatcherTrait {
    * @throws \Exception
    *   If any of the subscribers registered and Exception it is thrown.
    */
-  private function dispatchEvent($eventName, $data, $validator = NULL) {
+  private function dispatchEvent(mixed $eventName, mixed $data, mixed $validator = NULL) {
     if ($this->useLegacyDispatcher()) {
-      $data = $this->legacyDispatchEvent($eventName, $data, $validator);
-      return $data;
+      return $this->legacyDispatchEvent($eventName, $data, $validator);
     }
     $dispatcher = \Drupal::service('event_dispatcher');
 
@@ -73,7 +72,7 @@ private function useLegacyDispatcher() {
    * @throws \Exception
    *   If any of the subscribers registered and Exception it is thrown.
    */
-  private function legacyDispatchEvent($eventName, $data, $validator = NULL) {
+  private function legacyDispatchEvent(mixed $eventName, mixed $data, mixed $validator = NULL) {
     $dispatcher = \Drupal::service('event_dispatcher');
 
     if ($event = $dispatcher->dispatch($eventName, new Event($data, $validator))) {

modules/common/src/Plugin/DkanApiDocsBase.php

@@ -119,7 +119,7 @@ protected function getDoc($module = NULL, $docName = "openapi_spec") {
    *   Filtered schema.
    */
   protected static function filterJsonSchemaUnsupported(array $schema) {
-    $filteredSchema = self::nestedFilterKeys($schema, function ($prop) {
+    return self::nestedFilterKeys($schema, function ($prop) {
       $notSupported = [
         '$schema',
         'additionalItems',
@@ -139,8 +139,6 @@ protected static function filterJsonSchemaUnsupported(array $schema) {
       }
       return TRUE;
     });
-
-    return $filteredSchema;
   }
 
   /**

modules/common/src/Storage/AbstractDatabaseTable.php

@@ -105,11 +105,9 @@ public function retrieveAll(): array {
       return [];
     }
 
-    $result = array_map(function ($item) {
+    return array_map(function ($item) {
       return $item->{$this->primaryKey()};
     }, $result);
-
-    return $result;
   }
 
   /**
@@ -252,7 +250,7 @@ protected function sanitizedErrorMessage(string $unsanitizedMessage) {
       'Can\'t find FULLTEXT index matching the column list' => 'You have attempted a fulltext match against a column that is not indexed for fulltext searching',
     ];
     foreach ($messages as $portion => $message) {
-      if (strpos($unsanitizedMessage, $portion) !== FALSE) {
+      if (str_contains($unsanitizedMessage, $portion)) {
         return $message . '.';
       }
     }
@@ -271,7 +269,7 @@ protected function setTable() {
         try {
           $this->tableCreate($table_name, $schema);
         }
-        catch (SchemaObjectExistsException $e) {
+        catch (SchemaObjectExistsException) {
           // Table already exists, which is totally OK. Other throwables find
           // their way out to the caller.
         }

modules/common/src/Storage/DeprecatedJobStoreFactoryTrait.php

@@ -26,7 +26,7 @@ trait DeprecatedJobStoreFactoryTrait {
   protected function getTableName(string $identifier): string {
     // Check for either \ or class_exists() because not all class names will
     // contain a backslash.
-    if ((strpos($identifier, '\\') !== FALSE) || class_exists($identifier)) {
+    if ((str_contains($identifier, '\\')) || class_exists($identifier)) {
       $deprecatedTableName = $this->getDeprecatedTableName($identifier);
       if ($this->connection->schema()->tableExists($deprecatedTableName)) {
         return $deprecatedTableName;
@@ -48,12 +48,11 @@ protected function getHashedTableName(string $identifier): string {
     // Avoid table-name-too-long errors by hashing the FQN of the identifier,
     // assuming it's a class name.
     $exploded_class = explode('\\', $identifier);
-    $table_name = strtolower(implode('_', [
+    return strtolower(implode('_', [
       'jobstore',
       crc32($identifier),
       array_pop($exploded_class),
     ]));
-    return $table_name;
   }
 
   /**

modules/common/src/Storage/SelectFactory.php

@@ -116,7 +116,7 @@ private function addDateExpressions($db_query, $fields, $meta_data) {
    * @param mixed $property
    *   One property from a query properties array.
    */
-  private function setQueryProperty($property) {
+  private function setQueryProperty(mixed $property) {
 
     if (isset($property->expression)) {
       $expressionStr = $this->expressionToString($property->expression);
@@ -137,7 +137,7 @@ private function setQueryProperty($property) {
    * @return object
    *   Normalized property for conversion to field in select object.
    */
-  private function normalizeProperty($property): object {
+  private function normalizeProperty(mixed $property): object {
     if (is_string($property) && self::safeProperty($property)) {
       return (object) [
         "collection" => $this->alias,
@@ -227,7 +227,7 @@ private function getSupportedFunctions() {
    * @return mixed
    *   String or numeric operand for expression.
    */
-  private function normalizeOperand($operand) {
+  private function normalizeOperand(mixed $operand) {
     if (is_numeric($operand)) {
       return $operand;
     }
@@ -248,7 +248,7 @@ private function normalizeOperand($operand) {
    * @return string
    *   Property name with alias prefix.
    */
-  private function propertyToString($property) {
+  private function propertyToString(mixed $property) {
     $property = $this->normalizeProperty($property);
     return "{$property->collection}.{$property->property}";
   }

modules/common/src/Util/DrupalFiles.php

@@ -90,9 +90,8 @@ public function retrieveFile($url, $destination) {
       $filename = $this->getFilenameFromUrl($url);
       $dest = $this->getFilesystem()->realpath($destination) . "/{$filename}";
       copy($src, $dest);
-      $url = $this->fileCreateUrl("{$destination}/{$filename}");
 
-      return $url;
+      return $this->fileCreateUrl("{$destination}/{$filename}");
     }
     else {
       return system_retrieve_file($url, $destination, FALSE, FileSystemInterface::EXISTS_REPLACE);

modules/common/src/Util/ParentCallTrait.php

@@ -27,7 +27,7 @@ trait ParentCallTrait {
    * @return mixed
    *   Return of parent.
    */
-  protected function parentCall($method, ...$args) {
+  protected function parentCall($method, mixed ...$args) {
     return parent::$method(...$args);
   }
 

modules/common/src/Util/Timer.php

@@ -5,7 +5,7 @@
 /**
  * Timer utility object.
  */
-class Timer {
+class Timer implements \Stringable {
 
   /**
    * Start times in microseconds.
@@ -52,7 +52,7 @@ public function average($id) {
   /**
    * {@inheritdoc}
    */
-  public function __toString() {
+  public function __toString(): string {
     $strings = [];
     foreach ($this->ends as $id => $data) {
       $strings[] = "{$id} AVG: {$this->average($id)}";

modules/common/tests/src/Unit/Connection.php

@@ -25,7 +25,7 @@ class Connection extends CoreConnection {
   /**
    * {@inheritdoc}
    */
-  protected $statementClass = NULL;
+  protected $statementClass;
 
   /**
    * {@inheritdoc}
@@ -58,8 +58,8 @@ public function databaseType() {
   /**
    * {@inheritdoc}
    */
-  public function createDatabase($database) {
-    return;
+  public function createDatabase($database)
+  {
   }
 
   /**

modules/common/tests/src/Unit/DataResourceTest.php

@@ -82,7 +82,7 @@ public function testGetIdentifierAndVersionException() {
     $expectedMessage = "Could not find identifier and version for {$id}";
 
     $this->expectExceptionMessage($expectedMessage);
-    $result = DataResource::getIdentifierAndVersion($id);
+    DataResource::getIdentifierAndVersion($id);
   }
 
   /**

modules/common/tests/src/Unit/Events/EventTest.php

@@ -30,7 +30,7 @@ public function testDataIntegrityAcrossEventSubscribers() {
 
     \Drupal::setContainer($container);
 
-    $result = $this->dispatchEvent('test_event', 'hello', function($data) {
+    $this->dispatchEvent('test_event', 'hello', function($data) {
       return is_string($data);
     });
   }

modules/common/tests/src/Unit/Storage/QueryDataProvider.php

@@ -67,8 +67,7 @@ public function getAllData($return): array {
   public static function noPropertiesQuery($return) {
     switch ($return) {
       case self::QUERY_OBJECT:
-        $query = new Query();
-        return $query;
+        return new Query();
 
       case self::SQL:
         return "SELECT t.* FROM {table} t";

modules/common/tests/src/Unit/Util/DrupalFilesTest.php

@@ -36,20 +36,19 @@ private function getContainer(): ContainerInterface {
       ->add('stream_wrapper_manager', StreamWrapperManager::class)
       ->index(0);
 
-    $container = (new Chain($this))
+    return (new Chain($this))
       ->add(ContainerInterface::class, 'get', $options)
       ->add(FileSystemInterface::class, 'realpath', "/tmp")
       ->add(StreamWrapperManager::class, 'getViaUri', StreamWrapperInterface::class)
       ->add(StreamWrapperInterface::class, 'getExternalUrl', "blah")
       ->getMock();
-
-    return $container;
   }
 
   /**
    * Protected.
    */
   protected function tearDown(): void {
+    parent::tearDown();
     unlink("/tmp/hello.txt");
   }
 

modules/datastore/modules/datastore_mysql_import/src/Service/MysqlImport.php

@@ -128,7 +128,7 @@ protected function getColsFromFile(string $file_path, string $delimiter): array
     $f = fopen($file_path, 'r');
 
     // Ensure the file could be successfully opened.
-    if (!isset($f) || $f === FALSE) {
+    if ($f === FALSE) {
       throw new FileException(sprintf('Failed to open resource file "%s".', $file_path));
     }
 
@@ -141,8 +141,9 @@ protected function getColsFromFile(string $file_path, string $delimiter): array
 
     // Close the resource file, since it is no longer needed.
     fclose($f);
-    // Ensure the columns of the resource file were successfully read.
-    if (!isset($columns) || $columns === FALSE) {
+    // Ensure the columns of the resource file were successfully read. $columns
+    // could be array, FALSE, or NULL.
+    if (!$columns) {
       throw new FileException(sprintf('Failed to read columns from resource file "%s".', $file_path));
     }
 

modules/datastore/modules/datastore_mysql_import/tests/src/Kernel/Service/MysqlImportTest.php

@@ -100,7 +100,6 @@ public function testMysqlImporterWithCSVFileWithNewLinesInHeaders() {
     $import_factory = $this->container->get('dkan.datastore.service.factory.import');
     $this->assertInstanceOf(MysqlImportFactory::class, $import_factory);
 
-    /** @var \Drupal\datastore_mysql_import\Service\MysqlImport $mysql_import */
     $import_service = $import_factory->getInstance(
       $identifier,
       ['resource' => $data_resource]

modules/datastore/modules/datastore_mysql_import/tests/src/Kernel/Storage/MySqlDatabaseTableFactoryTest.php

@@ -29,7 +29,7 @@ public function testFactoryServiceResourceException() {
     $factory = $this->container->get('dkan.datastore_mysql_import.database_table_factory');
     $this->expectException(\Exception::class);
     $this->expectExceptionMessage("config['resource'] is required");
-    $table = $factory->getInstance('id', []);
+    $factory->getInstance('id', []);
   }
 
   public function testFactoryService() {