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

Adds test for PHPCS
matomo-org · Oct 16, 2024 · 669cd13 · 669cd13
2 parents 241d8cf + 1c8546e
commit 669cd13
Show file tree

Hide file tree

Showing 11 changed files with 123 additions and 41 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/devicedetectorcache --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/WarmDeviceDetectorCache.php b/Commands/WarmDeviceDetectorCache.php
@@ -18,7 +18,7 @@
 
 class WarmDeviceDetectorCache extends ConsoleCommand
 {
-    const COMMAND_NAME = 'device-detector-cache:warm-cache';
+    public const COMMAND_NAME = 'device-detector-cache:warm-cache';
     /**
      * @var Configuration
      */

diff --git a/Configuration.php b/Configuration.php
@@ -13,17 +13,17 @@
 
 class Configuration
 {
-    const KEY_NumEntriesToCache = 'num_cache_entries';
-    const DEFAULT_NumEntriesToCache = 200000;
+    public const KEY_NUM_ENTRIES_TO_CACHE = 'num_cache_entries';
+    public const DEFAULT_NUM_ENTRIES_TO_CACHE = 200000;
 
-    const KEY_AccessLogRegex = 'access_log_regex';
-    const DEFAULT_AccessLogRegex = '/^(\S+) (\S+) (\S+) (\S+) \[([^:]+):(\d+:\d+:\d+) ([^\]]+)\] \"(\S+) (.*?) (\S+)\" (\S+) (\S+) "([^"]*)" "([^"]*)" (\d+)$/';
+    public const KEY_ACCESS_LOG_REGEX = 'access_log_regex';
+    public const DEFAULT_ACCESS_LOG_REGEX = '/^(\S+) (\S+) (\S+) (\S+) \[([^:]+):(\d+:\d+:\d+) ([^\]]+)\] \"(\S+) (.*?) (\S+)\" (\S+) (\S+) "([^"]*)" "([^"]*)" (\d+)$/';
 
-    const KEY_AccessLogRegexMatchEntry = 'regex_match_entry';
-    const DEFAULT_AccessLogRegexMatchEntry = 14;
+    public const KEY_ACCESS_LOG_REGEX_MATCH_ENTRY = 'regex_match_entry';
+    public const DEFAULT_ACCESS_LOG_REGEX_MATCH_ENTRY = 14;
 
-    const KEY_AccessLogPath = 'access_log_path';
-    const DEFAULT_AccessLogPath = '/var/log/httpd/access_log';
+    public const KEY_ACCESS_LOG_PATH = 'access_log_path';
+    public const DEFAULT_ACCESS_LOG_PATH = '/var/log/httpd/access_log';
 
     public function install()
     {
@@ -36,17 +36,17 @@ public function install()
         $cache = $config->DeviceDetectorCache;
 
         // we make sure to set a value only if none has been configured yet, eg in common config.
-        if (empty($cache[self::KEY_NumEntriesToCache])) {
-            $cache[self::KEY_NumEntriesToCache] = self::DEFAULT_NumEntriesToCache;
+        if (empty($cache[self::KEY_NUM_ENTRIES_TO_CACHE])) {
+            $cache[self::KEY_NUM_ENTRIES_TO_CACHE] = self::DEFAULT_NUM_ENTRIES_TO_CACHE;
         }
-        if (empty($cache[self::KEY_AccessLogPath])) {
-            $cache[self::KEY_AccessLogPath] = self::DEFAULT_AccessLogPath;
+        if (empty($cache[self::KEY_ACCESS_LOG_PATH])) {
+            $cache[self::KEY_ACCESS_LOG_PATH] = self::DEFAULT_ACCESS_LOG_PATH;
         }
-        if (empty($cache[self::KEY_AccessLogRegex])) {
-            $cache[self::KEY_AccessLogRegex] = self::DEFAULT_AccessLogRegex;
+        if (empty($cache[self::KEY_ACCESS_LOG_REGEX])) {
+            $cache[self::KEY_ACCESS_LOG_REGEX] = self::DEFAULT_ACCESS_LOG_REGEX;
         }
-        if (empty($cache[self::KEY_AccessLogRegexMatchEntry])) {
-            $cache[self::KEY_AccessLogRegexMatchEntry] = self::DEFAULT_AccessLogRegexMatchEntry;
+        if (empty($cache[self::KEY_ACCESS_LOG_REGEX_MATCH_ENTRY])) {
+            $cache[self::KEY_ACCESS_LOG_REGEX_MATCH_ENTRY] = self::DEFAULT_ACCESS_LOG_REGEX_MATCH_ENTRY;
         }
 
         $config->DeviceDetectorCache = $cache;
@@ -66,31 +66,31 @@ public function uninstall()
      */
     public function getAccessLogPath()
     {
-        return $this->getConfigValue(self::KEY_AccessLogPath, self::DEFAULT_AccessLogPath);
+        return $this->getConfigValue(self::KEY_ACCESS_LOG_PATH, self::DEFAULT_ACCESS_LOG_PATH);
     }
 
     /**
      * @return string
      */
     public function getAccessLogRegex()
     {
-        return $this->getConfigValue(self::KEY_AccessLogRegex, self::DEFAULT_AccessLogRegex);
+        return $this->getConfigValue(self::KEY_ACCESS_LOG_REGEX, self::DEFAULT_ACCESS_LOG_REGEX);
     }
 
     /**
      * @return string
      */
     public function getRegexMatchEntry()
     {
-        return (int)$this->getConfigValue(self::KEY_AccessLogRegexMatchEntry, self::DEFAULT_AccessLogRegexMatchEntry);
+        return (int)$this->getConfigValue(self::KEY_ACCESS_LOG_REGEX_MATCH_ENTRY, self::DEFAULT_ACCESS_LOG_REGEX_MATCH_ENTRY);
     }
 
     /**
      * @return string
      */
     public function getNumEntriesToCache()
     {
-        return (int)$this->getConfigValue(self::KEY_NumEntriesToCache, self::DEFAULT_NumEntriesToCache);
+        return (int)$this->getConfigValue(self::KEY_NUM_ENTRIES_TO_CACHE, self::DEFAULT_NUM_ENTRIES_TO_CACHE);
     }
 
     private function getConfig()

diff --git a/DeviceDetector/CachedBrowserParser.php b/DeviceDetector/CachedBrowserParser.php
@@ -29,4 +29,4 @@ protected function parseBrowserFromUserAgent(): array
 
         return parent::parseBrowserFromUserAgent();
     }
-}
+}
diff --git a/DeviceDetector/CachedOperatingSystemParser.php b/DeviceDetector/CachedOperatingSystemParser.php
@@ -29,4 +29,4 @@ protected function parseOsFromUserAgent(): array
 
         return parent::parseOsFromUserAgent();
     }
-}
+}
diff --git a/Factory.php b/Factory.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -21,4 +22,4 @@ protected function getDeviceDetectionInfo($userAgent, array $clientHints = [])
 
         return parent::getDeviceDetectionInfo($userAgent, $clientHints);
     }
-}
+}
diff --git a/phpcs.xml b/phpcs.xml
@@ -0,0 +1,36 @@
+<?xml version="1.0"?>
+<ruleset name="deviceDetectorCache" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="vendor/squizlabs/php_codesniffer/phpcs.xsd">
+
+    <description>Matomo Coding Standard for DeviceDetectorCache 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/Integration/CachedEntryTest.php b/tests/Integration/CachedEntryTest.php
@@ -46,9 +46,9 @@ public function testConstructSomeValues()
         $values = [
             'bot' => 'testBot',
             'brand' => 'testBrand',
-            'client'=> 'testClient',
-            'device'=> 2,
-            'model'=> 'testModel',
+            'client' => 'testClient',
+            'device' => 2,
+            'model' => 'testModel',
             'os' => 'testOs',
         ];
 

diff --git a/tests/Integration/ConfigurationTest.php b/tests/Integration/ConfigurationTest.php
@@ -42,59 +42,59 @@ public function test_shouldInstallConfig()
         $this->assertEquals([
             'num_cache_entries' => '200000',
             'access_log_path'   => '/var/log/httpd/access_log',
-            'access_log_regex'  => Configuration::DEFAULT_AccessLogRegex,
+            'access_log_regex'  => Configuration::DEFAULT_ACCESS_LOG_REGEX,
             'regex_match_entry' => 14,
         ], $configs);
     }
 
     public function test_getRegexMatchEntry()
     {
-        $this->assertSame(Configuration::DEFAULT_AccessLogRegexMatchEntry, $this->configuration->getRegexMatchEntry());
+        $this->assertSame(Configuration::DEFAULT_ACCESS_LOG_REGEX_MATCH_ENTRY, $this->configuration->getRegexMatchEntry());
     }
 
     public function test_getRegexMatchEntry_customValue()
     {
         Config::getInstance()->DeviceDetectorCache = [
-            Configuration::KEY_AccessLogRegexMatchEntry => '5',
+            Configuration::KEY_ACCESS_LOG_REGEX_MATCH_ENTRY => '5',
         ];
         $this->assertEquals(5, $this->configuration->getRegexMatchEntry());
     }
 
     public function test_getAccessLogPath()
     {
-        $this->assertSame(Configuration::DEFAULT_AccessLogPath, $this->configuration->getAccessLogPath());
+        $this->assertSame(Configuration::DEFAULT_ACCESS_LOG_PATH, $this->configuration->getAccessLogPath());
     }
 
     public function test_getAccessLogPath_customValue()
     {
         Config::getInstance()->DeviceDetectorCache = [
-            Configuration::KEY_AccessLogPath => '/var/log/foo',
+            Configuration::KEY_ACCESS_LOG_PATH => '/var/log/foo',
         ];
         $this->assertEquals('/var/log/foo', $this->configuration->getAccessLogPath());
     }
 
     public function test_getNumEntriesToCache()
     {
-        $this->assertSame(Configuration::DEFAULT_NumEntriesToCache, $this->configuration->getNumEntriesToCache());
+        $this->assertSame(Configuration::DEFAULT_NUM_ENTRIES_TO_CACHE, $this->configuration->getNumEntriesToCache());
     }
 
     public function test_getNumEntriesToCache_customValue()
     {
         Config::getInstance()->DeviceDetectorCache = [
-            Configuration::KEY_NumEntriesToCache => '145',
+            Configuration::KEY_NUM_ENTRIES_TO_CACHE => '145',
         ];
         $this->assertEquals(145, $this->configuration->getNumEntriesToCache());
     }
 
     public function test_getAccessLogRegex()
     {
-        $this->assertSame(Configuration::DEFAULT_AccessLogRegex, $this->configuration->getAccessLogRegex());
+        $this->assertSame(Configuration::DEFAULT_ACCESS_LOG_REGEX, $this->configuration->getAccessLogRegex());
     }
 
     public function test_getAccessLogRegex_customValue()
     {
         Config::getInstance()->DeviceDetectorCache = [
-            Configuration::KEY_AccessLogRegex => '(.*)',
+            Configuration::KEY_ACCESS_LOG_REGEX => '(.*)',
         ];
         $this->assertEquals('(.*)', $this->configuration->getAccessLogRegex());
     }

diff --git a/tests/Integration/WarmDeviceDetectorCacheTest.php b/tests/Integration/WarmDeviceDetectorCacheTest.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Matomo - free/libre analytics platform
  *
@@ -26,7 +27,7 @@ class WarmDeviceDetectorCacheTest extends ConsoleCommandTestCase
     public function setUp(): void
     {
         parent::setUp();
-        CachedEntry::setCacheDir(PIWIK_DOCUMENT_ROOT. '/tmp/devicecachetests/');
+        CachedEntry::setCacheDir(PIWIK_DOCUMENT_ROOT . '/tmp/devicecachetests/');
         CachedEntry::clearCacheDir();
     }
 
@@ -41,15 +42,15 @@ private function setAccessLogFile($file)
     {
         $config                              = \Piwik\Config::getInstance();
         $d                                   = $config->DeviceDetectorCache;
-        $d[Configuration::KEY_AccessLogPath] = $file;
+        $d[Configuration::KEY_ACCESS_LOG_PATH] = $file;
         $config->DeviceDetectorCache         = $d;
     }
 
     private function setCountProcessNumEntries($numEntries)
     {
         $config                                  = \Piwik\Config::getInstance();
         $d                                       = $config->DeviceDetectorCache;
-        $d[Configuration::KEY_NumEntriesToCache] = $numEntries;
+        $d[Configuration::KEY_NUM_ENTRIES_TO_CACHE] = $numEntries;
         $config->DeviceDetectorCache             = $d;
     }
 
@@ -203,4 +204,4 @@ private function assertUserAgentWrittenToFile($userAgent)
         $this->assertEquals($deviceDetectionParsed->getModel(), $deviceDetectionFromFile->getModel());
         $this->assertEquals($deviceDetectionParsed->getOs(), $deviceDetectionFromFile->getOs());
     }
-}
+}
diff --git a/tests/System/CheckDirectDependencyUseCommandTest.php b/tests/System/CheckDirectDependencyUseCommandTest.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Piwik - free/libre analytics platform
  *
@@ -49,4 +50,4 @@ public function testCommand()
             ]
         ], $checkDirectDependencyUse->usesFoundList[$pluginName]);
     }
-}
+}