Skip to content

Commit

Permalink
Utils/PathHelper: split normalize_path() method
Browse files Browse the repository at this point in the history
... 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.
  • Loading branch information
jrfnl committed Nov 20, 2023
1 parent e6c1b98 commit 7690f3c
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 36 deletions.
2 changes: 1 addition & 1 deletion Yoast/Sniffs/Files/FileNameSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 ] );
}
Expand Down
76 changes: 51 additions & 25 deletions Yoast/Tests/Utils/PathHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, array<string, string>>
*/
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/',
],
];
}
Expand Down
2 changes: 1 addition & 1 deletion Yoast/Tests/Utils/PathValidationHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
23 changes: 16 additions & 7 deletions Yoast/Utils/PathHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) );
}

/**
Expand Down
4 changes: 2 additions & 2 deletions Yoast/Utils/PathValidationHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 7690f3c

Please sign in to comment.