Skip to content

Commit

Permalink
Priority: AlignMultilineComment should run before every Phpdoc Fixer
Browse files Browse the repository at this point in the history
I made use of the graph I generated with
https://github.com/dmvdbrugge/fixer-prio-graph

I also found a hidden priority between LineEnding and Braces, which I had to
add in this PR as well, because otherwise the @Symfony_whitespace.test would
fail.
  • Loading branch information
dmvdbrugge committed Nov 13, 2018
1 parent 5d82d40 commit 69003f1
Show file tree
Hide file tree
Showing 13 changed files with 103 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/AbstractNoUselessElseFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ abstract class AbstractNoUselessElseFixer extends AbstractFixer
public function getPriority()
{
// should be run before NoWhitespaceInBlankLineFixer, NoExtraBlankLinesFixer, BracesFixer and after NoEmptyStatementFixer.
return 25;
return 34;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Fixer/Basic/BracesFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public function bar($baz)
public function getPriority()
{
// should be run after the ElseIfFixer, NoEmptyStatementFixer and NoUselessElseFixer
return -25;
return 33;
}

/**
Expand Down
8 changes: 8 additions & 0 deletions src/Fixer/ControlStructure/ElseifFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ public function getDefinition()
);
}

/**
* {@inheritdoc}
*/
public function getPriority()
{
return 34;
}

/**
* {@inheritdoc}
*/
Expand Down
2 changes: 1 addition & 1 deletion src/Fixer/ControlStructure/NoAlternativeSyntaxFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public function isCandidate(Tokens $tokens)
*/
public function getPriority()
{
return 1;
return 35;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Fixer/ControlStructure/NoUnneededCurlyBracesFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public function getDefinition()
public function getPriority()
{
// must be run before NoUselessElseFixer and NoUselessReturnFixer.
return 26;
return 35;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/Fixer/Phpdoc/AlignMultilineCommentFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ public function getDefinition()
*/
public function getPriority()
{
// Should run after ArrayIndentationFixer
return -40;
// Should run after ArrayIndentationFixer but before every other Phpdoc Fixer
return 30;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Fixer/Semicolon/NoEmptyStatementFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function getPriority()
{
// should be run before the BracesFixer, CombineConsecutiveUnsetsFixer, NoExtraBlankLinesFixer, MultilineWhitespaceBeforeSemicolonsFixer, NoSinglelineWhitespaceBeforeSemicolonsFixer,
// NoTrailingCommaInListCallFixer, NoUselessReturnFixer, NoWhitespaceInBlankLineFixer, SpaceAfterSemicolonFixer, SwitchCaseSemicolonToColonFixer.
return 26;
return 35;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Fixer/Whitespace/ArrayIndentationFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function isCandidate(Tokens $tokens)
public function getPriority()
{
// should run after BracesFixer
return -30;
return 32;
}

protected function applyFix(\SplFileInfo $file, Tokens $tokens)
Expand Down
6 changes: 6 additions & 0 deletions src/Fixer/Whitespace/LineEndingFixer.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ public function getDefinition()
);
}

public function getPriority()
{
// Should run before braces
return 34;
}

/**
* {@inheritdoc}
*/
Expand Down
28 changes: 17 additions & 11 deletions tests/AutoReview/FixerFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public function provideFixersPriorityCases()
[$fixers['general_phpdoc_annotation_remove'], $fixers['no_empty_phpdoc']],
[$fixers['indentation_type'], $fixers['phpdoc_indent']],
[$fixers['is_null'], $fixers['yoda_style']],
[$fixers['line_ending'], $fixers['braces']],
[$fixers['list_syntax'], $fixers['binary_operator_spaces']],
[$fixers['list_syntax'], $fixers['ternary_operator_spaces']],
[$fixers['method_separation'], $fixers['braces']],
Expand Down Expand Up @@ -229,15 +230,18 @@ public function provideFixersPrioritySpecialPhpdocCases()
$cases = [];

// prepare bulk tests for phpdoc fixers to test that:
// * `comment_to_phpdoc` is first
// * `phpdoc_to_comment` is second
// * `phpdoc_indent` is third
// * `phpdoc_types` is fourth
// * `phpdoc_scalar` is fifth
// * `align_multiline_comment` is first
// * `comment_to_phpdoc` is second
// * `phpdoc_to_comment` is third
// * `phpdoc_indent` is fourth
// * `phpdoc_types` is fifth
// * `phpdoc_scalar` is sixth
// * `phpdoc_align` is last
// Add these cases in test-order instead of alphabetical
$cases[] = [$fixers['align_multiline_comment'], $fixers['comment_to_phpdoc']];
$cases[] = [$fixers['comment_to_phpdoc'], $fixers['phpdoc_to_comment']];
$cases[] = [$fixers['phpdoc_indent'], $fixers['phpdoc_types']];
$cases[] = [$fixers['phpdoc_to_comment'], $fixers['phpdoc_indent']];
$cases[] = [$fixers['phpdoc_indent'], $fixers['phpdoc_types']];
$cases[] = [$fixers['phpdoc_types'], $fixers['phpdoc_scalar']];

$docFixerNames = array_filter(
Expand All @@ -249,6 +253,7 @@ static function ($name) {

foreach ($docFixerNames as $docFixerName) {
if (!\in_array($docFixerName, ['comment_to_phpdoc', 'phpdoc_to_comment', 'phpdoc_indent', 'phpdoc_types', 'phpdoc_scalar'], true)) {
$cases[] = [$fixers['align_multiline_comment'], $fixers[$docFixerName]];
$cases[] = [$fixers['comment_to_phpdoc'], $fixers[$docFixerName]];
$cases[] = [$fixers['phpdoc_indent'], $fixers[$docFixerName]];
$cases[] = [$fixers['phpdoc_scalar'], $fixers[$docFixerName]];
Expand Down Expand Up @@ -289,13 +294,14 @@ public function testFixersPriorityPairsHaveIntegrationTest(FixerInterface $first
];

$integrationTestExists = $this->doesIntegrationTestExist($first, $second);
$integrationTestName = $this->generateIntegrationTestName($first, $second);

if (\in_array($this->generateIntegrationTestName($first, $second), $casesWithoutTests, true)) {
$this->assertFalse($integrationTestExists, sprintf('Case "%s" already has an integration test, so it should be removed from "$casesWithoutTests".', $this->generateIntegrationTestName($first, $second)));
$this->markTestIncomplete(sprintf('Case "%s" has no integration test yet, please help and add it.', $this->generateIntegrationTestName($first, $second)));
if (\in_array($integrationTestName, $casesWithoutTests, true)) {
$this->assertFalse($integrationTestExists, sprintf('Case "%s" already has an integration test, so it should be removed from "$casesWithoutTests".', $integrationTestName));
$this->markTestIncomplete(sprintf('Case "%s" has no integration test yet, please help and add it.', $integrationTestName));
}

$this->assertTrue($integrationTestExists, sprintf('There shall be an integration test "%s". How do you know that priority set up is good, if there is no integration test to check it?', $this->generateIntegrationTestName($first, $second)));
$this->assertTrue($integrationTestExists, sprintf('There shall be an integration test "%s". How do you know that priority set up is good, if there is no integration test to check it?', $integrationTestName));
}

public function provideFixersPriorityPairsHaveIntegrationTestCases()
Expand Down Expand Up @@ -371,7 +377,7 @@ public function testPriorityIntegrationTestFilesAreListedPriorityCases($fileName

$this->assertSame(
1,
preg_match('#^([a-z][a-z0-9_]*),([a-z][a-z_]*)(?:_\d{1,3})?\.test$#', $fileName, $matches),
preg_match('#^([a-z][a-z0-9_]*),([a-z][a-z_]*)(?:_\d{1,3})?\.test(-(in|out)\.php)?$#', $fileName, $matches),
sprintf('File with unexpected name "%s" in the priority integration test directory.', $fileName)
);

Expand Down
6 changes: 6 additions & 0 deletions tests/Fixtures/Integration/priority/line_ending,braces.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
--TEST--
Integration of @Symfony [whitespace version].
--RULESET--
{"line_ending": true, "braces": true}
--CONFIG--
{"indent": "\t", "lineEnding": "\r\n"}
30 changes: 30 additions & 0 deletions tests/Fixtures/Integration/priority/line_ending,braces.test-in.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

/**
* Coding standards demonstration.
*/
class FooBar
{


const SOME_CONST = 42;

private $fooBar;

/**
* @param string $dummy Some argument description
*/
public function __construct($dummy)
{
$this->fooBar = $this->transformText($dummy);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

/**
* Coding standards demonstration.
*/
class FooBar
{
const SOME_CONST = 42;

private $fooBar;

/**
* @param string $dummy Some argument description
*/
public function __construct($dummy)
{
$this->fooBar = $this->transformText($dummy);
}
}

0 comments on commit 69003f1

Please sign in to comment.