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

minor: Split BracesFixer #4884

Merged
merged 1 commit into from Aug 17, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/sca.yml
Expand Up @@ -113,9 +113,9 @@ jobs:
if: ${{ env.CHANGED_PHP_FILES }}
run: |
if [ '${{ github.event_name }}' == 'pull_request' ]; then
./dev-tools/vendor/bin/phpmd `echo "$CHANGED_PHP_FILES" | xargs | sed 's/ /,/g'` github phpmd.xml --exclude tests/Fixtures/
./dev-tools/vendor/bin/phpmd `echo "$CHANGED_PHP_FILES" | grep -v /Fixtures/ | xargs | sed 's/ /,/g'` github phpmd.xml
else
./dev-tools/vendor/bin/phpmd `echo "$CHANGED_PHP_FILES" | xargs | sed 's/ /,/g'` ansi phpmd.xml --exclude tests/Fixtures/
./dev-tools/vendor/bin/phpmd `echo "$CHANGED_PHP_FILES" | grep -v /Fixtures/ | xargs | sed 's/ /,/g'` ansi phpmd.xml
fi

- name: Check - ensure test files are not present in the archive
Expand Down
2 changes: 1 addition & 1 deletion phpstan.neon
Expand Up @@ -23,6 +23,6 @@ parameters:
-
message: '#^.+no value type specified in iterable type.+\.$#'
path: *
count: 540
count: 538
tipsOfTheDay: false
tmpDir: dev-tools/phpstan/cache
645 changes: 24 additions & 621 deletions src/Fixer/Basic/BracesFixer.php

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/Fixer/Basic/NoMultipleStatementsPerLineFixer.php
Expand Up @@ -44,11 +44,11 @@ public function getDefinition(): FixerDefinitionInterface
/**
* {@inheritdoc}
*
* Must run after NoEmptyStatementFixer.
* Must run after ControlStructureBracesFixer, NoEmptyStatementFixer.
*/
public function getPriority(): int
{
return 39;
return -1;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Fixer/ControlStructure/ControlStructureBracesFixer.php
Expand Up @@ -46,7 +46,7 @@ public function isCandidate(Tokens $tokens): bool
/**
* {@inheritdoc}
*
* Must run before ControlStructureContinuationPositionFixer, CurlyBracesPositionFixer.
* Must run before ControlStructureContinuationPositionFixer, CurlyBracesPositionFixer, NoMultipleStatementsPerLineFixer.
*/
public function getPriority(): int
{
Expand Down
2 changes: 1 addition & 1 deletion src/Fixer/FunctionNotation/MethodArgumentSpaceFixer.php
Expand Up @@ -123,7 +123,7 @@ public function configure(array $configuration): void
* {@inheritdoc}
*
* Must run before ArrayIndentationFixer.
* Must run after CombineNestedDirnameFixer, FunctionDeclarationFixer, ImplodeCallFixer, LambdaNotUsedImportFixer, MethodChainingIndentationFixer, NoMultilineWhitespaceAroundDoubleArrowFixer, NoUselessSprintfFixer, PowToExponentiationFixer, StrictParamFixer.
* Must run after CombineNestedDirnameFixer, FunctionDeclarationFixer, ImplodeCallFixer, LambdaNotUsedImportFixer, NoMultilineWhitespaceAroundDoubleArrowFixer, NoUselessSprintfFixer, PowToExponentiationFixer, StrictParamFixer.
*/
public function getPriority(): int
{
Expand Down
2 changes: 1 addition & 1 deletion src/Fixer/Whitespace/ArrayIndentationFixer.php
Expand Up @@ -54,7 +54,7 @@ public function isCandidate(Tokens $tokens): bool
* {@inheritdoc}
*
* Must run before AlignMultilineCommentFixer, BinaryOperatorSpacesFixer.
* Must run after MethodArgumentSpaceFixer, MethodChainingIndentationFixer.
* Must run after MethodArgumentSpaceFixer.
*/
public function getPriority(): int
{
Expand Down
10 changes: 0 additions & 10 deletions src/Fixer/Whitespace/LineEndingFixer.php
Expand Up @@ -54,16 +54,6 @@ public function getDefinition(): FixerDefinitionInterface
);
}

/**
* {@inheritdoc}
*
* Must run before BracesFixer.
*/
public function getPriority(): int
{
return 40;
}

/**
* {@inheritdoc}
*/
Expand Down
38 changes: 27 additions & 11 deletions src/Fixer/Whitespace/MethodChainingIndentationFixer.php
Expand Up @@ -40,17 +40,6 @@ public function getDefinition(): FixerDefinitionInterface
);
}

/**
* {@inheritdoc}
*
* Must run before ArrayIndentationFixer, MethodArgumentSpaceFixer.
* Must run after BracesFixer.
*/
public function getPriority(): int
{
return 34;
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -90,6 +79,33 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens): void
if ($currentIndent !== $expectedIndent) {
$tokens[$index - 1] = new Token([T_WHITESPACE, $lineEnding.$expectedIndent]);
}

$endParenthesisIndex = $tokens->findBlockEnd(
Tokens::BLOCK_TYPE_PARENTHESIS_BRACE,
$tokens->getNextTokenOfKind($index, ['('])
);

for ($searchIndex = $index + 1; $searchIndex < $endParenthesisIndex; ++$searchIndex) {
$searchToken = $tokens[$searchIndex];

if (!$searchToken->isWhitespace()) {
continue;
}

$content = $searchToken->getContent();

if (!Preg::match('/\R/', $content)) {
continue;
}

$content = Preg::replace(
'/(\R)'.$currentIndent.'(\h*)$/D',
'$1'.$expectedIndent.'$2',
$content
);

$tokens[$searchIndex] = new Token([$searchToken->getId(), $content]);
}
}
}

Expand Down
8 changes: 1 addition & 7 deletions src/Fixer/Whitespace/NoExtraBlankLinesFixer.php
Expand Up @@ -441,13 +441,7 @@ private function removeEmptyLinesAfterLineWithTokenAt(int $index): void
continue;
}

$pos = strrpos($content, "\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a clean up or can we maybe add a utest for this change to NoExtraBlankLinesFixerTest as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is only a refactoring that shouldn't change behavior.


if ($pos + 2 <= \strlen($content)) { // preserve indenting where possible
$newContent = $ending.substr($content, $pos + 1);
} else {
$newContent = $ending;
}
$newContent = Preg::replace('/^.*\R(\h*)$/s', $ending.'$1', $content);

$this->tokens[$i] = new Token([T_WHITESPACE, $newContent]);
}
Expand Down
9 changes: 1 addition & 8 deletions tests/AutoReview/FixerFactoryTest.php
Expand Up @@ -358,7 +358,6 @@ private static function getFixersPriorityGraph(): array
],
'braces' => [
'heredoc_indentation',
'method_chaining_indentation',
],
'class_attributes_separation' => [
'braces',
Expand Down Expand Up @@ -392,6 +391,7 @@ private static function getFixersPriorityGraph(): array
'control_structure_braces' => [
'control_structure_continuation_position',
'curly_braces_position',
'no_multiple_statements_per_line',
],
'declare_strict_types' => [
'blank_line_after_opening_tag',
Expand Down Expand Up @@ -473,20 +473,13 @@ private static function getFixersPriorityGraph(): array
'method_argument_space',
'no_spaces_inside_parenthesis',
],
'line_ending' => [
'braces',
],
'list_syntax' => [
'binary_operator_spaces',
'ternary_operator_spaces',
],
'method_argument_space' => [
'array_indentation',
],
'method_chaining_indentation' => [
'array_indentation',
'method_argument_space',
],
'modernize_strpos' => [
'binary_operator_spaces',
'no_extra_blank_lines',
Expand Down
49 changes: 29 additions & 20 deletions tests/Fixer/Basic/BracesFixerTest.php
Expand Up @@ -817,7 +817,8 @@ public function bar() {
<?php
if (true) {
echo \'x\';
} ?>
}
?>
<hr />
<?php
}',
Expand Down Expand Up @@ -1146,10 +1147,6 @@ public function provideFixMissingBracesAndIndentCases(): iterable
'<?php
class ClassName
{




/**
* comment
*/
Expand Down Expand Up @@ -1867,10 +1864,6 @@ public function __construct(
[
'<?php
class ClassName {




/**
* comment
*/
Expand All @@ -1895,10 +1888,6 @@ class ClassName
[
'<?php
class ClassName {




/**
* comment
*/
Expand Down Expand Up @@ -2200,9 +2189,9 @@ function mixedComplex()
[
'<?php
if (true)
// foo
// bar
{
// foo
// bar
if (true)
{
print("foo");
Expand Down Expand Up @@ -4851,13 +4840,12 @@ public function providePreserveLineAfterControlBraceCases(): iterable
}',
],
[
"<?php if (true) {\r\n\r\n // CRLF newline\n}",
"<?php if (true) {\n // CRLF newline\n}",
"<?php if (true) {\r\n\r\n// CRLF newline\n}",
],
[
'<?php
if (true) {

// The blank line helps with legibility in nested control structures
if (true) {
// if body
Expand All @@ -4870,9 +4858,30 @@ public function providePreserveLineAfterControlBraceCases(): iterable
],
[
'<?php
if (true) {
// The blank line helps with legibility in nested control structures
if (true) {
// if body
}

// if body
}',
'<?php
if (true) {

// The blank line helps with legibility in nested control structures
if (true) {
// if body
}

// if body
}',
self::CONFIGURATION_OOP_POSITION_SAME_LINE,
],
[
'<?php
if (true)
{

// The blank line helps with legibility in nested control structures
if (true)
{
Expand All @@ -4894,13 +4903,13 @@ public function providePreserveLineAfterControlBraceCases(): iterable
self::CONFIGURATION_OOP_POSITION_SAME_LINE + self::CONFIGURATION_CTRL_STRUCT_POSITION_NEXT_LINE,
],
[
"<?php if (true) {\n // CRLF newline\n}",
"<?php if (true) {\r\n\r\n // CRLF newline\n}",
null,
self::CONFIGURATION_OOP_POSITION_SAME_LINE,
],
[
"<?php if (true)
{\r\n\r\n // CRLF newline\n}",
{\n // CRLF newline\n}",
"<?php if (true){\r\n\r\n// CRLF newline\n}",
self::CONFIGURATION_OOP_POSITION_SAME_LINE + self::CONFIGURATION_CTRL_STRUCT_POSITION_NEXT_LINE,
],
Expand Down
8 changes: 4 additions & 4 deletions tests/Fixer/Whitespace/MethodChainingIndentationFixerTest.php
Expand Up @@ -214,12 +214,12 @@ public function provideFixCases(): array

$user->setFoo(1)
->setBar([
1 => 1,
])
1 => 1,
])
->setBaz(true)
->setX(array(
2 => 2,
))
2 => 2,
))
->setY();
',
'<?php
Expand Down
Expand Up @@ -9,9 +9,29 @@ Foo::bar([
])
->call();

function foo($foo)
{
$foo
->bar()
->baz([
'foo' => 'bar',
])
;
}

--INPUT--
<?php
Foo::bar([
'baz',
])
->call();

function foo($foo)
{
$foo
->bar()
->baz([
'foo' => 'bar',
])
;
}
@@ -0,0 +1,11 @@
--TEST--
Integration of fixers: control_structure_braces,no_multiple_statements_per_line.
--RULESET--
{"control_structure_braces": true, "no_multiple_statements_per_line": true}
--EXPECT--
<?php
$callback = function () { if ($a) { return true; } return false; };

--INPUT--
<?php
$callback = function () { if ($a) return true; return false; };

This file was deleted.