Issue #20339 fix: use expandedTabsColumnNo in NewHandler.getIndentImpl for tab sup… - #20342

@romani Hi! Since this is my first contribution to Checkstyle, my CI workflows are pending manual approval. Could you please help trigger the runs when you have a moment? I've verified the tests locally. Thanks!

@BobSong-dev please add test to https://github.com/checkstyle/checkstyle/blob/master/src/test/java/com/puppycrawl/tools/checkstyle/checks/indentation/IndentationCheckTest.java

and test file should go in
https://github.com/checkstyle/checkstyle/tree/master/src/test/resources/com/puppycrawl/tools/checkstyle/checks/indentation/indentation

@Atharv3221 Done, added two test cases:

testNewKeywordWithTab — non-strict mode, new at the first nesting level
testNewKeywordWithTabStrict — strict mode, new inside an if block (deeper nesting)
Both input files use actual tab characters. Let me know if there's anything I should adjust, and feel free to point out any issues. Thanks!

Github, generate report for Indentation/Example1

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

@BobSong-dev , according to issue, only tab based files should be affected, but diff reprot shows more changes.

please review all, and improve code if you see some false positives

@BobSong-dev , according to issue, only tab based files should be affected, but diff reprot shows more changes.

please review all, and improve code if you see some false positives

This update mainly includes two adjustments.

The previous change fixed the false positive reported in #20339, but its impact was broader than intended. It changed the indentation calculation whenever new appeared at the beginning of a tab-indented line, which caused many unrelated warnings in the regression report.

The condition has now been narrowed. The line-start indentation logic is used only when both the raw column and the tab-expanded column confirm that new is at the beginning of the line. This fixes the column-collision case in code such as \t\tthrow new, without changing the existing behavior of other tab-indented code.

I also updated the tests. The previous tests only covered a single-line expression:

Object obj = new Object();

Those tests passed even with the original implementation, so they did not actually reproduce the reported issue. The new tests use the multiline constructor case from the issue:

throw new RuntimeException(
    "Exception");

Both strict and non-strict configurations are now covered.

I ran the relevant tests as well as the full mvn clean verify, and everything passed.

Feedback and corrections are very welcome.

which caused many unrelated warnings in the regression report.

this is a big concern, as users wihtout tabs, majority of codebases will get false-positives.
We need to find way how to not affect others, and affect only source codes with tabs.

please add to our Inputs this snippet with violation asd fixed form of it https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/1c1ae3aec5d09ffb34fb5490ab42d29c45f19ae8_2026232108/reports/diff/java-design-patterns/index.html#A1

and https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/1c1ae3aec5d09ffb34fb5490ab42d29c45f19ae8_2026232108/reports/diff/spring-framework/index.html#A1
as is from them with violation and what is expected formatting.

ok ,thanks a lot, it looks like we resolved few other problems :) by this tab issue.
Ok ,lets move forward ... advanced CI is enabled, please check status of it.

@BobSong-dev , pelase glance at https://github.com/checkstyle/checkstyle/issues?q=is%3Aissue%20state%3Aopen%20label%3Aindentation do you see any issue that you resovled by your update ?

@BobSong-dev , pelase glance at https://github.com/checkstyle/checkstyle/issues?q=is%3Aissue%20state%3Aopen%20label%3Aindentation do you see any issue that you resovled by your update ?请浏览 https://github.com/checkstyle/checkstyle/issues?q=is%3Aissue%20state%3Aopen%20label%3Aindentation，您是否看到您通过更新解决的任何问题？

Thanks! I checked the advanced CI.

The functional checks look good, including the indentation tests and the test-resources compilation check. The only red job is the Java performance check. It looks like the run was faster than the baseline, but the performance threshold was still exceeded. The javadoc job was cancelled as a result. AppVeyor is still running.

I also checked the open indentation issues, and this change seems to resolve #18614 as well as #20339.

I reproduced the tab-indented cases from #18614 with the current PR. The correctly indented constructor arguments inside try now pass, while an incorrectly indented new is correctly reported.

The root cause appears to be the same raw-column versus tab-expanded-column mismatch in NewHandler.

ok, please add users code from #18614 to Input to make sure it is fixed.
and add link to it to PR description to not forget to close after merge.

But all such issues are from same Author and with TAB usage.
But diff testing shows us difference that is non-TAB code sources, and this worries me.

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/1c1ae3aec5d09ffb34fb5490ab42d29c45f19ae8_2026232108/reports/diff/spoon/index.html#A1
Please add this snippet to Inputs, new XXXXXX in method (without assignment to var) and correct form of it next to it.

and add https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/1c1ae3aec5d09ffb34fb5490ab42d29c45f19ae8_2026232108/reports/diff/spring-framework/index.html#A6

Looks like we affected new blocks. It is hard to judge on diff report as when sources are not compliant from the begining, diff can show side effects - just another mess over mess.

Lets take https://github.com/SpigotMC/BungeeCord project, looks like it uses default Indentation and enforce no error.
We need to make two config file, read https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#executing-generation-using-github-action , place them in PR descriptin and trigger diff report. If no diff on this project, we can assume we are good enough to merge

GitHub, generate report

BungeeCord default Indentation:
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/326bf1e_2026020700/reports/diff/index.html

Done, thanks. I completed the requested follow-up:

• added the original tab-indented code from False positive: indentation inside of constructor parameters inside try block #18614 to the input tests;
• linked False positive: indentation inside of constructor parameters inside try block #18614 in the PR description with Closes #18614;
• added the Spoon-style new block used directly inside a method, with both the violation and corrected formatting;
• added the Spring Framework A6 anonymous new block, also with violation and corrected formatting;
• added the BungeeCord diff config and project list to the PR description and generated the report.

The BungeeCord report completed with 0 differences:
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/326bf1e_2026020700/reports/diff/index.html

thanks a lot  多谢
thanks for your review

The Semaphore failure appears unrelated to this PR. The external
methods-distance project fails to compile because it calls TreeWalker
methods that now have protected access. This PR does not change TreeWalker.

The local full verification passes, and the BungeeCord diff report has
0 differences.

Semaphore got regression cause by previos PRs, CI is very unstable today .... that is why we got leak .
I will handle it, it wil not block this PR