[MAINTENANCE] Introduce usage of PHPStan checks for new PR (#1052)

Co-authored-by: Sebastian Meyer <[email protected]>

.github/phpstan.neon

@@ -0,0 +1,31 @@
+parameters:
+    ignoreErrors:
+            - '#Call to an undefined method Kitodo\\Dlf\\Domain\\Repository\\[a-zA-Z]+Repository::countByPid\(\)\.#'
+            - '#Call to an undefined method Kitodo\\Dlf\\Domain\\Repository\\[a-zA-Z]+Repository::findByIsListed\(\)\.#'
+            - '#Call to an undefined method Kitodo\\Dlf\\Domain\\Repository\\[a-zA-Z]+Repository::findByIsSortable\(\)\.#'
+            - '#Call to an undefined method Kitodo\\Dlf\\Domain\\Repository\\[a-zA-Z]+Repository::findOneByFeUserId\(\)\.#'
+            - '#Call to an undefined method Kitodo\\Dlf\\Domain\\Repository\\[a-zA-Z]+Repository::findOneByIndexName\(\)\.#'
+            - '#Call to an undefined method Kitodo\\Dlf\\Domain\\Repository\\[a-zA-Z]+Repository::findOneByPid\(\)\.#'
+            - '#Call to an undefined method Kitodo\\Dlf\\Domain\\Repository\\[a-zA-Z]+Repository::findOneByRecordId\(\)\.#'
+            - '#Call to an undefined method Kitodo\\Dlf\\Domain\\Repository\\[a-zA-Z]+Repository::findOneByRoot\(\)\.#'
+            - '#Call to an undefined method Kitodo\\Dlf\\Domain\\Repository\\[a-zA-Z]+Repository::findOneBySessionId\(\)\.#'
+            - '#Call to an undefined method Kitodo\\Dlf\\Domain\\Repository\\[a-zA-Z]+Repository::findOneByType\(\)\.#'
+            - '#Call to an undefined method Kitodo\\Dlf\\Domain\\Repository\\[a-zA-Z]+Repository::findOneByUid\(\)\.#'
+            - '#Call to an undefined method Psr\\Http\\Message\\RequestFactoryInterface::request\(\)\.#'
+            - '#Call to an undefined method Solarium\\Core\\Query\\DocumentInterface::setField\(\)\.#'
+            - '#Call to an undefined method Ubl\\Iiif\\Presentation\\Common\\Model\\Resources\\IiifResourceInterface::getHeight\(\)\.#'
+            - '#Call to an undefined method Ubl\\Iiif\\Presentation\\Common\\Model\\Resources\\IiifResourceInterface::getWidth\(\)\.#'
+            - '#Call to an undefined method Ubl\\Iiif\\Presentation\\Common\\Model\\Resources\\IiifResourceInterface::getPossibleTextAnnotationContainers\(\)\.#'
+            - '#Call to an undefined method Ubl\\Iiif\\Presentation\\Common\\Model\\Resources\\IiifResourceInterface::getTextAnnotations\(\)\.#'
+            - '#Call to an undefined method Ubl\\Iiif\\Presentation\\Common\\Model\\Resources\\ManifestInterface::getOriginalJsonArray\(\)\.#'
+            - '#Call to an undefined method Ubl\\Iiif\\Presentation\\Common\\Model\\Resources\\RangeInterface::getMemberRangesAndRanges\(\)\.#'
+            - '#Constant LOG_SEVERITY_ERROR not found\.#'
+            - '#Constant LOG_SEVERITY_NOTICE not found\.#'
+            - '#Constant LOG_SEVERITY_WARNING not found\.#'
+            - '#Constant TYPO3_MODE not found\.#'
+    level: 5
+    paths:
+        - ../Classes/
+    excludePaths:
+        - ../Classes/Controller/OaiPmhController.php
+    treatPhpDocTypesAsCertain: false

.github/workflows/phpstan.yml

@@ -0,0 +1,25 @@
+name: PHPStan
+
+on:
+    push:
+      branches: [ "master" ]
+    pull_request:
+      branches: [ "master" ]
+
+jobs:
+  phpstan:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v3
+
+      - name: Install dependencies
+        uses: php-actions/composer@v6
+        with:
+          command: update
+          php_version: "7.4"
+
+      - name: PHPStan Static Analysis
+        uses: php-actions/phpstan@v3
+        with:
+          configuration: ./.github/phpstan.neon

Classes/Api/Orcid/Client.php

@@ -31,12 +31,12 @@ class Client
     /**
      * @var string constant for API hostname
      **/
-    const HOSTNAME  = 'orcid.org';
+    const HOSTNAME = 'orcid.org';
 
     /**
      * @var string constant for API version
      **/
-    const VERSION   = '3.0';
+    const VERSION = '3.0';
 
     /**
      * @access protected

Classes/Api/Viaf/Client.php

@@ -35,8 +35,6 @@ class Client
     protected Logger $logger;
 
     /**
-     * The VIAF API endpoint
-     *
      * @access private
      * @var string The VIAF API endpoint
      **/
@@ -110,7 +108,7 @@ public function getData()
      *
      * @return string
      **/
-    private  function getApiEndpoint(): string
+    private function getApiEndpoint(): string
     {
         return $this->viafUrl . '/' . $this->endpoint;
     }

Classes/Command/BaseCommand.php

@@ -191,7 +191,7 @@ protected function getSolrCores(int $pageId): array
      *
      * @access protected
      *
-     * @param Document $doc The document instance
+     * @param Document $document The document instance
      *
      * @return bool true on success, false otherwise
      */

Classes/Command/HarvestCommand.php

@@ -187,6 +187,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         if (
             !is_array($input->getOption('set'))
             && !empty($input->getOption('set'))
+            && !empty($oai)
         ) {
             $setsAvailable = $oai->listSets();
             foreach ($setsAvailable as $setAvailable) {
@@ -201,9 +202,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             }
         }
 
+        $identifiers = [];
         // Get OAI record identifiers to process.
         try {
-            $identifiers = $oai->listIdentifiers('mets', $from, $until, $set);
+            if (!empty($oai)) {
+                $identifiers = $oai->listIdentifiers('mets', $from, $until, $set);
+            } else {
+                $io->error('ERROR: OAI interface does not exist.');
+            }
         } catch (BaseoaipmhException $exception) {
             $this->handleOaiError($exception, $io);
         }

Classes/Common/AbstractDocument.php

@@ -391,7 +391,7 @@ public abstract function getFileMimeType(string $id): string;
      * @param array $settings
      * @param bool $forceReload Force reloading the document instead of returning the cached instance
      *
-     * @return MetsDocument|IiifManifest|null Instance of this class, either MetsDocument or IiifManifest
+     * @return AbstractDocument|null Instance of this class, either MetsDocument or IiifManifest
      */
     public static function &getInstance(string $location, array $settings = [], bool $forceReload = false)
     {
@@ -441,9 +441,11 @@ public static function &getInstance(string $location, array $settings = [], bool
         // Sanitize input.
         $pid = max(intval($settings['storagePid']), 0);
         if ($documentFormat == 'METS') {
-            $instance = new MetsDocument($location, $pid, $xml);
+            $instance = new MetsDocument($pid, $location, $xml, $settings);
         } elseif ($documentFormat == 'IIIF') {
-            $instance = new IiifManifest($location, $pid, $iiif);
+            // TODO: Parameter $preloadedDocument of class Kitodo\Dlf\Common\IiifManifest constructor expects SimpleXMLElement|Ubl\Iiif\Presentation\Common\Model\Resources\IiifResourceInterface, Ubl\Iiif\Presentation\Common\Model\AbstractIiifEntity|null given.
+            // @phpstan-ignore-next-line
+            $instance = new IiifManifest($pid, $location, $iiif);
         }
 
         if (!is_null($instance)) {
@@ -750,10 +752,11 @@ public function getStructureDepth(string $logId)
      * @abstract
      *
      * @param string $location The location URL of the XML file to parse
+     * @param array $settings The extension settings
      *
      * @return void
      */
-    protected abstract function init(string $location): void;
+    abstract protected function init(string $location, array $settings): void;
 
     /**
      * Reuse any document object that might have been already loaded to determine whether document is METS or IIIF
@@ -1086,6 +1089,8 @@ protected function _getRootId(): int
     {
         if (!$this->rootIdLoaded) {
             if ($this->parentId) {
+                // TODO: Parameter $location of static method AbstractDocument::getInstance() expects string, int<min, -1>|int<1, max> given.
+                // @phpstan-ignore-next-line
                 $parent = self::getInstance($this->parentId, ['storagePid' => $this->pid]);
                 $this->rootId = $parent->rootId;
             }
@@ -1168,20 +1173,19 @@ protected function _setCPid(int $value): void
      *
      * @access protected
      *
-     * @param string $location The location URL of the XML file to parse
      * @param int $pid If > 0, then only document with this PID gets loaded
+     * @param string $location The location URL of the XML file to parse
      * @param \SimpleXMLElement|IiifResourceInterface $preloadedDocument Either null or the \SimpleXMLElement
      * or IiifResourceInterface that has been loaded to determine the basic document format.
      *
      * @return void
      */
-    protected function __construct(string $location, int $pid, $preloadedDocument)
+    protected function __construct(int $pid, string $location, $preloadedDocument, array $settings = [])
     {
         $this->pid = $pid;
         $this->setPreloadedDocument($preloadedDocument);
-        $this->init($location);
+        $this->init($location, $settings);
         $this->establishRecordId($pid);
-        return;
     }
 
     /**

Classes/Common/Helper.php

@@ -24,9 +24,6 @@
 use TYPO3\CMS\Core\Utility\ArrayUtility;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Core\Utility\MathUtility;
-use TYPO3\CMS\Extbase\Configuration\ConfigurationManager;
-use TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface;
-use TYPO3\CMS\Extbase\Object\ObjectManager;
 use TYPO3\CMS\Extbase\Utility\LocalizationUtility;
 use TYPO3\CMS\Core\Domain\Repository\PageRepository;
 
@@ -149,6 +146,8 @@ public static function checkIdentifier(string $id, string $type): bool
                 if (!preg_match('/\d{8}-\d{1}/i', $id)) {
                     return false;
                 } elseif ($checksum == 10) {
+                    //TODO: Binary operation "+" between string and 1 results in an error.
+                    // @phpstan-ignore-next-line
                     return self::checkIdentifier(($digits + 1) . substr($id, -2, 2), 'SWD');
                 } elseif (substr($id, -1, 1) != $checksum) {
                     return false;
@@ -218,11 +217,10 @@ public static function decrypt(string $encrypted)
      *
      * @static
      *
-     * @param string $content: content of file to read
+     * @param mixed $content content of file to read
      *
      * @return \SimpleXMLElement|false
      */
-    //TODO: make sure that this is called only with string then update
     public static function getXmlFileAsString($content)
     {
         // Don't make simplexml_load_string throw (when $content is an array
@@ -605,7 +603,7 @@ public static function getURN(string $base, string $id): string
         for ($i = 0, $j = strlen($digits); $i < $j; $i++) {
             $checksum += ($i + 1) * intval(substr($digits, $i, 1));
         }
-        $checksum = substr(intval($checksum / intval(substr($digits, -1, 1))), -1, 1);
+        $checksum = substr((string) floor($checksum / (int) substr($digits, -1, 1)), -1, 1);
         return $base . $id . $checksum;
     }
 
@@ -723,7 +721,7 @@ public static function translate(string $indexName, string $table, string $pid):
 
         // Check if "index_name" is an UID.
         if (MathUtility::canBeInterpretedAsInteger($indexName)) {
-            $indexName = self::getIndexNameFromUid($indexName, $table, $pid);
+            $indexName = self::getIndexNameFromUid((int) $indexName, $table, $pid);
         }
         /* $labels already contains the translated content element, but with the index_name of the translated content element itself
          * and not with the $indexName of the original that we receive here. So we have to determine the index_name of the

Classes/Common/IiifManifest.php

@@ -408,6 +408,8 @@ public function getFileLocation(string $id): string
         $resource = $this->iiif->getContainedResourceById($id);
         if (isset($resource)) {
             if ($resource instanceof CanvasInterface) {
+                // TODO: Cannot call method getSingleService() on array<Ubl\Iiif\Presentation\Common\Model\Resources\AnnotationInterface>.
+                // @phpstan-ignore-next-line
                 return (!empty($resource->getImageAnnotations()) && $resource->getImageAnnotations()->getSingleService() != null) ? $resource->getImageAnnotations()[0]->getSingleService()->getId() : $id;
             } elseif ($resource instanceof ContentResourceInterface) {
                 return $resource->getSingleService() != null && $resource->getSingleService() instanceof Service ? $resource->getSingleService()->getId() : $id;
@@ -458,6 +460,8 @@ public function getLogicalStructure(string $id, bool $recursive = false): array
         } else {
             $logUnits[] = $this->iiif;
         }
+        // TODO: Variable $logUnits in empty() always exists and is not falsy.
+        // @phpstan-ignore-next-line
         if (!empty($logUnits)) {
             if (!$recursive) {
                 $details = $this->getLogicalStructureInfo($logUnits[0]);
@@ -504,7 +508,7 @@ protected function getLogicalStructureInfo(IiifResourceInterface $resource, bool
         }
         $details['thumbnailId'] = $resource->getThumbnailUrl();
         $details['points'] = '';
-        // Load strucural mapping
+        // Load structural mapping
         $this->_getSmLinks();
         // Load physical structure.
         $this->_getPhysicalStructure();
@@ -516,7 +520,7 @@ protected function getLogicalStructureInfo(IiifResourceInterface $resource, bool
             $startCanvas = $resource->getStartCanvasOrFirstCanvas();
             $canvases = $resource->getAllCanvases();
         }
-        if ($startCanvas != null) {
+        if (isset($startCanvas)) {
             $details['pagination'] = $startCanvas->getLabel();
             $startCanvasIndex = array_search($startCanvas, $this->iiif->getDefaultCanvases());
             if ($startCanvasIndex !== false) {
@@ -681,8 +685,12 @@ public function getMetadata(string $id, int $cPid = 0): array
                     $resArray['format'] > 0 && !empty($resArray['xpath_sorting'])
                     && ($values = $iiifResource->jsonPath($resArray['xpath_sorting']) != null)
                 ) {
+                    // TODO: Call to function is_string() with true will always evaluate to false.
+                    // @phpstan-ignore-next-line
                     if (is_string($values)) {
                         $metadata[$resArray['index_name'] . '_sorting'][0] = [trim((string) $values)];
+                        // TODO: Instanceof between true and Flow\JSONPath\JSONPath will always evaluate to false.
+                        // @phpstan-ignore-next-line
                     } elseif ($values instanceof JSONPath && is_array($values->data()) && count($values->data()) > 1) {
                         $metadata[$resArray['index_name']] = [];
                         foreach ($values->data() as $value) {
@@ -836,7 +844,7 @@ public function getIiif(): IiifResourceInterface
     /**
      * @see AbstractDocument::init()
      */
-    protected function init(string $location): void
+    protected function init(string $location, array $settings = []): void
     {
         $this->logger = GeneralUtility::makeInstance(LogManager::class)->getLogger(static::class);
     }

Classes/Common/Indexer.php

@@ -532,8 +532,8 @@ protected static function processPhysical(Document $document, int $page, array $
     protected static function solrConnect(int $core, int $pid = 0): bool
     {
         // Get Solr instance.
-        // Connect to Solr server.
         $solr = Solr::getInstance($core);
+        // Connect to Solr server.
         if ($solr->ready) {
             self::$solr = $solr;
             // Load indexing configuration if needed.

Classes/Common/MetadataInterface.php

@@ -31,8 +31,9 @@ interface MetadataInterface
      *
      * @param \SimpleXMLElement $xml The XML to extract the metadata from
      * @param array &$metadata The metadata array to fill
+     * @param bool $useExternalApis true if external APIs should be called, false otherwise
      *
      * @return void
      */
-    public function extractMetadata(\SimpleXMLElement $xml, array &$metadata): void;
+    public function extractMetadata(\SimpleXMLElement $xml, array &$metadata, bool $useExternalApis): void;
 }

Classes/Common/MetsDocument.php

@@ -137,6 +137,12 @@ final class MetsDocument extends AbstractDocument
      */
     protected string $parentHref = '';
 
+    /**
+     * @access protected
+     * @var array the extension settings
+     */
+    protected array $settings = [];
+
     /**
      * This adds metadata from METS structural map to metadata array.
      *
@@ -467,7 +473,7 @@ public function getMetadata(string $id, int $cPid = 0): array
                         class_exists($class)
                         && ($obj = GeneralUtility::makeInstance($class)) instanceof MetadataInterface
                     ) {
-                        $obj->extractMetadata($this->mdSec[$dmdId]['xml'], $metadata);
+                        $obj->extractMetadata($this->mdSec[$dmdId]['xml'], $metadata, $this->settings['useExternalApisForMetadata']);
                     } else {
                         $this->logger->warning('Invalid class/method "' . $class . '->extractMetadata()" for metadata format "' . $this->mdSec[$dmdId]['type'] . '"');
                     }
@@ -711,9 +717,10 @@ public function getStructureDepth(string $logId)
     /**
      * @see AbstractDocument::init()
      */
-    protected function init(string $location): void
+    protected function init(string $location, array $settings): void
     {
         $this->logger = GeneralUtility::makeInstance(LogManager::class)->getLogger(get_class($this));
+        $this->settings = $settings;
         // Get METS node from XML file.
         $this->registerNamespaces($this->xml);
         $mets = $this->xml->xpath('//mets:mets');
@@ -1221,7 +1228,7 @@ public function __wakeup(): void
             $this->asXML = '';
             $this->xml = $xml;
             // Rebuild the unserializable properties.
-            $this->init('');
+            $this->init('', $this->settings);
         } else {
             $this->logger = GeneralUtility::makeInstance(LogManager::class)->getLogger(static::class);
             $this->logger->error('Could not load XML after deserialization');

Classes/Common/Solr/SearchResult/Highlight.php

@@ -29,12 +29,6 @@ class Highlight
      */
     private string $id;
 
-    /**
-     * @access private
-     * @var int The parent region's identifier
-     */
-    private int $parentRegionId;
-
     /**
      * @access private
      * @var int The horizontal beginning position of found highlight
@@ -70,7 +64,8 @@ class Highlight
      */
     public function __construct(array $highlight)
     {
-        $this->parentRegionId = $highlight['parentRegionIdx'];
+        // there is also possibility to access parentRegionIdx
+        // $this->parentRegionId = $highlight['parentRegionIdx'];
         $this->xBeginPosition = $highlight['ulx'];
         $this->xEndPosition = $highlight['lrx'];
         $this->yBeginPosition = $highlight['uly'];

Classes/Common/Solr/Solr.php

@@ -528,7 +528,7 @@ public function __get(string $var)
             || !method_exists($this, $method)
         ) {
             $this->logger->warning('There is no getter function for property "' . $var . '"');
-            return;
+            return null;
         } else {
             return $this->$method();
         }