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

Squiz.Functions.MultiLineFunctionDeclaration reports error if php attributes are declared #608

Closed
4 tasks done
vincent-paqt opened this issue Sep 8, 2024 · 3 comments · Fixed by #609
Closed
4 tasks done

Comments

@vincent-paqt
Copy link

vincent-paqt commented Sep 8, 2024

Describe the bug

When running phpcs with a class where multiple php attributes are declared Squiz.Functions.MultiLineFunctionDeclaration.OneParamPerLine reports an error.

Code sample

class MultiLineFunctionDeclarationFixesAttributes
{
    public function __construct(
        #[WithCast(DateTimeInterface::class), WithTransformer(DateTimeInterface::class, format: 'Y-m-d')]
        public readonly string $transferDate,
    ){
    }
}

Custom ruleset

<?xml version="1.0"?>
<ruleset name="My Custom Standard">
  <rule ref="Squiz.Functions.MultiLineFunctionDeclaration"/>
</ruleset>

To reproduce

Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above...
  2. Run phpcs test.php ...
  3. See error message displayed
----------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------------------
 5 | ERROR | [x] Multi-line function declarations must define one parameter per line (Squiz.Functions.MultiLineFunctionDeclaration.OneParamPerLine)
----------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------------

Expected behavior

Skip Squiz.Functions.MultiLineFunctionDeclaration when its between #[ and ]

Versions (please complete the following information)

Operating System Alpine Linux v3.19
PHP version 8.3.7
PHP_CodeSniffer version squizlabs/php_codesniffer 3.10.2
Standard PSR12, custom
Install type composer

Please confirm

  • I have searched the issue list and am not opening a duplicate issue.
  • I have read the Contribution Guidelines and this is not a support question.
  • I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
  • I have verified the issue still exists in the master branch of PHP_CodeSniffer.
@jrfnl
Copy link
Member

jrfnl commented Sep 8, 2024

Thanks @vincent-paqt for reporting this. I've been able to reproduce this. Of the three code sampes you provided, only the first one is relevant, the rest was unrelated code, so I've taken the liberty to remove this so as not to confuse people.

Based on some additional testing, this bug only happens with "multi-attributes", i.e. multiple comma separated attribute classes within a single set of attribute braces #[...].

  • "Single attributes" do not trigger the error.
  • Multiple "single attributes" do not trigger the error.

When the fixer is run, it can also clearly be seen where the sniff gets confused:

class MultiLineFunctionDeclarationFixesAttributes
{
    public function __construct(
        #[WithCast(DateTimeInterface::class),
 WithTransformer(DateTimeInterface::class, format: 'Y-m-d')]
        public readonly string $transferDate,
    ) {
    }
}

Other observations: while the Squiz sniff extends a PEAR sniff, this bug only occurs in the Squiz sniff. The PEAR sniff is not affected.

@jrfnl
Copy link
Member

jrfnl commented Sep 8, 2024

@vincent-paqt PR #609 should fix this. Testing appreciated.

@jrfnl jrfnl added this to the 3.10.x Next milestone Sep 8, 2024
@vincent-paqt
Copy link
Author

@jrfnl Thanks for the quick response and fix! I've tested #609 and it fixes the bug. I don't get the errors anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants