Skip to content

Issue #5685: Fix false positive indentation for try/catch with body on same line#19562

Merged
romani merged 1 commit into
checkstyle:masterfrom
Carbon14-48:issue-5685-fix-try-indentation
May 6, 2026
Merged

Issue #5685: Fix false positive indentation for try/catch with body on same line#19562
romani merged 1 commit into
checkstyle:masterfrom
Carbon14-48:issue-5685-fix-try-indentation

Conversation

@Carbon14-48

Copy link
Copy Markdown
Contributor

closes : #5685

  • Fixes a false positive in Indentation for inline statements on the same line as try, catch, and finally opening braces.
  • Adds a targeted child-filter in BlockParentHandler and applies it only in TryHandler, CatchHandler, and FinallyHandler so non-try/catch/finally behavior is unchanged.

Build Success withmvn clean verify

@Carbon14-48 Carbon14-48 force-pushed the issue-5685-fix-try-indentation branch from c5a6ffc to 97737ee Compare April 2, 2026 01:55
@Carbon14-48

Copy link
Copy Markdown
Contributor Author

GitHub, generate report for Indentation/Example3

@github-actions

github-actions Bot commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

@Carbon14-48 Carbon14-48 force-pushed the issue-5685-fix-try-indentation branch from 97737ee to 06920e9 Compare April 2, 2026 05:23
@Carbon14-48

Copy link
Copy Markdown
Contributor Author

GitHub, generate report for Indentation/Example3

@github-actions

github-actions Bot commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Report for Indentation/Example3:
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/06920e9d599d2695cfef6c6ae3adff9f5af4d092_2026070823/reports/diff/index.html

@Carbon14-48

Copy link
Copy Markdown
Contributor Author

it looks good i guess, i'm not sure about the added violation should it be reported or no . @romani can you please review

@romani

romani commented May 3, 2026

Copy link
Copy Markdown
Member

@Carbon14-48 , please resolve conflict.

@Carbon14-48

Copy link
Copy Markdown
Contributor Author

@Carbon14-48 , please resolve conflict.

i will resolve the conflict and also look into the new violations that appeared in the last report because i verified now and it should not be reported

@Carbon14-48 Carbon14-48 force-pushed the issue-5685-fix-try-indentation branch 2 times, most recently from 13de087 to 69208e3 Compare May 3, 2026 18:04
@Carbon14-48

Copy link
Copy Markdown
Contributor Author

GitHub, generate report for Indentation/Example3

@github-actions

github-actions Bot commented May 3, 2026

Copy link
Copy Markdown
Contributor

@Carbon14-48 Carbon14-48 force-pushed the issue-5685-fix-try-indentation branch 2 times, most recently from 4c1b6de to c7507af Compare May 4, 2026 07:04
@Carbon14-48

Copy link
Copy Markdown
Contributor Author

GitHub, generate report for Indentation/Example3

@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

@Carbon14-48 Carbon14-48 force-pushed the issue-5685-fix-try-indentation branch 3 times, most recently from 2a7315b to 36741ab Compare May 4, 2026 21:39
@Carbon14-48

Copy link
Copy Markdown
Contributor Author

GitHub, generate report for Indentation/Example3

@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

@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.

Items

} //indent:8 exp:8
} //indent:4 exp:4
void method3() { //indent:4 exp:4
try { invokeAndWait(new Runnable() { @Override //indent:8 exp:8

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

both snippets go through the same LineWrappingHandler path; the difference is the config: the test uses forceStrictCondition=false, so it only flags when indent is below expected, and column 46 is ≥ 8 so no violation. The OpenJDK diff uses forceStrictCondition=true, which i guess requires exact alignment, so column 46 becomes a violation.

@romani romani May 6, 2026

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.

Please create new input with such property as true, copy jdk code snippet in it.
Let's see what is going on.

Something strange, it use to be violation on 'try' now it is on '@'.
forceStrictCondition=true only applied to line wrapping, not sure how it becomes here involved

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Something strange, it use to be violation on 'try' now it is on '@'.

i think that comes down to the fact that checkstyle report one violation per line .before the fix #A4 (the try child violation) was reported first, which claimed line 643,That prevented #A5 (the @ violation) from being reported , After our fix, #A4 is correctly removed ( the false positive). But now line 643 is never claimed by TryHandler, so LineWrappingHandler freely reports #A5, which was always there, just previously masked.

i previously runned the all jar on this

import java.awt.Component;

public class Test {
    // Thread-safe Object.equals() called from native
    public static boolean doEquals(final Object a, final Object b, Component c) {
        if (a == b) return true;

        final boolean[] ret = new boolean[1];

        try {
            invokeAndWait(new Runnable() { @Override
                                              public void run() {
                                                  synchronized (ret) {
                                                      ret[0] = a.equals(b);
                                                  }
                                              }
            }, c);
        }
        catch (Exception e) {
            e.printStackTrace();
        }
        synchronized(ret) { return ret[0]; }
    }

    private static void invokeAndWait(Runnable r, Object o) {
    }
}

with this confix.xml :

<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
    "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
    "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
  <module name="TreeWalker">
    <module name="Indentation">
      <property name="forceStrictCondition" value="true"/>
    </module>
  </module>
</module>

and i get
 ~/gsoc/checkstyle > java -jar target/checkstyle-13.4.3-SNAPSHOT-all.jar -c config.xml Test.java

Starting audit...
[ERROR] /home/carbon14/gsoc/checkstyle/Test.java:11:44: '@' has incorrect indentation level 43, expected level should be 12. [Indentation]
[ERROR] /home/carbon14/gsoc/checkstyle/Test.java:13:51: 'synchronized' has incorrect indentation level 50, expected level should be one of the following: 20, 24. [Indentation]
[ERROR] /home/carbon14/gsoc/checkstyle/Test.java:14:55: 'synchronized' child has incorrect indentation level 54, expected level should be one of the following: 24, 28. [Indentation]
[ERROR] /home/carbon14/gsoc/checkstyle/Test.java:15:51: 'synchronized rcurly' has incorrect indentation level 50, expected level should be one of the following: 20, 24. [Indentation]
[ERROR] /home/carbon14/gsoc/checkstyle/Test.java:16:47: 'method def rcurly' has incorrect indentation level 46, expected level should be one of the following: 16, 20. [Indentation]
Audit done.
Checkstyle ends with 5 errors.

the first violation should not be reported right ?

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.

Please share output in same input but released version of Checkstyle

@romani romani May 6, 2026

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.

Let's add this input to tests, bit fix all violations except for line with @

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Please share output in same input but released version of Checkstyle

i fixed the input to have only the @ violation

import java.awt.Component;

public class Test {
    public static boolean doEquals(final Object a, final Object b, Component c) {
        if (a == b) return true;
        final boolean[] ret = new boolean[1];
        try {
            invokeAndWait(new Runnable() { @Override
                public void run() {
                    synchronized (ret) {
                        ret[0] = a.equals(b);
                    }
                }
            }, c);
        }
        catch (Exception e) {
            e.printStackTrace();
        }
        synchronized (ret) {
            return ret[0];
        }
    }

    private static void invokeAndWait(Runnable r, Object o) {
    }
}

with latest released we get :

     ~  > java -jar checkstyle-13.4.2-all.jar -c config.xml Test.java 
Starting audit...
[ERROR] /home/carbon14/Test.java:8:44: '@' has incorrect indentation level 43, expected level should be 12. [Indentation]
Audit done.
Checkstyle ends with 1 errors.

@Carbon14-48 Carbon14-48 force-pushed the issue-5685-fix-try-indentation branch from 36741ab to 752b989 Compare May 6, 2026 05:28

@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.

Ok. Let's move forward, this edge case is very rare and nobody writes code like this in modern world

@romani romani merged commit cfd6396 into checkstyle:master May 6, 2026
125 of 126 checks passed
@Carbon14-48 Carbon14-48 deleted the issue-5685-fix-try-indentation branch May 6, 2026 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Indentation: false positive for try child on the same line

2 participants