Skip to content

Commit 329dc21

Browse files
ViniTouViniTou
authored
Added rule for splitting multiple methods arguments into multiline if there is more than one (#13)
* Added rule for splitting multiple methods arguments into multiline if there is more than one * Fixed phpstan issues * make test php7.4 compliant * Added tests folder to phpstan * minor cr fixes * fixed closeParentIndex variable name
1 parent cfb9396 commit 329dc21

File tree

9 files changed

+316
-3
lines changed

9 files changed

+316
-3
lines changed

‎phpstan.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,4 @@ parameters:
44
checkMissingCallableSignature: true
55
paths:
66
- src
7+
- tests

‎src/lib/PhpCsFixer/Rule/MultilineParametersFixer.php‎

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
<?php
2+
3+
/**
4+
* @copyright Copyright (C) Ibexa AS. All rights reserved.
5+
* @license For full copyright and license information view LICENSE file distributed with this source code.
6+
*/
7+
declare(strict_types=1);
8+
9+
namespace Ibexa\CodeStyle\PhpCsFixer\Rule;
10+
11+
use PhpCsFixer\AbstractFixer;
12+
use PhpCsFixer\FixerDefinition\CodeSample;
13+
use PhpCsFixer\FixerDefinition\FixerDefinition;
14+
use PhpCsFixer\FixerDefinition\FixerDefinitionInterface;
15+
use PhpCsFixer\Tokenizer\Token;
16+
use PhpCsFixer\Tokenizer\Tokens;
17+
18+
final class MultilineParametersFixer extends AbstractFixer
19+
{
20+
public function getDefinition(): FixerDefinitionInterface
21+
{
22+
return new FixerDefinition(
23+
'Methods and functions with 2+ parameters must use multiline format',
24+
[
25+
new CodeSample(
26+
'<?php function foo(string $a, int $b): void {}'
27+
),
28+
]
29+
);
30+
}
31+
32+
public function isCandidate(Tokens $tokens): bool
33+
{
34+
return $tokens->isTokenKindFound(T_FUNCTION);
35+
}
36+
37+
protected function applyFix(
38+
\SplFileInfo $file,
39+
Tokens $tokens
40+
): void {
41+
for ($index = $tokens->count() - 1; $index >= 0; --$index) {
42+
if (!$tokens[$index]->isGivenKind(T_FUNCTION)) {
43+
continue;
44+
}
45+
46+
$openParentIndex = $tokens->getNextTokenOfKind($index, ['(']);
47+
if ($openParentIndex === null) {
48+
continue;
49+
}
50+
51+
$closeParentIndex = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_PARENTHESIS_BRACE, $openParentIndex);
52+
53+
// Count commas to determine parameter count
54+
$paramCount = $this->countParameters($tokens, $openParentIndex, $closeParentIndex);
55+
56+
// Only process if 2+ parameters
57+
if ($paramCount < 2) {
58+
continue;
59+
}
60+
61+
// Check if already multiline
62+
if ($this->isMultiline($tokens, $openParentIndex, $closeParentIndex)) {
63+
continue;
64+
}
65+
66+
// Apply multiline formatting
67+
$this->makeMultiline($tokens, $openParentIndex, $closeParentIndex);
68+
}
69+
}
70+
71+
private function countParameters(
72+
Tokens $tokens,
73+
int $start,
74+
int $end
75+
): int {
76+
$count = 0;
77+
$depth = 0;
78+
79+
for ($i = $start + 1; $i < $end; ++$i) {
80+
if ($tokens[$i]->equals('(') || $tokens[$i]->equals('[')) {
81+
++$depth;
82+
} elseif ($tokens[$i]->equals(')') || $tokens[$i]->equals(']')) {
83+
--$depth;
84+
} elseif ($depth === 0 && $tokens[$i]->equals(',')) {
85+
++$count;
86+
}
87+
}
88+
89+
// If we found any commas, param count is commas + 1
90+
// If no commas but there's content, it's 1 param
91+
if ($count > 0) {
92+
return $count + 1;
93+
}
94+
95+
// Check if there's any non-whitespace content
96+
for ($i = $start + 1; $i < $end; ++$i) {
97+
if (!$tokens[$i]->isWhitespace()) {
98+
return 1;
99+
}
100+
}
101+
102+
return 0;
103+
}
104+
105+
private function isMultiline(
106+
Tokens $tokens,
107+
int $start,
108+
int $end
109+
): bool {
110+
for ($i = $start; $i <= $end; ++$i) {
111+
if ($tokens[$i]->isGivenKind(T_WHITESPACE) && str_contains($tokens[$i]->getContent(), "\n")) {
112+
return true;
113+
}
114+
}
115+
116+
return false;
117+
}
118+
119+
private function makeMultiline(
120+
Tokens $tokens,
121+
int $openParentIndex,
122+
int $closeParentIndex
123+
): void {
124+
$indent = $this->detectIndent($tokens, $openParentIndex);
125+
$lineIndent = str_repeat(' ', 4); // 4 spaces for parameters
126+
127+
// Add newline after opening parenthesis
128+
$tokens->insertAt($openParentIndex + 1, new Token([T_WHITESPACE, "\n" . $indent . $lineIndent]));
129+
++$closeParentIndex;
130+
131+
// Find all commas and add newlines after them
132+
$depth = 0;
133+
for ($i = $openParentIndex + 1; $i < $closeParentIndex; ++$i) {
134+
if ($tokens[$i]->equals('(') || $tokens[$i]->equals('[')) {
135+
++$depth;
136+
} elseif ($tokens[$i]->equals(')') || $tokens[$i]->equals(']')) {
137+
--$depth;
138+
} elseif ($depth === 0 && $tokens[$i]->equals(',')) {
139+
// Remove any whitespace after comma
140+
$nextIndex = $i + 1;
141+
while ($nextIndex < $closeParentIndex && $tokens[$nextIndex]->isWhitespace()) {
142+
$tokens->clearAt($nextIndex);
143+
++$nextIndex;
144+
}
145+
146+
// Insert newline with proper indentation
147+
$tokens->insertAt($i + 1, new Token([T_WHITESPACE, "\n" . $indent . $lineIndent]));
148+
$closeParentIndex = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_PARENTHESIS_BRACE, $openParentIndex);
149+
}
150+
}
151+
152+
// Add newline before closing parenthesis
153+
$tokens->insertAt($closeParentIndex, new Token([T_WHITESPACE, "\n" . $indent]));
154+
155+
// Handle the opening brace
156+
$nextNonWhitespace = $tokens->getNextNonWhitespace($closeParentIndex);
157+
if ($nextNonWhitespace !== null && $tokens[$nextNonWhitespace]->equals('{')) {
158+
$tokens->ensureWhitespaceAtIndex($nextNonWhitespace - 1, 1, ' ');
159+
}
160+
}
161+
162+
private function detectIndent(
163+
Tokens $tokens,
164+
int $index
165+
): string {
166+
// Look backwards to find the indentation of the current line
167+
for ($i = $index - 1; $i >= 0; --$i) {
168+
if ($tokens[$i]->isGivenKind(T_WHITESPACE) && str_contains($tokens[$i]->getContent(), "\n")) {
169+
$lines = explode("\n", $tokens[$i]->getContent());
170+
171+
return end($lines);
172+
}
173+
}
174+
175+
return '';
176+
}
177+
178+
public function getName(): string
179+
{
180+
return 'Ibexa/multiline_parameters';
181+
}
182+
}

‎src/lib/PhpCsFixer/Sets/AbstractIbexaRuleSet.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace Ibexa\CodeStyle\PhpCsFixer\Sets;
1010

1111
use AdamWojs\PhpCsFixerPhpdocForceFQCN\Fixer\Phpdoc\ForceFQCNFixer;
12+
use Ibexa\CodeStyle\PhpCsFixer\Rule\MultilineParametersFixer;
1213
use PhpCsFixer\Config;
1314

1415
abstract class AbstractIbexaRuleSet implements RuleSetInterface
@@ -26,6 +27,7 @@ public function getRules(): array
2627
return [
2728
'@PSR12' => false,
2829
'AdamWojs/phpdoc_force_fqcn_fixer' => true,
30+
'Ibexa/multiline_parameters' => true,
2931
'array_syntax' => [
3032
'syntax' => 'short',
3133
],
@@ -204,7 +206,7 @@ public function getRules(): array
204206
public function buildConfig(): Config
205207
{
206208
$config = new Config();
207-
$config->registerCustomFixers([new ForceFQCNFixer()]);
209+
$config->registerCustomFixers([new ForceFQCNFixer(), new MultilineParametersFixer()]);
208210

209211
$config->setRules(array_merge(
210212
$config->getRules(),

‎tests/lib/PhpCsFixer/InternalConfigFactoryTest.php‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,22 @@ protected function setUp(): void
3737

3838
/**
3939
* @dataProvider provideRuleSetTestCases
40+
*
41+
* @param array{name: string, version: string, pretty_version?: string} $package
42+
* @param class-string $expectedRuleSetClass
4043
*/
41-
public function testVersionBasedRuleSetSelection(array $package, string $expectedRuleSetClass): void
42-
{
44+
public function testVersionBasedRuleSetSelection(
45+
array $package,
46+
string $expectedRuleSetClass
47+
): void {
4348
$ruleSet = $this->createRuleSetFromPackage->invoke($this->factory, $package);
4449

4550
self::assertInstanceOf($expectedRuleSetClass, $ruleSet);
4651
}
4752

53+
/**
54+
* @return array<string, array{0: array{name: string, version: string, pretty_version?: string}, 1: class-string}>
55+
*/
4856
public function provideRuleSetTestCases(): array
4957
{
5058
return [

‎tests/lib/PhpCsFixer/Rule/MultilineParametersFixerTest.php‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
<?php
2+
3+
/**
4+
* @copyright Copyright (C) Ibexa AS. All rights reserved.
5+
* @license For full copyright and license information view LICENSE file distributed with this source code.
6+
*/
7+
declare(strict_types=1);
8+
9+
namespace Ibexa\Tests\CodeStyle\PhpCsFixer\Rule;
10+
11+
use Ibexa\CodeStyle\PhpCsFixer\Rule\MultilineParametersFixer;
12+
use PhpCsFixer\Tokenizer\Tokens;
13+
use PHPUnit\Framework\TestCase;
14+
use SplFileInfo;
15+
16+
final class MultilineParametersFixerTest extends TestCase
17+
{
18+
private MultilineParametersFixer $fixer;
19+
20+
protected function setUp(): void
21+
{
22+
$this->fixer = new MultilineParametersFixer();
23+
}
24+
25+
/**
26+
* @dataProvider provideFixCases
27+
*/
28+
public function testFix(
29+
string $input,
30+
string $expected
31+
): void {
32+
$tokens = Tokens::fromCode($input);
33+
$this->fixer->fix(new SplFileInfo(__FILE__), $tokens);
34+
35+
self::assertSame($expected, $tokens->generateCode());
36+
}
37+
38+
/**
39+
* @return iterable<string, array{0: string, 1: string}>
40+
*/
41+
public static function provideFixCases(): iterable
42+
{
43+
yield 'single parameter should not be modified' => [
44+
'<?php
45+
function bar(array $package): void {
46+
}',
47+
'<?php
48+
function bar(array $package): void {
49+
}',
50+
];
51+
52+
yield 'single parameter with type hints should not be modified' => [
53+
'<?php
54+
function bar(?string $package = null): void {
55+
}',
56+
'<?php
57+
function bar(?string $package = null): void {
58+
}',
59+
];
60+
61+
yield 'multiple parameters should be split' => [
62+
'<?php
63+
function foo(array $package, string $expectedRuleSetClass): void {
64+
}',
65+
'<?php
66+
function foo(
67+
array $package,
68+
string $expectedRuleSetClass
69+
): void {
70+
}',
71+
];
72+
73+
yield 'multiple parameters with type hints should be split' => [
74+
'<?php
75+
function test(?string $foo = null, int $bar = 42): string {
76+
}',
77+
'<?php
78+
function test(
79+
?string $foo = null,
80+
int $bar = 42
81+
): string {
82+
}',
83+
];
84+
85+
yield 'constructor with properties should be split' => [
86+
'<?php
87+
class Test {
88+
public function __construct(string $foo, int $bar) {
89+
}
90+
}',
91+
'<?php
92+
class Test {
93+
public function __construct(
94+
string $foo,
95+
int $bar
96+
) {
97+
}
98+
}',
99+
];
100+
101+
yield 'already multiline should not be modified' => [
102+
'<?php
103+
function test(
104+
string $foo,
105+
int $bar
106+
): void {
107+
}',
108+
'<?php
109+
function test(
110+
string $foo,
111+
int $bar
112+
): void {
113+
}',
114+
];
115+
}
116+
}

‎tests/lib/PhpCsFixer/Sets/expected_rules/4_6_rule_set/local_rules.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,4 +195,5 @@
195195
'visibility_required' => true,
196196
'whitespace_after_comma_in_array' => true,
197197
'yoda_style' => false,
198+
'Ibexa/multiline_parameters' => true,
198199
];

‎tests/lib/PhpCsFixer/Sets/expected_rules/4_6_rule_set/php_cs_fixer_rules.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,4 +179,5 @@
179179
'no_trailing_comma_in_singleline_array' => true,
180180
'no_unneeded_curly_braces' => true,
181181
'single_blank_line_before_namespace' => true,
182+
'Ibexa/multiline_parameters' => true,
182183
];

‎tests/lib/PhpCsFixer/Sets/expected_rules/5_0_rule_set/local_rules.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,4 +218,5 @@
218218
'visibility_required' => true,
219219
'whitespace_after_comma_in_array' => true,
220220
'yoda_style' => false,
221+
'Ibexa/multiline_parameters' => true,
221222
];

‎tests/lib/PhpCsFixer/Sets/expected_rules/5_0_rule_set/php_cs_fixer_rules.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,4 +204,5 @@
204204
'types_spaces' => [
205205
'space' => 'single',
206206
],
207+
'Ibexa/multiline_parameters' => true,
207208
];

0 commit comments

Comments
 (0)