Use content instead of line number for the fingerprint

micheh · micheh · commit 2909ede81602 · 2024-12-30T20:53:09.000+01:00
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -26,7 +26,7 @@ jobs:
       - uses: shivammathur/setup-php@2.31.1
         with:
           php-version: ${{ matrix.php-versions }}
-          coverage: pcov
+          coverage: xdebug
         if: matrix.php-versions == '7.4' && matrix.dependencies == 'highest'
 
       - name: Setup problem matcher for PHPUnit
diff --git a/composer.json b/composer.json
@@ -24,7 +24,7 @@
     "require-dev": {
         "phpstan/phpstan": "^2.0",
         "phpunit/phpunit": "^8.5.14 || ^9.0",
-        "squizlabs/php_codesniffer": "^3.3.1"
+        "squizlabs/php_codesniffer": "^3.5.0"
     },
     "autoload": {
         "psr-4": {
diff --git a/phpcs.xml.dist b/phpcs.xml.dist
@@ -15,4 +15,5 @@
 
     <file>src</file>
     <file>tests</file>
+    <exclude-pattern>tests/_files/*</exclude-pattern>
 </ruleset>
diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -4,5 +4,7 @@ parameters:
   paths:
     - src
     - tests
+  excludePaths:
+    - tests/_files/
   scanDirectories:
     - vendor/squizlabs/php_codesniffer
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -7,6 +7,12 @@
          colors="true"
          forceCoversAnnotation="true"
 >
+    <php>
+        <const name="PHP_CODESNIFFER_IN_TESTS" value="true"/>
+        <const name="PHP_CODESNIFFER_CBF" value="false"/>
+        <const name="PHP_CODESNIFFER_VERBOSITY" value="0"/>
+    </php>
+
     <testsuites>
         <testsuite name="Project Test Suite">
             <directory>tests</directory>
diff --git a/src/Report/Gitlab.php b/src/Report/Gitlab.php
@@ -11,8 +11,11 @@
 
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Reports\Report;
+use SplFileObject;
 
+use function is_string;
 use function md5;
+use function preg_replace;
 use function rtrim;
 use function str_replace;
 
@@ -26,15 +29,25 @@ class Gitlab implements Report
     public function generateFileReport($report, File $phpcsFile, $showSources = false, $width = 80)
     {
         $hasOutput = false;
+        $fingerprints = [];
 
         foreach ($report['messages'] as $line => $lineErrors) {
-            foreach ($lineErrors as $column => $colErrors) {
+            $lineContent = $this->getContentOfLine($phpcsFile->getFilename(), $line);
+
+            foreach ($lineErrors as $colErrors) {
                 foreach ($colErrors as $error) {
+                    $fingerprint = md5($report['filename'] . $lineContent . $error['source']);
+                    if (isset($fingerprints[$fingerprint])) {
+                        ++$fingerprints[$fingerprint];
+                    } else {
+                        $fingerprints[$fingerprint] = 1;
+                    }
+
                     $issue = [
                         'type' => 'issue',
                         'categories' => ['Style'],
                         'check_name' => $error['source'],
-                        'fingerprint' => md5($report['filename'] . $error['message'] . $line . $column),
+                        'fingerprint' => $fingerprint . '-' . $fingerprints[$fingerprint],
                         'severity' => $error['type'] === 'ERROR' ? 'major' : 'minor',
                         'description' => str_replace(["\n", "\r", "\t"], ['\n', '\r', '\t'], $error['message']),
                         'location' => [
@@ -68,4 +81,25 @@ public function generate(
     ) {
         echo '[' . rtrim($cachedData, ',') . ']' . PHP_EOL;
     }
+
+    /**
+     * @param string $filename
+     * @param int $line
+     * @return string
+     */
+    private function getContentOfLine($filename, $line)
+    {
+        $file = new SplFileObject($filename);
+
+        if (!$file->eof()) {
+            $file->seek($line - 1);
+            $contents = $file->current();
+
+            if (is_string($contents)) {
+                return (string) preg_replace('/\s+/', '', $contents);
+            }
+        }
+
+        return '';
+    }
 }
diff --git a/tests/Fixtures/MixedViolations.php b/tests/Fixtures/MixedViolations.php
diff --git a/tests/Fixtures/ReportFixture.php b/tests/Fixtures/ReportFixture.php
diff --git a/tests/Fixtures/SingleViolation.php b/tests/Fixtures/SingleViolation.php
diff --git a/tests/Report/GitlabTest.php b/tests/Report/GitlabTest.php
@@ -10,23 +10,35 @@
 namespace MichehTest\PhpCodeSniffer\Report;
 
 use Micheh\PhpCodeSniffer\Report\Gitlab;
-use MichehTest\PhpCodeSniffer\Fixtures\MixedViolations;
-use MichehTest\PhpCodeSniffer\Fixtures\ReportFixture;
-use MichehTest\PhpCodeSniffer\Fixtures\SingleViolation;
+use PHP_CodeSniffer\Config;
 use PHP_CodeSniffer\Files\File;
+use PHP_CodeSniffer\Files\LocalFile;
+use PHP_CodeSniffer\Reporter;
+use PHP_CodeSniffer\Ruleset;
 use PHPUnit\Framework\TestCase;
 
+use function file_get_contents;
+
 class GitlabTest extends TestCase
 {
     /**
      * @var Gitlab
      */
     private $report;
 
+    /**
+     * @var Config
+     */
+    private $config;
+
 
     protected function setUp(): void
     {
         $this->report = new Gitlab();
+
+        $this->config = new Config();
+        $this->config->basepath = __DIR__ . '/../';
+        $this->config->standards = ['PSR12'];
     }
 
     /**
@@ -48,27 +60,49 @@ public function testGenerateWithEmpty(): void
     }
 
     /**
-     * @return array<string, array<class-string<ReportFixture>>>
+     * @return array<string, array<string>>
      */
     public function violations(): array
     {
         return [
-            'single' => [SingleViolation::class],
-            'mixed' => [MixedViolations::class],
+            'single' => ['Single'],
+            'mixed' => ['Mixed'],
+            'multiple' => ['Multiple'],
         ];
     }
 
     /**
-     * @param class-string<ReportFixture> $class
      * @covers \Micheh\PhpCodeSniffer\Report\Gitlab::generateFileReport
      * @dataProvider violations
      */
-    public function testGenerateFileReport(string $class): void
+    public function testGenerateFileReport(string $fileName): void
+    {
+        $phpPath = __DIR__ . '/../_files/' . $fileName . '.php';
+        self::assertFileExists($phpPath);
+
+        $outputPath = __DIR__ . '/../_files/' . $fileName . '.json';
+        self::assertFileExists($outputPath);
+
+        $file = $this->createFile($phpPath);
+
+        $this->expectOutputString((string) file_get_contents($outputPath));
+        $this->report->generateFileReport($this->getReportData($file), $file);
+    }
+
+    private function createFile(string $path): File
     {
-        $fixture = new $class();
-        self::assertInstanceOf(ReportFixture::class, $fixture);
+        $file = new LocalFile($path, new Ruleset($this->config), $this->config);
+        $file->process();
+
+        return $file;
+    }
 
-        $this->expectOutputString($fixture->getExpectedOutput());
-        $this->report->generateFileReport($fixture->getReportData(), $this->createMock(File::class));
+    /**
+     * @return array{filename: string, errors: int, warnings: int, fixable: int, messages: array<mixed>}
+     */
+    private function getReportData(File $file): array
+    {
+        $reporter = new Reporter($this->config);
+        return $reporter->prepareFileReport($file); // @phpstan-ignore return.type
     }
 }
diff --git a/tests/_files/Mixed.json b/tests/_files/Mixed.json
@@ -0,0 +1 @@
+{"type":"issue","categories":["Style"],"check_name":"Squiz.Functions.MultiLineFunctionDeclaration.BraceOnSameLine","fingerprint":"0da49d5a2f6b30f151e4abef2fccd4e2-1","severity":"major","description":"Opening brace should be on a new line","location":{"path":"_files/Mixed.php","lines":{"begin":7,"end":7}}},{"type":"issue","categories":["Style"],"check_name":"Generic.Files.LineLength.TooLong","fingerprint":"0be4b9620fcab36dcef7ba50bd3ac097-1","severity":"minor","description":"Line exceeds 120 characters; contains 143 characters","location":{"path":"_files/Mixed.php","lines":{"begin":8,"end":8}}},
diff --git a/tests/_files/Mixed.php b/tests/_files/Mixed.php
@@ -0,0 +1,10 @@
+<?php
+
+namespace DoNotUse;
+
+class Mixed
+{
+    public function __invoke(): string {
+        return 'mixed mixed mixed mixed mixed mixed mixed mixed mixed mixed mixed mixed mixed mixed mixed mixed mixed mixed mixed mixed mixed';
+    }
+}
diff --git a/tests/_files/Multiple.json b/tests/_files/Multiple.json
@@ -0,0 +1 @@
+{"type":"issue","categories":["Style"],"check_name":"PSR2.Methods.FunctionCallSignature.SpaceAfterOpenBracket","fingerprint":"e187fcc76a1b52d178447fea5370dfb1-1","severity":"major","description":"Space after opening parenthesis of function call prohibited","location":{"path":"_files/Multiple.php","lines":{"begin":11,"end":11}}},{"type":"issue","categories":["Style"],"check_name":"PSR2.Methods.FunctionCallSignature.SpaceAfterOpenBracket","fingerprint":"e187fcc76a1b52d178447fea5370dfb1-2","severity":"major","description":"Space after opening parenthesis of function call prohibited","location":{"path":"_files/Multiple.php","lines":{"begin":12,"end":12}}},
diff --git a/tests/_files/Multiple.php b/tests/_files/Multiple.php
@@ -0,0 +1,16 @@
+<?php
+
+namespace DoNotUse;
+
+use function min;
+
+class Multiple
+{
+    public function __invoke(): string
+    {
+        min( 1, 2);
+        min( 1, 2);
+
+        return 'multiple';
+    }
+}
diff --git a/tests/_files/Single.json b/tests/_files/Single.json
@@ -0,0 +1 @@
+{"type":"issue","categories":["Style"],"check_name":"Squiz.Functions.MultiLineFunctionDeclaration.BraceOnSameLine","fingerprint":"98816311d045b285f10765696fa38858-1","severity":"major","description":"Opening brace should be on a new line","location":{"path":"_files/Single.php","lines":{"begin":7,"end":7}}},
diff --git a/tests/_files/Single.php b/tests/_files/Single.php
@@ -0,0 +1,10 @@
+<?php
+
+namespace DoNotUse;
+
+class Single
+{
+    public function __invoke(): string {
+        return 'single';
+    }
+}
