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

Squiz.Strings.DoubleQuoteUsage replaces tabs with spaces when fixing #1640

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Sep 4, 2017

Squiz/DoubleQuoteUsage: Use original string when testing and fixing double quotes

If a string contains tabs, they were previously - unintentionally - replaced by spaces while that is not the responsibility of this sniff.

Includes unit test,


While examining the sniff code to fix the above issue, I also noticed two other optimizations which could benefit this sniff:

Squiz/DoubleQuoteUsage: Skip past subsequent tokens in a multi-line string

As the last token of the multi-line string is known, it can be returned to skip past subsequent tokens in a multi-line string instead of checking each string token.

Squiz/DoubleQuoteUsage: Make check for double quoted string a little more specific and strict

Only check strings which are finished double quotes strings.
This offers additional protection against examining unfinished strings during live code checking.

…ouble quotes

If a string contains tabs, they were previously replaced by spaces while that is not the responsibility of this sniff.
…tring

As the last token of the multi-line string is known, it can be returned to skip past subsequent tokens in a multi-line string instead of checking each string token.
…more specific and strict

Only check strings which are finished double quotes strings.
This offers additional protection against examining unfinished strings during live code checking.
@gsherwood gsherwood changed the title Squiz/DoubleQuoteUsage: fix whitespace replacement Squiz.Strings.DoubleQuoteUsage replaces tabs with spaces when fixing Sep 5, 2017
@gsherwood gsherwood merged commit ba6afb8 into squizlabs:master Sep 5, 2017
gsherwood added a commit that referenced this pull request Sep 5, 2017
@gsherwood
Copy link
Member

Thanks for fixing this + those two optimisations were a good idea.

@gsherwood gsherwood added this to the 3.1.0 milestone Sep 5, 2017
@jrfnl jrfnl deleted the feature/squiz-doublequote-usage-fix-whitespace-replacement branch September 6, 2017 02:06
@jrfnl
Copy link
Contributor Author

jrfnl commented Sep 6, 2017

Thanks for merging this in 👍

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

Successfully merging this pull request may close these issues.

2 participants