NewWithBracesFixer - option to remove braces

[jrmajor]

Although PSR-12 states that “when instantiating a new class, parentheses MUST always be present”, this rule clearly does not apply to anonymous classes, as there are three examples of anonymous classes in PSR-12 and none of them has parentheses.

This PR adds named_class and anonymous_class boolean options to NewWithBracesFixer. By default, both of them are set to true, which matches current behavior. However, I've changed the configuration in PSR-12 to ['anonymous_class' => false], as this is the code style used in all examples.

[coveralls]

Coverage increased (+0.008%) to 93.181% when pulling 877624a on jrmajor:new-with-braces-anonymous into 0999168 on FriendsOfPHP:master.

[mvorisek]

Although PSR-12 states that “when instantiating a new class, parentheses MUST always be present”, this rule clearly does not apply to anonymous classes, as there are three examples of anonymous classes in PSR-12 and none of them has parentheses.

I belive the parentheses MUST be present even for anonymous classes and the PSR-12 should receive an errata :)

The parentheses stress that a constructor is called and even needed when one or more arguments need to be passed.

[kubawerlos]

Given this comment: php-fig/fig-standards#1206 (review) I'd vote to reject this proposl.

[jrmajor]

@mvorisek @kubawerlos @keradus Thank you for linking to this discussion in php-fig, I wasn't aware of that. Would you be OK with adding these options without changing the PSR-12 rule set configuration then?

[SpacePossum]

If the changes are not following PSR12 (even if the spec was confusing and cannot be amended now because of reasons) than these should be coming from some other standard and/or supported/used by a bigger PHP community (this is a requirement for any rule or option).
I suspect these are not, so I think this would fit better in a 3rd party fixer and maintained by that 3rd party.

[jrmajor]

@SpacePossum I think Laravel does not use parentheses when there are no arguments, although I'm not sure whether it's a formal rule or they just use them randomly.

[jrmajor]

Sorry for force pushing, didn't know fabbot doesn't like merge commits.

[SpacePossum]

@GrahamCampbell do you know if this CS is part of the style of laravel?

[GrahamCampbell]

Laravel doesn't use this fixer, or particularly care about the code style here. There is no interest from the Laravel core team to normalize the code style in either direction, as far as I'm aware.

[SpacePossum]

Thanks Graham.

As this is not part of style defined by a popular standard or used in a bigger (F)OSS project I'm not in favor of adding this feature, I think a 3rd party fixer would be a better fit.

(also, would be better to utilize TokensAnalyzer::isAnonymousClass and not rewrite logic in the fixer)

[jrmajor]

@SpacePossum After seeing this comment by @sebastianbergmann, I think PHPUnit may be interested in using these options.

[jrmajor]

Also, would be better to utilize TokensAnalyzer::isAnonymousClass and not rewrite logic in the fixer

The $nextToken->isGivenKind(T_CLASS) check was there before. I'm not sure how to replace it, as TokensAnalyzer::isAnonymousClass() checks whether T_CLASS is preceded by T_NEW, and we need to check the opposite (whether T_NEW is followed by T_CLASS).

[sebastianbergmann]

I lack the expertise to comment on its implementation, but I would like to be able to use this fixer in my projects.

[SpacePossum]

Thank you @jrmajor.