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

Generic.Formatting.MultipleStatementAlignment.NotSame doesn't parse short list properly after upgrading to 3.2.3 #1988

Closed
lcobucci opened this issue Apr 12, 2018 · 8 comments

Comments

@lcobucci
Copy link

After updating to v3.2.3 this sniff is complaining about incorrect spacing regarding this code:

<?php
declare(strict_types=1);

$payload = $message->getPayload();

[$userInput, $errors] = $this->normalizeUserInput($payload);

Output:

$ phpcs aa.php 
E 1 / 1 (100%)



FILE: aa.php
------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------
 4 | ERROR | [x] Equals sign not aligned with surrounding assignments; expected 14 spaces but found 1 space
------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------

And it doesn't complain anymore if we use list() instead of [], like:

<?php
declare(strict_types=1);

$payload = $message->getPayload();

($userInput, $errors) = $this->normalizeUserInput($payload);

Output:

$ phpcs aa.php 
. 1 / 1 (100%)


Time: 131ms; Memory: 14Mb
@lcobucci
Copy link
Author

Maybe these are related: #1911 #1984

@jrfnl
Copy link
Contributor

jrfnl commented Apr 12, 2018

@lcobucci Hi Luís, #1870, #1848, #1924 are all related to this, but I'm surprised you are seeing the message as your example code looks to use blank lines between the lines and that would "break" a multi-statement block.
Could it be that the code you are testing on does not have the blank lines ?

So, this should trigger the notice:

<?php
declare(strict_types=1);

$payload = $message->getPayload();
[$userInput, $errors] = $this->normalizeUserInput($payload);

This should not:

<?php
declare(strict_types=1);

$payload = $message->getPayload();

[$userInput, $errors] = $this->normalizeUserInput($payload);

@jrfnl
Copy link
Contributor

jrfnl commented Apr 12, 2018

Just ran some tests which confirm what I said above.
Also: whether you use the short list syntax or the list() construct - correctly - has no influence on the results according to my test.

@lcobucci
Copy link
Author

@jrfnl the code is exactly the way I've posted (with the line in between), which indeed surprised me. It can sure be an influence of another sniff but I'm afraid it wouldn't have that output while only analysing the code.

@jrfnl
Copy link
Contributor

jrfnl commented Apr 12, 2018

@lcobucci Hmm... the blank line in between should ensure that this sniff isn't triggered.
Hang on... ok.. so I've tested this now on 3.2.3 and can confirm your findings. Looks like this has already been fixed in master via #1924 which fixed #1922.

@lcobucci
Copy link
Author

@jrfnl let me try it with master

@lcobucci
Copy link
Author

@jrfnl it's indeed fixed in master, we can close it as a duplicate then. Thanks for your help 👍

@jrfnl
Copy link
Contributor

jrfnl commented Apr 12, 2018

You're welcome.

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

No branches or pull requests

2 participants