From 7690f3c5ec554cd52c5e1f09bda344fbf96140ec Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 20 Nov 2023 01:05:12 +0100 Subject: [PATCH] Utils/PathHelper: split `normalize_path()` method ... into a `normalize_absolute_path()` and a `normalize_relative_path()` method. Linux paths are expected to start with a leading slash and functions like `file_exists()` will not work correctly without it. So... leading slashes should not be removed from absolute paths, but should be removed from relative paths (to allow for easy concatenation). Splitting the method and using the appropriate version in all the right places should fix this. Includes updated the tests. Includes updating function calls to the method. --- Yoast/Sniffs/Files/FileNameSniff.php | 2 +- Yoast/Tests/Utils/PathHelperTest.php | 76 +++++++++++++------ .../Tests/Utils/PathValidationHelperTest.php | 2 +- Yoast/Utils/PathHelper.php | 23 ++++-- Yoast/Utils/PathValidationHelper.php | 4 +- 5 files changed, 71 insertions(+), 36 deletions(-) diff --git a/Yoast/Sniffs/Files/FileNameSniff.php b/Yoast/Sniffs/Files/FileNameSniff.php index c08e1ac3..2946d390 100644 --- a/Yoast/Sniffs/Files/FileNameSniff.php +++ b/Yoast/Sniffs/Files/FileNameSniff.php @@ -283,7 +283,7 @@ private function is_file_excluded( File $phpcsFile, $path_to_file ) { return false; } - $path_to_file = PathHelper::normalize_path( $path_to_file ); + $path_to_file = PathHelper::normalize_absolute_path( $path_to_file ); return isset( $this->validated_excluded_files[ $path_to_file ] ); } diff --git a/Yoast/Tests/Utils/PathHelperTest.php b/Yoast/Tests/Utils/PathHelperTest.php index 93c448dd..70aec34e 100644 --- a/Yoast/Tests/Utils/PathHelperTest.php +++ b/Yoast/Tests/Utils/PathHelperTest.php @@ -15,64 +15,90 @@ final class PathHelperTest extends TestCase { /** - * Test normalizing a directory path. + * Test normalizing an absolute directory path. * * @dataProvider data_normalize_path - * @covers ::normalize_path + * @covers ::normalize_absolute_path * - * @param string $input The input string. - * @param string $expected The expected function output. + * @param string $input The input string. + * @param string $exp_absolute The expected function output. * * @return void */ - public function test_normalize_path( $input, $expected ) { - $this->assertSame( $expected, PathHelper::normalize_path( $input ) ); + public function test_normalize_absolute_path( $input, $exp_absolute ) { + $this->assertSame( $exp_absolute, PathHelper::normalize_absolute_path( $input ) ); + } + + /** + * Test normalizing a relative directory path. + * + * @dataProvider data_normalize_path + * @covers ::normalize_relative_path + * + * @param string $input The input string. + * @param string $unused Unused param. + * @param string $exp_relative The expected function output. + * + * @return void + */ + public function test_normalize_relative_path( $input, $unused, $exp_relative ) { + $this->assertSame( $exp_relative, PathHelper::normalize_relative_path( $input ) ); } /** * Data provider. * - * @see test_normalize_path() For the array format. + * @see test_normalize_absolute_path() For the array format. + * @see test_normalize_relative_path() For the array format. * * @return array> */ public static function data_normalize_path() { return [ 'path is dot' => [ - 'input' => '.', - 'expected' => './', + 'input' => '.', + 'exp_absolute' => './', + 'exp_relative' => './', ], 'path containing forward slashes only with trailing slash' => [ - 'input' => 'my/path/to/', - 'expected' => 'my/path/to/', + 'input' => 'my/path/to/', + 'exp_absolute' => 'my/path/to/', + 'exp_relative' => 'my/path/to/', ], 'path containing forward slashes only without trailing slash' => [ - 'input' => 'my/path/to', - 'expected' => 'my/path/to/', + 'input' => 'my/path/to', + 'exp_absolute' => 'my/path/to/', + 'exp_relative' => 'my/path/to/', ], 'path containing forward slashes only with leading and trailing slash' => [ - 'input' => '/my/path/to/', - 'expected' => 'my/path/to/', + 'input' => '/my/path/to/', + 'exp_absolute' => '/my/path/to/', + 'exp_relative' => 'my/path/to/', ], 'path containing back-slashes only with trailing slash' => [ - 'input' => 'my\path\to\\', - 'expected' => 'my/path/to/', + 'input' => 'my\path\to\\', + 'exp_absolute' => 'my/path/to/', + 'exp_relative' => 'my/path/to/', ], 'path containing back-slashes only without trailing slash' => [ - 'input' => 'my\path\to', - 'expected' => 'my/path/to/', + 'input' => 'my\path\to', + 'exp_absolute' => 'my/path/to/', + 'exp_relative' => 'my/path/to/', ], 'path containing back-slashes only with leading, no trailing slash' => [ - 'input' => '\my\path\to', - 'expected' => 'my/path/to/', + 'input' => '\my\path\to', + 'exp_absolute' => '/my/path/to/', + 'exp_relative' => 'my/path/to/', ], 'path containing a mix of forward and backslashes with leading and trailing slash' => [ - 'input' => '/my\path/to\\', - 'expected' => 'my/path/to/', + 'input' => '/my\path/to\\', + 'exp_absolute' => '/my/path/to/', + 'exp_relative' => 'my/path/to/', ], 'path containing a mix of forward and backslashes without trailing slash' => [ - 'input' => 'my\path/to', - 'expected' => 'my/path/to/', + 'input' => 'my\path/to', + 'exp_absolute' => 'my/path/to/', + 'exp_relative' => 'my/path/to/', ], ]; } diff --git a/Yoast/Tests/Utils/PathValidationHelperTest.php b/Yoast/Tests/Utils/PathValidationHelperTest.php index c5044992..b464305a 100644 --- a/Yoast/Tests/Utils/PathValidationHelperTest.php +++ b/Yoast/Tests/Utils/PathValidationHelperTest.php @@ -26,7 +26,7 @@ final class PathValidationHelperTest extends NonSniffTestCase { * * @var string */ - private const CLEAN_BASEPATH = 'base/path/'; + private const CLEAN_BASEPATH = '/base/path/'; /** * Test converting a set of relative paths to absolute paths when no basepath is present. diff --git a/Yoast/Utils/PathHelper.php b/Yoast/Utils/PathHelper.php index 4372aac8..866d41d0 100644 --- a/Yoast/Utils/PathHelper.php +++ b/Yoast/Utils/PathHelper.php @@ -20,17 +20,26 @@ final class PathHelper { /** - * Normalize a path to forward slashes and normalize the leading/trailing slashes. + * Normalize an absolute path to forward slashes and to include a trailing slash. * - * @param string $path File or directory path. - * Both absolute as well as relative paths are accepted. + * @param string $path Absolute file or directory path. * * @return string */ - public static function normalize_path( $path ) { - $path = self::normalize_directory_separators( $path ); - $path = self::remove_leading_slash( $path ); - return self::trailingslashit( $path ); + public static function normalize_absolute_path( $path ) { + return self::trailingslashit( self::normalize_directory_separators( $path ) ); + } + + /** + * Normalize a relative path to forward slashes and normalize the leading/trailing + * slashes (no leading, yes trailing). + * + * @param string $path Relative file or directory path. + * + * @return string + */ + public static function normalize_relative_path( $path ) { + return self::remove_leading_slash( self::normalize_absolute_path( $path ) ); } /** diff --git a/Yoast/Utils/PathValidationHelper.php b/Yoast/Utils/PathValidationHelper.php index 9852c1be..20a049b7 100644 --- a/Yoast/Utils/PathValidationHelper.php +++ b/Yoast/Utils/PathValidationHelper.php @@ -41,11 +41,11 @@ public static function relative_to_absolute( File $phpcsFile, array $relative_pa return $absolute; } - $base_path = PathHelper::normalize_path( $phpcsFile->config->basepath ); + $base_path = PathHelper::normalize_absolute_path( $phpcsFile->config->basepath ); foreach ( $relative_paths as $path ) { $result_path = \trim( $path ); - $result_path = PathHelper::normalize_path( $result_path ); + $result_path = PathHelper::normalize_relative_path( $result_path ); if ( $result_path === '' ) { continue;