log() method incorrectly calculates the column number when Tabs are used

[Zopsss]

log() method incorrectly calculates the column number when Tabs are used

[MANISH-K-07]

#14780 isn't truly about tabs, so I don't see how we are calculating tabs incorrectly

@rnveach , yep sorry, my bad.
The snapshots were hard to read and I misread '8' as '0', hence got bit confused :)

[rnveach]

#14643 (comment)

let’s reproduce it with an existing check

In addition, see https://checkstyle.org/report_issue.html#How_to_report_a_bug.3F .
Please do not post with pictures, it makes it hard to read and copy for reproduce testing.

[rnveach]

When a case is provided, it will have to be examined if this issue is related to #14780 as both are javadocs.

[MANISH-K-07]

When a case is provided, it will have to be examined if this issue is related to #14780 as both are javadocs.

@rnveach , your reference is accurate.
This issue reported here is an after effect of what we found of column number mismatch at #14780
It is getting reflected probably as we log violations by AST parsing. It makes me wonder where else we have been missing 😕

[rnveach]

If this is a duplicate of #14780 then we need to close this issue.

I can't argue against closing as too few details are given in this issue. #14780 isn't truly about tabs, so I don't see how we are calculating tabs incorrectly. All our AbstractCheck log methods use tab conversion code so you could show a case with nearly all our checks if one existed.

If the method is expecting 1-base and we are giving it 0-base then that is the issue and that is how it connects. However, looking at that method's code it does appear to be 0-based as well. So I am not seeing the connection yet.

[rnveach]

This may be the issue, which is unrelated to #14780 .

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/utils/CommonUtil.java

Line 209 in 05981e0

len = (len / tabWidth + 1) * tabWidth;

Shouldn't this be len += tabWidth? I am not understanding where this division is coming from.

It looks like it is saying tabs go up to a certain position and then stop. Meaning 2 spaces plus a tab stop at the same position as a tab alone. That does make sense as I have seen some tools do this.

Javadoc:

Each tab is counted as the number of characters is takes to jump to the next tab stop.

@Zopsss Please try to create a case.

[Zopsss]

Hey @rnveach Thanks for reviewing the issue. I have fixed the failing test case mentioned in this issue. I'll explain in detail why I opened this issue and what problem I was facing. I'll reply to your questions soon as it is already mid night here. Thanks again for showing your interest in this issue.

[Zopsss]

I have updated the issue description explaining the problem I was facing in a more precise way

---

I was passing the column number of leading asterisk with expanded tabs to the log() method. The log() method internally calls lengthExpandedTabs() which calculates the column number at a specific position of the provided string by expanding the tabs.

So I was already passing the asterisk's column number with the expanded tabs to the log() method as mentioned here:

log() method is receiving the asterisk's current column number

Here by current column number I meant the column number of the leading asterisk with expanded tabs.

At that time I wasn't sure what is the use of the lengthExpandedTabs() because I didn't knew the term expanding tabs in string. The log() was receiving the column number of the leading asterisk after expanding the tabs, this column number was being passed to the lengthExpandedTabs(), so it was receiving the wrong column number because I was passing the column number with tabs already expanded while the lengthExpandedTabs() method expects to receive the original column number ( without expanded tabs ), that's why it was calculating the column number incorrectly.

So to fix this, I modified the Check implementation to pass the column number of the leading asterisk without tabs expanded to the log() method. Now that the log() method is receiving the column number without tabs expanded, it gets passed to the lengthExpandedTabs() method and it calculates the column number by expanding tabs and everything works fine.

The lengthExpandedTabs() method is working fine, I was just passing the wrong column number to it that's why it was calculating the incorrect column number. All the problems I was facing has been resolved, I will close this issue now. And sorry for the delay I was busy with college stuff so it took a little longer to reply.