From f6da0aaae96a29d157be13d490058efa327b6ae2 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 23 Sep 2018 01:38:46 +0200 Subject: [PATCH] PrefixAllGlobals: verify that namespace names use a prefix 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. --- .../PrefixAllGlobalsSniff.php | 81 ++++++++++++++++++- .../PrefixAllGlobalsUnitTest.1.inc | 2 +- .../PrefixAllGlobalsUnitTest.4.inc | 30 +++++++ .../PrefixAllGlobalsUnitTest.php | 8 +- 4 files changed, 115 insertions(+), 6 deletions(-) create mode 100644 WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.4.inc diff --git a/WordPress/Sniffs/NamingConventions/PrefixAllGlobalsSniff.php b/WordPress/Sniffs/NamingConventions/PrefixAllGlobalsSniff.php index 5d3d255a1f..a23b82b52e 100644 --- a/WordPress/Sniffs/NamingConventions/PrefixAllGlobalsSniff.php +++ b/WordPress/Sniffs/NamingConventions/PrefixAllGlobalsSniff.php @@ -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 */ @@ -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. * @@ -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, @@ -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. @@ -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 ); @@ -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 ) @@ -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; } /** diff --git a/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.1.inc b/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.1.inc index 9408c55480..c2c8da5a01 100644 --- a/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.1.inc +++ b/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.1.inc @@ -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. } diff --git a/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.4.inc b/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.4.inc new file mode 100644 index 0000000000..cc466f8b3b --- /dev/null +++ b/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.4.inc @@ -0,0 +1,30 @@ + 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': @@ -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,