Skip to content

Commit

Permalink
Merge pull request #119 from PHPCSStandards/feature/generic-lowercase…
Browse files Browse the repository at this point in the history
…constants-performance-fix

Generic/LowerCaseConstant: improve performance
  • Loading branch information
jrfnl authored Dec 8, 2023
2 parents ec2709a + aae1797 commit 53c2838
Show file tree
Hide file tree
Showing 5 changed files with 175 additions and 17 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ The file documents changes to the PHP_CodeSniffer project.
- Runtime performance improvement for PHPCS CLI users. The improvement should be most noticeable for users on Windows.
- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch
- The following sniffs have received performance related improvements:
- Generic.PHP.LowerCaseConstant
- Generic.PHP.LowerCaseType
- PSR12.Files.OpenTag
- Thanks to Juliette Reinders Folmer (@jrfnl) for the patches
Expand Down
85 changes: 68 additions & 17 deletions src/Standards/Generic/Sniffs/PHP/LowerCaseConstantSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,29 @@ class LowerCaseConstantSniff implements Sniff
T_NULL => T_NULL,
];

/**
* Token types which can be encountered in a property type declaration.
*
* @var array<int|string, int|string>
*/
private $propertyTypeTokens = [
T_CALLABLE => T_CALLABLE,
T_SELF => T_SELF,
T_PARENT => T_PARENT,
T_FALSE => T_FALSE,
T_TRUE => T_TRUE,
T_NULL => T_NULL,
T_STRING => T_STRING,
T_NAME_QUALIFIED => T_NAME_QUALIFIED,
T_NAME_FULLY_QUALIFIED => T_NAME_FULLY_QUALIFIED,
T_NAME_RELATIVE => T_NAME_RELATIVE,
T_NS_SEPARATOR => T_NS_SEPARATOR,
T_NAMESPACE => T_NAMESPACE,
T_TYPE_UNION => T_TYPE_UNION,
T_TYPE_INTERSECTION => T_TYPE_INTERSECTION,
T_NULLABLE => T_NULLABLE,
];


/**
* Returns an array of tokens this test wants to listen for.
Expand All @@ -47,7 +70,13 @@ public function register()
{
$targets = $this->targets;

// Register function keywords to filter out type declarations.
// Register scope modifiers to filter out property type declarations.
$targets += Tokens::$scopeModifiers;
$targets[] = T_VAR;
$targets[] = T_STATIC;
$targets[] = T_READONLY;

// Register function keywords to filter out param/return type declarations.
$targets[] = T_FUNCTION;
$targets[] = T_CLOSURE;
$targets[] = T_FN;
Expand All @@ -64,12 +93,43 @@ public function register()
* @param int $stackPtr The position of the current token in the
* stack passed in $tokens.
*
* @return void|int
* @return void|int Optionally returns a stack pointer. The sniff will not be
* called again on the current file until the returned stack
* pointer is reached.
*/
public function process(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();

/*
* Skip over type declarations for properties.
*
* Note: for other uses of the visibility modifiers (functions, constants, trait use),
* nothing relevant will be skipped as the next non-empty token will be an "non-skippable"
* one.
* Functions are handled separately below (and then skip to their scope opener), so
* this should also not cause any confusion for constructor property promotion.
*
* For other uses of the "static" keyword, it also shouldn't be problematic as the only
* time the next non-empty token will be a "skippable" token will be in return type
* declarations, in which case, it is correct to skip over them.
*/

if (isset(Tokens::$scopeModifiers[$tokens[$stackPtr]['code']]) === true
|| $tokens[$stackPtr]['code'] === T_VAR
|| $tokens[$stackPtr]['code'] === T_STATIC
|| $tokens[$stackPtr]['code'] === T_READONLY
) {
$skipOver = (Tokens::$emptyTokens + $this->propertyTypeTokens);
$skipTo = $phpcsFile->findNext($skipOver, ($stackPtr + 1), null, true);
if ($skipTo !== false) {
return $skipTo;
}

// If we're at the end of the file, just return.
return;
}

// Handle function declarations separately as they may contain the keywords in type declarations.
if ($tokens[$stackPtr]['code'] === T_FUNCTION
|| $tokens[$stackPtr]['code'] === T_CLOSURE
Expand All @@ -79,9 +139,15 @@ public function process(File $phpcsFile, $stackPtr)
return;
}

// Make sure to skip over return type declarations.
$end = $tokens[$stackPtr]['parenthesis_closer'];
if (isset($tokens[$stackPtr]['scope_opener']) === true) {
$end = $tokens[$stackPtr]['scope_opener'];
} else {
$skipTo = $phpcsFile->findNext([T_SEMICOLON, T_OPEN_CURLY_BRACKET], ($end + 1), null, false, null, true);
if ($skipTo !== false) {
$end = $skipTo;
}
}

// Do a quick check if any of the targets exist in the declaration.
Expand Down Expand Up @@ -114,21 +180,6 @@ public function process(File $phpcsFile, $stackPtr)
return $end;
}//end if

// Handle property declarations separately as they may contain the keywords in type declarations.
if (isset($tokens[$stackPtr]['conditions']) === true) {
$conditions = $tokens[$stackPtr]['conditions'];
$lastCondition = end($conditions);
if (isset(Tokens::$ooScopeTokens[$lastCondition]) === true) {
// This can only be an OO constant or property declaration as methods are handled above.
$equals = $phpcsFile->findPrevious(T_EQUAL, ($stackPtr - 1), null, false, null, true);
if ($equals !== false) {
$this->processConstant($phpcsFile, $stackPtr);
}

return;
}
}

// Handle everything else.
$this->processConstant($phpcsFile, $stackPtr);

Expand Down
49 changes: 49 additions & 0 deletions src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,52 @@ class TypedThings {
}

$cl = function (int|FALSE $param = NULL, Type|NULL $obj = new MyObj(FALSE)) : string|FALSE|NULL {};

// Adding some extra tests to safeguard that function declarations which don't create scope are handled correctly.
interface InterfaceMethodsWithReturnTypeNoScopeOpener {
private function typed($param = TRUE) : string|FALSE|NULL;
}

abstract class ClassMethodsWithReturnTypeNoScopeOpener {
abstract public function typed($param = FALSE) : TRUE;
}

// Additional tests to safeguard improved property type skip logic.
readonly class Properties {
use SomeTrait {
sayHello as private myPrivateHello;
}

public Type|FALSE|NULL $propertyA = array(
'itemA' => TRUE,
'itemB' => FALSE,
'itemC' => NULL,
), $propertyB = FALSE;

protected \FullyQualified&Partially\Qualified&namespace\Relative $propertyC;
var ?TRUE $propertyD;
static array|callable|FALSE|self|parent $propertyE = TRUE;
private
// phpcs:ignore Stnd.Cat.Sniff -- for reasons.
TRUE /*comment*/
$propertyF = TRUE;

public function __construct(
public FALSE|NULL $promotedPropA,
readonly callable|TRUE $promotedPropB,
) {
static $var;
echo static::class;
static::foo();
$var = $var instanceof static;
$obj = new static();
}

public static function foo(): static|self|FALSE {
$callable = static function() {};
}
}

// Last coding/parse error.
// This has to be the last test in the file.
function UnclosedCurly (): FALSE {
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,52 @@ class TypedThings {
}

$cl = function (int|FALSE $param = null, Type|NULL $obj = new MyObj(false)) : string|FALSE|NULL {};

// Adding some extra tests to safeguard that function declarations which don't create scope are handled correctly.
interface InterfaceMethodsWithReturnTypeNoScopeOpener {
private function typed($param = true) : string|FALSE|NULL;
}

abstract class ClassMethodsWithReturnTypeNoScopeOpener {
abstract public function typed($param = false) : TRUE;
}

// Additional tests to safeguard improved property type skip logic.
readonly class Properties {
use SomeTrait {
sayHello as private myPrivateHello;
}

public Type|FALSE|NULL $propertyA = array(
'itemA' => true,
'itemB' => false,
'itemC' => null,
), $propertyB = false;

protected \FullyQualified&Partially\Qualified&namespace\Relative $propertyC;
var ?TRUE $propertyD;
static array|callable|FALSE|self|parent $propertyE = true;
private
// phpcs:ignore Stnd.Cat.Sniff -- for reasons.
TRUE /*comment*/
$propertyF = true;

public function __construct(
public FALSE|NULL $promotedPropA,
readonly callable|TRUE $promotedPropB,
) {
static $var;
echo static::class;
static::foo();
$var = $var instanceof static;
$obj = new static();
}

public static function foo(): static|self|FALSE {
$callable = static function() {};
}
}

// Last coding/parse error.
// This has to be the last test in the file.
function UnclosedCurly (): FALSE {
8 changes: 8 additions & 0 deletions src/Standards/Generic/Tests/PHP/LowerCaseConstantUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ public function getErrorList($testFile='LowerCaseConstantUnitTest.inc')
94 => 2,
95 => 1,
100 => 2,
104 => 1,
108 => 1,
118 => 1,
119 => 1,
120 => 1,
121 => 1,
125 => 1,
129 => 1,
];

case 'LowerCaseConstantUnitTest.js':
Expand Down

0 comments on commit 53c2838

Please sign in to comment.