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

Error in tab expansion results in white-space of incorrect size #1373

Closed
MarkMaldaba opened this issue Feb 26, 2017 · 3 comments
Closed

Error in tab expansion results in white-space of incorrect size #1373

MarkMaldaba opened this issue Feb 26, 2017 · 3 comments

Comments

@MarkMaldaba
Copy link

MarkMaldaba commented Feb 26, 2017

Hi there,

I have just upgrade from CodeSniffer 1.x to 2.x and have come across what appears to be a bug in the way tabs are expanded. As far as I can see, this has existed since 2.0 and still exists in the latest 2.8 version. I have not tested on 3.x.

Consider the following (invalid, but illustrative) line in a file:
123 56 (the gap in the middle is a tab character, i.e. 123\t56)

Expected result (assuming a tab-width of 4 characters):

  • Translated content: " " (single space character)
  • Reported line length = 6

Actual result:

  • Translated content: "     " (5 space characters)
  • Reported line length = 10

As far as I can see, the problem is in the calculation of $firstTabSize in Files.php on line 1542.
Instead of:

$firstTabSize = ($tabWidth - ($currColumn % $tabWidth) + 1);

it should be

$firstTabSize = ($tabWidth - (($currColumn - 1) % $tabWidth));

The reason I spotted this was because I suddenly started getting errors from Generic.Files.LineLength.MaxExceeded which I wasn't getting prior to the upgrade.

I am surprised no-one else has spotted this issue - I know this code path is only travelled for white-space composed entirely of tabs, but I wouldn't expect that to be particularly uncommon.

@orlangur
Copy link

I am surprised no-one else has spotted this issue

With PSR-2 expansion tabs could be met rarely in source code whilst LineLength.MaxExceeded is just a warning in it :) Also, meeting such error there is a reflex just to change the line instead of calculating whether PHPCS calculation is correct or not. Nice boundary check!

@gsherwood gsherwood changed the title Error in tab expansion results in white-space of incorrect size. Error in tab expansion results in white-space of incorrect size Mar 3, 2017
@gsherwood
Copy link
Member

Thanks a lot for reporting and fixing this. It is strange that it has never come up, but was clearly wrong on those tab boundaries.

@MarkMaldaba
Copy link
Author

:-) No problem. Thanks for accepting it so quickly.

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