Misplaced highlights when tabs are used instead of spaces

[rusproject]

Hello!
I don't know if it is a SublimeLinter-phpcs or SublimeLinter or even PHP_Code_Sniffer problem, so If I opened the issue in a wrong place, please point me where I should do it.

The problem is:
When CodeSniffer highlight errors on lines that is indented with tabs, not spaces, even if the tabs are allowed by rules, we get misplaced highlights.

Here's what I mean:

Expected look (tabs are converted to spaces):

[kaste]

Hm, probably https://github.com/squizlabs/PHP_CodeSniffer/wiki/Configuration-Options#setting-the-default-tab-width

It looks like phpcs reports the same line:column pair for both cases, which would mean it interprets one tab has a width of 4 spaces/characters. Maybe setting a tab-width can fix the problem.

[rusproject]

It was already tab-width=4 in my ruleset:

    <arg name="tab-width" value="0"/>

I've tried to set tab-width=0 or delete it, and then highlights were in right places. But it's leading us to another problem, incorrect work of Generic.WhiteSpace.ScopeIndent rule:

I tried to configure rule like this:

    <rule ref="Generic.WhiteSpace.ScopeIndent">
        <properties>
            <property name="indent" value="4" />
            <property name="tabIndent" value="true" />
        </properties>
    </rule>

and this:

    <rule ref="Generic.WhiteSpace.ScopeIndent">
        <properties>
            <!-- <property name="indent" value="4" /> -->
            <property name="tabIndent" value="true" />
        </properties>
    </rule>

But it gives the same result: phpcs tells expected 1 tabs, found 1 spaces although there are really 1 tab in the code.

So I don't think that it's the proper way to configure indent rules with tab-width=0.

According to this gist we really need to set tab-width=4, as it was in my rules from the beginning.

Btw, if we use ruleset from that gist above, we get exactly same problem with misplaced highlights.

So I think it is not a configuration problem, but some bug. Can't figure out where - phpcs, st-linter, or st-linter-phpcs.

[kaste]

I looked at ale, a vim plugin, and they actually do the same as we do here. I did not find a similar issue over there, so I would suggest asking on the phpcs forum for some advice and help. I don't see what we could do wrong here, and it looks like phpcs reports columns with tabs expanded. Did you check this happens also on the command line?

[rusproject]

looks like phpcs reports columns with tabs expanded

I don't think this is the problem, LSP reports same line:column but highlights correctly:

[kaste]

Heh? Here it looks like it reports 9:3 which is clearly tabs-not-expanded style. In the original post it looked like it reported 9:9. What does it report on the command line in --report=emacs mode?

[kaste]

Also this is intelephense. Does this use phpcs under the hood?

[rusproject]

Heh? Here it looks like it reports 9:3 which is clearly tabs-not-expanded style. In the original post it looked like it reported 9:9. What does it report on the command line in --report=emacs mode?

Yes, sorry, looked at the wrong screenshot.

--report=emacs reports same line:column with tabs-expanded style, just like Sublime linter panel:

C:\Users\Alex>phpcs --report=emacs --standard=D:\example\phpcs.xml D:\example\test.php
D:\example\test.php:7:5: error - Visibility must be declared on method "__construct"
D:\example\test.php:9:9: warning - Inline control structures are not allowed

Also this is intelephense. Does this use phpcs under the hood?

Can't find that information quickly.

[kaste]

As I understand the documentation, tab-width MUST NOT be set because it dictates if tabs are converted to spaces. Of course you then have to change the scope-ident rule because its defaults are wrong after that, e.g. you expect 1 indent as it now counts 1 tab but the default is 4, typically 4 spaces but 4 tabs if tabIndent is set.

If you see "found n spaces" it indicates it converted tabs to spaces. But it might translate 1 tab to 1 space instead of 4. Then the markers are drawn correctly as the column is reported as we expect it but other rules have to be adjusted as well.

[rusproject]

Thank you for detailed response.
Anyway, why is it so complicated... Can't it be like ESlint for example? With clear list of available rules, and obvious ways to use them.

[kaste]

I don't know why phpcs made their decisions.

[braver]

This discussion has me a bit confused. Perhaps we can start over and determine what to do from there?

When using tabs for indentation, phpcs reports:

<file path>:88:16: error - TRUE, FALSE and NULL must be lowercase; expected "false" but found "FALSE"

When using spaces for indentation, phpcs reports:

<file path>:88:16: error - TRUE, FALSE and NULL must be lowercase; expected "false" but found "FALSE"

Ie. the output is identical. The output is identical in both JSON and Emacs style reports.

When using tabs, the squiggles are mis-aligned:

When using spaces, they're fine:

Note that I did not configure anything around tab widths here, but I also don't think that's relevant: phpcs is reporting the correct columns for the errors. Your column position after a tab is 5, just like it would be after 4 spaces. So as far as I know phpcs is reporting correctly here. And it looks like we're misplacing the highlight region.

I don't know of any other linters with a similar problem. I've been using a mix of tabs and spaces since forever and never noticed anything like it (that I can remember).

[braver]

Note also that the column reported in the panel is correct, in this screenshot 16 (where my cursor is), but the squiggly is at 22.

Edit:

When using tabs the begin of the region is (95, 15) and the end is (96, 0), whereas when using spaces it's also (95, 15), but with (95, 20) as the end. Not sure if relevant because I don't fully understand what's going in the entire pipeline around draw_view_region.

[kaste]

phpcs has this option https://github.com/squizlabs/PHP_CodeSniffer/wiki/Configuration-Options#setting-the-default-tab-width Maybe setting this to the actual used tab-width solves this problem.

We also had SublimeLinter/SublimeLinter@7e145114

@@ -1383,22 +1379,6 @@ class Linter(metaclass=LinterMeta):
         # type: (str) -> bool
         return filename in ["stdin", "<stdin>", "-"]
 
-    def maybe_fix_tab_width(self, line, col, vv):
-        # type: (int, int, VirtualView) -> int
-        # Adjust column numbers to match the linter's tabs if necessary
-        if self.tab_width > 1:
-            code_line = vv.select_line(line)
-            diff = 0
-
-            for i in range(len(code_line)):
-                if code_line[i] == '\t':
-                    diff += (self.tab_width - 1)
-
-                if col - diff <= i:
-                    col = i
-                    break
-        return col
-
     def reposition_match(self, line, col, m, vv):
         # type: (int, Optional[int], LintMatch, VirtualView) -> Tuple[int, int, int]
         """Chance to reposition the error.

This was unused code, no body set tab_width ever, but does likely do the same thing.

Depending on the implementation or interpretation, in the example line $foo = ... , is the "$" at the 3rd position of that line -- tab tab $ -- or on the, hm, 9th -- 8 spaces plus $?

I think it is reasonable that Sublime would say 9 in the status bar, but if we count the bytes/chars or the Sublime's Point it may report differently.

Edit: E.g. the deleted code would have subtracted 6 from the column position if tab_width is set to 4.

[braver]

Maybe setting this to the actual used tab-width solves this problem.

Setting it on the fly neither solves the issue nor changes the line/col in the output (but line col in the output was already correct, I don't think we want to change that).

I think it is reasonable that Sublime would say 9 in the status bar, but if we count the bytes/chars or the Sublime's Point it may report differently.

The line/col for the point we have for the start of this error is actually correct (see my previous comment).

To re-iterate: the linter says column 16, which, minus one, matches column 15 which we have as the point for the error, and we have 16 in the output panel, which lines up with the reported cursor position, 16, in the status bar. All data I can see (that I know to look for) is in agreement on where the error is (column 16), but the squiggly is wrong.

[kaste]

The squiggly is probably plus 6 because we just count it from the line_start point. (I guess.) (I think we just do line_start plus col because that's what we already have.)