Skip to content

Commit c901484

Browse files
shudd3rshudd3r
committed
Added custom variable names validation (phpcs)
Embeded sniffs allow private variables to start with an underscore and validate class/object referenced variables which might come from 3rd party code. Those in our codebase will fail at definition.
1 parent 80a7bcc commit c901484

File tree

5 files changed

+222
-3
lines changed

5 files changed

+222
-3
lines changed

‎src/Sniffer/Sniffs/NamingConventions/ValidVariableNameSniff.php‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
<?php declare(strict_types=1);
2+
3+
/*
4+
* This file is part of Polymorphine/Dev package.
5+
*
6+
* (c) Shudd3r <q3.shudder@gmail.com>
7+
*
8+
* This source file is subject to the MIT license that is bundled
9+
* with this source code in the file LICENSE.
10+
*/
11+
12+
namespace Polymorphine\Dev\Sniffer\Sniffs\NamingConventions;
13+
14+
use PHP_CodeSniffer\Sniffs\AbstractVariableSniff;
15+
use PHP_CodeSniffer\Util\Common;
16+
use PHP_CodeSniffer\Files\File;
17+
18+
19+
class ValidVariableNameSniff extends AbstractVariableSniff
20+
{
21+
private File $phpcsFile;
22+
private int $varPointer;
23+
private array $tokens;
24+
25+
protected function processVariable(File $phpcsFile, $stackPtr)
26+
{
27+
$varName = $this->contents($phpcsFile, $stackPtr);
28+
if (isset($this->phpReservedVars[$varName])) { return; }
29+
30+
$prev = $this->phpcsFile->findPrevious([T_WHITESPACE], $stackPtr - 1, null, true);
31+
if ($this->tokens[$prev]['code'] === T_PAAMAYIM_NEKUDOTAYIM) {
32+
return;
33+
}
34+
35+
$this->validateCamelCaps($varName, 'NotCamelCaps');
36+
$this->validateNumbersInName($varName, 'ContainsNumbers');
37+
}
38+
39+
protected function processMemberVar(File $phpcsFile, $stackPtr)
40+
{
41+
$varName = $this->contents($phpcsFile, $stackPtr);
42+
$this->validateCamelCaps($varName, 'ClassVarNotCamelCaps');
43+
$this->validateNumbersInName($varName, 'ClassVarContainsNumbers');
44+
}
45+
46+
protected function processVariableInString(File $phpcsFile, $stackPtr)
47+
{
48+
$varPattern = '#[^\\\]\${?([a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)#';
49+
$contents = $this->contents($phpcsFile, $stackPtr, false);
50+
if (!preg_match_all($varPattern, $contents, $matches)) { return; }
51+
52+
foreach ($matches[1] as $varName) {
53+
if (isset($this->phpReservedVars[$varName])) { continue; }
54+
$this->validateCamelCaps($varName, 'StringVarNotCamelCaps');
55+
$this->validateNumbersInName($varName, 'StringVarContainsNumbers');
56+
}
57+
}
58+
59+
private function contents(File $phpcsFile, int $varPointer, bool $isVar = true): string
60+
{
61+
$this->varPointer = $varPointer;
62+
if (!isset($this->file) || $this->file !== $phpcsFile) {
63+
$this->phpcsFile = $phpcsFile;
64+
$this->tokens = $phpcsFile->getTokens();
65+
}
66+
$contents = $this->tokens[$this->varPointer]['content'];
67+
return $isVar ? ltrim($contents, '$') : $contents;
68+
}
69+
70+
private function validateCamelCaps(string $varName, string $type): void
71+
{
72+
if (Common::isCamelCaps($varName, false, true, false)) { return; }
73+
74+
$message = 'Variable `%s` is not in valid camel caps format';
75+
$this->phpcsFile->addError($message, $this->varPointer, $type, [$varName]);
76+
}
77+
78+
private function validateNumbersInName($varName, string $type): void
79+
{
80+
if (preg_match('#[0-9]#', $varName) !== 1) { return; }
81+
if ($type === 'ClassVarContainsNumbers') {
82+
$message = 'Variable `%s` cannot contain numbers';
83+
$this->phpcsFile->addError($message, $this->varPointer, $type, [$varName]);
84+
return;
85+
}
86+
87+
$message = 'Variable `%s` should not contain numbers';
88+
$this->phpcsFile->addWarning($message, $this->varPointer, $type, [$varName]);
89+
}
90+
}

‎src/Sniffer/ruleset.xml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,4 @@
3030
<rule ref="PSR1.Methods.CamelCapsMethodName">
3131
<exclude-pattern>*/tests/*</exclude-pattern>
3232
</rule>
33-
<rule ref="Squiz.NamingConventions.ValidVariableName">
34-
<exclude name="Squiz.NamingConventions.ValidVariableName.PrivateNoUnderscore"/>
35-
</rule>
3633
</ruleset>

‎tests/Fixtures/code-samples/Sniffs/InvalidVariableNames.php‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
<?php
2+
3+
// reserved names
4+
echo $_SERVER['var_name'];
5+
echo $_REQUEST['var_name'];
6+
echo $_GET['var_name'];
7+
echo $_POST['var_name'];
8+
echo $GLOBALS['var_name'];
9+
10+
// local variables
11+
$camelCaps = new SomeClass();
12+
$technicallycorrect = true;
13+
$not_camel = new SomeClass();
14+
$Capital = 'city';
15+
$_likePrivate = false;
16+
$hasNumber1 = false;
17+
$has2number = 'false';
18+
19+
// class members
20+
class SomeClass
21+
{
22+
public static $staticVar;
23+
protected static array $technicallycorrect;
24+
public static $not_camel = 100;
25+
private static $_oldSchoolPrivate;
26+
public static $with100numerInside = 100;
27+
public static $endsWithNumber6 = 6;
28+
29+
public Closure $camelCaps;
30+
private array $_privateLike = [];
31+
public int $shudd3r = 3;
32+
public array $endsWithNumber1 = [];
33+
34+
public function doSomething($not_camel = false, $has2Number = false): void
35+
{
36+
$cameCase = $not_camel && $has2Number;
37+
38+
// not validated object reference
39+
$this->_privateLike = [$cameCase];
40+
}
41+
}
42+
43+
$not_camel->doSomething();
44+
$cameCase->doSomething($bad_arg_name);
45+
46+
// object/class referenced var names are not validated
47+
// 1. Might be 3rd party code
48+
// 2. Ours will fail at definition anyway
49+
$camelCaps->endsWithNumber1 = [];
50+
SomeClass::$not_camel = 220;
51+
52+
// in strings
53+
$str = "Hello $name_or_world!";
54+
echo 'This is first line
55+
line with $_likePrivate variable, but in single quote
56+
which is ' . $concatenated2;
57+
58+
echo "And here double ${quote}
59+
with $Capital variable
60+
in second line ${wrong_name}
61+
this is $variable_name}";
62+
63+
$nowDoc = <<<'EOF'
64+
Variables in $NOWDOC are not validated
65+
EOF;
66+
67+
$hereDoc = <<<EOF
68+
But they are validated in HEREDOC - $has2Number
69+
This line {$should_have} error
70+
this is $\{not variable name.
71+
EOF;
72+
73+
$error = "format is \$GLOBALS['$varName']";
74+
75+
echo $_SESSION['var_name'];
76+
echo $_FILES['var_name'];
77+
echo $_ENV['var_name'];
78+
echo $_COOKIE['var_name'];
79+
80+
$XML = 'hello';
81+
$myXML = 'hello';
82+
$XMLParser = 'hello';
83+
$xmlParser = 'hello';
84+
85+
echo "{$_SERVER['HOSTNAME']} $var_name";
86+
echo "{$_SERVER['HOSTNAME']} $varName";
87+
88+
var_dump($http_response_header);
89+
var_dump($HTTP_RAW_POST_DATA);
90+
var_dump($php_errormsg);

‎tests/Sniffer/Sniffs/NamingConventions/ValidVariableNameSniffTest.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php declare(strict_types=1);
2+
3+
/*
4+
* This file is part of Polymorphine/Dev package.
5+
*
6+
* (c) Shudd3r <q3.shudder@gmail.com>
7+
*
8+
* This source file is subject to the MIT license that is bundled
9+
* with this source code in the file LICENSE.
10+
*/
11+
12+
namespace Polymorphine\Dev\Tests\Sniffer\Sniffs\NamingConventions;
13+
14+
use Polymorphine\Dev\Tests\SnifferTest;
15+
use Polymorphine\Dev\Sniffer\Sniffs\NamingConventions\ValidVariableNameSniff;
16+
17+
18+
class ValidVariableNameSniffTest extends SnifferTest
19+
{
20+
public function testNumberInLocalVariableName_GivesWarning()
21+
{
22+
$lines = [16, 17, 34, 36, 56, 68];
23+
$this->assertWarningLines('./tests/Fixtures/code-samples/Sniffs/InvalidVariableNames.php', $lines);
24+
}
25+
26+
public function testInvalidCamelCaseOrNumberInClassVariable_GivesError(): void
27+
{
28+
$lines = [13, 14, 15, 24, 25, 26, 27, 30, 31, 32, 34, 36, 43, 44, 53, 59, 60, 61, 69, 85];
29+
$this->assertErrorLines('./tests/Fixtures/code-samples/Sniffs/InvalidVariableNames.php', $lines);
30+
}
31+
32+
protected function sniffer(): string
33+
{
34+
return ValidVariableNameSniff::class;
35+
}
36+
}

‎tests/SnifferTest.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,11 @@ public function assertWarningLines(string $filename, array $expectedWarningLines
3535
$this->assertEquals($expectedWarningLines, array_keys($fileWarnings));
3636
}
3737

38+
public function assertErrorLines(string $filename, array $expectedErrorLines): void
39+
{
40+
$fileErrors = $this->runner->sniff($filename)->getErrors();
41+
$this->assertEquals($expectedErrorLines, array_keys($fileErrors));
42+
}
43+
3844
abstract protected function sniffer(): string;
3945
}

0 commit comments

Comments
 (0)