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.PHP.DisallowMultipleAssignments false positive when using namespaces with static assignments #1949

Closed
dbarr opened this issue Mar 21, 2018 · 4 comments
Milestone

Comments

@dbarr
Copy link

dbarr commented Mar 21, 2018

In the following snippet:

<?php

class A {
    public static $a;
}

A::$a = 'b'; // works
\A::$a = 'c'; // fails
\A\B\C::$d = 'e'; // fails

The last two assignments triggers "Assignments must be the first block of code on a line (Squiz.PHP.DisallowMultipleAssignments.Found)" due to T_NS_SEPARATOR not being considered.

@gsherwood gsherwood added this to the 3.3.0 milestone Mar 21, 2018
@gsherwood gsherwood changed the title Static class variable assignment fails with namespace separator Squiz.PHP.DisallowMultipleAssignments false positive when using namespaces with static assignments Mar 21, 2018
gsherwood added a commit that referenced this issue Mar 21, 2018
…e when using namespaces with static assignments
@gsherwood
Copy link
Member

This was a tough one as it was quite an old sniff and required a rewrite to get this working. But all tests are passing and I think it is working now. Thanks for the bug report. The fix will be in the 3.3.0 version, but if you are able to test on your code base before release, it would be really helpful given the large change to the sniff.

@dbarr
Copy link
Author

dbarr commented Mar 22, 2018

Not related to namespaces, but I noticed the error suppression operator (yep, cringe) causes the same sniff:

@$a = 1;

@dbarr
Copy link
Author

dbarr commented Mar 22, 2018

Also, foreach without curly braces or semi-column:

$a = [];
foreach ($a as $b)
    $c = 'd';

gsherwood added a commit that referenced this issue Mar 22, 2018
@gsherwood
Copy link
Member

Thanks a lot for reporting those as well. I've fixed them up.

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

No branches or pull requests

2 participants