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

Incorrect Squiz.WhiteSpace.ControlStructureSpacing.NoLineAfterClose error between catch and finally statements #1890

Closed
photodude opened this issue Feb 8, 2018 · 5 comments
Milestone

Comments

@photodude
Copy link
Contributor

The following error was recorded under Squiz.WhiteSpace.ControlStructureSpacing.NoLineAfterClose

449 | ERROR | [x] No blank line found after control structure (Squiz.WhiteSpace.ControlStructureSpacing.NoLineAfterClose)

The code is a try/catch/finally block with a set of if control structures inside the catch.
link to code section: https://github.com/joomla-framework/console/blob/13a8498930831b88c5ece0327a1c46e331a54fb8/src/Application.php#L390-L466

The error occurs at the catch close bracket before the finally keyword

I believe the Expected result is no error for this code

@gsherwood gsherwood changed the title [2.9.1] new line errors with Errors on try/catch/finally with if Squiz.WhiteSpace.ControlStructureSpacing.NoLineAfterClose Incorrect Squiz.WhiteSpace.ControlStructureSpacing.NoLineAfterClose error between catch and finally statements Feb 8, 2018
gsherwood added a commit that referenced this issue Feb 8, 2018
…NoLineAfterClose error between catch and finally statements

Also fixed up a few other sniff that were not checking finally statements
@gsherwood gsherwood added this to the 3.2.3 milestone Feb 8, 2018
@gsherwood
Copy link
Member

Yes, it should be no error. I also found a couple of other sniffs that were not checking finally blocks, so I've fixed those up as well. Thanks for reporting this.

@photodude
Copy link
Contributor Author

You are welcome. Thank you for addressing this issue.

@photodude
Copy link
Contributor Author

One question, Will this be backported to 2.9.x?
Should I backport it and submit a PR?

@gsherwood
Copy link
Member

One question, Will this be backported to 2.9.x?

I'm not maintaining the 2.9 branch any more, so it wont be backported.

But, I still need to tag and release the very final 2.9 version, so if you want to create a PR for this, I'll merge it into 2.9 before I do that. But once that's done, I wont be able to fix any more 2.9 issues and you may need to maintain a fork if you want to backport any more.

photodude added a commit to photodude/PHP_CodeSniffer that referenced this issue Feb 9, 2018
photodude added a commit to photodude/PHP_CodeSniffer that referenced this issue Feb 9, 2018
photodude added a commit to photodude/PHP_CodeSniffer that referenced this issue Feb 9, 2018
photodude added a commit to photodude/PHP_CodeSniffer that referenced this issue Feb 9, 2018
gsherwood added a commit that referenced this issue Feb 11, 2018
@Majkl578
Copy link
Contributor

This change (based on bisect) introduces a BC break in 3.2.3 bugfix release.

The following code did not produce any error in 3.2.2:

<?php


try {
    doSomething();
} catch (SomeException $e) {
    // ignore
}

It is now producing the following error in 3.2.3:

FILE: test.php
------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------
 6 | ERROR | Empty CATCH statement detected (Generic.CodeAnalysis.EmptyStatement.DetectedCatch)
------------------------------------------------------------------------------------------------

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

3 participants