Merge pull request WordPress#558 from jrfnl/feature/improve-error-dif…

…ferentiation [VariableName Sniff] Improve error message differentiation in the sniff.
jrfnl · Jun 28, 2016 · 15460b4 · 15460b4
2 parents 3c23ba9 + 79b2170
commit 15460b4
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 27 deletions.
diff --git a/WordPress/Sniffs/NamingConventions/ValidVariableNameSniff.php b/WordPress/Sniffs/NamingConventions/ValidVariableNameSniff.php
@@ -140,29 +140,35 @@ protected function processVariable( PHP_CodeSniffer_File $phpcs_file, $stack_ptr
 			}//end if
 		}//end if
 
+		$in_class     = false;
+		$obj_operator = $phpcs_file->findPrevious( array( T_WHITESPACE ), ( $stack_ptr - 1 ), null, true );
+		if ( T_DOUBLE_COLON === $tokens[ $obj_operator ]['code'] || T_OBJECT_OPERATOR === $tokens[ $obj_operator ]['code'] ) {
+			// The variable lives within a class, and is referenced like
+			// this: MyClass::$_variable or $class->variable.
+			$in_class = true;
+		}
+
 		// There is no way for us to know if the var is public or private,
 		// so we have to ignore a leading underscore if there is one and just
 		// check the main part of the variable name.
 		$original_var_name = $var_name;
-		if ( substr( $var_name, 0, 1 ) === '_' ) {
-			$obj_operator = $phpcs_file->findPrevious( array( T_WHITESPACE ), ( $stack_ptr - 1 ), null, true );
-			if ( T_DOUBLE_COLON === $tokens[ $obj_operator ]['code'] ) {
-				// The variable lives within a class, and is referenced like
-				// this: MyClass::$_variable, so we don't know its scope.
-				$in_class = true;
-			} else {
-				$in_class = $phpcs_file->hasCondition( $stack_ptr, array( T_CLASS, T_INTERFACE, T_TRAIT ) );
-			}
-
-			if ( true === $in_class ) {
-				$var_name = substr( $var_name, 1 );
-			}
+		if ( substr( $var_name, 0, 1 ) === '_' && true === $in_class ) {
+			$var_name = substr( $var_name, 1 );
 		}
 
 		if ( self::isSnakeCase( $var_name ) === false ) {
-			$error = 'Variable "%s" is not in valid snake_case format';
-			$data  = array( $original_var_name );
-			$phpcs_file->addError( $error, $stack_ptr, 'NotSnakeCase', $data );
+			if ( $in_class && ! in_array( $var_name, $this->whitelisted_mixed_case_member_var_names, true ) ) {
+				$error      = 'Object property "%s" is not in valid snake_case format';
+				$error_name = 'NotSnakeCaseMemberVar';
+			} elseif ( ! $in_class ) {
+				$error      = 'Variable "%s" is not in valid snake_case format';
+				$error_name = 'NotSnakeCase';
+			}
+
+			if ( isset( $error, $error_name ) ) {
+				$data  = array( $original_var_name );
+				$phpcs_file->addError( $error, $stack_ptr, $error_name, $data );
+			}
 		}
 
 	}
@@ -219,7 +225,7 @@ protected function processVariableInString( PHP_CodeSniffer_File $phpcs_file, $s
 				}
 
 				if ( self::isSnakeCase( $var_name ) === false ) {
-					$error = 'Variable "%s" is not in snake_case format';
+					$error = 'Variable "%s" is not in valid snake_case format';
 					$data  = array( $var_name );
 					$phpcs_file->addError( $error, $stack_ptr, 'StringNotSnakeCase', $data );
 				}

diff --git a/WordPress/Tests/NamingConventions/ValidVariableNameUnitTest.inc b/WordPress/Tests/NamingConventions/ValidVariableNameUnitTest.inc
@@ -1,6 +1,6 @@
 <?php
-$varName  = 'hello';
-$var_name = 'hello'; // Bad
+$varName  = 'hello'; // Bad
+$var_name = 'hello';
 $varname  = 'hello';
 $_varName = 'hello'; // Bad
 
@@ -52,6 +52,7 @@ echo MyClass::$varName; // Bad
 echo MyClass::$var_name;
 echo MyClass::$varname;
 echo MyClass::$_varName; // Bad
+echo MyClass::$VAR_name; // Bad
 
 echo $this->varName2; // Bad
 echo $this->var_name2;
@@ -61,6 +62,7 @@ echo $object->varName2; // Bad
 echo $object->var_name2;
 echo $object_name->varname2;
 echo $object_name->_varName2; // Bad
+echo $object_name->VAR_name; // Bad
 
 echo $this->myFunction($one, $two);
 echo $object->myFunction($one_two);
@@ -93,6 +95,24 @@ echo $comment->comment_author_IP;
 class Foo {
 	public $_public_leading_underscore;
 	private $private_no_underscore_loading;
+
+	function Bar( $VARname ) { // Bad
+		$localVariable = false; // Bad
+		echo Some_Class::$VarName; // Bad
+		echo $this->VAR_name; // Bad
+		$_localVariable = false; // Bad
+		echo Some_Class::$_VarName; // Bad
+		echo $this->_VAR_name; // Bad
+	}
+
+	function Baz( $var_name ) { // Ok
+		$local_variable = false; // Ok
+		echo Some_Class::$var_name; // Ok
+		echo $this->var_name; // Ok
+		$_local_variable = false; // Ok
+		echo Some_Class::$_var_name; // Ok
+		echo $this->_var_name; // Ok
+	}
 }
 
 if ( is_category() ) {

diff --git a/WordPress/Tests/NamingConventions/ValidVariableNameUnitTest.php b/WordPress/Tests/NamingConventions/ValidVariableNameUnitTest.php
@@ -60,20 +60,29 @@ public function getErrorList() {
 			43   => 1,
 			51   => 1,
 			54   => 1,
-			56   => 1,
-			59   => 1,
+			55   => 1,
+			57   => 1,
 			60   => 1,
-			63   => 1,
-			68   => 1,
-			75   => 1,
-			76   => 1,
+			61   => 1,
+			64   => 1,
+			65   => 1,
+			70   => 1,
 			77   => 1,
 			78   => 1,
+			79   => 1,
 			80   => 1,
-			84   => 1,
-			85   => 1,
+			82   => 1,
 			86   => 1,
+			87   => 1,
+			88   => 1,
+			99   => 1,
+			100  => 1,
 			101  => 1,
+			102  => 1,
+			103  => 1,
+			104  => 1,
+			105  => 1,
+			121  => 1,
 		);
 
 		return $errors;