roslov / psr12ext

PSR-12 Extended Coding Standard
MIT License
3 stars 1 forks source link

`Generic.WhiteSpace.ScopeIndent` with `exact`=`true` does not require indentation for ternaries and multiline strings #35

Closed roslov closed 5 months ago

roslov commented 6 months ago

This

$where = in_array($cISO, ['PT', 'MX', 'ES', 'IT', 'LV', 'LT', 'DK', 'CH', 'CZ'], true)
    ? ' and cc.is_act=1'
    : ' and cc.is_vis=1';

fails and is fixed to

$where = in_array($cISO, ['PT', 'MX', 'ES', 'IT', 'LV', 'LT', 'DK', 'CH', 'CZ'], true)
? ' and cc.is_act=1'
: ' and cc.is_vis=1';

It can be fixed by

    <rule ref="Generic.WhiteSpace.ScopeIndent">
        <properties>
            <property name="exact" value="true" />
            <property name="ignoreIndentationTokens" type="array">
                <element value="T_COMMENT"/>
                <element value="T_DOC_COMMENT_OPEN_TAG"/>
                <element value="T_INLINE_THEN"/>
                <element value="T_INLINE_ELSE"/>
            </property>
        </properties>
    </rule>

But it also allows

$where = in_array($cISO, ['PT', 'MX', 'ES', 'IT', 'LV', 'LT', 'DK', 'CH', 'CZ'], true)
        ? ' and cc.is_act=1'
    : ' and cc.is_vis=1';

that is not good.

See also https://github.com/squizlabs/PHP_CodeSniffer/issues/3711

martinjoiner commented 6 months ago

Your example is very specific to your personal choice to use ternaries on multiple lines. There is nothing fundamentally wrong with the sniff and it would not be a good investment of the maintainers time to change the behaviour just for you.

Rewriting your code below not only brings the ternary onto a single line but it gives a name to the array of ISO codes and thus makes it more expressive and easier to read:

$actCountries = ['PT', 'MX', 'ES', 'IT', 'LV', 'LT', 'DK', 'CH', 'CZ'];
$where .= in_array($cISO, $actCountries, true) ? ' and cc.is_act=1' : ' and cc.is_vis=1';

If you still insist on using long ternaries on multiple lines, maybe consider if using a standards library is the right choice for you.

Open source maintainers time is precious and they have bigger fish to fry. Please understand, I only comment out of a desire to protect the CodeSniffer project and keep it sustainable.

roslov commented 5 months ago

@martinjoiner, probably you’re right regarding the current implementation of the Generic.WhiteSpace.ScopeIndent rule. Anyway, some IDEs (like PHPStorm) do such an indentation. Maybe, there exists a third party sniff that does what I what. But I still haven’t found it :-( @martinjoiner, if you know something about such a sniff, I’d be grateful if you let me know. Thank you for your comment.

For these case, I will disable exact indentation for ternaries and multiline strings.

roslov commented 5 months ago

Disabled the exact indentation for ternaries and multiline strings by #40.