Make single_line_empty_body fix control structures

[mvorisek]

Bug report

only the last catch/finally body should be fixed as otherwise the formatting will be broken

expected behaviour:

 try {
     new Foo();
 } catch (E1 $e) {
-} catch (E2 $e) {
-}
+} catch (E2 $e) {}

 try {
     new Foo();
 } catch (E $e) {
     // comment
 }

[julienfalque]

Currently the rule does not fix control structures at all, I can't reproduce the issue. However, we may want to extend the rule's scope to control structures.

[mvorisek]

I can't reproduce the issue

the current behaviour is:

try {
    new Foo();
} catch (E $e) {
}

intended behaviour/format is:

try {
    new Foo();
} catch (E $e) {}

[julienfalque]

Do I understand correctly that your actual request is to extend the scope of the rule to control structures as I suggested?

[mvorisek]

Yes. Maybe under an option.

[Wirone]

I don't see the need for an option here, it's also a "body" - the rule's name does not refer to functions, classes or anything particular, so it can also cover catch, finally or other parts of code IMHO. However an option would be nice to have, so people could select which "empty bodies" should be handled.

[VincentLanglet]

I don't see the need for an option here, it's also a "body" - the rule's name does not refer to functions, classes or anything particular, so it can also cover catch, finally or other parts of code IMHO. However an option would be nice to have, so people could select which "empty bodies" should be handled.

To me the rule was first introduced to respect PER standard https://www.php-fig.org/per/coding-style/
The standard explicitly refer to functions and classes ; but say nothing about other bodies.

So the option seems a requirement or the rule will start doing more than the PER standard.
(Or the standard need to be updated first)

[Wirone]

IMHO if PER does not specify that, it means either style is valid and the rule can propose the standard 🙂. If PER specifies it at some point, then it may be required to make the rule configurable. If PER have specified it differently for each kind of body, then a config option would be required.

[mvorisek]

IMHO the option should be present to a) satisfy PER only, b) satisfy users more, but optionally.

[Wirone]

Doing more than PER specifies is not a violation, hence the config option is superfluous at this point (nice to have, but not required). Also, such an option should not be based on PER/not PER , but on available places where fix can be applied, then it can be configured accordingly in any ruleset.

[VincentLanglet]

Doing more than PER specifies is not a violation

I disagree with this as soon as the rule is (or will be) inside the PER Ruleset.

When I add the "Foo" ruleset in my php-cs-fixer config, I expect to strictly respect the "Foo" standard without any extra-rules added inside it.

So, if we add single_line_empty_body inside the PER ruleset, it require an option to only fix empty class/function body, in order to not add extra fixer.

[Wirone]

I still think that doing more than described in PER is not against it, it would be if fixer did something that is described as "SHOULD NOT" or "MUST NOT", though. Anyway, the config option is nice to have and as we see here, more than welcome 🙂.