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

PSR12.Classes.ClassInstantiation incorrectly fix when using member vars and some variable formats #2053

Closed
jrfnl opened this issue Jun 8, 2018 · 4 comments
Milestone

Comments

@jrfnl
Copy link
Contributor

jrfnl commented Jun 8, 2018

WPCS already contained a ClassInstantiation sniff, so after seeing #2047 I've run the PSR12 version over the WPCS unit tests for the sniff to see if we could switch over at some point.

The test run identified a number of bugs in the PSR12 sniffs:

Test cases:

$a = new self::$transport[$cap_string];
$renderer = new $this->inline_diff_renderer;
$a = new ${$varHoldingClassName};

Fixed as:

$a = new self()::$transport[$cap_string];
$renderer = new $this()->inline_diff_renderer;
$a = new $(){$varHoldingClassName};

Expected fixes:

$a = new self::$transport[$cap_string]();
$renderer = new $this->inline_diff_renderer();
$a = new ${$varHoldingClassName}();

👉 Would you like me to look into fixing these ?

It also raises the question Why does FIG ignore this rule for anonymous classes ?, though that's outside the scope of PHPCS.

Other than the above, the WPCS sniff has some additional features.

  • Detect class instantiation without parentheses in JS.
  • Throw error when "Assigning new by reference" is detected (PHP only).
  • Throw fixable error for whitespace between class name and open parenthesis (PHP + JS).
  • Metrics for all errors.

👉 Would you be interested in any of these ?

@gsherwood
Copy link
Member

Would you like me to look into fixing these ?

I'm working on this sniff right now due to another bug report, so I can take these ones. Thanks for reporting them.

Would you be interested in any of these ?

They are good checks, but I don't think they belong in the PSR-12 sniff. I prefer these PSR ones to be as focused as possible due to the arguments that they tend to bring up.

@gsherwood gsherwood changed the title PSR12/ClassInstantiation: various bugs PSR12.Classes.ClassInstantiation incorrectly fix when using member vars and some variable formats Jun 8, 2018
@gsherwood gsherwood added this to the 3.3.1 milestone Jun 8, 2018
gsherwood added a commit that referenced this issue Jun 8, 2018
…en using member vars and some variable formats
@gsherwood
Copy link
Member

I've fixed these cases. If you find others, please report new issues and I'll fix them up too.

@jrfnl
Copy link
Contributor Author

jrfnl commented Jun 8, 2018

@gsherwood Thanks for the quick turn-around. ❤️

They are good checks, but I don't think they belong in the PSR-12 sniff. I prefer these PSR ones to be as focused as possible due to the arguments that they tend to bring up.

So leave off ? or would you like me to pull these as separate sniffs in the Generic standard ?

@gsherwood
Copy link
Member

or would you like me to pull these as separate sniffs in the Generic standard ?

If you want to contrib them, and these checks don't exist elsewhere, then absolutely.

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