Merge pull request #903 from frankdekker/Improve_baseline_basepath_ca…

…lculation Improve baseline basepath calculation
phpmd · Jul 20, 2021 · 1190179 · 1190179
2 parents 42fb77b + 18d2246
commit 1190179
Show file tree

Hide file tree

Showing 11 changed files with 61 additions and 123 deletions.
diff --git a/README.rst b/README.rst
@@ -133,13 +133,14 @@ Command line options
     violations are found.
 
   - ``--generate-baseline`` - will generate a ``phpmd.baseline.xml`` for existing violations
-    next to the ruleset definition file.
+    next to the ruleset definition file. The file paths of the violations will be relative to the current
+    working directory.
 
   - ``--update-baseline`` - will remove all violations from an existing ``phpmd.baseline.xml``
-    that no longer exist. New violations will _not_ be added.
+    that no longer exist. New violations will _not_ be added. The file path of the violations will be relative
+    to the current working directory.
 
-  - ``--baseline-file`` - the filepath to a custom baseline xml file. The filepath
-    of all baselined files must be relative to this file location.
+  - ``--baseline-file`` - the filepath to a custom baseline xml file.
 
   An example command line: ::
 

diff --git a/src/main/php/PHPMD/Baseline/BaselineSet.php b/src/main/php/PHPMD/Baseline/BaselineSet.php
@@ -28,7 +28,7 @@ public function contains($ruleName, $fileName, $methodName)
         $fileName = str_replace('\\', '/', $fileName);
 
         foreach ($this->violations[$ruleName] as $baseline) {
-            if ($baseline->getFileName() === $fileName && $baseline->getMethodName() === $methodName) {
+            if ($baseline->matches($fileName, $methodName)) {
                 return true;
             }
         }

diff --git a/src/main/php/PHPMD/Baseline/BaselineSetFactory.php b/src/main/php/PHPMD/Baseline/BaselineSetFactory.php
@@ -2,7 +2,6 @@
 
 namespace PHPMD\Baseline;
 
-use PHPMD\Utility\Paths;
 use RuntimeException;
 
 class BaselineSetFactory
@@ -26,9 +25,7 @@ public static function fromFile($fileName)
             throw new RuntimeException('Unable to read xml from: ' . $fileName);
         }
 
-        $basePath    = dirname($fileName);
         $baselineSet = new BaselineSet();
-
         foreach ($xml->children() as $node) {
             if ($node->getName() !== 'violation') {
                 continue;
@@ -42,14 +39,12 @@ public static function fromFile($fileName)
                 throw new RuntimeException('Missing `file` attribute in `violation` in ' . $fileName);
             }
 
-            $ruleName   = (string)$node['rule'];
-            $filePath   = Paths::concat($basePath, (string)$node['file']);
             $methodName = null;
             if (isset($node['method']) === true && ((string)$node['method']) !== '') {
                 $methodName = (string)($node['method']);
             }
 
-            $baselineSet->addEntry(new ViolationBaseline($ruleName, $filePath, $methodName));
+            $baselineSet->addEntry(new ViolationBaseline((string)$node['rule'], (string)$node['file'], $methodName));
         }
 
         return $baselineSet;

diff --git a/src/main/php/PHPMD/Baseline/ViolationBaseline.php b/src/main/php/PHPMD/Baseline/ViolationBaseline.php
@@ -10,6 +10,9 @@ class ViolationBaseline
     /** @var string */
     private $fileName;
 
+    /** @var int */
+    private $fileNameLength;
+
     /** @var string|null */
     private $methodName;
 
@@ -20,9 +23,10 @@ class ViolationBaseline
      */
     public function __construct($ruleName, $fileName, $methodName)
     {
-        $this->ruleName   = $ruleName;
-        $this->fileName   = $fileName;
-        $this->methodName = $methodName;
+        $this->ruleName       = $ruleName;
+        $this->fileName       = $fileName;
+        $this->methodName     = $methodName;
+        $this->fileNameLength = strlen($fileName);
     }
 
     /**
@@ -34,18 +38,15 @@ public function getRuleName()
     }
 
     /**
-     * @return string
-     */
-    public function getFileName()
-    {
-        return $this->fileName;
-    }
-
-    /**
-     * @return string|null
+     * Test if the given filepath and method matches the baseline
+     *
+     * @param string      $filepath   the full filepath to match against
+     * @param string|null $methodName the name of the method of the method, if any
+     *
+     * @return bool
      */
-    public function getMethodName()
+    public function matches($filepath, $methodName)
     {
-        return $this->methodName;
+        return $this->methodName === $methodName && substr($filepath, -$this->fileNameLength) === $this->fileName;
     }
 }
diff --git a/src/main/php/PHPMD/Renderer/RendererFactory.php b/src/main/php/PHPMD/Renderer/RendererFactory.php
@@ -2,7 +2,6 @@
 
 namespace PHPMD\Renderer;
 
-use PHPMD\Utility\Paths;
 use PHPMD\Writer\StreamWriter;
 use RuntimeException;
 
@@ -14,9 +13,8 @@ class RendererFactory
      */
     public static function createBaselineRenderer(StreamWriter $writer)
     {
-        // determine basedir based on stream output filepath
-        $absolutePath = Paths::getAbsolutePath($writer->getStream());
-        $renderer     = new BaselineRenderer(dirname($absolutePath));
+        // set base path to current working directory
+        $renderer = new BaselineRenderer(getcwd());
         $renderer->setWriter($writer);
 
         return $renderer;

diff --git a/src/main/php/PHPMD/Utility/Paths.php b/src/main/php/PHPMD/Utility/Paths.php
@@ -6,20 +6,6 @@
 
 class Paths
 {
-    /**
-     * Append $pathB to $pathA and apply the correct amount of slashes between them
-     *
-     * @param string $pathA
-     * @param string $pathB
-     * @return string
-     */
-    public static function concat($pathA, $pathB)
-    {
-        $pathA = rtrim(str_replace('\\', '/', $pathA), '/');
-        $pathB = ltrim(str_replace('\\', '/', $pathB), '/');
-        return $pathA . '/' . $pathB;
-    }
-
     /**
      * Transform the given absolute path to the relative path based on the given base path.
      *
@@ -41,18 +27,6 @@ public static function getRelativePath($basePath, $filePath)
         return $filePath;
     }
 
-    /**
-     * Derive the absolute path from the given resource
-     * @param resource $resource
-     * @return string
-     * @throws RuntimeException
-     */
-    public static function getAbsolutePath($resource)
-    {
-        $metaData = stream_get_meta_data($resource);
-        return self::getRealPath($metaData['uri']);
-    }
-
     /**
      * Get the realpath of the given path or exception on failure
      * @param string $path

diff --git a/src/site/rst/documentation/index.rst b/src/site/rst/documentation/index.rst
@@ -65,13 +65,14 @@ Command line options
     violations are found.
 
   - ``--generate-baseline`` - will generate a ``phpmd.baseline.xml`` for existing violations
-    next to the ruleset definition file.
+    next to the ruleset definition file. The file paths of the violations will be relative to the current
+    working directory.
 
   - ``--update-baseline`` - will remove all violations from an existing ``phpmd.baseline.xml``
-    that no longer exist. New violations will _not_ be added.
+    that no longer exist. New violations will _not_ be added. The file path of the violations will be relative
+    to the current working directory.
 
-  - ``--baseline-file`` - the filepath to a custom baseline xml file. The filepath
-    of all baselined files must be relative to this file location.
+  - ``--baseline-file`` - the filepath to a custom baseline xml file.
 
   An example command line: ::
 

diff --git a/src/test/php/PHPMD/Baseline/ViolationBaselineTest.php b/src/test/php/PHPMD/Baseline/ViolationBaselineTest.php
@@ -12,24 +12,42 @@ class ViolationBaselineTest extends TestCase
     /**
      * @covers ::__construct
      * @covers ::getRuleName
-     * @covers ::getFileName
-     * @covers ::getMethodName
      */
-    public function testAccessorsWithoutMethod()
+    public function testGetRuleName()
     {
         $violation = new ViolationBaseline('rule', 'foobar', null);
         static::assertSame('rule', $violation->getRuleName());
-        static::assertSame('foobar', $violation->getFileName());
-        static::assertNull($violation->getMethodName());
     }
 
     /**
+     * Test the give file matches the baseline correctly
+     *
      * @covers ::__construct
-     * @covers ::getMethodName
+     * @covers ::matches
+     * @return void
      */
-    public function testAccessorsWithMethod()
+    public function testMatchesWithMethod()
     {
-        $violation = new ViolationBaseline('rule', 'foobar', 'method');
-        static::assertSame('method', $violation->getMethodName());
+        $violation = new ViolationBaseline('sniff', 'foobar.txt', 'method');
+        static::assertTrue($violation->matches('foobar.txt', 'method'));
+        static::assertTrue($violation->matches('/test/foobar.txt', 'method'));
+        static::assertFalse($violation->matches('foo.txt', 'method'));
+        static::assertFalse($violation->matches('foobar.txt', 'unknown'));
+    }
+
+    /**
+     * Test the give file matches the baseline correctly
+     *
+     * @covers ::__construct
+     * @covers ::matches
+     * @return void
+     */
+    public function testMatchesWithoutMethod()
+    {
+        $violation = new ViolationBaseline('sniff', 'foobar.txt', null);
+        static::assertTrue($violation->matches('foobar.txt', null));
+        static::assertTrue($violation->matches('/test/foobar.txt', null));
+        static::assertFalse($violation->matches('foobar.txt', 'method'));
+        static::assertFalse($violation->matches('/test/unknown.txt', null));
     }
 }
diff --git a/src/test/php/PHPMD/Renderer/RendererFactoryTest.php b/src/test/php/PHPMD/Renderer/RendererFactoryTest.php
@@ -4,7 +4,6 @@
 
 use PHPMD\AbstractTest;
 use PHPMD\Writer\StreamWriter;
-use RuntimeException;
 
 /**
  * @coversDefaultClass \PHPMD\Renderer\RendererFactory
@@ -21,15 +20,4 @@ public function testCreateBaselineRendererSuccessfully()
 
         static::assertSame($writer, $renderer->getWriter());
     }
-
-    /**
-     * @covers ::createBaselineRenderer
-     * @expectedException RuntimeException
-     * @expectedExceptionMessage Unable to determine the realpath for
-     */
-    public function testCreateBaselineRendererThrowsExceptionForInvalidStream()
-    {
-        $writer = new StreamWriter(STDOUT);
-        RendererFactory::createBaselineRenderer($writer);
-    }
 }
diff --git a/src/test/php/PHPMD/TextUI/CommandTest.php b/src/test/php/PHPMD/TextUI/CommandTest.php
@@ -18,6 +18,7 @@
 namespace PHPMD\TextUI;
 
 use PHPMD\AbstractTest;
+use PHPMD\Utility\Paths;
 
 /**
  * Test case for the {@link \PHPMD\TextUI\Command} class.
@@ -246,7 +247,7 @@ public function testMainGenerateBaseline()
 
         static::assertSame(Command::EXIT_SUCCESS, $exitCode);
         static::assertFileExists($temp);
-        static::assertContains($uri, file_get_contents($temp));
+        static::assertContains(Paths::getRelativePath(getcwd(), $uri), file_get_contents($temp));
     }
 
     /**
@@ -262,6 +263,9 @@ public function testMainUpdateBaseline()
     {
         $sourceTemp   = self::createTempFileUri('ClassWithMultipleViolations.php');
         $baselineTemp = self::createTempFileUri();
+        // set work directory to the temp dir
+        self::changeWorkingDirectory(dirname($baselineTemp));
+
         copy(static::createResourceUriForTest('UpdateBaseline/ClassWithMultipleViolations.php'), $sourceTemp);
         copy(static::createResourceUriForTest('UpdateBaseline/phpmd.baseline.xml'), $baselineTemp);
 

diff --git a/src/test/php/PHPMD/Utility/PathsTest.php b/src/test/php/PHPMD/Utility/PathsTest.php
@@ -10,30 +10,6 @@
  */
 class PathsTest extends AbstractTest
 {
-    /**
-     * @covers ::concat
-     */
-    public function testConcatShouldConcatTwoPaths()
-    {
-        static::assertSame('/foo/bar', Paths::concat('/foo', '/bar'));
-    }
-
-    /**
-     * @covers ::concat
-     */
-    public function testConcatShouldDeduplicateSlashes()
-    {
-        static::assertSame('/foo/bar', Paths::concat('/foo/', '/bar'));
-    }
-
-    /**
-     * @covers ::concat
-     */
-    public function testConcatShouldForwardAllSlashes()
-    {
-        static::assertSame('/foo/bar/text.txt', Paths::concat('/foo\\', '/bar\\text.txt'));
-    }
-
     /**
      * @covers ::getRelativePath
      */
@@ -58,24 +34,6 @@ public function testGetRelativePathShouldNotSubtractOnInfixPath()
         static::assertSame('/foo/bar/text.txt', Paths::getRelativePath('/bar', '/foo/bar/text.txt'));
     }
 
-    /**
-     * @covers ::getAbsolutePath
-     * @expectedException RuntimeException
-     */
-    public function testGetAbsolutePathShouldReturnNullForIrregularStream()
-    {
-        Paths::getAbsolutePath(fopen('php://stdout', 'wb'));
-    }
-
-    /**
-     * @covers ::getAbsolutePath
-     */
-    public function testGetAbsolutePathShouldReturnPath()
-    {
-        $path = static::createResourceUriForTest('resource.txt');
-        static::assertSame(realpath($path), Paths::getAbsolutePath(fopen($path, 'rb')));
-    }
-
     /**
      * @covers ::getRealPath
      */