From bb762dbec2608e585e1f31141a97a011b81863f4 Mon Sep 17 00:00:00 2001 From: Alex Skrypnyk Date: Thu, 18 Dec 2025 15:08:31 +1100 Subject: [PATCH] [#15] Fixed false positives on variables prefixed with underscore. Variables like `$_static_value` are now completely skipped from naming convention checks as they are commonly used for internal/special variables. --- .../AbstractVariableNamingSniff.php | 17 +++++++++ .../LocalVariableNamingSniff.php | 5 +++ .../ParameterNamingSniff.php | 5 +++ tests/Fixtures/Valid.php | 22 ++++++++++- .../Unit/AbstractVariableNamingSniffTest.php | 38 +++++++++++++++++++ tests/Unit/LocalVariableNamingSniffTest.php | 8 ++++ tests/Unit/ParameterNamingSniffTest.php | 8 ++++ 7 files changed, 102 insertions(+), 1 deletion(-) diff --git a/src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php b/src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php index ca82d6f..eef8e31 100644 --- a/src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php +++ b/src/DrevOps/Sniffs/NamingConventions/AbstractVariableNamingSniff.php @@ -64,6 +64,23 @@ protected function isReserved(string $name): bool { return in_array($name, $this->reservedVariables, TRUE); } + /** + * Check if a variable name has a leading underscore. + * + * Variables with leading underscores (e.g., $_static_value) are typically + * used as a naming convention for internal/special variables and should + * be excluded from naming convention checks. + * + * @param string $name + * Variable name (without $). + * + * @return bool + * TRUE if has leading underscore, FALSE otherwise. + */ + protected function hasLeadingUnderscore(string $name): bool { + return str_starts_with($name, '_'); + } + /** * Check if a variable name follows snake_case format. * diff --git a/src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php b/src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php index 7ab91e6..0840e11 100644 --- a/src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php +++ b/src/DrevOps/Sniffs/NamingConventions/LocalVariableNamingSniff.php @@ -37,6 +37,11 @@ public function process(File $phpcsFile, $stackPtr): void { return; } + // Skip variables with leading underscores (e.g., $_static_value). + if ($this->hasLeadingUnderscore($var_name)) { + return; + } + // Skip static property accesses (self::$prop, static::$prop, etc.). if ($this->isStaticPropertyAccess($phpcsFile, $stackPtr)) { return; diff --git a/src/DrevOps/Sniffs/NamingConventions/ParameterNamingSniff.php b/src/DrevOps/Sniffs/NamingConventions/ParameterNamingSniff.php index 5ac689f..43e8cdf 100644 --- a/src/DrevOps/Sniffs/NamingConventions/ParameterNamingSniff.php +++ b/src/DrevOps/Sniffs/NamingConventions/ParameterNamingSniff.php @@ -38,6 +38,11 @@ public function process(File $phpcsFile, $stackPtr): void { return; } + // Skip variables with leading underscores (e.g., $_static_value). + if ($this->hasLeadingUnderscore($var_name)) { + return; + } + // Only process parameters (declaration only, not usage in body). // Local variables handled by LocalVariableNamingSniff. if (!$this->isParameter($phpcsFile, $stackPtr, FALSE)) { diff --git a/tests/Fixtures/Valid.php b/tests/Fixtures/Valid.php index 87a07f0..4f9d320 100644 --- a/tests/Fixtures/Valid.php +++ b/tests/Fixtures/Valid.php @@ -7,7 +7,17 @@ function functionWithValidSnakeCaseParams($valid_param_one, $valid_param_two) { $valid_local_variable = 100; $another_valid_local = $valid_local_variable * $valid_param_two; - return $valid_local_variable + $another_valid_local; + // Variables with leading underscores should be skipped. + $_static_value = 50; + $_internalVar = 25; + return $valid_local_variable + $another_valid_local + $_static_value + $_internalVar; +} + +/** + * Function with underscore-prefixed parameter (should be skipped). + */ +function functionWithUnderscorePrefixedParam($_prefixed_param) { + return $_prefixed_param * 2; } class ClassWithValidSnakeCaseNaming { @@ -16,7 +26,17 @@ class ClassWithValidSnakeCaseNaming { public function methodWithValidSnakeCaseParam($valid_param) { $valid_local_variable = strtolower($valid_param); + // Variables with leading underscores should be skipped. + $_internal_cache = []; + $_tempValue = NULL; return $valid_local_variable; } + /** + * Method with underscore-prefixed parameter (should be skipped). + */ + public function methodWithUnderscorePrefixedParam($_prefixed_param) { + return $_prefixed_param; + } + } diff --git a/tests/Unit/AbstractVariableNamingSniffTest.php b/tests/Unit/AbstractVariableNamingSniffTest.php index 2c249df..86517a2 100644 --- a/tests/Unit/AbstractVariableNamingSniffTest.php +++ b/tests/Unit/AbstractVariableNamingSniffTest.php @@ -683,4 +683,42 @@ public function testToFormatThrowsExceptionForInvalidFormat(): void { $method->invoke($sniff, 'test'); } + /** + * Test leading underscore detection. + * + * @param string $name + * The variable name to test. + * @param bool $expected + * Expected result. + */ + #[DataProvider('providerHasLeadingUnderscore')] + public function testHasLeadingUnderscore(string $name, bool $expected): void { + $sniff = new LocalVariableNamingSniff(); + $reflection = new \ReflectionClass($sniff); + $method = $reflection->getMethod('hasLeadingUnderscore'); + + $result = $method->invoke($sniff, $name); + $this->assertSame($expected, $result, 'Failed for: ' . $name); + } + + /** + * Data provider for hasLeadingUnderscore tests. + * + * @return array> + * Test cases. + */ + public static function providerHasLeadingUnderscore(): array { + return [ + 'leading_underscore_snake' => ['_static_value', TRUE], + 'leading_underscore_camel' => ['_staticValue', TRUE], + 'leading_underscore_only' => ['_', TRUE], + 'leading_double_underscore' => ['__internal', TRUE], + 'no_leading_underscore_snake' => ['static_value', FALSE], + 'no_leading_underscore_camel' => ['staticValue', FALSE], + 'underscore_in_middle' => ['static_value', FALSE], + 'trailing_underscore' => ['value_', FALSE], + 'single_letter' => ['a', FALSE], + ]; + } + } diff --git a/tests/Unit/LocalVariableNamingSniffTest.php b/tests/Unit/LocalVariableNamingSniffTest.php index f8f87ba..fb220c7 100644 --- a/tests/Unit/LocalVariableNamingSniffTest.php +++ b/tests/Unit/LocalVariableNamingSniffTest.php @@ -118,6 +118,14 @@ public static function dataProviderProcess(): array { 'camelCaseProperty; } }', FALSE, ], + 'underscore_prefixed_variable' => [ + ' [ + ' [ + ' [ + '