Skip to content

Commit

Permalink
DisallowSpaceIndent: Fix the case of the moving space
Browse files Browse the repository at this point in the history
Given this code:
```php
	 		wp_die( $api ); // Tab - Space - Tab - Tab
```
with tab width set to `4`, `wp_die()` would start in column 13.

The sniff was currently fixing this to:
```php
			 wp_die( $api ); // Tab - Tab - Tab - Space
```
which changed the start column to 14 and changed the "space hidden before a tab" to precision alignment.

This commit fixes that and is a further iteration building onto the improvements in 1404.

The above code will now be fixed as:
```php
			wp_die( $api ); // Tab - Tab - Tab
```

Notes:
* As the tabs in whitespace at the start of `T_INLINE_HTML` and `T_COMMENT` tokens is not replaced by spaces in the `content` by the tokenizer, this has to be done within the sniff to determine what the correct length of the whitespace should be.
* Basing the correction of the space-based length of the whitespace allows for fixing with higher precision.
* Incidentally, this also fixes one of the metrics being recorded incorrectly. For in-depth details of the effect on the metrics of this fix, please see: https://gist.github.com/jrfnl/5e2d75894c8e60a8f314b9fcb0ad3f62
* The `tabWidth` is now set in the unit test file.
  • Loading branch information
jrfnl committed Nov 22, 2017
1 parent 52bee3d commit eb3c55f
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ public function process(File $phpcsFile, $stackPtr)

$recordMetrics = true;

$expectedIndentSize = $tokens[$i]['length'];

// If this is an inline HTML token or a subsequent line of a multi-line comment,
// split the content into indentation whitespace and the actual HTML/text.
$nonWhitespace = '';
Expand All @@ -100,6 +102,10 @@ public function process(File $phpcsFile, $stackPtr)
) {
if (isset($matches[1]) === true) {
$content = $matches[1];

// Tabs are not replaced in content, so the "length" is wrong.
$matches[1] = str_replace("\t", str_repeat(' ', $this->tabWidth), $matches[1]);
$expectedIndentSize = strlen($matches[1]);
}

if (isset($matches[2]) === true) {
Expand All @@ -114,17 +120,17 @@ public function process(File $phpcsFile, $stackPtr)

// Don't record metrics for empty lines.
$recordMetrics = false;
}
}//end if

$hasSpaces = strpos($content, ' ');
$hasTabs = strpos($content, "\t");
$foundSpaces = substr_count($content, ' ');
$foundTabs = substr_count($content, "\t");

if ($hasSpaces === false && $hasTabs === false) {
if ($foundSpaces === 0 && $foundTabs === 0) {
// Empty line.
continue;
}

if ($hasSpaces === false && $hasTabs !== false) {
if ($foundSpaces === 0 && $foundTabs > 0) {
// All ok, nothing to do.
if ($recordMetrics === true) {
$phpcsFile->recordMetric($i, 'Line indent', 'tabs');
Expand All @@ -145,22 +151,23 @@ public function process(File $phpcsFile, $stackPtr)
// We just don't know yet whether they need to be replaced or
// are precision indentation, nor whether they are correctly
// placed at the end of the whitespace.
$trimmed = str_replace(' ', '', $content);
$numSpaces = (strlen($content) - strlen($trimmed));
$numTabs = (int) floor($numSpaces / $this->tabWidth);
$tabAfterSpaces = strpos($content, "\t", $hasSpaces);
$tabAfterSpaces = strpos($content, "\t", strpos($content, ' '));

// Calculate the expected tabs and spaces.
$expectedTabs = (int) floor($expectedIndentSize / $this->tabWidth);
$expectedSpaces = ($expectedIndentSize % $this->tabWidth);

if ($hasTabs === false) {
if ($foundTabs === 0) {
if ($recordMetrics === true) {
$phpcsFile->recordMetric($i, 'Line indent', 'spaces');
}

if ($numTabs === 0) {
if ($foundTabs === $expectedTabs && $foundSpaces === $expectedSpaces) {
// Ignore: precision indentation.
continue;
}
} else {
if ($numTabs === 0) {
if ($foundTabs === $expectedTabs && $foundSpaces === $expectedSpaces) {
// Precision indentation.
if ($recordMetrics === true) {
if ($tabAfterSpaces !== false) {
Expand All @@ -183,10 +190,9 @@ public function process(File $phpcsFile, $stackPtr)
$error = 'Tabs must be used to indent lines; spaces are not allowed';
$fix = $phpcsFile->addFixableError($error, $i, 'SpacesUsed');
if ($fix === true) {
$remaining = ($numSpaces % $this->tabWidth);
$padding = str_repeat("\t", $numTabs);
$padding .= str_repeat(' ', $remaining);
$phpcsFile->fixer->replaceToken($i, $trimmed.$padding.$nonWhitespace);
$padding = str_repeat("\t", $expectedTabs);
$padding .= str_repeat(' ', $expectedSpaces);
$phpcsFile->fixer->replaceToken($i, $padding.$nonWhitespace);
}
}//end for

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ EOF;
$world = '';
// here the indention is mixed with tabs and spaces
// [tab][space][space][space][tab]return "<$name$xmlns$type_str $atts xsi:nil=\"true\"/>";
return "<$name$xmlns$type_str $atts xsi:nil=\"true\"/>";
return "<$name$xmlns$type_str $atts xsi:nil=\"true\"/>";
// [space][space][space][tab]return "<$name$xmlns$type_str $atts xsi:nil=\"true\"/>";
return "<$name$xmlns$type_str $atts xsi:nil=\"true\"/>";
return "<$name$xmlns$type_str $atts xsi:nil=\"true\"/>";
// Doc comments are indent with tabs and one space
//[tab]/**
//[tab][space]*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,21 @@ class DisallowSpaceIndentUnitTest extends AbstractSniffUnitTest
{


/**
* Get a list of CLI values to set before the file is tested.
*
* @param string $testFile The name of the file being tested.
* @param \PHP_CodeSniffer\Config $config The config data for the test run.
*
* @return void
*/
public function setCliValues($testFile, $config)
{
$config->tabWidth = 4;

}//end setCliValues()


/**
* Returns the lines where errors should occur.
*
Expand Down

0 comments on commit eb3c55f

Please sign in to comment.