Presence of a goto statement or brace blocks mess the indentation

[Geolim4]

Describe the bug PHPCS keeps wrongly indent my code when a goto or an additional set of brackets is added.

https://github.com/PHPSocialNetwork/phpfastcache/commit/37f3ca4ed592858648517371994875adb9eae33c

Code sample

<?php
{
    {
        if(true)
        {
            {
                $someCode = null;
            }
        }
    }
}

Reformatted code

<?php
{
    {
if (true) {
    {
        $someCode = null;
    }
}
    }
}

Custom ruleset

<?xml version="1.0"?>
<ruleset name="Phpfastcache Standards">
    <description>The Phpfastcache Coding Standards</description>

    <arg name="colors"/>
    <arg value="p"/>
    <ini name="memory_limit" value="256M"/>
    <rule ref="PSR2"/>

    <exclude-pattern>tests/*</exclude-pattern>
    <exclude-pattern>bin/*</exclude-pattern>
    <exclude-pattern>docs/*</exclude-pattern>
    <exclude-pattern>vendor/*</exclude-pattern>

    <rule ref="Generic.Files.LineLength">
        <properties>
            <property name="lineLimit" value="170"/>
            <property name="absoluteLineLimit" value="170"/>
        </properties>
    </rule>
</ruleset>

To reproduce Check out the CI build: https://app.travis-ci.com/github/PHPSocialNetwork/phpfastcache/jobs/565171795#L1053

Expected behavior No error and no wrong reindentation

Versions (please complete the following information):

• OS: Ubuntu 18.04/Win 10 x64
• PHP: 7.4/8
• PHPCS: 3.6.2
• Standard: PSR12

Additional context

[sybrew]

Another example, where a message is displayed at // Generic.WhiteSpace.ScopeIndent.IncorrectExact

<?php

gotolabel: {
	this_is_fine();
	if ( false_positive() ) { // Generic.WhiteSpace.ScopeIndent.IncorrectExact
		this_is_fine();
} else {
	false_negative();
}

	$b = this_is_fine();
}

Desired code (with no warning of this sort):

<?php

gotolabel: {
	this_is_fine();
	if ( this_is_fine() ) { // Generic.WhiteSpace.ScopeIndent.IncorrectExact
		this_is_fine();
	} else { // Generic.WhiteSpace.ScopeIndent.IncorrectExact
		this_is_fine();
	} // Generic.WhiteSpace.ScopeIndent.IncorrectExact

	$b = this_is_fine();
}

[Geolim4]

Hello @gsherwood can this lexer bug be fixed please ? 😢

It's an major flaw blocking builds :(

[gsherwood]

The ScopeIndent sniff doesn't see unnecessary braces as scopes for indenting. Just adding braces around your code when they aren't owned by any control structure is not supported by this sniff.

Changing that is likely going to be pretty difficult to get right due to the amount of new test cases to try and come up with and the fact that it would need to be optional. Unless it's something impacting a lot of developers, it's not something I can prioritise.

[Geolim4]

Hello,

Thank you for your reply. It's also impacting go-to statements as well as namespace braces, not only non-statement braces 😅

On Tue, 28 Jun 2022, 12:36 am Greg Sherwood, @.***> wrote:

The ScopeIndent sniff doesn't see unnecessary braces as scopes for indenting. Just adding braces around your code when they aren't owned by any control structure is not supported by this sniff.

Changing that is likely going to be pretty difficult to get right due to the amount of new test cases to try and come up with and the fact that it would need to be optional. Unless it's something impacting a lot of developers, it's not something I can prioritise.

— Reply to this email directly, view it on GitHub https://github.com/squizlabs/PHP_CodeSniffer/issues/3571#issuecomment-1167995172, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKFGZ73EXGSG45LCLBIXALVRIUGPANCNFSM5SACJBKQ . You are receiving this because you authored the thread.Message ID: @.***>

[gsherwood]

Thank you for your reply. It's also impacting go-to statements as well as namespace braces, not only non-statement braces

Goto statements are not block structures and do not use braces. The braces in your example are purely decorative.

I do not believe there is any issue with indentation of namespace blocks. Namespace blocks can have braces in PHP and PHPCS assigns them correctly. The ScopeIndent sniff has tests for namespace blocks with braces, so any issues specifically with these is something I'd look at.

[sybrew]

Whoop, my mistake on the terminology. I use scope-indents in combination with goto handles so I can see what's where once collapsed without adding comment lines.

In procedural programming, rather than conventional objective or functional programming, these scope-indents are lifesavers. Procedural programming is becoming more of a thing because modern IDEs/code-editors allow collapsing scopes. I don't believe SOLID has a long future once more people catch on.

An example is visible on line 101 below (skips to 113). Line 115 won't allow this because there are no braces.

Detecting scope-indents, whether preceded with a goto handle or not, would be tremendously helpful in these scenarios. If you welcome PRs, I'd love to help get the ball rolling.

[Geolim4]

Detecting scope-indents, whether preceded with a goto handle or not, would be tremendously helpful in these scenarios. If you welcome PRs, I'd love to help get the ball rolling.

I agree, a scope-indent sometimes allows (aside goto statements) for better code clarity in complex scenarios. This change would be welcomed by lot of developers including me obviously.