Code Quality

terminal42 · May 21, 2024 · 60e81d5 · 60e81d5
1 parent 1efdc6f
commit 60e81d5
Show file tree

Hide file tree

Showing 24 changed files with 137 additions and 102 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -1,32 +1,16 @@
 name: CI
 
 on:
+    push: ~
     pull_request: ~
-    schedule:
-        - cron: 0 13 * * MON,THU
+#    schedule:
+#        - cron: 0 13 * * MON,THU
 
 permissions: read-all
 
 jobs:
-    ecs:
-        name: ECS
-        runs-on: ubuntu-latest
-        steps:
-            - name: Setup PHP
-              uses: shivammathur/setup-php@v2
-              with:
-                  php-version: 8.1
-                  extensions: dom, fileinfo, filter, gd, hash, intl, json, mbstring, mysqli, pcre, pdo_mysql, zlib
-                  coverage: none
-
-            - name: Checkout
-              uses: actions/checkout@v3
-
-            - name: Install the dependencies
-              run: composer install --no-interaction --no-progress
-
-            - name: Run ECS
-              run: composer run check-cs
+    build-tools:
+        uses: 'terminal42/contao-build-tools/.github/workflows/build-tools.yml@main'
 
     tests:
         name: PHP ${{ matrix.php }} / Contao ${{ matrix.contao }}

diff --git a/composer.json b/composer.json
@@ -41,7 +41,8 @@
         "phpunit/phpunit": "^9.3",
         "symfony/phpunit-bridge": "^5.4",
         "contao/test-case": "^4.13 || ^5.0",
-        "symfony/monolog-bundle": "^3.1"
+        "symfony/monolog-bundle": "^3.1",
+        "terminal42/service-annotation-bundle": "^1.1"
     },
     "conflict": {
         "contao/manager-plugin": "<2.0 || >=3.0",
@@ -66,9 +67,10 @@
     },
     "config": {
         "allow-plugins": {
-            "contao-components/installer": true,
-            "contao/manager-plugin": true,
-            "terminal42/contao-build-tools": true
+            "terminal42/contao-build-tools": true,
+            "contao-components/installer": false,
+            "contao/manager-plugin": false,
+            "php-http/discovery": false
         }
     }
 }
diff --git a/contao/dca/tl_page.php b/contao/dca/tl_page.php
@@ -15,15 +15,12 @@
     'inputType' => 'pageTree',
     'eval' => ['fieldType' => 'radio', 'multiple' => false, 'rootNodes' => [0], 'tl_class' => 'w50 clr'],
     'sql' => "int(10) unsigned NOT NULL default '0'",
-    // 'load_callback'           => [['Terminal42\ChangeLanguage\EventListener\DataContainer\PageFieldsListener', 'onLoadLanguageMain']],
-    // 'save_callback'           => [['Terminal42\ChangeLanguage\EventListener\DataContainer\PageFieldsListener', 'onSaveLanguageMain']],
 ];
 
 $GLOBALS['TL_DCA']['tl_page']['fields']['languageRoot'] = [
     'label' => &$GLOBALS['TL_LANG']['tl_page']['languageRoot'],
     'exclude' => true,
     'inputType' => 'select',
-    // 'options_callback'        => array('Terminal42\ChangeLanguage\EventListener\DataContainer\PageFieldsListener', 'onLanguageRootOptions'),
     'eval' => ['includeBlankOption' => true, 'blankOptionLabel' => &$GLOBALS['TL_LANG']['tl_page']['languageRoot'][2], 'tl_class' => 'w50'],
     'sql' => "int(10) unsigned NOT NULL default '0'",
 ];

diff --git a/depcheck.php b/depcheck.php
@@ -0,0 +1,26 @@
+<?php
+
+use ShipMonk\ComposerDependencyAnalyser\Config\Configuration;
+use ShipMonk\ComposerDependencyAnalyser\Config\ErrorType;
+
+return (new Configuration())
+    ->ignoreErrorsOnPackage('composer/semver', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('doctrine/dbal', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('doctrine/doctrine-bundle', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('knplabs/knp-menu-bundle', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('knplabs/knp-time-bundle', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('psr/log', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('scheb/2fa-bundle', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('symfony-cmf/routing-bundle', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('symfony/config', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('symfony/dependency-injection', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('symfony/event-dispatcher-contracts', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('symfony/filesystem', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('symfony/framework-bundle', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('symfony/http-kernel', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('symfony/routing', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('symfony/security-bundle', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('symfony/security-core', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('symfony/service-contracts', [ErrorType::SHADOW_DEPENDENCY])
+    ->ignoreErrorsOnPackage('symfony/twig-bundle', [ErrorType::SHADOW_DEPENDENCY])
+;
diff --git a/ecs.php b/ecs.php
@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+use PhpCsFixer\Fixer\Phpdoc\NoSuperfluousPhpdocTagsFixer;
+use Symplify\EasyCodingStandard\Config\ECSConfig;
+
+return static function (ECSConfig $ecsConfig): void {
+    $ecsConfig->skip([NoSuperfluousPhpdocTagsFixer::class]);
+};
diff --git a/phpstan.neon b/phpstan.neon
@@ -0,0 +1,4 @@
+parameters:
+    ignoreErrors:
+        - '#Access to an undefined property Contao\\DataContainer::\$activeRecord.#'
+        - '#Parameter \#2 \$models#'
diff --git a/src/EventListener/BackendView/ArticleViewListener.php b/src/EventListener/BackendView/ArticleViewListener.php
@@ -39,7 +39,7 @@ protected function getCurrentPage(): ?PageModel
 
                 $this->currentArticle = ArticleModel::findOneBy(['tl_article.id=(SELECT pid FROM tl_content WHERE id=?)'], [$this->dataContainer->id]);
             } else {
-                $this->currentArticle = ArticleModel::findByPk($this->dataContainer->id);
+                $this->currentArticle = ArticleModel::findById($this->dataContainer->id);
             }
         }
 

diff --git a/src/EventListener/BackendView/PageViewListener.php b/src/EventListener/BackendView/PageViewListener.php
@@ -26,7 +26,7 @@ protected function getCurrentPage(): ?PageModel
             return null;
         }
 
-        return PageModel::findByPk($node);
+        return PageModel::findById($node);
     }
 
     protected function getAvailableLanguages(PageModel $page): array

diff --git a/src/EventListener/BackendView/ParentChildViewListener.php b/src/EventListener/BackendView/ParentChildViewListener.php
@@ -17,7 +17,7 @@
 class ParentChildViewListener extends AbstractViewListener
 {
     /**
-     * @var Model
+     * @var Model|false
      */
     private $current = false;
 
@@ -44,7 +44,7 @@ protected function getCurrentPage()
                 $t = $class::getTable();
                 $this->current = $class::findOneBy(["$t.id=(SELECT pid FROM ".$this->getTable().' WHERE id=?)'], [$this->dataContainer->id]);
             } else {
-                $this->current = $class::findByPk($this->dataContainer->id);
+                $this->current = $class::findById($this->dataContainer->id);
             }
         }
 
@@ -146,7 +146,7 @@ private function getModelClass(): string
         return Model::getClassFromTable($GLOBALS['TL_DCA'][$this->getTable()]['config']['ptable']);
     }
 
-    private function hasParent()
+    private function hasParent(): bool
     {
         /** @var Model $class */
         $class = $this->getModelClass();

diff --git a/src/EventListener/CallbackSetupListener.php b/src/EventListener/CallbackSetupListener.php
@@ -20,7 +20,7 @@
  */
 class CallbackSetupListener
 {
-    private static $listeners;
+    private static ?array $listeners = null;
 
     public function __invoke(string $table): void
     {

diff --git a/src/EventListener/DataContainer/AbstractChildTableListener.php b/src/EventListener/DataContainer/AbstractChildTableListener.php
@@ -90,7 +90,7 @@ protected function getModel($id)
         /** @var Model $class */
         $class = Model::getClassFromTable($this->table);
 
-        return $class::findByPk($id);
+        return $class::findById($id);
     }
 
     /**

diff --git a/src/EventListener/DataContainer/ArticleListener.php b/src/EventListener/DataContainer/ArticleListener.php
@@ -33,10 +33,10 @@ public function onLoad(DataContainer $dc): void
         if ('editAll' === $action) {
             $this->addFieldsToPalettes();
         } elseif ('edit' === $action) {
-            $article = ArticleModel::findByPk($dc->id);
+            $article = ArticleModel::findById($dc->id);
 
             if (null !== $article) {
-                $page = PageModel::findByPk($article->pid);
+                $page = PageModel::findById($article->pid);
 
                 if (null !== $page && null !== (new PageFinder())->findAssociatedInMaster($page)) {
                     $this->addFieldsToPalettes();
@@ -45,11 +45,11 @@ public function onLoad(DataContainer $dc): void
         }
     }
 
-    public function onLanguageMainOptions(DataContainer $dc)
+    public function onLanguageMainOptions(DataContainer $dc): array
     {
         $pageFinder = new PageFinder();
-        $current = ArticleModel::findByPk($dc->id);
-        $page = PageModel::findByPk($current->pid);
+        $current = ArticleModel::findById($dc->id);
+        $page = PageModel::findById($current->pid);
 
         if (null === $page || null === ($master = $pageFinder->findAssociatedInMaster($page))) {
             return [];

diff --git a/src/EventListener/DataContainer/MissingLanguageIconListener.php b/src/EventListener/DataContainer/MissingLanguageIconListener.php
@@ -23,12 +23,14 @@
  */
 class MissingLanguageIconListener implements ResetInterface
 {
-    private static $callbacks;
+    private static ?array $callbacks = null;
 
     private Security $security;
+
     private Connection $connection;
 
     private ?array $pageCache = null;
+
     private ?array $translationCache = null;
 
     public function __construct(Security $security, Connection $connection)
@@ -60,6 +62,8 @@ public function reset(): void
 
     /**
      * Adds missing translation warning to page tree.
+     *
+     * @param string|array|null $previousResult;
      */
     private function onPageLabel(array $args, $previousResult = null): string
     {
@@ -85,7 +89,7 @@ private function onPageLabel(array $args, $previousResult = null): string
             ($translation['languageMain'] ?? null) > 0
             && $user instanceof BackendUser
             && \is_array($user->pageLanguageLabels)
-            && \in_array(($translation['rootId'] ?? null), $user->pageLanguageLabels, false)
+            && \in_array($translation['rootId'] ?? null, $user->pageLanguageLabels, false)
         ) {
             return sprintf(
                 '%s <span style="color:#999;padding-left:3px">(<a href="%s" title="%s" style="color:#999">%s</a>)</span>',
@@ -101,6 +105,8 @@ private function onPageLabel(array $args, $previousResult = null): string
 
     /**
      * Adds missing translation warning to article tree.
+     *
+     * @param string|array|null $previousResult
      */
     private function onArticleLabel(array $args, $previousResult = null): string
     {
@@ -123,6 +129,8 @@ private function onArticleLabel(array $args, $previousResult = null): string
 
     /**
      * Generate missing translation warning for news child records.
+     *
+     * @param string|array|null $previousResult
      */
     private function onNewsChildRecords(array $args, $previousResult = null): string
     {
@@ -147,14 +155,15 @@ private function onNewsChildRecords(array $args, $previousResult = null): string
 
     /**
      * Generate missing translation warning for calendar events child records.
+     *
+     * @param string|array|null $previousResult
      */
     private function onCalendarEventChildRecords(array $args, $previousResult = null): string
     {
         $row = $args[0];
         $label = (string) $previousResult;
 
         if (0 === $this->getChildTranslation((int) $row['id'], 'tl_calendar_events', 'tl_calendar', 'master')) {
-            //return $this->generateLabelWithWarning($label);
             return preg_replace(
                 '#</div>#',
                 $this->generateLabelWithWarning('', 'position:absolute;top:6px').'</div>',
@@ -168,6 +177,8 @@ private function onCalendarEventChildRecords(array $args, $previousResult = null
 
     /**
      * Generate missing translation warning for faq child records.
+     *
+     * @param string|array|null $previousResult
      */
     private function onFaqChildRecords(array $args, $previousResult = null): string
     {
@@ -186,13 +197,7 @@ private function onFaqChildRecords(array $args, $previousResult = null): string
         return $label;
     }
 
-    /**
-     * @param string $label
-     * @param string $imgStyle
-     *
-     * @return string
-     */
-    private function generateLabelWithWarning($label, $imgStyle = '')
+    private function generateLabelWithWarning(string $label, string $imgStyle = ''): string
     {
         return $label.sprintf(
             '<span style="padding-left:3px"><img src="%s" alt="%s" title="%s" style="%s"></span>',
@@ -241,17 +246,17 @@ private function getPageTranslation(int $id): ?array
         $childRecords = function (array $parentIds, int $rootId, $return = []) use (&$childRecords) {
             $children = $this->connection->fetchAllAssociativeIndexed(
                 <<<SQL
-                    SELECT
-                        c.id,
-                        IFNULL(t.id, 0) AS languageMain,
-                        t.title AS mainTitle,
-                        $rootId as rootId
-                    FROM tl_page c
-                        LEFT JOIN tl_page t ON c.languageMain=t.id
-                    WHERE c.pid IN(?)
-                SQL,
+                        SELECT
+                            c.id,
+                            IFNULL(t.id, 0) AS languageMain,
+                            t.title AS mainTitle,
+                            $rootId as rootId
+                        FROM tl_page c
+                            LEFT JOIN tl_page t ON c.languageMain=t.id
+                        WHERE c.pid IN(?)
+                    SQL,
                 [array_keys($parentIds)],
-                [ArrayParameterType::INTEGER]
+                [ArrayParameterType::INTEGER],
             );
 
             if ($children) {
@@ -278,15 +283,15 @@ private function getChildTranslation(int $id, string $table, string $ptable, str
 
         $this->translationCache[$table] = $this->connection->fetchAllKeyValue(
             <<<SQL
-                SELECT
-                    c.id,
-                    IFNULL(ct.id, 0)
-                FROM $table c
-                    JOIN $ptable p ON c.pid=p.id
-                    LEFT JOIN $table ct ON c.languageMain=ct.id
-                    LEFT JOIN $ptable pt ON p.$parentField=pt.id
-                WHERE pt.id > 0
-            SQL
+                    SELECT
+                        c.id,
+                        IFNULL(ct.id, 0)
+                    FROM $table c
+                        JOIN $ptable p ON c.pid=p.id
+                        LEFT JOIN $table ct ON c.languageMain=ct.id
+                        LEFT JOIN $ptable pt ON p.$parentField=pt.id
+                    WHERE pt.id > 0
+                SQL,
         );
 
         return $this->translationCache[$table][$id] ?? null;