Spacing around T_BREAK

[stlrnz]

I have some trouble configuring spacing around T_BREAK.

Consider the following code as valid:

<?php

declare(strict_types=1);

namespace Demo;

use Exception;

class BreakTest
{
    public function switchTest(): void
    {
        $var = null;

        switch (true) {
            case 1:
                $var = 1;
                break;

            case 2:
                $var = 2;
                break;

            default:
                throw new Exception();
        }
    }

    public function loopTest(): void
    {
        $vars = [0, 1, 2];
        $sum = 0;
        $msg = null;

        foreach ($vars as $var) {
            if ($var === 0) {
                break;
            }

            if ($var === 1) {
                $msg = '';

                break;
            }

            $sum += $var;
        }
    }
}

The expected configuration should force:

• 0 empty lines before T_BREAK in switch
• 1 empty line after T_BREAK in switch
• 1 empty line before T_BREAK in loop (unless its the first statement in a control structure)
• 0 empty lines after T_BREAK in loop

First, I've tried the following rule:

    <rule ref="SlevomatCodingStandard.ControlStructures.JumpStatementsSpacing">
        <properties>
            <property name="linesCountBeforeWhenFirstInCaseOrDefault" value="0"/>
            <property name="linesCountAfterWhenLastInCaseOrDefault" value="1"/>
            <property name="linesCountAfterWhenLastInLastCaseOrDefault" value="0"/>
        </properties>
    </rule>

It always expects an empty line before T_BREAK resulting in:

FOUND 2 ERRORS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------
 18 | ERROR | [x] Expected 1 lines before "break", found 0.
    |       |     (SlevomatCodingStandard.ControlStructures.JumpStatementsSpacing.IncorrectLinesCountBeforeControlStructure)

If I change the configuration to fix this problem for the switsch stamement, it doesnt fit for loops anymore.

    <rule ref="SlevomatCodingStandard.ControlStructures.JumpStatementsSpacing">
        <properties>
            <property name="linesCountBeforeControlStructure" value="0"/>
            <property name="linesCountBeforeWhenFirstInCaseOrDefault" value="0"/>
            <property name="linesCountAfterWhenLastInCaseOrDefault" value="1"/>
            <property name="linesCountAfterWhenLastInLastCaseOrDefault" value="0"/>
        </properties>
    </rule>

FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------
 42 | ERROR | [x] Expected 0 lines before "break", found 1.
    |       |     (SlevomatCodingStandard.ControlStructures.JumpStatementsSpacing.IncorrectLinesCountBeforeControlStructure)

In https://github.com/slevomat/coding-standard/issues/867 you discussed a related problem resulting in three new options for JumpStatementsSpacing (which are helpfull indeed). However, the issue was fixed for doctrine, but not for my scenario.

Doctrine is ignoring T_BREAK in their standard. Thats why they don't have any issues like this. However, ignoring T_BREAK opens the door for some other creative variants which I try to avoid.

Is there any way to configure the expected behaviour like described? If thats not the case, would you consider implementing SwitchSpacingSniff?

[kukulich]

            case 2:
                $var = 2;
                break;

            if ($var === 1) {
                $msg = '';

                break;
            }

These two examples are same and should work equally as reported in your first configuration. There's always some code before break.

[stlrnz]

Yes, I know what you mean. Technically, its really the same. But for me, it does not feel the same.

A break inside switch is like a terminating statement. I see case and break more like forming a block. Thats why I prefer break direcly after the last statement (just like I forbid an empty line before }).

A break inside a loop is however more like any other jump statement. So I prefer the same behaviour as I would expect from return.

For me the context really matters. So I tried to figure out how others are handling this. As I said, doctrine is ignoring this case. In consequence you can find different implementations, including the way described above (break in loop, break in switch). Maybe doctrine would profit from this rule, too?

[kukulich]

I've looked at it again and it still does not make sense to me.

	case 2:
		if ($something) {
			return;
		}

		break;

In this example I expect you don't want this:

	case 2:
		if ($something) {
			return;
		}
		break;

[stlrnz]

You are right, that would be an inconsistency as our standard forces an empty line after the if (via BlockControlStructureSpacing rule). So only your first example is valid. But that's in conflict with the behaviour I described above.

So, what about the following: You could add an option maxLinesCountBeforeWhenLastInCaseOrDefault which defaults to null (= ignored). But if set, it supersedes the linesCountBeforeControlStructure rule only inside case or default.

I could set the new option to 1. That way for loops the "exactly one line before break" (linesCountBeforeControlStructure = 1) rule will still be valid. But inside case or default there could be one or zero empty lines. So both of your examples will be possible. But the "one empty line after if" rule from BlockControlStructureSpacing would still force your first example.

The only downside is that without an if two possibilities are left (coming from the max in maxLinesCountBeforeWhenLastInCaseOrDefault):

            case 1:
                $var = 1;

                break;

            case 2:
                $var = 2;
                break;

But that would be okay for me.