Skip to content

Commit

Permalink
Squiz/FunctionDeclarationArgumentSpacing: special case "spacing after…
Browse files Browse the repository at this point in the history
… comma" vs constructor property promotion

While incorrect spacing after a comma for constructor property promotion parameters would already be flagged and fixed by the sniff, the error message and code were incorrect/unclear.

Given the following test code:
```php
class PropertyPromotionSpacingAfterComma {
    public function __construct(private string|int $propA, protected bool $correctSpace, public MyClass $tooMuchSpace, readonly string $noSpace) {}
}
```

Previously the following would be reported:
```
 198 | ERROR | [x] Expected 1 space between comma and type hint "MyClass"; 2 found
     |       |     (Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingBeforeHint)
 198 | ERROR | [x] Expected 1 space between comma and type hint "string"; 0 found
     |       |     (Squiz.Functions.FunctionDeclarationArgumentSpacing.NoSpaceBeforeHint)
```

Take note of the "type hint" in the message and the "Hint" suffix for the error codes.

With the fix from this commit in place, this will now be reported as follows:
```
 198 | ERROR | [x] Expected 1 space between comma and property modifier "public"; 2 found
     |       |     (Squiz.Functions.FunctionDeclarationArgumentSpacing.SpacingBeforePropertyModifier)
 198 | ERROR | [x] Expected 1 space between comma and property modifier "readonly"; 0 found
     |       |     (Squiz.Functions.FunctionDeclarationArgumentSpacing.NoSpaceBeforePropertyModifier)
```

Includes tests.
  • Loading branch information
jrfnl committed Jan 23, 2025
1 parent 23e551b commit 68c96b5
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,12 @@ public function processBracket($phpcsFile, $openBracket)
$typeOfNextShort = 'Arg';
$contentOfNext = $param['name'];

if ($param['type_hint_token'] !== false) {
if (isset($param['property_visibility']) === true) {
$typeOfNext = 'property modifier';
$typeOfNextShort = 'PropertyModifier';
$modifier = $phpcsFile->findNext(Tokens::$emptyTokens, ($commaToken + 1), $param['token'], true);
$contentOfNext = $tokens[$modifier]['content'];
} else if ($param['type_hint_token'] !== false) {
$typeOfNext = 'type hint';
$typeOfNextShort = 'Hint';
$contentOfNext = $param['type_hint'];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,3 +195,7 @@ function newlineBeforeCommaFixerRespectsComments(
, $paramC=30
, string $paramC='foo'
) {}

class PropertyPromotionSpacingAfterComma {
public function __construct(private string|int $propA, protected bool $correctSpace, public MyClass $tooMuchSpace,readonly string $noSpace) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,7 @@ function newlineBeforeCommaFixerRespectsComments(
$paramC=30,
string $paramC='foo'
) {}

class PropertyPromotionSpacingAfterComma {
public function __construct(private string|int $propA, protected bool $correctSpace, public MyClass $tooMuchSpace, readonly string $noSpace) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ public function getErrorList($testFile='')
193 => 1,
195 => 1,
196 => 1,
200 => 2,
];

default:
Expand Down

0 comments on commit 68c96b5

Please sign in to comment.