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

LineLength check documentation incorrectly shows how to work with tabWidth property #13005

Closed
danielrcollins1 opened this issue Apr 19, 2023 · 9 comments

Comments

@danielrcollins1
Copy link

danielrcollins1 commented Apr 19, 2023

Sad documentation defect

https://checkstyle.org/config_sizes.html#LineLength_Notes

To specify a different number of spaces, the user can set TreeWalker property tabWidth which applies to all Checks, including LineLength; or can set property tabWidth for LineLength alone.

We need to change Treewalker to Checker.

Line to change

<a href="config.html#TreeWalker"><code>TreeWalker</code></a> property


I have read check documentation: https://checkstyle.org/config_xxxxxx.html#NameOfAffectedCheck
I have downloaded the latest checkstyle from: https://checkstyle.org/cmdline.html#Download_and_Run
I have executed the cli and showed it below, as cli describes the problem better than 1,000 words

D:\Dwnl\CheckStyle\BugReports\BadTabWidth>type Config.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
    "-//Puppy Crawl//DTD Check Configuration 1.2//EN"
    "https://checkstyle.org/dtds/configuration_1_2.dtd">

<module name="Checker">
   <module name="LineLength"/>
   <module name="TreeWalker">
      <property name="tabWidth" value="3" />
   </module>
</module>

D:\Dwnl\CheckStyle\BugReports\BadTabWidth>type TestLongLine.java
/*
                1234567890123456789012345678901234567890123456789012345678901234567890
*/

D:\Dwnl\CheckStyle\BugReports\BadTabWidth>java -jar checkstyle-10.9.3-all.jar -c Config.xml TestLongLine.java
Starting audit...
[ERROR] D:\Dwnl\CheckStyle\BugReports\BadTabWidth\TestLongLine.java:2: Line is longer than 80 characters (found 86). [LineLength]
Audit done.
Checkstyle ends with 1 errors.

The documentation for LineLength says that it honors the TreeWalker tabWidth property, but it does not seem to do so in practice (https://checkstyle.org/config_sizes.html#LineLength).
The sample code file in its second line has 2 tab characters, followed by 70 digits. The config file has tabWidth set to 3, so this should be counted as 76 total characters, and within the default limit of 80 for LineLength. But instead this is counted as 86 characters (what you get if tabs are counted as the default 8 spaces wide each; 70 + 8 + 8 = 86), and thus triggers the LineLength error.
I've also tried setting the tabWidth property within the LineLenth module (as documentation says should work), but that has the same result.


Config.xml.txt
TestLongLine.java.txt

@danielrcollins1 danielrcollins1 changed the title LineLength check incorrectly handles tabwidth value LineLength check incorrectly handles tabWidth value Apr 19, 2023
@romani
Copy link
Member

romani commented Apr 19, 2023

attention that LineLength is not a child of TreeWalker to reuse or be affected by its property.

   <module name="LineLength"/>
   <module name="TreeWalker">
      <property name="tabWidth" value="3" />
   </module>

https://checkstyle.org/config.html#Checker has property tabWidth , use it.

@danielrcollins1
Copy link
Author

Okay, thanks. I guess that must be fairly new?
Documentation linked above still says, "the user can set TreeWalker property tabWidth which applies to all Checks, including LineLength"
Additionally, I got that snippet from the default config file included with the jGRASP IDE install (https://www.jgrasp.org/).

@romani
Copy link
Member

romani commented Apr 20, 2023

Sad documentation defect

https://checkstyle.org/config_sizes.html#LineLength_Notes

To specify a different number of spaces, the user can set TreeWalker property tabWidth which applies to all Checks, including LineLength; or can set property tabWidth for LineLength alone.

We need to change Treewalker to Checker.

@romani romani changed the title LineLength check incorrectly handles tabWidth value LineLength check documentation incorrectly shows how to work with tabWidth property Apr 20, 2023
@romani
Copy link
Member

romani commented Apr 20, 2023

@danielrcollins1, do you have time to send Pull Request to fix it ?
I put in issue description where to fix it.

@PavanKumarCB
Copy link

Hey i would like to take up this issue.
Do you assign the issues or am i free to pick them and raise PR?

@romani
Copy link
Member

romani commented Apr 23, 2023

no assignments, just do comment "I am on it" and start working on any issue that is approved.

@ghost
Copy link

ghost commented Apr 25, 2023

i am on it.

@ghost ghost mentioned this issue Apr 25, 2023
Andromeda227799 pushed a commit to Andromeda227799/checkstyle that referenced this issue May 21, 2023
@Andromeda227799
Copy link

I am on it

Andromeda227799 pushed a commit to Andromeda227799/checkstyle that referenced this issue May 21, 2023
Andromeda227799 pushed a commit to Andromeda227799/checkstyle that referenced this issue May 21, 2023
Andromeda227799 pushed a commit to Andromeda227799/checkstyle that referenced this issue May 21, 2023
Andromeda227799 pushed a commit to Andromeda227799/checkstyle that referenced this issue May 22, 2023
…ows how to work with tabWidth property

Issue checkstyle#13005: LineLength check documentation incorrectly shows how to work with tabWidth property
Andromeda227799 pushed a commit to Andromeda227799/checkstyle that referenced this issue May 22, 2023
…ows how to work with tabWidth property

Issue checkstyle#13005: LineLength check documentation incorrectly shows how to work with tabWidth property
Andromeda227799 pushed a commit to Andromeda227799/checkstyle that referenced this issue May 22, 2023
romani pushed a commit that referenced this issue May 22, 2023
@romani
Copy link
Member

romani commented May 22, 2023

Fix is merged

@romani romani closed this as completed May 22, 2023
@github-actions github-actions bot added this to the 10.12.0 milestone May 22, 2023
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

4 participants