Skip to content

Commit

Permalink
bug #4109 NoBlankLines*: fix removing lines consisting only of spaces…
Browse files Browse the repository at this point in the history
… (kubawerlos, keradus)

This PR was squashed before being merged into the 2.12 branch (closes #4109).

Discussion
----------

NoBlankLines*: fix removing lines consisting only of spaces

We have 3 `NoBlankLines*` fixers and they behave differently for lines consisting only of spaces:
- `NoBlankLinesAfterClassOpeningFixer` - removes line of spaces only when next line is indented
- `NoBlankLinesAfterPhpdocFixer` - same as above
- `NoBlankLinesBeforeNamespaceFixer` - removes indentation of the next line

What would be the proper behaviour (apart of removing indentation of next line what is obviously a bug) - to keep lines consisting only of spaces or to remove them?

_Tests are to demonstrate the problem, obviously have to be fixed as well._

Commits
-------

7df251c NoBlankLines*: fix removing lines consisting only of spaces
  • Loading branch information
keradus committed Dec 28, 2018
2 parents 88c0f72 + 7df251c commit 990fc94
Show file tree
Hide file tree
Showing 8 changed files with 94 additions and 10 deletions.
5 changes: 2 additions & 3 deletions src/AbstractLinesBeforeNamespaceFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,12 @@ protected function fixLinesBeforeNamespace(Tokens $tokens, $index, $expectedMin,

return;
}
$newWhitespaceToken = new Token([T_WHITESPACE, str_repeat($lineEnding, $newlinesForWhitespaceToken)]);
if ($previous->isWhitespace()) {
// Fix the previous whitespace token
$tokens[$previousIndex] = $newWhitespaceToken;
$tokens[$previousIndex] = new Token([T_WHITESPACE, str_repeat($lineEnding, $newlinesForWhitespaceToken).substr($previous->getContent(), strrpos($previous->getContent(), "\n") + 1)]);
} else {
// Add a new whitespace token
$tokens->insertAt($index, $newWhitespaceToken);
$tokens->insertAt($index, new Token([T_WHITESPACE, str_repeat($lineEnding, $newlinesForWhitespaceToken)]));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
use PhpCsFixer\FixerDefinition\FixerDefinition;
use PhpCsFixer\Tokenizer\Token;
use PhpCsFixer\Tokenizer\Tokens;
use PhpCsFixer\Utils;

/**
* @author Ceeram <ceeram@cakephp.org>
Expand Down Expand Up @@ -87,8 +86,7 @@ private function fixWhitespace(Tokens $tokens, $index)
// if there is more than one new line in the whitespace, then we need to fix it
if (substr_count($content, "\n") > 1) {
// the final bit of the whitespace must be the next statement's indentation
$lines = Utils::splitLines($content);
$tokens[$index] = new Token([T_WHITESPACE, $this->whitespacesConfig->getLineEnding().end($lines)]);
$tokens[$index] = new Token([T_WHITESPACE, $this->whitespacesConfig->getLineEnding().substr($content, strrpos($content, "\n") + 1)]);
}
}
}
4 changes: 1 addition & 3 deletions src/Fixer/Phpdoc/NoBlankLinesAfterPhpdocFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
use PhpCsFixer\FixerDefinition\FixerDefinition;
use PhpCsFixer\Tokenizer\Token;
use PhpCsFixer\Tokenizer\Tokens;
use PhpCsFixer\Utils;

/**
* @author Graham Campbell <graham@alt-three.com>
Expand Down Expand Up @@ -107,8 +106,7 @@ private function fixWhitespace(Tokens $tokens, $index)
// if there is more than one new line in the whitespace, then we need to fix it
if (substr_count($content, "\n") > 1) {
// the final bit of the whitespace must be the next statement's indentation
$lines = Utils::splitLines($content);
$tokens[$index] = new Token([T_WHITESPACE, "\n".end($lines)]);
$tokens[$index] = new Token([T_WHITESPACE, substr($content, strrpos($content, "\n"))]);
}
}
}
1 change: 0 additions & 1 deletion tests/AutoReview/FixerFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ public function provideFixersPriorityCases()
[$fixers['no_useless_return'], $fixers['blank_line_before_statement']],
[$fixers['no_useless_return'], $fixers['no_extra_blank_lines']],
[$fixers['no_useless_return'], $fixers['no_whitespace_in_blank_line']],
[$fixers['no_whitespace_in_blank_line'], $fixers['no_blank_lines_after_phpdoc']],
[$fixers['ordered_class_elements'], $fixers['class_attributes_separation']],
[$fixers['ordered_class_elements'], $fixers['method_separation']],
[$fixers['ordered_class_elements'], $fixers['no_blank_lines_after_class_opening']],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,40 @@ function firstMethod()
}',
];

// check if line with spaces is removed when next token is indented
$cases[] = [
'<?php
class Foo
{
function bar() {}
}
',
'<?php
class Foo
{
'.'
function bar() {}
}
',
];

// check if line with spaces is removed when next token is not indented
$cases[] = [
'<?php
class Foo
{
function bar() {}
}
',
'<?php
class Foo
{
'.'
function bar() {}
}
',
];

return $cases;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,28 @@ public function provideFixCases()
["<?php\nnamespace X;", "<?php\r\n\r\n\r\n\r\nnamespace X;"],
["<?php\r\nnamespace X;", "<?php\r\n\r\n\r\n\r\nnamespace X;", new WhitespacesFixerConfig(' ', "\r\n")],
["<?php\n\nnamespace\\Sub\\Foo::bar();"],
[
'<?php
// Foo
namespace Foo;
',
'<?php
// Foo
'.'
namespace Foo;
',
],
[
'<?php
// Foo
namespace Foo;
',
'<?php
// Foo
'.'
namespace Foo;
',
],
];
}

Expand Down
34 changes: 34 additions & 0 deletions tests/Fixer/Phpdoc/NoBlankLinesAfterPhpdocFixerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,40 @@ public function testLineBeforeDeclareIsNotBeRemoved()
$this->doTest($expected);
}

public function testLineWithSpacesIsRemovedWhenNextTokenIsIndented()
{
$this->doTest(
'<?php
/**
* PHPDoc with a line with space
*/
class Foo {}',
'<?php
/**
* PHPDoc with a line with space
*/
'.'
class Foo {}'
);
}

public function testLineWithSpacesIsRemovedWhenNextTokenIsNotIndented()
{
$this->doTest(
'<?php
/**
* PHPDoc with a line with space
*/
class Foo {}',
'<?php
/**
* PHPDoc with a line with space
*/
'.'
class Foo {}'
);
}

public function testFixesSimpleClass()
{
$expected = <<<'EOF'
Expand Down

0 comments on commit 990fc94

Please sign in to comment.