Skip to content

Issue #18614: Fix tab-based indentation validation in getIndentImpl#19062

Open
MiniPiku wants to merge 1 commit into
checkstyle:masterfrom
MiniPiku:issue-18614-fix-tab-indentation-bug
Open

Issue #18614: Fix tab-based indentation validation in getIndentImpl#19062
MiniPiku wants to merge 1 commit into
checkstyle:masterfrom
MiniPiku:issue-18614-fix-tab-indentation-bug

Conversation

@MiniPiku

@MiniPiku MiniPiku commented Feb 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Fixes #18614 - incorrect indentation validation for tab-indented files in IndentationCheck.getIndentImpl()

Root Cause:
The previous implementation compared:

getLineStart(mainAst) → visual column (tab-expanded)

mainAst.getColumnNo() → raw character column (tab counts as 1)

For tab-indented files, these values never matched, causing the branch responsible for computing expected indentation to become unreachable. As a result, incorrect indentation for the new keyword inside a try block was not reported (false negative).

Fix:
Replaced the inconsistent visual vs raw column comparison with isOnStartOfLine(mainAst), which uses consistent visual column logic and properly handles tab expansion.

This ensures:
Correct tab-indented code is validated properly
Incorrect tab indentation now reports violations
Space-indented behavior remains unchanged

@MiniPiku MiniPiku force-pushed the issue-18614-fix-tab-indentation-bug branch from 1a7f507 to 0a54ab9 Compare February 28, 2026 20:20
This was referenced Feb 28, 2026
Open
Closed
romani
romani requested changes Mar 1, 2026
View reviewed changes

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need tests extension to prove that it fixing issue

@MiniPiku

MiniPiku commented Mar 1, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the feedback.

The required test extension is currently blocked by issue #19063, which is directly related to this problem. At the moment, adding new regression inputs causes IndentationTrailingCommentsVerticalAlignmentTest to fail due to the vertical alignment enforcement.

Because of this coupling, we cannot properly add or validate the necessary tests until #19063 is resolved.

Could you please review #19063 first? Once that issue is addressed, I will immediately proceed with adding the test extensions to fully cover this case.

Let me know if you would prefer a different approach.

@romani

romani commented Mar 8, 2026

Copy link
Copy Markdown
Member

There must be test method with Input that is copy from issue description.
You need to prove that fix is working

@MiniPiku

MiniPiku commented Mar 9, 2026

Copy link
Copy Markdown
Contributor Author

There must be test method with Input that is copy from issue description. You need to prove that fix is working

Before adding tests we need to resolve #19063 or else the tabs has to be resolved to 4/8 spaces and it defeats the whole purpose of the test.

@MiniPiku MiniPiku force-pushed the issue-18614-fix-tab-indentation-bug branch from 0a54ab9 to 7f50337 Compare March 18, 2026 18:54
@MiniPiku MiniPiku requested a review from romani March 18, 2026 18:59
@MiniPiku

MiniPiku commented Mar 18, 2026

Copy link
Copy Markdown
Contributor Author

Hi @romani,
I’ve added the requested tests. This PR should now be ready for review.

@romani

romani commented Mar 19, 2026

Copy link
Copy Markdown
Member

Github, generate report for Indentation/all-examples-in-one

@romani romani force-pushed the issue-18614-fix-tab-indentation-bug branch from 7f50337 to 655c804 Compare March 19, 2026 02:01
@github-actions

github-actions Bot commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

Report generation failed. Please check the logs for more details.

Link: https://github.com/checkstyle/checkstyle/actions/runs/23276189731

@MiniPiku

MiniPiku commented Mar 19, 2026

Copy link
Copy Markdown
Contributor Author

@romani is this good to merge or any changes are required?

@romani

romani commented Mar 20, 2026

Copy link
Copy Markdown
Member

Github, generate report for Indentation/Example1

@romani

romani commented Mar 20, 2026

Copy link
Copy Markdown
Member

We need report to see diff on big scale of projects
https://github.com/checkstyle/test-configs/tree/main/Indentation

@github-actions

github-actions Bot commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Report for Indentation/Example1:
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/655c804ee597f80bbf316a4cb71cef494df6ee37_2026022228/reports/diff/index.html

@romani

romani commented Mar 20, 2026

Copy link
Copy Markdown
Member

Please review diff report

@MiniPiku MiniPiku force-pushed the issue-18614-fix-tab-indentation-bug branch 5 times, most recently from 0d6a04b to c38a88f Compare March 20, 2026 21:16
@MiniPiku

MiniPiku commented Mar 20, 2026

Copy link
Copy Markdown
Contributor Author

Github, generate report for Indentation/Example1

@github-actions

github-actions Bot commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Report for Indentation/Example1:
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/c38a88f94269071d8016bc68ec11c196f65e4a89_2026222724/reports/diff/index.html

@romani

romani commented Mar 21, 2026

Copy link
Copy Markdown
Member

Why I don't see exclude of file with tab, tab is forbidden, so there should be suppression of checkstyle violation. Do you know why ?

@MiniPiku

MiniPiku commented Mar 21, 2026

Copy link
Copy Markdown
Contributor Author

Why I don't see exclude of file with tab, tab is forbidden, so there should be suppression of checkstyle violation. Do you know why ?

@romani

Tabs are not suppressed here because IndentationCheck supports tabs via the tabWidth property and still processes such files.

The rule that forbids tabs is FileTabCharacter, which is not part of this configuration. Because of that, tab-based files are not excluded and are still validated by IndentationCheck.

With this fix, these files are now handled correctly instead of being skipped due to the mismatch between raw and visual column calculations.

Please let me know if suppression is expected for these configs.

@MiniPiku MiniPiku force-pushed the issue-18614-fix-tab-indentation-bug branch from c38a88f to acb0a12 Compare March 21, 2026 07:24
@MiniPiku

MiniPiku commented Mar 21, 2026

Copy link
Copy Markdown
Contributor Author

Github, generate report for Indentation/Example1

@github-actions

github-actions Bot commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Failed to download or process the specified configuration(s).
Error details:

Please ensure you've used one of the following commands correctly:

@MiniPiku

MiniPiku commented Mar 21, 2026

Copy link
Copy Markdown
Contributor Author

Github, generate report for Indentation/Example1

@github-actions

github-actions Bot commented Mar 21, 2026

Copy link
Copy Markdown
Contributor

Report for Indentation/Example1:
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/acb0a12f5068b4ff4fd144a53c2e7eaa950a67ac_2026090505/reports/diff/index.html

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

Reviewers

@romani romani Awaiting requested review from romani

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

False positive: indentation inside of constructor parameters inside try block

2 participants

@MiniPiku @romani