From 13678766eabad768d844c1bd10a1a978c2cb773a Mon Sep 17 00:00:00 2001 From: webimpress Date: Mon, 21 Oct 2019 14:04:08 +0100 Subject: [PATCH 1/6] Feature: all variables must be camel case - strict Defined variables, variables used in strings and class properties must be now camelCased (strict) - so two capital letters next to each other are not allowed Disable `PSR2.Classes.PropertyDeclaration.Underscore` as it is now covered in `Webimpress\NamingConventions\ValidVariableName` sniff. --- .../ValidVariableNameSniff.php | 30 ++++++++++++++++--- src/WebimpressCodingStandard/ruleset.xml | 2 ++ .../ValidVariableNameUnitTest.inc | 9 ++++-- .../ValidVariableNameUnitTest.php | 8 ++++- 4 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/WebimpressCodingStandard/Sniffs/NamingConventions/ValidVariableNameSniff.php b/src/WebimpressCodingStandard/Sniffs/NamingConventions/ValidVariableNameSniff.php index 4c53037b..4e759135 100644 --- a/src/WebimpressCodingStandard/Sniffs/NamingConventions/ValidVariableNameSniff.php +++ b/src/WebimpressCodingStandard/Sniffs/NamingConventions/ValidVariableNameSniff.php @@ -9,6 +9,7 @@ use PHP_CodeSniffer\Util\Common; use function ltrim; +use function preg_match_all; use const T_DOUBLE_COLON; use const T_WHITESPACE; @@ -48,8 +49,8 @@ protected function processVariable(File $phpcsFile, $stackPtr) : void return; // skip MyClass::$variable, there might be no control over the declaration } - if (! Common::isCamelCaps($varName, false, true, false)) { - $error = 'Variable "%s" is not in valid camel caps format'; + if (! Common::isCamelCaps($varName, false, true, true)) { + $error = 'Variable "$%s" is not in valid camel caps format'; $data = [$varName]; $phpcsFile->addError($error, $stackPtr, 'NotCamelCaps', $data); } @@ -60,7 +61,14 @@ protected function processVariable(File $phpcsFile, $stackPtr) : void */ protected function processMemberVar(File $phpcsFile, $stackPtr) : void { - // handled by PSR2.Classes.PropertyDeclaration + $tokens = $phpcsFile->getTokens(); + $varName = ltrim($tokens[$stackPtr]['content'], '$'); + + if (! Common::isCamelCaps($varName, false, true, true)) { + $error = 'Property "$%s" is not in valid camel caps format'; + $data = [$varName]; + $phpcsFile->addError($error, $stackPtr, 'NotCamelCapsProperty', $data); + } } /** @@ -68,6 +76,20 @@ protected function processMemberVar(File $phpcsFile, $stackPtr) : void */ protected function processVariableInString(File $phpcsFile, $stackPtr) : void { - // handled by Squiz.Strings.DoubleQuoteUsage + $tokens = $phpcsFile->getTokens(); + $content = $tokens[$stackPtr]['content']; + + $pattern = '|(?addError($error, $stackPtr, 'NotCamelCapsInString', $data); + } + } } } diff --git a/src/WebimpressCodingStandard/ruleset.xml b/src/WebimpressCodingStandard/ruleset.xml index 1ef90dae..6ed5b641 100755 --- a/src/WebimpressCodingStandard/ruleset.xml +++ b/src/WebimpressCodingStandard/ruleset.xml @@ -27,6 +27,8 @@ + + diff --git a/test/Sniffs/NamingConventions/ValidVariableNameUnitTest.inc b/test/Sniffs/NamingConventions/ValidVariableNameUnitTest.inc index 1fa89be9..24db167c 100644 --- a/test/Sniffs/NamingConventions/ValidVariableNameUnitTest.inc +++ b/test/Sniffs/NamingConventions/ValidVariableNameUnitTest.inc @@ -21,9 +21,14 @@ echo \Library::$_variable; echo \Library::$_another_variable; class Foo { - protected $_this_is_not_handled_by_this_sniff; + protected $_invalid; + protected $userID; } -$string = "This $_some_variable is not {$handled_by} this sniff."; +$string = "This {$_some_variable} is now ${handled_by} this $sniff."; $string .= $_some_variable; $string .= $camelCase; +$string .= "{$myIP}"; + +$userID = 'invalid due to two uppercase characters next to each other'; +$InvalidVariable = 'as first letter cannot be uppercase'; diff --git a/test/Sniffs/NamingConventions/ValidVariableNameUnitTest.php b/test/Sniffs/NamingConventions/ValidVariableNameUnitTest.php index 91075f3c..69669b01 100644 --- a/test/Sniffs/NamingConventions/ValidVariableNameUnitTest.php +++ b/test/Sniffs/NamingConventions/ValidVariableNameUnitTest.php @@ -13,7 +13,13 @@ protected function getErrorList(string $testFile = '') : array return [ 15 => 1, 16 => 1, - 28 => 1, + 24 => 1, + 25 => 1, + 28 => 2, + 29 => 1, + 31 => 1, + 33 => 1, + 34 => 1, ]; } From 6ca5e76640d7a7f23c8be1b306abda9f22fa30bc Mon Sep 17 00:00:00 2001 From: webimpress Date: Mon, 21 Oct 2019 14:12:42 +0100 Subject: [PATCH 2/6] CS fixes - update variable names to strict camelCase --- src/WebimpressCodingStandard/Helper/MethodsTrait.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/WebimpressCodingStandard/Helper/MethodsTrait.php b/src/WebimpressCodingStandard/Helper/MethodsTrait.php index bc693eae..38b2ca6e 100644 --- a/src/WebimpressCodingStandard/Helper/MethodsTrait.php +++ b/src/WebimpressCodingStandard/Helper/MethodsTrait.php @@ -308,10 +308,10 @@ private function isClassName(string $name) : bool $ns = strtolower($this->currentNamespace); $lowerClassName = strtolower($this->className); - $lowerFQCN = ($ns ? '\\' . $ns : '') . '\\' . $lowerClassName; + $lowerFqcn = ($ns ? '\\' . $ns : '') . '\\' . $lowerClassName; $lower = strtolower($name); - return $lower === $lowerFQCN + return $lower === $lowerFqcn || $lower === $lowerClassName; } @@ -322,10 +322,10 @@ private function isParentClassName(string $name) : bool } $lowerParentClassName = strtolower($this->parentClassName); - $lowerFQCN = strtolower($this->getFQCN($lowerParentClassName)); + $lowerFqcn = strtolower($this->getFQCN($lowerParentClassName)); $lower = strtolower($name); - return $lower === $lowerFQCN + return $lower === $lowerFqcn || $lower === $lowerParentClassName; } From b37706da724e9b7a5502083d1fecb7a15a238ded Mon Sep 17 00:00:00 2001 From: webimpress Date: Mon, 21 Oct 2019 14:20:02 +0100 Subject: [PATCH 3/6] Rename method name to be also strict camelCase Method names are validated by PSR-2 sniff which doesn't require strict camelCase names. There is no any option to change it to strict as well. --- src/WebimpressCodingStandard/Helper/MethodsTrait.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/WebimpressCodingStandard/Helper/MethodsTrait.php b/src/WebimpressCodingStandard/Helper/MethodsTrait.php index 38b2ca6e..eb207c73 100644 --- a/src/WebimpressCodingStandard/Helper/MethodsTrait.php +++ b/src/WebimpressCodingStandard/Helper/MethodsTrait.php @@ -263,13 +263,13 @@ private function typesMatch(string $typeHint, string $typeStr) : bool return false; } - $fqcnTypeHint = strtolower($this->getFQCN($lowerTypeHint)); + $fqcnTypeHint = strtolower($this->getFqcn($lowerTypeHint)); foreach ($types as $key => $type) { if ($type === 'null') { continue; } - $types[$key] = strtolower($this->getFQCN($type)); + $types[$key] = strtolower($this->getFqcn($type)); } $fqcnTypes = implode('|', $types); @@ -279,7 +279,7 @@ private function typesMatch(string $typeHint, string $typeStr) : bool || $fqcnTypeHint . '|null' === $fqcnTypes)); } - private function getFQCN(string $class) : string + private function getFqcn(string $class) : string { // It is a simple type if (in_array(strtolower($class), $this->simpleReturnTypes, true)) { @@ -322,7 +322,7 @@ private function isParentClassName(string $name) : bool } $lowerParentClassName = strtolower($this->parentClassName); - $lowerFqcn = strtolower($this->getFQCN($lowerParentClassName)); + $lowerFqcn = strtolower($this->getFqcn($lowerParentClassName)); $lower = strtolower($name); return $lower === $lowerFqcn From 873292e69710c07a6c0b8074a610b50007eefbc4 Mon Sep 17 00:00:00 2001 From: webimpress Date: Mon, 21 Oct 2019 15:18:50 +0100 Subject: [PATCH 4/6] Updated test - string without variables inside It should increase coverage --- test/Sniffs/NamingConventions/ValidVariableNameUnitTest.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Sniffs/NamingConventions/ValidVariableNameUnitTest.inc b/test/Sniffs/NamingConventions/ValidVariableNameUnitTest.inc index 24db167c..d0dbe874 100644 --- a/test/Sniffs/NamingConventions/ValidVariableNameUnitTest.inc +++ b/test/Sniffs/NamingConventions/ValidVariableNameUnitTest.inc @@ -31,4 +31,4 @@ $string .= $camelCase; $string .= "{$myIP}"; $userID = 'invalid due to two uppercase characters next to each other'; -$InvalidVariable = 'as first letter cannot be uppercase'; +$InvalidVariable = "as first letter cannot be uppercase"; From f8afacf4d207f85945e6190cfded35b361163da5 Mon Sep 17 00:00:00 2001 From: webimpress Date: Mon, 21 Oct 2019 15:34:25 +0100 Subject: [PATCH 5/6] Remove redundant condition and use coalesce op instead --- .../Sniffs/NamingConventions/ValidVariableNameSniff.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/WebimpressCodingStandard/Sniffs/NamingConventions/ValidVariableNameSniff.php b/src/WebimpressCodingStandard/Sniffs/NamingConventions/ValidVariableNameSniff.php index 4e759135..0e89cc6f 100644 --- a/src/WebimpressCodingStandard/Sniffs/NamingConventions/ValidVariableNameSniff.php +++ b/src/WebimpressCodingStandard/Sniffs/NamingConventions/ValidVariableNameSniff.php @@ -80,11 +80,9 @@ protected function processVariableInString(File $phpcsFile, $stackPtr) : void $content = $tokens[$stackPtr]['content']; $pattern = '|(? Date: Fri, 25 Oct 2019 21:37:32 +0100 Subject: [PATCH 6/6] Adds CHANGELOG entries for #42 --- CHANGELOG.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1608818..9d3159db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,9 +17,12 @@ All notable changes to this project will be documented in this file, in reverse - [#39](https://github.com/webimpress/coding-standard/pull/39) adds `Arrays\DuplicateKey` sniff which detects duplicated keys in arrays +- [#42](https://github.com/webimpress/coding-standard/pull/42) adds requiring camelCase names for class members and variables used in strings - extended sniff `NamingConventions\ValidVariableName`. + Disallowed are two capital letters next to each other (strict mode). + ### Changed -- Nothing. +- [#42](https://github.com/webimpress/coding-standard/pull/42) changes `NamingConventions\ValidVariableName` to require variable names be in strict camelCase. It means two capital letters next to each other are not allowed. ### Deprecated @@ -27,7 +30,7 @@ All notable changes to this project will be documented in this file, in reverse ### Removed -- Nothing. +- [#42](https://github.com/webimpress/coding-standard/pull/42) excludes `PSR2.Classes.PropertyDeclaration.Underscore` check, as it is now covered by `NamingConventions\ValidVariableName` sniff ### Fixed