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

IF statement with no braces and double catch turned into syntax error by auto-fixer #3345

Closed
PierreAntoineGuillaume opened this issue May 11, 2021 · 5 comments · Fixed by #3347
Milestone

Comments

@PierreAntoineGuillaume
Copy link

PierreAntoineGuillaume commented May 11, 2021

When using cbf to clean up following sample code, it turns it

<?php

function testParse()
{
    if (true)
        try {
        } catch (\LogicException $e) {
        } catch (\Exception $e) {
        }
}

into

<?php

function testParse()
{
    if (true) {
        try {
        } catch (\LogicException $e) {
        }
    } catch (\Exception $e) {
    }
}

To reproduce
Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above...
  2. Run phpcbf test.php ...
  3. now has a syntax error
$ vendor/bin/phpcbf test.php 

PHPCBF RESULT SUMMARY
----------------------------------------------------------------------
FILE                                                  FIXED  REMAINING
----------------------------------------------------------------------
test.php                                              5      0
----------------------------------------------------------------------
A TOTAL OF 5 ERRORS WERE FIXED IN 1 FILE
----------------------------------------------------------------------

Time: 48ms; Memory: 10MB

Expected behavior
Either a fatal error and no code modification, or this code :

<?php

function testParse()
{
    if (true) {
        try {
        } catch (\LogicException $e) {
        } catch (\Exception $e) {
        }
    }
}

Versions (please complete the following information):

  • OS: Ubuntu 20 (standard apt repo)
  • PHP: 7.4
  • PHPCS: 3.6.0
  • Standard: PSR12

Here's a full version : https://github.com/GPierre-Antoine/phpcbf-bug

@jrfnl
Copy link
Contributor

jrfnl commented May 11, 2021

Confirmed.

A phpcs run yields the following issues:

-------------------------------------------------------------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
-------------------------------------------------------------------------------------------------------------------------
 5 | ERROR | [x] Inline control structures are not allowed
   |       |     (Generic.ControlStructures.InlineControlStructure.NotAllowed)
 6 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
   |       |     (Generic.WhiteSpace.ScopeIndent.IncorrectExact)
 7 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
   |       |     (Generic.WhiteSpace.ScopeIndent.IncorrectExact)
 8 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
   |       |     (Generic.WhiteSpace.ScopeIndent.IncorrectExact)
 9 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
   |       |     (Generic.WhiteSpace.ScopeIndent.IncorrectExact)
-------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------------------------------

The problem looks to be in the Generic.ControlStructures.InlineControlStructure sniff getting confused over the multi-catch.

If curly braces would have been used for the if (true) condition in the original code sample, PHPCS would not report any issues.

@jrfnl
Copy link
Contributor

jrfnl commented May 11, 2021

Just checked, same problem exists when using finally:

function testFinally()
{
    if (true)
        try {
        } catch (\LogicException $e) {
        } finally {
        }
}

@jrfnl
Copy link
Contributor

jrfnl commented May 11, 2021

PR #3347 should fix this. Testing appreciated.

@PierreAntoineGuillaume
Copy link
Author

I tested it in my test repo, it works.

Thanks.

@jrfnl
Copy link
Contributor

jrfnl commented May 11, 2021

@GPierre-Antoine Thanks for testing!

@gsherwood gsherwood added this to the 3.6.1 milestone May 11, 2021
@gsherwood gsherwood changed the title in phpcbf 3.6.0, php7.4.14, bug when if with no parenthesis and double catch turned into syntax error IF statement with no braces and double catch turned into syntax error by auto-fixer May 27, 2021
gsherwood added a commit that referenced this issue May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants