-
Notifications
You must be signed in to change notification settings - Fork 0
Added rule for splitting multiple methods arguments into multiline if there is more than one #13
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
Changes from all commits
da678f7
5c33b15
99d3877
6f44627
a55b44c
f143ece
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,3 +4,4 @@ parameters: | |
checkMissingCallableSignature: true | ||
paths: | ||
- src | ||
- tests |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,182 @@ | ||
<?php | ||
|
||
/** | ||
* @copyright Copyright (C) Ibexa AS. All rights reserved. | ||
* @license For full copyright and license information view LICENSE file distributed with this source code. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Ibexa\CodeStyle\PhpCsFixer\Rule; | ||
|
||
use PhpCsFixer\AbstractFixer; | ||
use PhpCsFixer\FixerDefinition\CodeSample; | ||
use PhpCsFixer\FixerDefinition\FixerDefinition; | ||
use PhpCsFixer\FixerDefinition\FixerDefinitionInterface; | ||
use PhpCsFixer\Tokenizer\Token; | ||
use PhpCsFixer\Tokenizer\Tokens; | ||
|
||
final class MultilineParametersFixer extends AbstractFixer | ||
{ | ||
public function getDefinition(): FixerDefinitionInterface | ||
{ | ||
return new FixerDefinition( | ||
'Methods and functions with 2+ parameters must use multiline format', | ||
[ | ||
new CodeSample( | ||
'<?php function foo(string $a, int $b): void {}' | ||
), | ||
] | ||
); | ||
} | ||
|
||
public function isCandidate(Tokens $tokens): bool | ||
{ | ||
return $tokens->isTokenKindFound(T_FUNCTION); | ||
} | ||
|
||
protected function applyFix( | ||
\SplFileInfo $file, | ||
Tokens $tokens | ||
): void { | ||
for ($index = $tokens->count() - 1; $index >= 0; --$index) { | ||
if (!$tokens[$index]->isGivenKind(T_FUNCTION)) { | ||
continue; | ||
} | ||
|
||
$openParentIndex = $tokens->getNextTokenOfKind($index, ['(']); | ||
if ($openParentIndex === null) { | ||
continue; | ||
} | ||
|
||
$closeParentIndex = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_PARENTHESIS_BRACE, $openParentIndex); | ||
|
||
// Count commas to determine parameter count | ||
$paramCount = $this->countParameters($tokens, $openParentIndex, $closeParentIndex); | ||
|
||
// Only process if 2+ parameters | ||
if ($paramCount < 2) { | ||
continue; | ||
} | ||
|
||
// Check if already multiline | ||
if ($this->isMultiline($tokens, $openParentIndex, $closeParentIndex)) { | ||
continue; | ||
} | ||
|
||
// Apply multiline formatting | ||
$this->makeMultiline($tokens, $openParentIndex, $closeParentIndex); | ||
} | ||
} | ||
|
||
private function countParameters( | ||
Tokens $tokens, | ||
int $start, | ||
int $end | ||
): int { | ||
$count = 0; | ||
$depth = 0; | ||
|
||
for ($i = $start + 1; $i < $end; ++$i) { | ||
if ($tokens[$i]->equals('(') || $tokens[$i]->equals('[')) { | ||
++$depth; | ||
} elseif ($tokens[$i]->equals(')') || $tokens[$i]->equals(']')) { | ||
--$depth; | ||
} elseif ($depth === 0 && $tokens[$i]->equals(',')) { | ||
++$count; | ||
} | ||
} | ||
|
||
// If we found any commas, param count is commas + 1 | ||
// If no commas but there's content, it's 1 param | ||
if ($count > 0) { | ||
return $count + 1; | ||
} | ||
|
||
// Check if there's any non-whitespace content | ||
for ($i = $start + 1; $i < $end; ++$i) { | ||
if (!$tokens[$i]->isWhitespace()) { | ||
return 1; | ||
} | ||
} | ||
|
||
return 0; | ||
} | ||
|
||
private function isMultiline( | ||
Tokens $tokens, | ||
int $start, | ||
int $end | ||
): bool { | ||
for ($i = $start; $i <= $end; ++$i) { | ||
if ($tokens[$i]->isGivenKind(T_WHITESPACE) && str_contains($tokens[$i]->getContent(), "\n")) { | ||
return true; | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
|
||
private function makeMultiline( | ||
Tokens $tokens, | ||
int $openParentIndex, | ||
int $closeParentIndex | ||
): void { | ||
$indent = $this->detectIndent($tokens, $openParentIndex); | ||
$lineIndent = str_repeat(' ', 4); // 4 spaces for parameters | ||
|
||
// Add newline after opening parenthesis | ||
$tokens->insertAt($openParentIndex + 1, new Token([T_WHITESPACE, "\n" . $indent . $lineIndent])); | ||
++$closeParentIndex; | ||
|
||
// Find all commas and add newlines after them | ||
$depth = 0; | ||
for ($i = $openParentIndex + 1; $i < $closeParentIndex; ++$i) { | ||
if ($tokens[$i]->equals('(') || $tokens[$i]->equals('[')) { | ||
++$depth; | ||
} elseif ($tokens[$i]->equals(')') || $tokens[$i]->equals(']')) { | ||
--$depth; | ||
} elseif ($depth === 0 && $tokens[$i]->equals(',')) { | ||
// Remove any whitespace after comma | ||
$nextIndex = $i + 1; | ||
while ($nextIndex < $closeParentIndex && $tokens[$nextIndex]->isWhitespace()) { | ||
$tokens->clearAt($nextIndex); | ||
++$nextIndex; | ||
} | ||
|
||
// Insert newline with proper indentation | ||
$tokens->insertAt($i + 1, new Token([T_WHITESPACE, "\n" . $indent . $lineIndent])); | ||
$closeParentIndex = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_PARENTHESIS_BRACE, $openParentIndex); | ||
} | ||
} | ||
|
||
// Add newline before closing parenthesis | ||
$tokens->insertAt($closeParentIndex, new Token([T_WHITESPACE, "\n" . $indent])); | ||
|
||
// Handle the opening brace | ||
$nextNonWhitespace = $tokens->getNextNonWhitespace($closeParentIndex); | ||
if ($nextNonWhitespace !== null && $tokens[$nextNonWhitespace]->equals('{')) { | ||
$tokens->ensureWhitespaceAtIndex($nextNonWhitespace - 1, 1, ' '); | ||
} | ||
} | ||
|
||
private function detectIndent( | ||
Tokens $tokens, | ||
int $index | ||
): string { | ||
// Look backwards to find the indentation of the current line | ||
for ($i = $index - 1; $i >= 0; --$i) { | ||
if ($tokens[$i]->isGivenKind(T_WHITESPACE) && str_contains($tokens[$i]->getContent(), "\n")) { | ||
$lines = explode("\n", $tokens[$i]->getContent()); | ||
|
||
return end($lines); | ||
} | ||
} | ||
|
||
return ''; | ||
} | ||
|
||
public function getName(): string | ||
{ | ||
return 'Ibexa/multiline_parameters'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,116 @@ | ||
<?php | ||
|
||
/** | ||
* @copyright Copyright (C) Ibexa AS. All rights reserved. | ||
* @license For full copyright and license information view LICENSE file distributed with this source code. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Ibexa\Tests\CodeStyle\PhpCsFixer\Rule; | ||
|
||
use Ibexa\CodeStyle\PhpCsFixer\Rule\MultilineParametersFixer; | ||
use PhpCsFixer\Tokenizer\Tokens; | ||
use PHPUnit\Framework\TestCase; | ||
use SplFileInfo; | ||
|
||
final class MultilineParametersFixerTest extends TestCase | ||
{ | ||
private MultilineParametersFixer $fixer; | ||
|
||
protected function setUp(): void | ||
{ | ||
$this->fixer = new MultilineParametersFixer(); | ||
} | ||
|
||
/** | ||
* @dataProvider provideFixCases | ||
*/ | ||
public function testFix( | ||
string $input, | ||
string $expected | ||
): void { | ||
$tokens = Tokens::fromCode($input); | ||
$this->fixer->fix(new SplFileInfo(__FILE__), $tokens); | ||
|
||
self::assertSame($expected, $tokens->generateCode()); | ||
} | ||
|
||
/** | ||
* @return iterable<string, array{0: string, 1: string}> | ||
*/ | ||
public static function provideFixCases(): iterable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing iterable type. Looks like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, that was the case, added to phpstan configuration and fixed all issues. |
||
{ | ||
yield 'single parameter should not be modified' => [ | ||
'<?php | ||
function bar(array $package): void { | ||
}', | ||
'<?php | ||
function bar(array $package): void { | ||
}', | ||
]; | ||
|
||
yield 'single parameter with type hints should not be modified' => [ | ||
'<?php | ||
function bar(?string $package = null): void { | ||
}', | ||
'<?php | ||
function bar(?string $package = null): void { | ||
}', | ||
]; | ||
|
||
yield 'multiple parameters should be split' => [ | ||
'<?php | ||
function foo(array $package, string $expectedRuleSetClass): void { | ||
}', | ||
'<?php | ||
function foo( | ||
array $package, | ||
string $expectedRuleSetClass | ||
): void { | ||
}', | ||
]; | ||
|
||
yield 'multiple parameters with type hints should be split' => [ | ||
'<?php | ||
function test(?string $foo = null, int $bar = 42): string { | ||
}', | ||
'<?php | ||
function test( | ||
?string $foo = null, | ||
int $bar = 42 | ||
): string { | ||
}', | ||
]; | ||
|
||
yield 'constructor with properties should be split' => [ | ||
'<?php | ||
class Test { | ||
public function __construct(string $foo, int $bar) { | ||
} | ||
}', | ||
'<?php | ||
class Test { | ||
public function __construct( | ||
string $foo, | ||
int $bar | ||
) { | ||
} | ||
}', | ||
]; | ||
|
||
yield 'already multiline should not be modified' => [ | ||
'<?php | ||
function test( | ||
string $foo, | ||
int $bar | ||
): void { | ||
}', | ||
'<?php | ||
function test( | ||
string $foo, | ||
int $bar | ||
): void { | ||
}', | ||
]; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -204,4 +204,5 @@ | |
'types_spaces' => [ | ||
'space' => 'single', | ||
], | ||
'Ibexa/multiline_parameters' => true, | ||
]; |
Uh oh!
There was an error while loading. Please reload this page.