Skip to content

Commit

Permalink
PrefixAllGlobals: verify that namespace names use a prefix
Browse files Browse the repository at this point in the history
Until now, namespaced classes, functions and constants would be exempt from the "prefix all globals" rule, but the namespace name _itself_ was not examined.

As namespace names can, of course, also conflict, the namespace name should be prefixed with a plugin/theme specific prefix.

This PR adds that feature to the sniff. The PR builds onto the changes made in PR 1491 and 1492.

Notes:
* The sniff allows for underscores and (other) non-word characters in a passed prefix to be converted to namespace separators when used in a namespace name.
    In other words, if a prefix of `my_plugin` is passed as the value of the custom property, a namespace name of `My\Plugin` will be accepted automatically.
* Passing a prefix property value containing namespace separators will now also be allowed and will no longer trigger a warning.
  • Loading branch information
jrfnl committed Oct 31, 2018
1 parent 3f6ae0d commit f6da0aa
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 6 deletions.
81 changes: 77 additions & 4 deletions WordPress/Sniffs/NamingConventions/PrefixAllGlobalsSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
*
* @since 0.12.0
* @since 0.13.0 Class name changed: this class is now namespaced.
* @since 1.2.0 Now also checks whether namespaces are prefixed.
*
* @uses \WordPress\Sniff::$custom_test_class_whitelist
*/
Expand Down Expand Up @@ -65,6 +66,20 @@ class PrefixAllGlobalsSniff extends AbstractFunctionParameterSniff {
*/
private $validated_prefixes = array();

/**
* Target namespace prefixes after validation with regex indicator.
*
* All prefixes are lowercased for case-insensitive compare.
* If the prefix doesn't already contain a namespace separator, but does contain
* non-word characters, these will have been replaced with regex syntax to allow
* for namespace separators and the `is_regex` indicator will have been set to `true`.
*
* @since 1.2.0
*
* @var array
*/
private $validated_namespace_prefixes = array();

/**
* Cache of previously set prefixes.
*
Expand Down Expand Up @@ -167,6 +182,7 @@ class PrefixAllGlobalsSniff extends AbstractFunctionParameterSniff {
*/
public function register() {
$targets = array(
\T_NAMESPACE => \T_NAMESPACE,
\T_FUNCTION => \T_FUNCTION,
\T_CLASS => \T_CLASS,
\T_INTERFACE => \T_INTERFACE,
Expand Down Expand Up @@ -274,6 +290,46 @@ public function process_token( $stackPtr ) {

return $this->process_variable_assignment( $stackPtr );

} elseif ( \T_NAMESPACE === $this->tokens[ $stackPtr ]['code'] ) {
$namespace_name = $this->get_declared_namespace_name( $stackPtr );

if ( false === $namespace_name || '' === $namespace_name || '\\' === $namespace_name ) {
return;
}

foreach ( $this->validated_namespace_prefixes as $key => $prefix_info ) {
if ( false === $prefix_info['is_regex'] ) {
if ( stripos( $namespace_name, $prefix_info['prefix'] ) === 0 ) {
$this->phpcsFile->recordMetric( $stackPtr, 'Prefix all globals: allowed prefixes', $key );
return;
}
} else {
// Ok, so this prefix should be used as a regex.
$regex = '`^' . $prefix_info['prefix'] . '`i';
if ( preg_match( $regex, $namespace_name ) > 0 ) {
$this->phpcsFile->recordMetric( $stackPtr, 'Prefix all globals: allowed prefixes', $key );
return;
}
}
}

// Still here ? In that case, we have a non-prefixed namespace name.
$recorded = $this->phpcsFile->addError(
self::ERROR_MSG,
$stackPtr,
'NonPrefixedNamespaceFound',
array(
'Namespaces declared',
$namespace_name,
)
);

if ( true === $recorded ) {
$this->record_potential_prefix_metric( $stackPtr, $namespace_name );
}

return;

} else {

// Namespaced methods, classes and constants do not need to be prefixed.
Expand Down Expand Up @@ -824,6 +880,7 @@ private function validate_prefixes() {
$this->previous_prefixes = $this->prefixes;

// Validate the passed prefix(es).
$ns_prefixes = array();
foreach ( $this->prefixes as $key => $prefix ) {
$prefixLC = strtolower( $prefix );

Expand All @@ -839,9 +896,9 @@ private function validate_prefixes() {
}

// Validate the prefix against characters allowed for function, class, constant names etc.
if ( preg_match( '`^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$`', $prefix ) !== 1 ) {
if ( preg_match( '`^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff\\\\]*$`', $prefix ) !== 1 ) {
$this->phpcsFile->addWarning(
'The "%s" prefix is not a valid function/class/variable/constant prefix in PHP.',
'The "%s" prefix is not a valid namespace/function/class/variable/constant prefix in PHP.',
0,
'InvalidPrefixPassed',
array( $prefix )
Expand All @@ -851,10 +908,26 @@ private function validate_prefixes() {

// Lowercase the prefix to allow for direct compare.
$this->prefixes[ $key ] = $prefixLC;

/*
* Replace non-word characters in the prefix with a regex snippet, but only if the
* string doesn't already contain namespace separators.
*/
$is_regex = false;
if ( strpos( $prefix, '\\' ) === false && preg_match( '`[_\W]`', $prefix ) > 0 ) {
$prefix = preg_replace( '`([_\W])`', '[\\\\\\\\$1]', $prefixLC );
$is_regex = true;
}

$ns_prefixes[ $prefixLC ] = array(
'prefix' => $prefix,
'is_regex' => $is_regex,
);
}

// Set the validated prefixes cache.
$this->validated_prefixes = $this->prefixes;
// Set the validated prefixes caches.
$this->validated_prefixes = $this->prefixes;
$this->validated_namespace_prefixes = $ns_prefixes;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ define( '\OtherNameSpace\PLUGIN_FILE', __FILE__ ); // OK.
define( __NAMESPACE__ . '\PLUGIN_FILE', __FILE__ );
define( '\PLUGIN_FILE', __FILE__ );

namespace Testing {
namespace TGMPA\Testing {
define( 'MY' . __NAMESPACE__, __FILE__ ); // Error, not actually namespaced.
define( 'MY\\' . __NAMESPACE__, __FILE__ ); // OK, even though strangely setup, the constant is in a namespace.
}
Expand Down
30 changes: 30 additions & 0 deletions WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.4.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php
// @codingStandardsChangeSetting WordPress.NamingConventions.PrefixAllGlobals prefixes acronym,tgmpa,test_this,test\that

namespace TGMPA;

namespace Acronym\B\C;

namespace AcronymPlugin\B\C;

namespace Test_This\C;

namespace Test\This\C;

namespace Test\That\C;

namespace Test \ /* comment */ This \ C;

namespace A\B\C; // Bad.

use namespace\DEF; // Not our target.
namespace\cname::method(); // Not our target.
$acronym_result = namespace \ blah \ mine(); // Not our target.

// @codingStandardsChangeSetting WordPress.NamingConventions.PrefixAllGlobals prefixes wordpress
namespace wordpress\myplugin;

// @codingStandardsChangeSetting WordPress.NamingConventions.PrefixAllGlobals prefixes wordpress\myplugin
namespace wordpress\myplugin;

// @codingStandardsChangeSetting WordPress.NamingConventions.PrefixAllGlobals prefixes false
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ public function getErrorList( $testFile = 'PrefixAllGlobalsUnitTest.1.inc' ) {
403 => 1,
);

case 'PrefixAllGlobalsUnitTest.4.inc':
return array(
1 => 1, // 1 x error for blacklisted prefix passed.
18 => 1,
);

case 'PrefixAllGlobalsUnitTest.2.inc':
// Namespaced - all OK, fall through to the default case.
case 'PrefixAllGlobalsUnitTest.3.inc':
Expand All @@ -90,7 +96,7 @@ public function getWarningList( $testFile = 'PrefixAllGlobalsUnitTest.1.inc' ) {
switch ( $testFile ) {
case 'PrefixAllGlobalsUnitTest.1.inc':
return array(
1 => 3, // 3 x error for potentially incorrect prefix passed.
1 => 2, // 2 x warning for potentially incorrect prefix passed.
249 => 1,
250 => 1,
253 => 1,
Expand Down

0 comments on commit f6da0aa

Please sign in to comment.