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

PSR2/UseDeclaration: allow comments after the last USE statement #1893

Conversation

iammattcoleman
Copy link
Contributor

Resolves #1891

@@ -142,7 +142,7 @@ public function process(File $phpcsFile, $stackPtr)
return;
}

$next = $phpcsFile->findNext(T_WHITESPACE, ($end + 1), null, true);
$next = $phpcsFile->findNext([T_WHITESPACE, T_PHPCS_IGNORE, T_COMMENT], ($end + 1), null, true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to use Tokens::$EmptyTokens instead for completeness.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Updated!

@iammattcoleman iammattcoleman force-pushed the fix-UseDeclarationSniff-commentLastLine branch from d7dc100 to b569625 Compare February 10, 2018 03:53
@iammattcoleman iammattcoleman force-pushed the fix-UseDeclarationSniff-commentLastLine branch from b569625 to f147caf Compare February 10, 2018 03:53
Copy link
Contributor

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this should go into 3.2.3 as it involved the new PHPCS annotations (aside from normal comments).

@gsherwood gsherwood added this to the 3.2.3 milestone Feb 11, 2018
@gsherwood gsherwood merged commit f147caf into squizlabs:master Feb 11, 2018
@gsherwood
Copy link
Member

Thanks a lot for fixing this, and for the test.

@iammattcoleman iammattcoleman deleted the fix-UseDeclarationSniff-commentLastLine branch February 12, 2018 01:53
@dhensby
Copy link
Contributor

dhensby commented Feb 15, 2018

This appears to have caused a regression, please see #1898

dhensby added a commit to dhensby/PHP_CodeSniffer that referenced this pull request Feb 15, 2018
@dhensby
Copy link
Contributor

dhensby commented Feb 15, 2018

OK - the regression here is that this allows you to do:

use Class; // comment

but it now causes this to fail:

use Class;

//comment

Because the test fixture tried to evaluate both at once it appeared to fix the problem, but it actually caused a regression instead

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

Successfully merging this pull request may close these issues.

4 participants