Skip to content

Commit

Permalink
WhiteSpace/PrecisionAlignment: bug fix - improved handling of heredoc…
Browse files Browse the repository at this point in the history 
…/nowdoc closers

This commit fixes two bugs:
1. When a heredoc/nowdoc closer (PHP 7.3+ flexible syntax) is indented with tabs, PHPCS does not replace tabs with spaces in the token.
    This means the token `'length'` key will also be tab-based, which leads to incorrrect calculations and false positives with incorrect fixes.
    Fixed now by handling tab replacement in heredoc/nowdoc closer tokens in the sniff itself.
2. A heredoc/nowdoc closer using flexible syntax is not allowed to have a larger indent than any of the content of the heredoc/nowdoc. Doing so results in a parse error.
    With the "best guess" rounding of the indent, the sniff had a risk of introducing these kind of parse errors.
    Fixed now by always rounding the expected indent down to the nearest tabstop for these tokens.

Includes additional unit tests to cover the changes.
  • Loading branch information
@jrfnl @dingo-d
jrfnl authored and dingo-d committed May 28, 2023
1 parent d6ce6ef commit 58fe9e5
Show file tree
Hide file tree
Showing 4 changed files with 183 additions and 9 deletions.
45 changes: 38 additions & 7 deletions Universal/Sniffs/WhiteSpace/PrecisionAlignmentSniff.php
View file
Expand Up @@ -270,12 +270,10 @@ public function process(File $phpcsFile, $stackPtr)
case \T_PHPCS_SET:
case \T_PHPCS_IGNORE:
case \T_PHPCS_IGNORE_FILE:
case \T_END_HEREDOC:
case \T_END_NOWDOC:
/*
* Indentation is included in the contents of the token for:
* - inline HTML
* - PHP 7.3+ flexible heredoc/nowdoc closer identifiers;
* - PHP 7.3+ flexible heredoc/nowdoc closer identifiers (see below);
* - subsequent lines of multi-line comments;
* - PHPCS native annotations when part of a multi-line comment.
*/
Expand Down Expand Up @@ -303,6 +301,24 @@ public function process(File $phpcsFile, $stackPtr)
--$spaces;
}
break;

case \T_END_HEREDOC:
case \T_END_NOWDOC:
/*
* PHPCS does not execute tab replacement in heredoc/nowdoc closer
* tokens (last checked: PHPCS 3.7.1), so handle this ourselves.
*/
$content = $tokens[$i]['content'];
if (\strpos($tokens[$i]['content'], "\t") !== false) {
$origContent = $content;
$content = \str_replace("\t", \str_repeat(' ', $this->tabWidth), $content);
}

$closer = \ltrim($content);
$whitespace = \str_replace($closer, '', $content);
$length = \strlen($whitespace);
$spaces = ($length % $indent);
break;
}

if ($spaces === 0) {
Expand All @@ -317,7 +333,13 @@ public function process(File $phpcsFile, $stackPtr)
);

if ($fix === true) {
$tabstops = (int) \round($spaces / $indent, 0);
if ($tokens[$i]['code'] === \T_END_HEREDOC || $tokens[$i]['code'] === \T_END_NOWDOC) {
// For heredoc/nowdoc, always round down to prevent introducing parse errors.
$tabstops = (int) \floor($spaces / $indent);
} else {
// For everything else, use "best guess".
$tabstops = (int) \round($spaces / $indent, 0);
}

switch ($tokens[$i]['code']) {
case \T_WHITESPACE:
Expand Down Expand Up @@ -357,8 +379,6 @@ public function process(File $phpcsFile, $stackPtr)
case \T_PHPCS_SET:
case \T_PHPCS_IGNORE:
case \T_PHPCS_IGNORE_FILE:
case \T_END_HEREDOC:
case \T_END_NOWDOC:
$replaceLength = (((int) ($length / $indent) + $tabstops) * $indent);
$replace = $this->getReplacement($replaceLength, $origContent);

Expand All @@ -376,6 +396,14 @@ public function process(File $phpcsFile, $stackPtr)

$phpcsFile->fixer->replaceToken($i, $newContent);
break;

case \T_END_HEREDOC:
case \T_END_NOWDOC:
$replaceLength = (((int) ($length / $indent) + $tabstops) * $indent);
$replace = $this->getReplacement($replaceLength, $origContent);

$phpcsFile->fixer->replaceToken($i, $replace . $closer);
break;
}
}
}
Expand All @@ -397,7 +425,10 @@ private function getReplacement($length, $origContent)
if ($origContent !== null) {
// Check whether tabs were part of the indent or inline alignment.
$content = \ltrim($origContent);
$whitespace = \str_replace($content, '', $origContent);
$whitespace = $origContent;
if ($content !== '') {
$whitespace = \str_replace($content, '', $origContent);
}

if (\strpos($whitespace, "\t") !== false) {
// Original indent used tabs. Use tabs in replacement too.
Expand Down
68 changes: 68 additions & 0 deletions Universal/Tests/WhiteSpace/PrecisionAlignmentUnitTest.11.inc
View file
Expand Up @@ -17,6 +17,8 @@
* The text lines are not checked and the closer MUST be at the start of the line,
* as otherwise the code will result in a parse error.
*/

// Space indent.
$heredoc = <<<EOD
some text
some text
Expand All @@ -29,12 +31,33 @@ some text
some text
EOD;

// Tab indent.
$heredoc = <<<EOD
some text
some text
some text
EOD;

$nowdoc = <<<'EOD'
some text
some text
some text
EOD;

/*
* PHP >= 7.3: flexible heredoc/nowdocs.
*
* In this case, the indentation before the closing identifier will be checked.
* As before, the text lines will not be checked.
*
* Notes:
* - These tests also safeguard that for nowdoc/heredoc closers, the indent is always rounded DOWN
* as rounding up could cause a parse error (invalid body indentation level)!
* - These tests also verify correct handling of spaces vs tabs in tokens in which PHPCS
* may not have done the replacement natively.
*/

// Space indent.
$heredoc = <<<END
a
b
Expand All @@ -58,3 +81,48 @@ $nowdoc = <<<'END'
b
c
END; // Warning.

$heredoc = <<<END
a
b
END; // Warning.

$nowdoc = <<<'END'
a
b
END; // Warning.

// Tab indent.
$heredoc = <<<END
a
b
c
END; // OK.

$nowdoc = <<<'END'
a
b
c
END; // OK.

$heredoc = <<<"END"
a
b
c
END; // Warning.

$nowdoc = <<<'END'
a
b
c
END; // Warning.

$heredoc = <<<END
a
b
END; // Warning.

$nowdoc = <<<'END'
a
b
END; // Warning.
68 changes: 68 additions & 0 deletions Universal/Tests/WhiteSpace/PrecisionAlignmentUnitTest.11.inc.fixed
View file
Expand Up @@ -17,6 +17,8 @@
* The text lines are not checked and the closer MUST be at the start of the line,
* as otherwise the code will result in a parse error.
*/

// Space indent.
$heredoc = <<<EOD
some text
some text
Expand All @@ -29,12 +31,33 @@ some text
some text
EOD;

// Tab indent.
$heredoc = <<<EOD
some text
some text
some text
EOD;

$nowdoc = <<<'EOD'
some text
some text
some text
EOD;

/*
* PHP >= 7.3: flexible heredoc/nowdocs.
*
* In this case, the indentation before the closing identifier will be checked.
* As before, the text lines will not be checked.
*
* Notes:
* - These tests also safeguard that for nowdoc/heredoc closers, the indent is always rounded DOWN
* as rounding up could cause a parse error (invalid body indentation level)!
* - These tests also verify correct handling of spaces vs tabs in tokens in which PHPCS
* may not have done the replacement natively.
*/

// Space indent.
$heredoc = <<<END
a
b
Expand All @@ -58,3 +81,48 @@ $nowdoc = <<<'END'
b
c
END; // Warning.

$heredoc = <<<END
a
b
END; // Warning.

$nowdoc = <<<'END'
a
b
END; // Warning.

// Tab indent.
$heredoc = <<<END
a
b
c
END; // OK.

$nowdoc = <<<'END'
a
b
c
END; // OK.

$heredoc = <<<"END"
a
b
c
END; // Warning.

$nowdoc = <<<'END'
a
b
c
END; // Warning.

$heredoc = <<<END
a
b
END; // Warning.

$nowdoc = <<<'END'
a
b
END; // Warning.
11 changes: 9 additions & 2 deletions Universal/Tests/WhiteSpace/PrecisionAlignmentUnitTest.php
View file
Expand Up @@ -34,6 +34,7 @@ final class PrecisionAlignmentUnitTest extends AbstractSniffUnitTest
'PrecisionAlignmentUnitTest.7.inc' => 3,
'PrecisionAlignmentUnitTest.8.inc' => 2,
'PrecisionAlignmentUnitTest.10.inc' => 4,
'PrecisionAlignmentUnitTest.11.inc' => 4,
'PrecisionAlignmentUnitTest.15.inc' => 4,
'PrecisionAlignmentUnitTest.17.inc' => 4,
'PrecisionAlignmentUnitTest.2.css' => 4,
Expand Down Expand Up @@ -138,8 +139,14 @@ public function getWarningList($testFile = '')
case 'PrecisionAlignmentUnitTest.11.inc':
if (\PHP_VERSION_ID >= 70300) {
return [
54 => 1,
60 => 1,
77 => 1,
83 => 1,
88 => 1,
93 => 1,
112 => 1,
118 => 1,
123 => 1,
128 => 1,
];
}

Expand Down

0 comments on commit 58fe9e5

Please sign in to comment.