Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow parsing mixed HTML and PHP #234

Merged
merged 4 commits into from
Jun 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions Tests/VariableAnalysisSniff/ClosingPhpTagsTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php
namespace VariableAnalysis\Tests\VariableAnalysisSniff;

use VariableAnalysis\Tests\BaseTestCase;

class ClosingPhpTagsTest extends BaseTestCase {
public function testVariableWarningsWhenClosingTagsAreUsed() {
$fixtureFile = $this->getFixture('ClosingPhpTagsFixture.php');
$phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
$phpcsFile->process();
$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
$expectedWarnings = [
6,
8,
13,
16,
];
$this->assertEquals($expectedWarnings, $lines);
}

public function testVariableWarningsHaveCorrectSniffCodesWhenClosingTagsAreUsed() {
$fixtureFile = $this->getFixture('ClosingPhpTagsFixture.php');
$phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
$phpcsFile->process();
$warnings = $phpcsFile->getWarnings();
$this->assertEquals('VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable', $warnings[6][1][0]['source']);
$this->assertEquals('VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable', $warnings[8][6][0]['source']);
$this->assertEquals('VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable', $warnings[13][1][0]['source']);
$this->assertEquals('VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable', $warnings[16][6][0]['source']);
}
}
18 changes: 18 additions & 0 deletions Tests/VariableAnalysisSniff/fixtures/ClosingPhpTagsFixture.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<html>
<h1>Page Title</h1>
<?php
$foo = 'hello';
$blue = 'hello';
$bar = 'bye'; // unused variable
echo $foo;
echo $baz; // undefined variable
?>
<p>More stuff</p>
<?php
$foo2 = 'hello';
$bar2 = 'bye'; // unused variable
echo $foo2;
echo $blue;
echo $baz2; // undefined variable
?>
</html>
45 changes: 29 additions & 16 deletions VariableAnalysis/Lib/Helpers.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,19 @@
use PHP_CodeSniffer\Util\Tokens;

class Helpers {
/**
* return int[]
*/
public static function getEmptyTokens() {
return array_merge(
array_values(Tokens::$emptyTokens),
[
T_INLINE_HTML,
T_CLOSE_TAG,
]
);
Comment on lines +16 to +22
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return array_merge(
array_values(Tokens::$emptyTokens),
[
T_INLINE_HTML,
T_CLOSE_TAG,
]
);
$empty = Tokens::$emptyTokens;
$empty[T_INLINE_HTML] = T_INLINE_HTML;
$empty[T_CLOSE_TAG] = T_CLOSE_TAG;
return $empty;

Array functions like array_merge() and array_values() are "expensive".

Changing it to the above will be more efficient performance + memory-use-wise.

You could even choose to optimize it further like this:

Suggested change
return array_merge(
array_values(Tokens::$emptyTokens),
[
T_INLINE_HTML,
T_CLOSE_TAG,
]
);
static $empty;
if (isset($empty) === false) {
$empty = Tokens::$emptyTokens;
$empty[T_INLINE_HTML] = T_INLINE_HTML;
$empty[T_CLOSE_TAG] = T_CLOSE_TAG;
}
return $empty;

It's not as if this compound array will change at any time during the run.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! Good to know. I reduced how often this function is called in #236 but I made #237 to make sure I remember to clean this up.

}

/**
* @param int|bool $value
*
Expand Down Expand Up @@ -146,7 +159,7 @@ public static function getFunctionIndexForFunctionArgument(File $phpcsFile, $sta
return null;
}

$nonFunctionTokenTypes = array_values(Tokens::$emptyTokens);
$nonFunctionTokenTypes = self::getEmptyTokens();
$nonFunctionTokenTypes[] = T_STRING;
$nonFunctionTokenTypes[] = T_BITWISE_AND;
$functionPtr = self::getIntOrNull($phpcsFile->findPrevious($nonFunctionTokenTypes, $startOfArguments - 1, null, true, null, true));
Expand Down Expand Up @@ -186,7 +199,7 @@ public static function isTokenInsideFunctionUseImport(File $phpcsFile, $stackPtr
public static function getUseIndexForUseImport(File $phpcsFile, $stackPtr) {
$tokens = $phpcsFile->getTokens();

$nonUseTokenTypes = array_values(Tokens::$emptyTokens);
$nonUseTokenTypes = self::getEmptyTokens();
$nonUseTokenTypes[] = T_VARIABLE;
$nonUseTokenTypes[] = T_ELLIPSIS;
$nonUseTokenTypes[] = T_COMMA;
Expand Down Expand Up @@ -215,7 +228,7 @@ public static function findFunctionCall(File $phpcsFile, $stackPtr) {
$openPtr = Helpers::findContainingOpeningBracket($phpcsFile, $stackPtr);
if (is_int($openPtr)) {
// First non-whitespace thing and see if it's a T_STRING function name
$functionPtr = $phpcsFile->findPrevious(Tokens::$emptyTokens, $openPtr - 1, null, true, null, true);
$functionPtr = $phpcsFile->findPrevious(self::getEmptyTokens(), $openPtr - 1, null, true, null, true);
if (is_int($functionPtr) && $tokens[$functionPtr]['code'] === T_STRING) {
return $functionPtr;
}
Expand All @@ -242,7 +255,7 @@ public static function findFunctionCallArguments(File $phpcsFile, $stackPtr) {
}

// $stackPtr is the function name, find our brackets after it
$openPtr = $phpcsFile->findNext(Tokens::$emptyTokens, $stackPtr + 1, null, true, null, true);
$openPtr = $phpcsFile->findNext(self::getEmptyTokens(), $stackPtr + 1, null, true, null, true);
if (($openPtr === false) || ($tokens[$openPtr]['code'] !== T_OPEN_PARENTHESIS)) {
return [];
}
Expand Down Expand Up @@ -280,7 +293,7 @@ public static function getNextAssignPointer(File $phpcsFile, $stackPtr) {
$tokens = $phpcsFile->getTokens();

// Is the next non-whitespace an assignment?
$nextPtr = $phpcsFile->findNext(Tokens::$emptyTokens, $stackPtr + 1, null, true, null, true);
$nextPtr = $phpcsFile->findNext(self::getEmptyTokens(), $stackPtr + 1, null, true, null, true);
if (is_int($nextPtr)
&& isset(Tokens::$assignmentTokens[$tokens[$nextPtr]['code']])
// Ignore double arrow to prevent triggering on `foreach ( $array as $k => $v )`.
Expand Down Expand Up @@ -508,14 +521,14 @@ public static function isArrowFunction(File $phpcsFile, $stackPtr) {
return false;
}
// Make sure next non-space token is an open parenthesis
$openParenIndex = $phpcsFile->findNext(Tokens::$emptyTokens, $stackPtr + 1, null, true);
$openParenIndex = $phpcsFile->findNext(self::getEmptyTokens(), $stackPtr + 1, null, true);
if (! is_int($openParenIndex) || $tokens[$openParenIndex]['code'] !== T_OPEN_PARENTHESIS) {
return false;
}
// Find the associated close parenthesis
$closeParenIndex = $tokens[$openParenIndex]['parenthesis_closer'];
// Make sure the next token is a fat arrow
$fatArrowIndex = $phpcsFile->findNext(Tokens::$emptyTokens, $closeParenIndex + 1, null, true);
$fatArrowIndex = $phpcsFile->findNext(self::getEmptyTokens(), $closeParenIndex + 1, null, true);
if (! is_int($fatArrowIndex)) {
return false;
}
Expand Down Expand Up @@ -543,14 +556,14 @@ public static function getArrowFunctionOpenClose(File $phpcsFile, $stackPtr) {
return null;
}
// Make sure next non-space token is an open parenthesis
$openParenIndex = $phpcsFile->findNext(Tokens::$emptyTokens, $stackPtr + 1, null, true);
$openParenIndex = $phpcsFile->findNext(self::getEmptyTokens(), $stackPtr + 1, null, true);
if (! is_int($openParenIndex) || $tokens[$openParenIndex]['code'] !== T_OPEN_PARENTHESIS) {
return null;
}
// Find the associated close parenthesis
$closeParenIndex = $tokens[$openParenIndex]['parenthesis_closer'];
// Make sure the next token is a fat arrow
$fatArrowIndex = $phpcsFile->findNext(Tokens::$emptyTokens, $closeParenIndex + 1, null, true);
$fatArrowIndex = $phpcsFile->findNext(self::getEmptyTokens(), $closeParenIndex + 1, null, true);
if (! is_int($fatArrowIndex)) {
return null;
}
Expand Down Expand Up @@ -600,7 +613,7 @@ public static function getListAssignments(File $phpcsFile, $listOpenerIndex) {
}

// Find the assignment (equals sign) which, if this is a list assignment, should be the next non-space token
$assignPtr = $phpcsFile->findNext(Tokens::$emptyTokens, $closePtr + 1, null, true);
$assignPtr = $phpcsFile->findNext(self::getEmptyTokens(), $closePtr + 1, null, true);

// If the next token isn't an assignment, check for nested brackets because we might be a nested assignment
if (! is_int($assignPtr) || $tokens[$assignPtr]['code'] !== T_EQUAL) {
Expand Down Expand Up @@ -715,7 +728,7 @@ public static function isVariableANumericVariable($varName) {
*/
public static function isVariableInsideElseCondition(File $phpcsFile, $stackPtr) {
$tokens = $phpcsFile->getTokens();
$nonFunctionTokenTypes = array_values(Tokens::$emptyTokens);
$nonFunctionTokenTypes = self::getEmptyTokens();
$nonFunctionTokenTypes[] = T_OPEN_PARENTHESIS;
$nonFunctionTokenTypes[] = T_VARIABLE;
$nonFunctionTokenTypes[] = T_ELLIPSIS;
Expand Down Expand Up @@ -837,7 +850,7 @@ public static function getScopeCloseForScopeOpen(File $phpcsFile, $scopeStartInd
public static function getLastNonEmptyTokenIndexInFile(File $phpcsFile) {
$tokens = $phpcsFile->getTokens();
foreach (array_reverse($tokens, true) as $index => $token) {
if (! in_array($token['code'], Tokens::$emptyTokens, true)) {
if (! in_array($token['code'], self::getEmptyTokens(), true)) {
return $index;
}
}
Expand Down Expand Up @@ -921,7 +934,7 @@ public static function getFunctionIndexForFunctionCallArgument(File $phpcsFile,
return null;
}

$nonFunctionTokenTypes = array_values(Tokens::$emptyTokens);
$nonFunctionTokenTypes = self::getEmptyTokens();
$functionPtr = self::getIntOrNull($phpcsFile->findPrevious($nonFunctionTokenTypes, $startOfArguments - 1, null, true, null, true));
if (! is_int($functionPtr) || ! isset($tokens[$functionPtr]['code'])) {
return null;
Expand Down Expand Up @@ -965,7 +978,7 @@ public static function isVariableInsideIssetOrEmpty(File $phpcsFile, $stackPtr)
*/
public static function isVariableArrayPushShortcut(File $phpcsFile, $stackPtr) {
$tokens = $phpcsFile->getTokens();
$nonFunctionTokenTypes = array_values(Tokens::$emptyTokens);
$nonFunctionTokenTypes = self::getEmptyTokens();

$arrayPushOperatorIndex1 = self::getIntOrNull($phpcsFile->findNext($nonFunctionTokenTypes, $stackPtr + 1, null, true, null, true));
if (! is_int($arrayPushOperatorIndex1)) {
Expand Down Expand Up @@ -1063,7 +1076,7 @@ public static function isTokenInsideAssignmentLHS(File $phpcsFile, $stackPtr) {
public static function isTokenVariableVariable(File $phpcsFile, $stackPtr) {
$tokens = $phpcsFile->getTokens();

$prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($stackPtr - 1), null, true);
$prev = $phpcsFile->findPrevious(self::getEmptyTokens(), ($stackPtr - 1), null, true);
if ($prev === false) {
return false;
}
Expand All @@ -1074,7 +1087,7 @@ public static function isTokenVariableVariable(File $phpcsFile, $stackPtr) {
return false;
}

$prevPrev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($prev - 1), null, true);
$prevPrev = $phpcsFile->findPrevious(self::getEmptyTokens(), ($prev - 1), null, true);
if ($prevPrev !== false && $tokens[$prevPrev]['code'] === T_DOLLAR) {
return true;
}
Expand Down