Skip to content

Commit

Permalink
Bug fix: DisallowSpaceIndent did not recognize nor fix mixed tab/spac…
Browse files Browse the repository at this point in the history 
…e indentations.

Details:
* Now also detects & fixes spaces if used after tabs and not used for precision indentation.
* Now also corrects the whitespace order if needed. Order should be tabs first, precision spaces second. For this a new error message and error code has been added.
* Adds the `Line endings: mixed` metric to this sniff in line with how it's used in the `DisallowTabIndent` sniff
* Adds `.fixed` files for the CSS and JS tests.
* Minor documentation clarifications.
  • Loading branch information
@jrfnl
jrfnl committed Apr 4, 2017
1 parent 29646e5 commit bf763be
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 33 deletions.
100 changes: 75 additions & 25 deletions CodeSniffer/Standards/Generic/Sniffs/WhiteSpace/DisallowSpaceIndentSniff.php
View file
Expand Up @@ -15,7 +15,7 @@
/**
* Generic_Sniffs_WhiteSpace_DisallowSpaceIndentSniff.
*
* Throws errors if spaces are used for indentation.
* Throws errors if spaces are used for indentation other than precision indentation.
*
* @category PHP
* @package PHP_CodeSniffer
Expand Down Expand Up @@ -94,42 +94,92 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
continue;
}

// If tabs are being converted to spaces, the original content
// should be used instead of the converted content.
// As tabs are being converted to spaces by the Tokenizer, the
// original content should be used instead of the converted content.
if (isset($tokens[$i]['orig_content']) === true) {
$content = $tokens[$i]['orig_content'];
} else {
$content = $tokens[$i]['content'];
}

if ($content[0] === ' ') {
if ($tokens[$i]['code'] === T_DOC_COMMENT_WHITESPACE && $content === ' ') {
// Ignore file/class-level DocBlock.
continue;
// If this is an inline HTML token, split the content into
// indentation whitespace and the actual HTML/text.
$nonWhitespace = '';
if ($tokens[$i]['code'] === T_INLINE_HTML && preg_match('`^(\s*)(\S.*)`s', $content, $matches) > 0) {
if (isset($matches[1]) === true) {
$content = $matches[1];
}

if (isset($matches[2]) === true) {
$nonWhitespace = $matches[2];
}
}

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

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

// Space are considered ok if they are proceeded by tabs and not followed
// by tabs, as is the case with standard docblock comments.
if ($hasSpaces === false && $hasTabs !== false) {
// All ok, nothing to do.
$phpcsFile->recordMetric($i, 'Line indent', 'tabs');
continue;
}

if ($tokens[$i]['code'] === T_DOC_COMMENT_WHITESPACE && $content === ' ') {
// Ignore file/class-level DocBlock, especially for recording metrics.
continue;
}

// OK, by now we know there will be spaces.
// 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);

if ($hasTabs === false) {
$phpcsFile->recordMetric($i, 'Line indent', 'spaces');
$error = 'Tabs must be used to indent lines; spaces are not allowed';
$fix = $phpcsFile->addFixableError($error, $i, 'SpacesUsed');
if ($fix === true) {
$trimmed = ltrim($content, ' ');
$numSpaces = (strlen($content) - strlen($trimmed));
if ($numSpaces < $this->_tabWidth) {
$numTabs = 1;
$padding = "\t";
} else {
$numTabs = floor($numSpaces / $this->_tabWidth);
$remaining = ($numSpaces - ($numTabs * $this->_tabWidth));
$padding = str_repeat("\t", $numTabs).$padding = str_repeat(' ', $remaining);
}

$phpcsFile->fixer->replaceToken($i, $padding.$trimmed);
if ($numTabs === 0) {
// Ignore: precision indentation.
continue;
}
} else {
if ($numTabs === 0) {
// Precision indentation.
$phpcsFile->recordMetric($i, 'Line indent', 'tabs');

if ($tabAfterSpaces === false) {
// Ignore: precision indentation is already at the
// end of the whitespace.
continue;
}
} else {
$phpcsFile->recordMetric($i, 'Line indent', 'mixed');
}
} else if ($content[0] === "\t") {
$phpcsFile->recordMetric($i, 'Line indent', 'tabs');
}//end if

$error = 'Tabs must be used to indent lines; spaces are not allowed';
$errorCode = 'SpacesUsed';

if ($tabAfterSpaces !== false) {
$error = 'Tabs must be used to indent lines; spaces are only allowed at the end for precision indentation';
$errorCode = 'TabsAfterSpaces';
}

$fix = $phpcsFile->addFixableError($error, $i, $errorCode);
if ($fix === true) {
$remaining = ($numSpaces % $this->_tabWidth);
$padding = str_repeat("\t", $numTabs);
$padding .= str_repeat(' ', $remaining);
$phpcsFile->fixer->replaceToken($i, $trimmed.$padding.$nonWhitespace);
}
}//end for

// Ignore the rest of the file.
Expand Down
4 changes: 4 additions & 0 deletions CodeSniffer/Standards/Generic/Tests/WhiteSpace/DisallowSpaceIndentUnitTest.css.fixed
View file
@@ -0,0 +1,4 @@
#login-container {
margin-left: -225px;
width: 450px;
}
9 changes: 9 additions & 0 deletions CodeSniffer/Standards/Generic/Tests/WhiteSpace/DisallowSpaceIndentUnitTest.inc
View file
Expand Up @@ -59,3 +59,12 @@ $x = 1;
</div>
</body>
</html>

<?php

// Issue #1404
// This is a line with mixed tabs and spaces.
echo 'And here is another line with mixed tabs and spaces.'
echo 'And another one.'
echo 'And another one.'
echo 'And another one.'
21 changes: 15 additions & 6 deletions CodeSniffer/Standards/Generic/Tests/WhiteSpace/DisallowSpaceIndentUnitTest.inc.fixed
View file
Expand Up @@ -15,13 +15,13 @@ EOF;
$correctVar = true;

// Indent with spaces is not allowed
$hello = array();
$world = '';
$hello = array();
$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 Expand Up @@ -54,8 +54,17 @@ $x = 1;
<div>
<div>
<div>
</div>
</div>
</div>
</div>
</div>
</body>
</html>

<?php

// Issue #1404
// This is a line with mixed tabs and spaces.
echo 'And here is another line with mixed tabs and spaces.'
echo 'And another one.'
echo 'And another one.'
echo 'And another one.'
9 changes: 9 additions & 0 deletions CodeSniffer/Standards/Generic/Tests/WhiteSpace/DisallowSpaceIndentUnitTest.js.fixed
View file
@@ -0,0 +1,9 @@
var x = {
abc: 1,
zyz: 2,
abc: 5,
mno: {
abc: 4
},
abc: 5
}
10 changes: 8 additions & 2 deletions CodeSniffer/Standards/Generic/Tests/WhiteSpace/DisallowSpaceIndentUnitTest.php
View file
Expand Up @@ -48,16 +48,22 @@ public function getErrorList($testFile='DisallowSpaceIndentUnitTest.inc')
5 => 1,
9 => 1,
15 => 1,
18 => 1,
19 => 1,
22 => 1,
24 => 1,
30 => 1,
35 => 1,
50 => 1,
55 => 1,
57 => 1,
58 => 1,
59 => 1,
60 => 1,
65 => 1,
66 => 1,
67 => 1,
68 => 1,
69 => 1,
70 => 1,
);
break;
case 'DisallowSpaceIndentUnitTest.js':
Expand Down
2 changes: 2 additions & 0 deletions package.xml
View file
Expand Up @@ -804,9 +804,11 @@ http://pear.php.net/dtd/package-2.0.xsd">
</dir>
<dir name="WhiteSpace">
<file baseinstalldir="PHP" name="DisallowSpaceIndentUnitTest.css" role="test" />
<file baseinstalldir="PHP" name="DisallowSpaceIndentUnitTest.css.fixed" role="test" />
<file baseinstalldir="PHP" name="DisallowSpaceIndentUnitTest.inc" role="test" />
<file baseinstalldir="PHP" name="DisallowSpaceIndentUnitTest.inc.fixed" role="test" />
<file baseinstalldir="PHP" name="DisallowSpaceIndentUnitTest.js" role="test" />
<file baseinstalldir="PHP" name="DisallowSpaceIndentUnitTest.js.fixed" role="test" />
<file baseinstalldir="PHP" name="DisallowSpaceIndentUnitTest.php" role="test">
<tasks:replace from="@package_version@" to="version" type="package-info" />
</file>
Expand Down

0 comments on commit bf763be

Please sign in to comment.