Skip to content

Generic/InlineControlStructure: small bug when fixing a control structure without body #599

@rodrigoprimo

Description

@rodrigoprimo
Contributor

Describe the bug

The Generic.ControlStructures.InlineControlStructure sniff removes a newline from the code when adding braces to an inline control structure without body and with a trailing comment.

Code sample

if ($emptyBody); // comment

$unrelatedCode = 1;

To reproduce

Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above.
  2. Run phpcbf --standard=Generic --sniffs=Generic.ControlStructures.InlineControlStructure test.php
  3. Check the modified file

Expected behavior

I would expect the modified file to be:

if ($emptyBody) { 
// comment
}

$unrelatedCode = 1;

But instead it is:

if ($emptyBody) { 
// comment
}
$unrelatedCode = 1;

Note there is no newline between the if and the variable assignment.

Versions (please complete the following information)

Operating System Ubuntu 24.04
PHP version 8.3
PHP_CodeSniffer version master
Standard Generic
Install type git clone

Additional context

I found this while working on finalizing the items discussed in #482 (review), specifically the sections about commits 6 and 7. I can work on a PR to fix this small issue. Since @jrfnl mentioned the current behavior of the sniff regarding how to handle inline control structures without body, I'm not planning to change it anymore as I did in commit 6 or remove part of the fixer code as I did in commit 7.

Please confirm

  • I have searched the issue list and am not opening a duplicate issue.
    I have read the Contribution Guidelines and this is not a support question.
    I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
    I have verified the issue still exists in the master branch of PHP_CodeSniffer.

Activity

jrfnl

jrfnl commented on Aug 17, 2024

@jrfnl
Member

@rodrigoprimo It is unclear to me what you are reporting here or what you are proposing as a fix.

if ($emptyBody); // comment

... is valid code (not useful code, but that's not the concern of the sniff), so this code should not be flagged by the sniff nor touched by the fixer, which is exactly what I suggested in my feedback on commit 6 in PR 482.

And if the code is no longer flagged by the sniff, the new line issue doesn't come into play.

rodrigoprimo

rodrigoprimo commented on Aug 20, 2024

@rodrigoprimo
ContributorAuthor

@jrfnl, since you mentioned in your feedback in PR 482 that in the past there was a decision not to handle those inline control structures without a body, I was planning on dropping commit 6. I should have mentioned this in this issue or before opening this issue. Without commit 6, I believe we should just fix this tiny issue in the fixer. Does this help clarify things?

what you are proposing as a fix

I'm proposing that the sniff fixes

if ($emptyBody); // comment

$unrelatedCode = 1;

As

if ($emptyBody) { 
// comment
}

$unrelatedCode = 1;

But let me know if you prefer that I work on improving commit 6 to address the points that you raised. If that is the case, we can close this issue.

jrfnl

jrfnl commented on Aug 20, 2024

@jrfnl
Member

But let me know if you prefer that I work on improving commit 6 to address the points that you raised.

Yes.

rodrigoprimo

rodrigoprimo commented on Aug 20, 2024

@rodrigoprimo
ContributorAuthor

Ok, closing as not planned (I wrongly closed as completed initially).

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @rodrigoprimo@jrfnl

        Issue actions

          Generic/InlineControlStructure: small bug when fixing a control structure without body · Issue #599 · PHPCSStandards/PHP_CodeSniffer