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 🙂.

[github-actions[bot]]

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 30 days.

[github-actions[bot]]

The fact this was automatically closed doesn't mean that the idea got rejected - it simply didn't get any priority for way too long to keep it open. If you're still interested in this, please let as know, we can consider re-opening it.