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

Tokenizer: support hash comment for ignore annotations #3071

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Aug 31, 2020

Tokenizer: support hash comment for ignore annotations

Until now, for the PHPCS native ignore annotations, only // slash or /* */ star-style comments were supported.

This adds support for PHPCS native ignore annotations using # hash-style comments.

Includes unit tests and syncing of the ltrim() used in File::process() with the ltrim() used in Tokenizer::createPositionMap().

@gsherwood gsherwood added this to the 3.6.0 milestone Aug 31, 2020
@jrfnl
Copy link
Contributor Author

jrfnl commented Sep 20, 2020

Let's put this PR on hold for now until support for PHP 8.0 attributes has been investigated as it may complicate that.

@gsherwood
Copy link
Member

Sorry, I never came back to this after attribute support was added.

@gsherwood gsherwood added this to the 3.6.1 milestone Apr 27, 2021
@jrfnl
Copy link
Contributor Author

jrfnl commented Apr 27, 2021

Shall I rebase and add some tests to make sure they pass ?

@jrfnl
Copy link
Contributor Author

jrfnl commented Apr 27, 2021

@gsherwood I'm also tempted to refactor that test file to use data providers to see more easily which cases are being tested. Would you be okay with that ? If so, I'll do it after we got this PR out of the way.

Until now, for the PHPCS native ignore annotations, only `//` slash or `/* */` star-style comments were supported.

This adds support for PHPCS native ignore annotations using `#` hash-style comments.

Includes unit tests and syncing of the `ltrim()` used in `File::process()` with the `ltrim()` used in `Tokenizer::createPositionMap()`.
@jrfnl jrfnl force-pushed the feature/tokenizer-phpcs-annotations-support-hash-comments branch from 855ed03 to 807c10c Compare April 27, 2021 23:42
@jrfnl
Copy link
Contributor Author

jrfnl commented Apr 27, 2021

PR updated, now including allowance for "late suppression" with phpcs:ignoreFile and extensive unit tests.

All seems to work fine.

@gsherwood
Copy link
Member

@gsherwood I'm also tempted to refactor that test file to use data providers to see more easily which cases are being tested. Would you be okay with that ? If so, I'll do it after we got this PR out of the way.

Sounds like a good idea.

gsherwood added a commit that referenced this pull request Apr 28, 2021
@gsherwood gsherwood merged commit 807c10c into squizlabs:master Apr 28, 2021
@gsherwood
Copy link
Member

Thanks a lot for finishing this off. Merged now.

@jrfnl jrfnl deleted the feature/tokenizer-phpcs-annotations-support-hash-comments branch April 28, 2021 11:41
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.

2 participants