Issue #14654: fix catch parameter wrong indentation #16627

mohitsatr · 2025-03-22T02:02:17Z

Diff Regression config: https://gist.githubusercontent.com/mohitsatr/d55485b980e5c7ae9c883ba8f79dcbd6/raw/c824102d90368dbd060f41a1f6bda249cdcf0a5b/master_google_style.xml

Diff Regression projects: https://gist.githubusercontent.com/mohitsatr/68b0817ac52ff7773d35585ee12fc1cc/raw/f9c7774ca78df1f48f34bf30799bd3952abd9e2a/list-of-projects.properties

mohitsatr · 2025-03-22T02:02:51Z

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

github-actions · 2025-03-22T03:13:53Z

Report for Indentation/all-examples-in-one:
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/8ead1b4bf20ecd48e8fb4a2f1302c657282911fe_2025031232/reports/diff/index.html

romani · 2025-03-22T14:20:05Z

do not forget to add code to ITs of google style, to let formatter do formatting on code and checkstyle run on same file.

checkstyle/.github/workflows/google-java-format.yml

Line 48 in d73922d

    
                     ./.ci/google-java-format.sh .ci-temp/google-java-format-${{env.VERSION}}-all-deps.jar

be aware of files that are excluded due to conflict of formatter and checkstyle:

checkstyle/.ci/google-java-format.sh

Line 45 in d73922d

| grep -v "rule42blockindentation/ClassWithChainedMethods.java" \

if you put a lot of samples to ITs of google it will be good prove that formatter and checkstyle validate same code and not conflicting with each other

mohitsatr · 2025-03-24T02:45:58Z

Github, generate report

mohitsatr · 2025-03-25T02:09:00Z

Github, generate report

github-actions · 2025-03-25T02:40:17Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/8ead1b4_2025024003/reports/diff/index.html

romani · 2025-03-27T13:12:41Z

@mohitsatr , is it still wip ?

mohitsatr · 2025-03-28T04:10:46Z

yes, I was busy researching for proposal. i will get to back to it as soon as Im done with that

mohitsatr · 2025-04-15T10:33:33Z

@Zopsss @romani ready for review

mohitsatr · 2025-04-15T10:33:51Z

Github, generate report

github-actions · 2025-04-15T11:17:04Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/81073fc_2025110852/reports/diff/index.html

mohitsatr · 2025-04-15T11:40:13Z

nevermind, I'm checking the diffs

mohitsatr · 2025-04-20T03:20:40Z

there are conflicts created by the google-formatter.

   public void test1() {
     try {
       System.out.println("try0");
     } catch (NullPointerException
-| IllegalArgumentException t) { // violation '.* incorrect indentation level 0, expected .* 8.'
+        | IllegalArgumentException
+            t) { // violation '.* incorrect indentation level 0, expected .* 8.'
     }
   }

even though I have specified wrong indentation in violation message, it thinks code's formatted wrong.

        System.out.println("try");
     } catch (
         @SuppressWarnings("PMD.AvoidCatchingGenericException")
-    Exception e) { // violation '.* incorrect indentation level 4, expected .* 8.'
+        Exception e) { // violation '.* incorrect indentation level 4, expected .* 8.'
       java.util.logging.Logger.getAnonymousLogger().severe(e.toString());
     }

src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/AbstractExpressionHandler.java

mohitsatr · 2025-04-20T04:04:51Z

@romani @Zopsss please review

romani · 2025-04-20T15:56:27Z

google formatter is more strict and opinionated.
Please apply patch from google formatter, our goal is to not put violation that what google formatter say is good.

in regular test area Inputs we can have any crazy indentations and see how Check place violation on it.

in IT for google style, we apply auto-formatter first, and must not produce any violation on what formatter did with code.
There might some defects in formatter also, and we should report to them and we will think what to do.

romani · 2025-04-22T13:15:29Z

formatter is still red, please fix.

mohitsatr · 2025-04-23T09:51:19Z

I suppose pitest one is not related to my changes?
can you please review the diff report
#16627 (comment)

Zopsss · 2025-04-24T03:17:41Z

Yes it is not related to your PR.

romani · 2025-04-26T15:55:31Z

from semaphore:
https://checkstyle.semaphoreci.com/jobs/a7fb1b76-14ec-4825-9c9d-30672bee1b69#L836

> Task :postgresql:checkstyleMain 04:22
[ant:checkstyle] [ERROR] /home/semaphore/checkstyle/.ci-temp/pgjdbc/pgjdbc/src/main/java/org/postgresql/util/PasswordUtil.java:69:9: 'try' child has incorrect indentation level 8, expected level should be 10. [Indentation] 04:22
[ant:checkstyle] [ERROR] /home/semaphore/checkstyle/.ci-temp/pgjdbc/pgjdbc/src/main/java/org/postgresql/util/PasswordUtil.java:70:9: 'try' child has incorrect indentation level 8, expected level should be 10. [Indentation] 04:22
[ant:checkstyle] [ERROR] /home/semaphore/checkstyle/.ci-temp/pgjdbc/pgjdbc/src/main/java/org/postgresql/util/PasswordUtil.java:71:9: 'try' child has incorrect indentation level 8, expected level should be 10. [Indentation] 04:22
[ant:checkstyle] [ERROR] /home/semaphore/checkstyle/.ci-temp/pgjdbc/pgjdbc/src/main/java/org/postgresql/util/PasswordUtil.java:72:9: 'try' child has incorrect indentation level 8, expected level should be 10. [Indentation]

if this is are not false positives, please send PR to pgjdbc to fix them to make CI green fully (after they merge).

execution from:

checkstyle/.ci/validation.sh

Lines 635 to 646 in c784535

    
           no-error-pgjdbc) 
        
             CS_POM_VERSION="$(getCheckstylePomVersion)" 
        
             echo CS_version: "${CS_POM_VERSION}" 
        
             mvn -e --no-transfer-progress clean install -Pno-validations 
        
             echo "Checkout target sources ..." 
        
             checkout_from https://github.com/pgjdbc/pgjdbc.git 
        
             cd .ci-temp/pgjdbc 
        
             ./gradlew --no-parallel --no-daemon checkstyleAll \ 
        
                       -PenableMavenLocal -Pcheckstyle.version="${CS_POM_VERSION}" 
        
             cd ../ 
        
             removeFolderWithProtectedFiles pgjdbc 
        
             ;;

target code looks ok:
https://github.com/pgjdbc/pgjdbc/blob/1e0c88d8bb292417f499d9a4821af67fc5740966/pgjdbc/src/main/java/org/postgresql/util/PasswordUtil.java#L60-L76
it looks like false positive, but it is actually good vioaltions, as it is line wrap on statement inside try not a try itself.

please sen PR to pgjdbc

sdavids · 2025-04-26T16:03:55Z

Just a small nitpick:

parameters is not spelled correctly in the commit message

From: checkstyle/checkstyle#16627 (comment)

mohitsatr · 2025-04-28T08:20:07Z

please sen PR to pgjdbc

done pgjdbc/pgjdbc#3611

github-actions · 2025-04-30T14:08:21Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/7d8f4ef_2025140756/reports/diff/index.html

romani · 2025-04-30T16:05:08Z

Ok, reports are same, let's extend Inputs and be prepared for merge

romani · 2025-05-01T12:20:45Z

@mohitsatr , CI is red, please address.

romani

Items

.../checkstyle/test/chapter4formatting/rule4841indentation/InputMultilineStatementsCorrect.java

mohitsatr · 2025-05-03T06:39:34Z

I don't understand why javac11 is failing even though javac17 has passed successfully.

romani · 2025-05-03T13:15:24Z

jdk version matters a lot, this syntax is for jdk17

src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/checks/indentation/indentation/InputIndentationMultilineStatements.java:56: error: unclosed string literal
    final String simpleProperty = """
                                    ^

romani

item:

...awl/tools/checkstyle/checks/indentation/indentation/InputIndentationMultilineStatements.java

romani

thanks a lot !!!

@Aziz-755, can you review this PR ?

Aziz-755

Great work!
Just minors

src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/CatchHandler.java

...awl/tools/checkstyle/checks/indentation/indentation/InputIndentationMultilineStatements.java

...ools/checkstyle/checks/indentation/indentation/InputIndentationCatchParametersOnNewLine.java

romani

Items

...awl/tools/checkstyle/checks/indentation/indentation/InputIndentationMultilineStatements.java

romani · 2025-05-07T12:08:39Z

Github, generate report

github-actions · 2025-05-07T12:39:57Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/f98ed1d_2025123917/reports/diff/index.html

romani

last items:

romani · 2025-05-07T13:04:34Z

...ools/checkstyle/checks/indentation/indentation/InputIndentationCatchParametersOnNewLine.java

+    try {                                                                     //indent:4 exp:4
+      System.out.println("try");                                              //indent:6 exp:6
+    } catch (                                                                 //indent:4 exp:4
+            @SuppressWarnings("PMD.AvoidCatchingGenericException")          //indent:12 exp:8 warn


please fix vertical alignment of trailing comment

done all.
just curious, can we somehow automate alignment of comments? it's quite tedious doing manually 🥲

romani · 2025-05-07T13:04:43Z

...ools/checkstyle/checks/indentation/indentation/InputIndentationCatchParametersOnNewLine.java

+    try {                                                                     //indent:4 exp:4
+      System.out.println("try0");                                             //indent:6 exp:6
+    } catch (NullPointerException                                             //indent:4 exp:4
+| IllegalArgumentException t) {                                              //indent:0 exp:8 warn


please fix vertical alignment of trailing comment

romani · 2025-05-07T13:04:53Z

...awl/tools/checkstyle/checks/indentation/indentation/InputIndentationMultilineStatements.java

@@ -0,0 +1,82 @@
+//non-compiled with javac: Compilable with Java17                            //indent:0 exp:0
+package com.puppycrawl.tools.checkstyle.checks.indentation.indentation;       //indent:0 exp:0


please fix vertical alignment of trailing comment

romani · 2025-05-07T13:05:08Z

...ools/checkstyle/checks/indentation/indentation/InputIndentationCatchParametersOnNewLine.java

+    try {                                                                     //indent:4 exp:4
+      System.out.println("try1");                                             //indent:6 exp:6
+    } catch (                                                                 //indent:4 exp:4
+    @SuppressWarnings("PMD.AvoidCatchingGenericException")                  //indent:4 exp:8 warn


please fix vertical alignment of trailing comment

romani · 2025-05-07T13:05:15Z

...ools/checkstyle/checks/indentation/indentation/InputIndentationCatchParametersOnNewLine.java

+      System.out.println("try");                                              //indent:6 exp:6
+    } catch (                                                                 //indent:4 exp:4
+        @SuppressWarnings("PMD.AvoidCatchingGenericException")                //indent:8 exp:8
+    Exception e) {                                                           //indent:4 exp:8 warn


please fix vertical alignment of trailing comment

romani · 2025-05-07T13:06:27Z

@Aziz-755 , please finalize review.

romani

last last:

romani · 2025-05-07T13:09:41Z

.../checkstyle/test/chapter4formatting/rule4841indentation/InputMultilineStatementsCorrect.java

+        1;
+  }
+
+  int testIfConditionMultiline(int newState, int tableName) { //indent:2 exp:2


please remove trailing comment

romani

diff report is good.
thanks a lot for your hard work!!!

Aziz-755 · 2025-05-07T13:46:27Z

Looks good !

… new line

romani

Thanks a lot!!!!

just curious, can we somehow automate alignment of comments? it's quite tedious doing manually 🥲

Please create issue on this, it should be trivial, we parsing this comment, so we definitely know it's position, but there might be exceptions.

mohitsatr force-pushed the indent-catch-parameter branch from 8ead1b4 to 81073fc Compare April 15, 2025 10:32

mohitsatr commented Apr 20, 2025

View reviewed changes

src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/AbstractExpressionHandler.java Show resolved Hide resolved

mohitsatr force-pushed the indent-catch-parameter branch from 81073fc to 193d44d Compare April 21, 2025 07:12

mohitsatr force-pushed the indent-catch-parameter branch 2 times, most recently from 262b27f to 501aa8d Compare April 23, 2025 08:06

romani force-pushed the indent-catch-parameter branch from 501aa8d to 7d8f4ef Compare April 26, 2025 15:53

mohitsatr added a commit to mohitsatr/pgjdbc that referenced this pull request Apr 28, 2025

fix indentation of return child to allow built pass in Checkstyle'CIs

949a067

From: checkstyle/checkstyle#16627 (comment)

This was referenced Apr 28, 2025

fix indentation of return child to allow built pass in Checkstyle'CIs mohitsatr/pgjdbc#1

Closed

fix indentation of return child to allow built pass in Checkstyle's CIs pgjdbc/pgjdbc#3611

Merged

mohitsatr force-pushed the indent-catch-parameter branch from 7d8f4ef to 0198e5f Compare May 1, 2025 04:51

mohitsatr force-pushed the indent-catch-parameter branch 2 times, most recently from 00f269a to aeb481c Compare May 3, 2025 03:31

romani requested changes May 3, 2025

View reviewed changes

.../checkstyle/test/chapter4formatting/rule4841indentation/InputMultilineStatementsCorrect.java Outdated Show resolved Hide resolved

.../checkstyle/test/chapter4formatting/rule4841indentation/InputMultilineStatementsCorrect.java Outdated Show resolved Hide resolved

mohitsatr force-pushed the indent-catch-parameter branch from aeb481c to 701b911 Compare May 3, 2025 05:56

romani requested changes May 3, 2025

View reviewed changes

...awl/tools/checkstyle/checks/indentation/indentation/InputIndentationMultilineStatements.java Outdated Show resolved Hide resolved

mohitsatr force-pushed the indent-catch-parameter branch from 701b911 to d39ee66 Compare May 3, 2025 13:47

romani approved these changes May 3, 2025

View reviewed changes

Aziz-755 suggested changes May 5, 2025

View reviewed changes

romani requested changes May 5, 2025

View reviewed changes

...awl/tools/checkstyle/checks/indentation/indentation/InputIndentationMultilineStatements.java Show resolved Hide resolved

mohitsatr force-pushed the indent-catch-parameter branch from d39ee66 to f98ed1d Compare May 7, 2025 03:47

romani requested changes May 7, 2025

View reviewed changes

romani reviewed May 7, 2025

View reviewed changes

Aziz-755 approved these changes May 7, 2025

View reviewed changes

Issue checkstyle#14654: fix wrong-indentation for catch parameters on…

048fc8f

… new line

mohitsatr force-pushed the indent-catch-parameter branch from f98ed1d to 048fc8f Compare May 8, 2025 02:56

romani approved these changes May 8, 2025

View reviewed changes

romani merged commit edbfc66 into checkstyle:master May 8, 2025
117 checks passed

mohitsatr deleted the indent-catch-parameter branch May 8, 2025 12:17

mohitsatr mentioned this pull request May 23, 2025

test to check indentation trailing comments are vertically aligned #17128

Open

		@@ -0,0 +1,82 @@
		//non-compiled with javac: Compilable with Java17 //indent:0 exp:0
		package com.puppycrawl.tools.checkstyle.checks.indentation.indentation; //indent:0 exp:0

Uh oh!

Issue #14654: fix catch parameter wrong indentation #16627

Issue #14654: fix catch parameter wrong indentation #16627

Uh oh!

Conversation

mohitsatr commented Mar 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mohitsatr commented Mar 22, 2025

Uh oh!

github-actions bot commented Mar 22, 2025

Uh oh!

romani commented Mar 22, 2025

Uh oh!

mohitsatr commented Mar 24, 2025

Uh oh!

mohitsatr commented Mar 25, 2025

Uh oh!

github-actions bot commented Mar 25, 2025

Uh oh!

romani commented Mar 27, 2025

Uh oh!

mohitsatr commented Mar 28, 2025

Uh oh!

mohitsatr commented Apr 15, 2025

Uh oh!

mohitsatr commented Apr 15, 2025

Uh oh!

github-actions bot commented Apr 15, 2025

Uh oh!

mohitsatr commented Apr 15, 2025

Uh oh!

mohitsatr commented Apr 20, 2025

Uh oh!

Uh oh!

mohitsatr commented Apr 20, 2025

Uh oh!

romani commented Apr 20, 2025

Uh oh!

romani commented Apr 22, 2025

Uh oh!

mohitsatr commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Zopsss commented Apr 24, 2025

Uh oh!

romani commented Apr 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sdavids commented Apr 26, 2025

Uh oh!

mohitsatr commented Apr 28, 2025

Uh oh!

github-actions bot commented Apr 30, 2025

Uh oh!

romani commented Apr 30, 2025

Uh oh!

romani commented May 1, 2025

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mohitsatr commented May 3, 2025

Uh oh!

romani commented May 3, 2025

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Aziz-755 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mohitsatr commented Mar 22, 2025 •

edited

Loading

mohitsatr commented Apr 23, 2025 •

edited

Loading

romani commented Apr 26, 2025 •

edited

Loading