Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiline ternaries not being indented correctly #3711

Open
paigedwight opened this issue Nov 8, 2022 · 5 comments
Open

Multiline ternaries not being indented correctly #3711

paigedwight opened this issue Nov 8, 2022 · 5 comments

Comments

@paigedwight
Copy link

Describe the bug
phpcbf will format multiline ternaries to the same level of indentation, instead of having the lines after the first indented an additional level.

Code sample

<?php

$x = $a === $b
    ? true
    : false;

Custom ruleset

<?xml version="1.0"?>
<ruleset name="My Custom Standard">
    <rule ref="Generic.WhiteSpace.ScopeIndent">
        <properties>
            <property name="exact" value="true" />
        </properties>
    </rule>
</ruleset>

To reproduce
Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above...
  2. Run phpcs test.php ...
  3. See error message displayed
-----------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------------
 4 | ERROR | [x] Line indented incorrectly; expected 0 spaces, found 4
   |       |     (Generic.WhiteSpace.ScopeIndent.IncorrectExact)
 5 | ERROR | [x] Line indented incorrectly; expected 0 spaces, found 4
   |       |     (Generic.WhiteSpace.ScopeIndent.IncorrectExact)
-----------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------

Expected behavior
Multiline ternaries should be indented

Versions (please complete the following information):

  • OS: MacOS 12.5
  • PHP: 7.4
  • PHPCS: 3.7.1
  • Standard: n/a

Additional context
n/a

@martinjoiner
Copy link

I suggest this issue is a candidate to be closed with no action required.

Multi-line ternaries are poor readability and although PSR-12 - 6.3 Ternary Operators does not explicitly mention multi-line ternaries it certainly doesn't use any in it's examples. I'm sure consensus opinion would be against breaking them onto multiple lines.

If you want a multi-line conditional, use an if-statement or a match expression. If your ternary is too long to fit on 1 line, it should probably be an if-statement or a match expression.

Also, the ternary in your code sample is redundant and can be re-written as:

$x = $a === $b;

Ternaries are elegant for cases where the code blocks are so small they would look silly in an if-statement and would definitely read better were it all on 1 line such as this:

$color = $accountBalance < 0 ? 'red' : 'black';

But they are inappropriate in any other situation.

Only my opinion, I hope you understand. I'm just hoping that with my input, a few low-priority requests can be closed and allow some higher priority ones to get fixed.

@ErikBrendel
Copy link

ErikBrendel commented Oct 18, 2023

Not indenting them at all makes it worse, though.

The pear style also has examples of indenting them: https://pear.php.net/pepr/pepr-proposal-show.php?id=538#toc5

If this is not officially taken up to get supported, is there a way of automatically disabling the sniff on ternary statements so that the user can at least manually keep indentation here?

@jrfnl
Copy link
Contributor

jrfnl commented Oct 18, 2023

Not indenting them at all makes it worse, though.

The pear style also has examples of indenting them: https://pear.php.net/pepr/pepr-proposal-show.php?id=538#toc5

If this is not officially taken up to get supported, is there a way of automatically disabling the sniff on ternary statements so that the user can at least manually keep indentation here?

The simple solution here is to set the exact property to false.

The exact property being set to true in the ruleset is the problem, not this sniff.

@roslov
Copy link

roslov commented Mar 13, 2024

I also need to have this fixed. exact=false cannot be applied as it ignores other cases to be fixed.

@martinjoiner
Copy link

@roslov This has already been marked as wontfix. As I have commended on the issue on your repo, I am yet to see someone present a good use case for multi-line ternaries so I don't think it is a good investment of time for open source maintainers to to support them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants