Skip to content

Commit

Permalink
NamingConventions/NamespaceName: add support for strict PSR-4 complia…
Browse files Browse the repository at this point in the history 
…nce checking

As the Yoast plugin test directories will start to follow PSR-4. the `NamespaceName` sniff will need to be able to enforce this.

This commit adds this ability to the sniff.

Notes:
* It adds a new `public` `psr4_paths` ruleset property via the `PSR4PathsTrait` utility.
* If the file being examined is in a path indicated as a PSR-4 path, PSR-4 based namespace names will be enforced.
    The differences between the "old-style" enforcement and PSR-4 are in case-sensitivity and in how characters which are allowed in paths, but not allowed in namespace names are handled.
* Includes updated error message/code for the "missing prefix" check when the file is in a PSR-4 path.
* Includes updated/adjusted logic for the "not counting of `Tests`/`Doubles`" directories as the `Tests` may be part of the PSR-4 namespace.

Also note that when both a `psr4_paths` as well as the `src_directory` and `prefixes` properties are set, the `psr4_paths` property will take precedence and the sniff will only fall back to the previous logic if the file is not in a path matching one of the PSR-4 directories.

Includes ample tests for this new functionality.
Includes updated XML documentation.

Includes updating the YoastCS native PHPCS ruleset to indicate that the YoastCS repo follows PSR-4 (as per the PHPCS file name rules).

Note: if the name in use in the file is causing a problematic parse error (like in the test with the `#` in the namespace name), the sniff will stay silent. This is the normal behaviour for a PHPCS check when encountering parse errors.

:point_right: The sniff changes are probably easiest to review while ignoring whitespace.
  • Loading branch information
@jrfnl
jrfnl committed Nov 20, 2023
1 parent 0d870dc commit 02b03dd
Show file tree
Hide file tree
Showing 13 changed files with 544 additions and 60 deletions.
7 changes: 2 additions & 5 deletions .phpcs.xml.dist
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,8 @@

<rule ref="Yoast.NamingConventions.NamespaceName">
<properties>
<property name="prefixes" type="array">
<element value="YoastCS\Yoast"/>
</property>
<property name="src_directory" type="array">
<element value="Yoast"/>
<property name="psr4_paths" type="array">
<element key="YoastCS\Yoast\\" value="Yoast"/>
</property>
</properties>
</rule>
Expand Down
28 changes: 26 additions & 2 deletions Yoast/Docs/NamingConventions/NamespaceNameStandard.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,18 @@ namespace Yoast\WP\Plugin\Tests\<em>Foo\Bar\Flo\Sub</em>;
<standard>
<![CDATA[
The levels in a namespace name should reflect the path to the file.
Optional, strict PSR-4 compliance for the path to namespace name translation can be enforced by setting the `psr4_paths` property.
If a path to name translation would result in an invalid namespace name based on the naming rules for PHP, an error will be thrown to rename the problem directory.
The differences between the "basic" path to namespace translation and strict PSR-4 path to namespace translation, are as follows:
* The "basic" translation is case-insensitive, while the PSR-4 translation is case-sensitive.
* The "basic" translation will convert dashes and other punctuation characters in the path to underscores, while the PSR-4 translation enforces that the names match exactly.
* The "basic" translation will accept any of the provided prefixes, while the strict PSR-4 translation will require the exact prefix assigned to the matched PSR-4 path.
* The "basic" translation is suitable for use in combination with a Composer classmap for autoloading.
The PSR-4 compliant translation is suitable for use in combination with the Composer `psr4` autoload directive.
If the `psr4_paths` property is provided, the PSR-4 based path translation will always take precedence for a file matching any of the PSR-4 paths.
The plugin specific prefix is disregarded for this check.
If a path to name translation would result in an invalid namespace name based on the naming rules for PHP, an error will be thrown to rename the problem directory.
]]>
</standard>
<code_comparison>
Expand All @@ -72,4 +80,20 @@ namespace Yoast\WP\Plugin\<em>Unrelated</em>;
]]>
</code>
</code_comparison>
<code_comparison>
<code title="Valid: PSR-4 compliant namespace name reflects the path to the file exactly.">
<![CDATA[
<!-- Path to file: <em>User_Forms/</em>file.php -->
<?php
namespace Yoast\WP\Plugin\<em>User_Forms</em>;
]]>
</code>
<code title="Invalid: non-PSR-4 compliant namespace name does not reflect the path to the file exactly.">
<![CDATA[
<!-- Path to file: <em>User_forms/</em>file.php -->
<?php
namespace Yoast\WP\Plugin\<em>user_Forms</em>;
]]>
</code>
</code_comparison>
</documentation>
188 changes: 135 additions & 53 deletions Yoast/Sniffs/NamingConventions/NamespaceNameSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use YoastCS\Yoast\Utils\CustomPrefixesTrait;
use YoastCS\Yoast\Utils\PathHelper;
use YoastCS\Yoast\Utils\PathValidationHelper;
use YoastCS\Yoast\Utils\PSR4PathsTrait;

/**
* Check namespace name declarations.
Expand All @@ -23,13 +24,16 @@
* placed in.
*
* @since 2.0.0
* @since 3.0.0 Added new check to verify a prefix is used.
* @since 3.0.0 - Added new check to verify a prefix is used.
* - The sniff now also has the ability to check for PSR-4 compliant namespace names.
*
* @uses \YoastCS\Yoast\Utils\CustomPrefixesTrait::$prefixes
* @uses \YoastCS\Yoast\Utils\PSR4PathsTrait::$psr4_paths
*/
final class NamespaceNameSniff implements Sniff {

use CustomPrefixesTrait;
use PSR4PathsTrait;

/**
* Double/Mock/Fixture directories to allow for.
Expand Down Expand Up @@ -135,14 +139,42 @@ public function process( File $phpcsFile, $stackPtr ) {
return;
}

$this->validate_prefixes();
// Stripping potential quotes to ensure `stdin_path` passed by IDEs does not include quotes.
$file = TextStrings::stripQuotes( $phpcsFile->getFileName() );
if ( $file === 'STDIN' ) {
$file = ''; // @codeCoverageIgnore
}
else {
$file = PathHelper::normalize_absolute_path( $file );
}

$valid_prefixes = [];
$psr4_info = false;
if ( $file !== '' ) {
$psr4_info = $this->get_psr4_info( $phpcsFile, $file );
}

if ( \is_array( $psr4_info ) ) {
// If a PSR4 path matched, there will only ever be one valid prefix for the matched path.
$valid_prefixes = [ $psr4_info['prefix'] . '\\' ];
}
else {
// Safeguard that the PSR-4 prefixes are always included.
// Makes sure level depth check still happens even if there is no basepath or path doesn't match PSR-4 path.
if ( empty( $this->prefixes ) && ! empty( $this->psr4_paths ) ) {
$this->prefixes = \array_keys( $this->psr4_paths );
}

$this->validate_prefixes();
$valid_prefixes = $this->validated_prefixes;
}

// Strip off the (longest) plugin prefix.
$namespace_name_no_prefix = $namespace_name;
$found_prefix = '';
if ( ! empty( $this->validated_prefixes ) ) {
if ( ! empty( $valid_prefixes ) ) {
$name = $namespace_name . '\\'; // Validated prefixes always have a \ at the end.
foreach ( $this->validated_prefixes as $prefix ) {
foreach ( $valid_prefixes as $prefix ) {
if ( \strpos( $name, $prefix ) === 0 ) {
$namespace_name_no_prefix = \rtrim( \substr( $name, \strlen( $prefix ) ), '\\' );
$found_prefix = \rtrim( $prefix, '\\' );
Expand All @@ -153,19 +185,31 @@ public function process( File $phpcsFile, $stackPtr ) {
}

// Check if a prefix is used.
if ( ! empty( $this->validated_prefixes ) && $found_prefix === '' ) {
if ( \count( $this->validated_prefixes ) === 1 ) {
$error = 'A namespace name is required to start with the "%s" prefix.';
if ( ! empty( $valid_prefixes ) && $found_prefix === '' ) {
$prefixes = $valid_prefixes;

if ( $psr4_info !== false ) {
$error = 'PSR-4 namespace name for this path is required to start with the "%1$s" prefix.';
$errorcode = 'MissingPSR4Prefix';
}
else {
$error = 'A namespace name is required to start with one of the following prefixes: "%s"';
$error = 'A namespace name is required to start with one of the following prefixes: "%s"';
$errorcode = 'MissingPrefix';

$prefixes = \array_merge( $prefixes, \array_keys( $this->psr4_paths ) );
$prefixes = \array_unique( $prefixes );

if ( \count( $prefixes ) === 1 ) {
$error = 'A namespace name is required to start with the "%s" prefix.';
}
else {
\natcasesort( $prefixes );
}
}

$prefixes = $this->validated_prefixes;
\natcasesort( $prefixes );
$data = [ \implode( '", "', $prefixes ) ];

$phpcsFile->addError( $error, $stackPtr, 'MissingPrefix', $data );
$phpcsFile->addError( $error, $stackPtr, $errorcode, $data );
}

/*
Expand All @@ -175,8 +219,9 @@ public function process( File $phpcsFile, $stackPtr ) {
$namespace_for_level_check = $namespace_name_no_prefix;

// Allow for a variation of `Tests\` and `Tests\*\Doubles\` after the prefix.
$starts_with_tests = ( \strpos( $namespace_for_level_check, 'Tests\\' ) === 0 );
if ( $starts_with_tests === true ) {
$starts_with_tests = ( \strpos( $namespace_for_level_check, 'Tests\\' ) === 0 );
$prefix_ends_with_tests = ( \substr( $found_prefix, -6 ) === '\Tests' );
if ( $starts_with_tests === true || $prefix_ends_with_tests === true ) {
$stripped = false;
foreach ( self::DOUBLE_DIRS as $dir => $length ) {
if ( \strpos( $namespace_for_level_check, $dir ) !== false ) {
Expand All @@ -187,16 +232,27 @@ public function process( File $phpcsFile, $stackPtr ) {
}

if ( $stripped === false ) {
// No double dir found, now check/strip typical test dirs.
if ( \strpos( $namespace_for_level_check, 'Tests\WP\\' ) === 0 ) {
$namespace_for_level_check = \substr( $namespace_for_level_check, 9 );
}
elseif ( \strpos( $namespace_for_level_check, 'Tests\Unit\\' ) === 0 ) {
$namespace_for_level_check = \substr( $namespace_for_level_check, 11 );
if ( $starts_with_tests === true ) {
// No double dir found, now check/strip typical test dirs.
if ( \strpos( $namespace_for_level_check, 'Tests\WP\\' ) === 0 ) {
$namespace_for_level_check = \substr( $namespace_for_level_check, 9 );
}
elseif ( \strpos( $namespace_for_level_check, 'Tests\Unit\\' ) === 0 ) {
$namespace_for_level_check = \substr( $namespace_for_level_check, 11 );
}
else {
// Okay, so this only has the `Tests` prefix, just strip it.
$namespace_for_level_check = \substr( $namespace_for_level_check, 6 );
}
}
else {
// Okay, so this only has the `Tests` prefix, just strip it.
$namespace_for_level_check = \substr( $namespace_for_level_check, 6 );
elseif ( $prefix_ends_with_tests === true ) {
// Prefix which already includes `Tests`.
if ( \strpos( $namespace_for_level_check, 'WP\\' ) === 0 ) {
$namespace_for_level_check = \substr( $namespace_for_level_check, 3 );
}
elseif ( \strpos( $namespace_for_level_check, 'Unit\\' ) === 0 ) {
$namespace_for_level_check = \substr( $namespace_for_level_check, 5 );
}
}
}
}
Expand Down Expand Up @@ -237,40 +293,50 @@ public function process( File $phpcsFile, $stackPtr ) {
return;
}

// Stripping potential quotes to ensure `stdin_path` passed by IDEs does not include quotes.
$file = TextStrings::stripQuotes( $phpcsFile->getFileName() );
if ( $file === 'STDIN' ) {
if ( $file === '' ) {
// STDIN.
return; // @codeCoverageIgnore
}

$directory = PathHelper::normalize_absolute_path( \dirname( $file ) );
$relative_directory = '';
if ( \is_array( $psr4_info ) ) {
$relative_directory = $psr4_info['relative'];
}
else {
$directory = PathHelper::normalize_absolute_path( \dirname( $file ) );

$this->validate_src_directory( $phpcsFile );
$this->validate_src_directory( $phpcsFile );

if ( empty( $this->validated_src_directory ) === false ) {
foreach ( $this->validated_src_directory as $absolute_src_path ) {
if ( PathHelper::path_starts_with( $directory, $absolute_src_path ) === false ) {
continue;
}
if ( empty( $this->validated_src_directory ) === false ) {
foreach ( $this->validated_src_directory as $absolute_src_path ) {
if ( PathHelper::path_starts_with( $directory, $absolute_src_path ) === false ) {
continue;
}

$relative_directory = PathHelper::strip_basepath( $directory, $absolute_src_path );
if ( $relative_directory === '.' ) {
$relative_directory = '';
$relative_directory = PathHelper::strip_basepath( $directory, $absolute_src_path );
break;
}
break;
}
}

if ( $relative_directory === '.' ) {
$relative_directory = '';
}

// Now any potential src directory has been stripped, remove surrounding slashes.
$relative_directory = \trim( $relative_directory, '/' );

// Directory to namespace translation.
$expected = '[Plugin\Prefix]';
if ( $found_prefix !== '' ) {
$expected = $found_prefix;
}
elseif ( \count( $this->validated_prefixes ) === 1 ) {
$expected = \rtrim( $this->validated_prefixes[0], '\\' );
// Namespace name doesn't have the correct prefix, but we do know what the prefix should be.
elseif ( \is_array( $psr4_info ) ) {
$expected = $psr4_info['prefix'];
}
elseif ( \count( $valid_prefixes ) === 1 ) {
$expected = \rtrim( $valid_prefixes[0], '\\' );
}

$clean = [];
Expand All @@ -281,10 +347,13 @@ public function process( File $phpcsFile, $stackPtr ) {
$levels = \array_filter( $levels ); // Remove empties, just in case.

foreach ( $levels as $level ) {
$cleaned_level = \preg_replace( '`[[:punct:]]`', '_', $level );
$words = \explode( '_', $cleaned_level );
$words = \array_map( 'ucfirst', $words );
$cleaned_level = \implode( '_', $words );
$cleaned_level = $level;
if ( $psr4_info === false ) {
$cleaned_level = \preg_replace( '`[[:punct:]]`', '_', $cleaned_level );
$words = \explode( '_', $cleaned_level );
$words = \array_map( 'ucfirst', $words );
$cleaned_level = \implode( '_', $words );
}

if ( NamingConventions::isValidIdentifierName( $cleaned_level ) === false ) {
$phpcsFile->addError(
Expand All @@ -304,23 +373,36 @@ public function process( File $phpcsFile, $stackPtr ) {
$name_for_compare = \implode( '\\', $clean );
}

if ( \strcasecmp( $name_for_compare, $namespace_name_no_prefix ) === 0 ) {
return;
// Check whether the namespace name complies with the rules.
if ( $psr4_info !== false ) {
// Check for PSR-4 compliant namespace.
if ( \strcmp( $name_for_compare, $namespace_name_no_prefix ) === 0 ) {
return;
}

$error = 'Directory marked as a PSR-4 path. The namespace name should match the path exactly in a case-sensitive manner. Expected namespace name: "%s"; found: "%s"';
$code = 'NotPSR4Compliant';
}
else {
// Check for "old-style" namespace.
if ( \strcasecmp( $name_for_compare, $namespace_name_no_prefix ) === 0 ) {
return;
}

$error = 'The namespace (sub)level(s) should reflect the directory path to the file. Expected: "%s"; Found: "%s"';
$code = 'Invalid';
}

if ( $name_for_compare !== '' ) {
$expected .= '\\' . $name_for_compare;
}

$phpcsFile->addError(
'The namespace (sub)level(s) should reflect the directory path to the file. Expected: "%s"; Found: "%s"',
$stackPtr,
'Invalid',
[
$expected,
$namespace_name,
]
);
$data = [
$expected,
$namespace_name,
];

$phpcsFile->addError( $error, $stackPtr, $code, $data );
}

/**
Expand Down
Loading

0 comments on commit 02b03dd

Please sign in to comment.