Merge pull request #71 from matomo-org/spice-psr

Adds test for PHPCS
matomo-org · Oct 17, 2024 · b3e7dfb · b3e7dfb
2 parents 5110417 + 96e7066
commit b3e7dfb
Show file tree

Hide file tree

Showing 24 changed files with 129 additions and 25 deletions.
diff --git a/.github/workflows/phpcs.yml b/.github/workflows/phpcs.yml
@@ -0,0 +1,43 @@
+name: PHPCS check
+
+on: pull_request
+
+permissions:
+  actions: read
+  checks: read
+  contents: read
+  deployments: none
+  issues: read
+  packages: none
+  pull-requests: read
+  repository-projects: none
+  security-events: none
+  statuses: read
+
+jobs:
+  phpcs:
+    name: PHPCS
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          lfs: false
+          persist-credentials: false
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: '7.4'
+          tools: cs2pr
+      - name: Install dependencies
+        run:
+          composer init --name=matomo/migration --quiet;
+          composer --no-plugins config allow-plugins.dealerdirect/phpcodesniffer-composer-installer true -n;
+          composer config repositories.matomo-coding-standards vcs https://github.com/matomo-org/matomo-coding-standards -n;
+          composer require matomo-org/matomo-coding-standards:dev-master;
+          composer install --dev --prefer-dist --no-progress --no-suggest
+      - name: Check PHP code styles
+        id: phpcs
+        run: ./vendor/bin/phpcs --report-full --standard=phpcs.xml --report-checkstyle=./phpcs-report.xml
+      - name: Show PHPCS results in PR
+        if: ${{ always() && steps.phpcs.outcome == 'failure' }}
+        run: cs2pr ./phpcs-report.xml --prepend-filename
diff --git a/Commands/Migrate.php b/Commands/Migrate.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -132,6 +133,6 @@ private function makeTargetDb()
 
     private function confirmMigration($idSite): bool
     {
-        return $this->askForConfirmation('<question>Are you sure you want to migrate the data for idSite '.(int) $idSite.'. (y/N)</question>');
+        return $this->askForConfirmation('<question>Are you sure you want to migrate the data for idSite ' . (int) $idSite . '. (y/N)</question>');
     }
 }
diff --git a/Db/BatchQuery.php b/Db/BatchQuery.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

diff --git a/Migration.php b/Migration.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

diff --git a/Migrations.php b/Migrations.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -41,7 +42,11 @@ public function migrate($migrations, Request $request, TargetDb $targetDb)
                 $this->log('Processed ' . $migration->getName());
             }
         } catch (\Exception $e) {
-            //Since php8, PDO::inTransaction() now reports the actual transaction state of the connection, rather than an approximation maintained by PDO. If a query that is subject to "implicit commit" is executed, PDO::inTransaction() will subsequently return false, as a transaction is no longer active
+            /**
+             * Since php8, PDO::inTransaction() now reports the actual transaction state of the connection, rather than
+             * an approximation maintained by PDO. If a query that is subject to "implicit commit" is executed,
+             * PDO::inTransaction() will subsequently return false, as a transaction is no longer active
+             */
             //inTransaction check fixes warning raised due to implicit commit change
             if ($this->isInTransaction($targetDb)) {
                 $targetDb->rollBack();
@@ -51,7 +56,11 @@ public function migrate($migrations, Request $request, TargetDb $targetDb)
             }
             throw $e;
         }
-        //Since php8, PDO::inTransaction() now reports the actual transaction state of the connection, rather than an approximation maintained by PDO. If a query that is subject to "implicit commit" is executed, PDO::inTransaction() will subsequently return false, as a transaction is no longer active
+        /**
+         * Since php8, PDO::inTransaction() now reports the actual transaction state of the connection, rather than an
+         * approximation maintained by PDO. If a query that is subject to "implicit commit" is executed,
+         * PDO::inTransaction() will subsequently return false, as a transaction is no longer active
+         */
         //inTransaction check fixes warning raised due to implicit commit change
         if ($this->isInTransaction($targetDb)) {
             $targetDb->commit();
@@ -72,7 +81,7 @@ public function onLog($callback)
 
     private function isInTransaction($targetDb)
     {
-        $inTransactionMethodExists = method_exists($targetDb->getDb()->getConnection(),'inTransaction');
+        $inTransactionMethodExists = method_exists($targetDb->getDb()->getConnection(), 'inTransaction');
 
         return (!$inTransactionMethodExists || $targetDb->getDb()->getConnection()->inTransaction());
     }

diff --git a/Migrations/AnnotationsMigration.php b/Migrations/AnnotationsMigration.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

diff --git a/Migrations/ArchiveMigration.php b/Migrations/ArchiveMigration.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -9,7 +10,6 @@
 namespace Piwik\Plugins\Migration\Migrations;
 
 use Piwik\Common;
-use Piwik\Config;
 use Piwik\DataAccess\ArchiveTableCreator;
 use Piwik\Plugins\Migration\Db\BatchQuery;
 use Piwik\Plugins\Migration\TargetDb;

diff --git a/Migrations/BaseMigration.php b/Migrations/BaseMigration.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -33,7 +34,7 @@ abstract public function migrate(Request $request, TargetDb $targetDb);
 
     protected function migrateEntities(Request $request, TargetDb $targetDb, $tableUnprefixed, $entityName, $unsetId = null, $idSiteColumnName = 'idsite')
     {
-        $rows = Db::fetchAll('SELECT * FROM ' . Common::prefixTable($tableUnprefixed) . ' WHERE '.$idSiteColumnName.' = ?', array($request->sourceIdSite));
+        $rows = Db::fetchAll('SELECT * FROM ' . Common::prefixTable($tableUnprefixed) . ' WHERE ' . $idSiteColumnName . ' = ?', array($request->sourceIdSite));
 
         $this->log(sprintf('Found %s %s', count($rows), $entityName));
 

diff --git a/Migrations/CustomDimensionMigration.php b/Migrations/CustomDimensionMigration.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

diff --git a/Migrations/GoalsMigration.php b/Migrations/GoalsMigration.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

diff --git a/Migrations/LogActionMigration.php b/Migrations/LogActionMigration.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

diff --git a/Migrations/LogMigration.php b/Migrations/LogMigration.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -41,7 +42,7 @@ public function migrate(Request $request, TargetDb $targetDb)
 
         $loggedAt = array(0.05, 0.1, 0.2, 0.4, 0.6, 0.8, 0.9);
 
-        foreach ($batchQuery->generateQuery('SELECT * FROM ' . Common::prefixTable('log_visit') . ' WHERE idsite = ? ORDER BY idvisit ASC',  array($request->sourceIdSite)) as $visitRows) {
+        foreach ($batchQuery->generateQuery('SELECT * FROM ' . Common::prefixTable('log_visit') . ' WHERE idsite = ? ORDER BY idvisit ASC', array($request->sourceIdSite)) as $visitRows) {
             $count += count($visitRows);
             $this->migrateVisits($visitRows, $request, $logActionMigration, $targetDb);
 
@@ -74,7 +75,7 @@ private function migrateVisits($visitRows, Request $request, LogActionMigration
         $visitorIds = implode(',', $visitorIds);
 
         $batchQuery = new BatchQuery();
-        foreach ($batchQuery->generateQuery('SELECT * FROM ' . Common::prefixTable('log_link_visit_action') . ' WHERE idvisit in ('.$visitorIds.') ORDER BY idlink_va ASC') as $actionRows) {
+        foreach ($batchQuery->generateQuery('SELECT * FROM ' . Common::prefixTable('log_link_visit_action') . ' WHERE idvisit in (' . $visitorIds . ') ORDER BY idlink_va ASC') as $actionRows) {
             foreach ($actionRows as $row) {
                 $oldIdLinkAction = $row['idlink_va'];
                 $row['idvisit'] = $visitorIdMap[$row['idvisit']];
@@ -91,7 +92,7 @@ private function migrateVisits($visitRows, Request $request, LogActionMigration
         }
         unset($actionRows);
 
-        $rows = Db::fetchAll('SELECT * FROM ' . Common::prefixTable('log_conversion') . ' WHERE idvisit in ('.$visitorIds.')');
+        $rows = Db::fetchAll('SELECT * FROM ' . Common::prefixTable('log_conversion') . ' WHERE idvisit in (' . $visitorIds . ')');
         foreach ($rows as $row) {
             $row['idvisit'] = $visitorIdMap[$row['idvisit']];
             if (isset($row['idlink_va'])) {
@@ -106,7 +107,7 @@ private function migrateVisits($visitRows, Request $request, LogActionMigration
 
         unset($rows);
 
-        $rows = Db::fetchAll('SELECT * FROM ' . Common::prefixTable('log_conversion_item') . ' WHERE idvisit in ('.$visitorIds.')');
+        $rows = Db::fetchAll('SELECT * FROM ' . Common::prefixTable('log_conversion_item') . ' WHERE idvisit in (' . $visitorIds . ')');
         foreach ($rows as $row) {
             $row['idvisit'] = $visitorIdMap[$row['idvisit']];
             $row['idsite'] = $request->targetIdSite;
@@ -136,5 +137,4 @@ private function migrateVisits($visitRows, Request $request, LogActionMigration
 
         unset($visitRows);
     }
-
 }
diff --git a/Migrations/Provider.php b/Migrations/Provider.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -29,8 +30,10 @@ public function getAllMigrations($skipLogs, $skipArchives)
         ];
 
         $pluginManager = Plugin\Manager::getInstance();
-        if ($pluginManager->isPluginActivated('CustomDimensions')
-            && $pluginManager->isPluginLoaded('CustomDimensions')) {
+        if (
+            $pluginManager->isPluginActivated('CustomDimensions')
+            && $pluginManager->isPluginLoaded('CustomDimensions')
+        ) {
             $migrations[] = new CustomDimensionMigration();
         }
 

diff --git a/Migrations/Request.php b/Migrations/Request.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -19,5 +20,4 @@ class Request
      * @var int
      */
     public $targetIdSite;
-
 }
diff --git a/Migrations/SegmentsMigration.php b/Migrations/SegmentsMigration.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

diff --git a/Migrations/SiteMigration.php b/Migrations/SiteMigration.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

diff --git a/Migrations/SiteSettingMigration.php b/Migrations/SiteSettingMigration.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

diff --git a/Migrations/SiteUrlMigration.php b/Migrations/SiteUrlMigration.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

diff --git a/TargetDb.php b/TargetDb.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *

diff --git a/phpcs.xml b/phpcs.xml
@@ -0,0 +1,36 @@
+<?xml version="1.0"?>
+<ruleset name="migration" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="vendor/squizlabs/php_codesniffer/phpcs.xsd">
+
+    <description>Matomo Coding Standard for Migration plugin</description>
+
+    <arg name="extensions" value="php" />
+
+    <file>.</file>
+
+    <exclude-pattern>tests/javascript/*</exclude-pattern>
+    <exclude-pattern>*/vendor/*</exclude-pattern>
+
+    <rule ref="Matomo"></rule>
+
+    <rule ref="Generic.Files.LineLength">
+        <properties>
+            <property name="lineLimit" value="250" />
+        </properties>
+        <exclude-pattern>tests/*</exclude-pattern>
+    </rule>
+
+    <rule ref="Squiz.Classes.ValidClassName.NotCamelCaps">
+        <!-- Classnames for our update files don't match PascalCase, this can't be changed easily -->
+        <exclude-pattern>Updates/*</exclude-pattern>
+    </rule>
+
+    <rule ref="PSR1.Methods.CamelCapsMethodName.NotCamelCaps">
+        <!-- Allow using method name without camel caps in tests as long as some methods are named test_* -->
+        <exclude-pattern>tests/*</exclude-pattern>
+    </rule>
+
+    <rule ref="PSR1.Classes.ClassDeclaration.MultipleClasses">
+        <!-- Allow using multiple classes in one file for tests -->
+        <exclude-pattern>tests/*</exclude-pattern>
+    </rule>
+</ruleset>
diff --git a/tests/Fixtures/MigrationFixture.php b/tests/Fixtures/MigrationFixture.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -31,7 +32,7 @@ class MigrationFixture extends Fixture
     public $dateTime = '2013-01-23 01:23:45';
     public $idSite = 1;
 
-    const TARGET_DB_PREFIX = 'targetdb_';
+    public const TARGET_DB_PREFIX = 'targetdb_';
 
     private $goals = array(
         array('name' => 'Download Software',  'match' => 'url', 'pattern' => 'download',   'patternType' => 'contains', 'revenue' => 0.10),
@@ -64,7 +65,7 @@ public function setUp(): void
         $this->trackSecondVisit();
 
         // make sure to archive data
-        Request::processRequest('API.get',array(
+        Request::processRequest('API.get', array(
             'period' => 'year',
             'date' => '2013-01-23',
             'idSite' => $this->idSite
@@ -74,9 +75,9 @@ public function setUp(): void
     private function setUpCustomDimensions()
     {
         $configuration = new Configuration();
-        $configuration->configureNewDimension($this->idSite,  'MyName1', CustomDimensions::SCOPE_VISIT, 1, $active = true, $extractions = array(), $caseSensitive = true);
-        $configuration->configureNewDimension($this->idSite,  'MyName2', CustomDimensions::SCOPE_ACTION, 1, $active = true, $extractions = array(), $caseSensitive = true);
-        $configuration->configureNewDimension($this->idSite,  'MyName3', CustomDimensions::SCOPE_ACTION, 2, $active = false, $extractions = array(), $caseSensitive = true);
+        $configuration->configureNewDimension($this->idSite, 'MyName1', CustomDimensions::SCOPE_VISIT, 1, $active = true, $extractions = array(), $caseSensitive = true);
+        $configuration->configureNewDimension($this->idSite, 'MyName2', CustomDimensions::SCOPE_ACTION, 1, $active = true, $extractions = array(), $caseSensitive = true);
+        $configuration->configureNewDimension($this->idSite, 'MyName3', CustomDimensions::SCOPE_ACTION, 2, $active = false, $extractions = array(), $caseSensitive = true);
     }
 
     private function setUpSegments()
@@ -109,7 +110,7 @@ public static function copyTableStructure(TargetDb $targetDb)
 
             $row = Db::fetchRow('SHOW CREATE TABLE `' . $table . '`');
             $sql = $row['Create Table'];
-            $sql = str_replace('`'.$table.'`', '`'.$tablePrefixed.'`', $sql);
+            $sql = str_replace('`' . $table . '`', '`' . $tablePrefixed . '`', $sql);
             $targetDb->getDb()->query($sql);
         }
     }
@@ -190,4 +191,4 @@ protected function trackSecondVisit()
         $t->addEcommerceItem($sku = 'SKU_ID2', $name = 'A durable item', $category = 'Best seller', $price = 321);
         self::checkResponse($t->doTrackEcommerceCartUpdate($grandTotal = 33 * 77));
     }
-}
+}
diff --git a/tests/Integration/Migrations/LogActionMigrationTest.php b/tests/Integration/Migrations/LogActionMigrationTest.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -76,5 +77,4 @@ public function test_migrateAction()
         $targetId3 = $this->migration->migrateAction($id3, $this->targetDb);
         $this->assertEquals(3, $targetId3);
     }
-
 }
diff --git a/tests/Integration/TargetDbTest.php b/tests/Integration/TargetDbTest.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -136,5 +137,4 @@ public function test_update()
             array ('idsite' => '4', 'url' => 'https://www.foobaz.com'),
         ), $urls);
     }
-
 }