Skip to content

Commit

Permalink
Merge pull request #173 from PHPCSStandards/universal/noreservedkeywo…
Browse files Browse the repository at this point in the history 
…rdparamname-various-improvements

Universal/NoReservedKeywordParameterNames: various improvements
  • Loading branch information
@jrfnl
jrfnl committed Dec 4, 2022
2 parents 7cafca7 + 35a2e51 commit becd4f0
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 117 deletions.
230 changes: 117 additions & 113 deletions Universal/Sniffs/NamingConventions/NoReservedKeywordParameterNamesSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,19 @@

namespace PHPCSExtra\Universal\Sniffs\NamingConventions;

use PHP_CodeSniffer\Exceptions\RuntimeException;
use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;
use PHPCSUtils\Tokens\Collections;
use PHPCSUtils\Utils\FunctionDeclarations;

/**
* Verifies that parameters in function declarations do not use PHP reserved keywords.
* Verifies that parameters in function declarations do not use PHP reserved keywords
* as this can lead to confusing code when using PHP 8.0+ named parameters in function calls.
*
* Note: while parameters (variables) are case-sensitive in PHP, keywords are not,
* so this sniff checks for the keywords used in parameter names in a
* case-insensitive manner to make this sniff independent of code style rules
* regarding the case for parameter names.
*
* @link https://www.php.net/manual/en/reserved.keywords.php
*
Expand All @@ -31,108 +36,108 @@ final class NoReservedKeywordParameterNamesSniff implements Sniff
*
* @since 1.0.0
*
* @var array(string => string)
* @var array<string => string> Key is the lowercased keyword, value the "proper" cased keyword.
*/
protected $reservedNames = [
'abstract' => true,
'and' => true,
'array' => true,
'as' => true,
'break' => true,
'callable' => true,
'case' => true,
'catch' => true,
'class' => true,
'clone' => true,
'const' => true,
'continue' => true,
'declare' => true,
'default' => true,
'die' => true,
'do' => true,
'echo' => true,
'else' => true,
'elseif' => true,
'empty' => true,
'enddeclare' => true,
'endfor' => true,
'endforeach' => true,
'endif' => true,
'endswitch' => true,
'endwhile' => true,
'enum' => true,
'eval' => true,
'exit' => true,
'extends' => true,
'final' => true,
'finally' => true,
'fn' => true,
'for' => true,
'foreach' => true,
'function' => true,
'global' => true,
'goto' => true,
'if' => true,
'implements' => true,
'include' => true,
'include_once' => true,
'instanceof' => true,
'insteadof' => true,
'interface' => true,
'isset' => true,
'list' => true,
'match' => true,
'namespace' => true,
'new' => true,
'or' => true,
'print' => true,
'private' => true,
'protected' => true,
'public' => true,
'readonly' => true,
'require' => true,
'require_once' => true,
'return' => true,
'static' => true,
'switch' => true,
'throw' => true,
'trait' => true,
'try' => true,
'unset' => true,
'use' => true,
'var' => true,
'while' => true,
'xor' => true,
'yield' => true,
'__CLASS__' => true,
'__DIR__' => true,
'__FILE__' => true,
'__FUNCTION__' => true,
'__LINE__' => true,
'__METHOD__' => true,
'__NAMESPACE__' => true,
'__TRAIT__' => true,
'int' => true,
'float' => true,
'bool' => true,
'string' => true,
'true' => true,
'false' => true,
'null' => true,
'void' => true,
'iterable' => true,
'object' => true,
'resource' => true,
'mixed' => true,
'numeric' => true,
'never' => true,
private $reservedNames = [
'abstract' => 'abstract',
'and' => 'and',
'array' => 'array',
'as' => 'as',
'break' => 'break',
'callable' => 'callable',
'case' => 'case',
'catch' => 'catch',
'class' => 'class',
'clone' => 'clone',
'const' => 'const',
'continue' => 'continue',
'declare' => 'declare',
'default' => 'default',
'die' => 'die',
'do' => 'do',
'echo' => 'echo',
'else' => 'else',
'elseif' => 'elseif',
'empty' => 'empty',
'enddeclare' => 'enddeclare',
'endfor' => 'endfor',
'endforeach' => 'endforeach',
'endif' => 'endif',
'endswitch' => 'endswitch',
'endwhile' => 'endwhile',
'enum' => 'enum',
'eval' => 'eval',
'exit' => 'exit',
'extends' => 'extends',
'final' => 'final',
'finally' => 'finally',
'fn' => 'fn',
'for' => 'for',
'foreach' => 'foreach',
'function' => 'function',
'global' => 'global',
'goto' => 'goto',
'if' => 'if',
'implements' => 'implements',
'include' => 'include',
'include_once' => 'include_once',
'instanceof' => 'instanceof',
'insteadof' => 'insteadof',
'interface' => 'interface',
'isset' => 'isset',
'list' => 'list',
'match' => 'match',
'namespace' => 'namespace',
'new' => 'new',
'or' => 'or',
'print' => 'print',
'private' => 'private',
'protected' => 'protected',
'public' => 'public',
'readonly' => 'readonly',
'require' => 'require',
'require_once' => 'require_once',
'return' => 'return',
'static' => 'static',
'switch' => 'switch',
'throw' => 'throw',
'trait' => 'trait',
'try' => 'try',
'unset' => 'unset',
'use' => 'use',
'var' => 'var',
'while' => 'while',
'xor' => 'xor',
'yield' => 'yield',
'__class__' => '__CLASS__',
'__dir__' => '__DIR__',
'__file__' => '__FILE__',
'__function__' => '__FUNCTION__',
'__line__' => '__LINE__',
'__method__' => '__METHOD__',
'__namespace__' => '__NAMESPACE__',
'__trait__' => '__TRAIT__',
'int' => 'int',
'float' => 'float',
'bool' => 'bool',
'string' => 'string',
'true' => 'true',
'false' => 'false',
'null' => 'null',
'void' => 'void',
'iterable' => 'iterable',
'object' => 'object',
'resource' => 'resource',
'mixed' => 'mixed',
'numeric' => 'numeric',
'never' => 'never',

/*
* Not reserved keywords, but equally confusing when used in the context of function calls
* with named parameters.
*/
'parent' => true,
'self' => true,
'parent' => 'parent',
'self' => 'self',
];

/**
Expand Down Expand Up @@ -161,25 +166,24 @@ public function register()
public function process(File $phpcsFile, $stackPtr)
{
// Get all parameters from method signature.
try {
$parameters = FunctionDeclarations::getParameters($phpcsFile, $stackPtr);
if (empty($parameters)) {
return;
}
} catch (RuntimeException $e) {
$parameters = FunctionDeclarations::getParameters($phpcsFile, $stackPtr);
if (empty($parameters)) {
return;
}

$paramNames = [];
$message = 'It is recommended not to use reserved keyword "%s" as function parameter name. Found: %s';

foreach ($parameters as $param) {
$name = \ltrim($param['name'], '$');
if (isset($this->reservedNames[$name]) === true) {
$phpcsFile->addWarning(
'It is recommended not to use reserved keywords as function parameter names. Found: %s',
$param['token'],
$name . 'Found',
[$param['name']]
);
$name = \ltrim($param['name'], '$');
$nameLC = \strtolower($name);
if (isset($this->reservedNames[$nameLC]) === true) {
$errorCode = $nameLC . 'Found';
$data = [
$this->reservedNames[$nameLC],
$param['name'],
];

$phpcsFile->addWarning($message, $param['token'], $errorCode, $data);
}
}
}
Expand Down
47 changes: 43 additions & 4 deletions Universal/Tests/NamingConventions/NoReservedKeywordParameterNamesUnitTest.inc
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,54 @@
<?php

function foo( $parameter, $descriptive_name ) {} // OK.
function noReservedKeywords( $parameter, $descriptive_name ) {} // OK.

function foo( $string, $echo = true ) {} // Bad x 2.
function hasReservedKeywords( $string, $echo = true ) {} // Bad x 2.
$closure = function ( $foreach, $array, $require ) {}; // Bad x 3.
$fn = fn($callable, $list) => $callable($list); // Bad x 2.
$fn = fn($callable, $__FILE__) => $callable($__FILE__); // Bad x 2.

abstract class Foo {
abstract public function bar(
abstract public function abstractMethod(
string &$string,
Foo|false $exit,
?int $parent
); // Bad x 3.
}

/*
* Tests using less conventional param name casing.
*/
function noReservedKeywordsCasedParams( $Parameter, $DescriptiveName ) {} // OK.

function hasReservedKeywordsCasedParams( $String, $ECHO = true ) {} // Bad x 2.
$closure = function ( $forEach, $Array, $REQUIRE ) {}; // Bad x 3.
$fn = fn($Callable, $__file__) => $Callable($__file__); // Bad x 2.

abstract class Bar {
abstract public function abstractMethodCasedParams(
string &$STRING,
Foo|false $Exit,
?int $PaReNt
); // Bad x 3.
}

/*
* Also flag properties declared via constructor property promotion as those can also be
* passed to the class constructor as named parameters.
*/
class ConstructorPropertyPromotionNoTypes {
public function __construct(
public $const = 0.0,
protected $do = '',
private $eval = null,
) {}
}

class ConstructorPropertyPromotionWithTypes {
public function __construct(protected float|int $Function, public ?string &$GLOBAL = 'test', private mixed $match) {}
}

// No parameters, nothing to do.
function noParam() {}

// Live coding/parse error. This has to be the last test in the file.
function liveCoding($echo
10 changes: 10 additions & 0 deletions Universal/Tests/NamingConventions/NoReservedKeywordParameterNamesUnitTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,16 @@ public function getWarningList()
11 => 1,
12 => 1,
13 => 1,
22 => 2,
23 => 3,
24 => 2,
28 => 1,
29 => 1,
30 => 1,
40 => 1,
41 => 1,
42 => 1,
47 => 3,
];
}
}

0 comments on commit becd4f0

Please sign in to comment.