Indentation incorrect level for chained method with bracket on new line

[ati90ati]

Hello,
https://checkstyle.org/config_misc.html#Indentation
I'm using checkstyle 8.14 and I have a problem with Indentation check.

/var/tmp $ cat config.xml

        <module name="Indentation">
            <property name="lineWrappingIndentation" value="8"/> <!-- default is 4 -->
        </module>

/var/tmp $ cat MyClass.java

        assertThat(result).hasSize(2)
                .extracting(
                        MyDto::getField1,
                        MyDto::getField2,
                        MyDto::getField3)
                .containsOnly(     // <- this is at level 16
                        tuple(ID_1, expectedField2, expectedField3),
                        tuple(ID_2, expectedField2, expectedField3)
                );          // <- 'method call rparen' has incorrect indentation level 16, expected level should be 8. [Indentation]     <<-- line 77

/var/tmp $ RUN_LOCALE="-Duser.language=en -Duser.country=US"
/var/tmp $ java $RUN_LOCALE -jar checkstyle-X.XX-all.jar -c config.xml YOUR_FILE.java

[ant:checkstyle] [ERROR] .../MyClass.java:77: 'method call rparen' has incorrect indentation level 16, expected level should be 8. [Indentation]

---

For chained methods I expect that line 77 should start at level 16 but the checkstyle reports that it should start at 8.
Maybe a fix can be added for this. Or a different parameter if needed. Or some parameter to disable these kind of checks (tokens) and allow only indentation for others.

NOTE: assertThat is from assertj-core library.
NOTE 2: at this moment because of this bug the Indentation check is completely unusable (because we even cannot disable this rparen check).

[rnveach]

@ati90ati Please give us full code and configuration so we can reproduce issue without problem.
See https://checkstyle.org/report_issue.html#How_to_report_a_bug.3F

[johndaniels]

Hey there. I made a github repo to demonstrate this.

https://github.com/johndaniels/checkstyle-bug

I tried to make the config and code as minimal as possible. Let me know if that's specific enough.

[rnveach]

Case from repo is:

$ cat TestClass.java
package indentation;

public class Indentation {

  public Indentation indentation(Object... args) {
    return this;
  }

  public static void main(String[] args) {
    Indentation i = new Indentation();
    i.indentation()
        .indentation(
            "a",
            "b"
        );
  }
}

$ cat TestConfig.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">
    <property name="charset" value="UTF-8"/>

    <property name="severity" value="error"/>

    <property name="fileExtensions" value="java, properties, xml"/>

    <module name="TreeWalker">
        <module name="Indentation">
            <property name="basicOffset" value="2"/>
            <property name="braceAdjustment" value="0"/>
            <property name="caseIndent" value="2"/>
            <property name="throwsIndent" value="4"/>
            <property name="lineWrappingIndentation" value="4"/>
            <property name="arrayInitIndent" value="2"/>
        </module>
    </module>
</module>

$ java -jar checkstyle-8.17-all.jar -c TestConfig.xml TestClass.java
Starting audit...
[ERROR] TestClass.java:15: 'method call rparen' has incorrect indentation level 8, expected level should be 6. [Indentation]
Audit done.
Checkstyle ends with 1 errors.

[johndaniels]

Sorry about that. Thanks for putting it in the more proper format!

[davidalayachew]

Just poking the thread to see if there has been any updates.

For those who just don't want to deal with the error (like me), well, Indentation is more a readability issue in Java than anything else. If it's going to clog up my list of errors, better to simply remove the module until the fix is released. You can do so by ctrl+f "Indentation", and removing that line and it's property name's from the xml file.

EDIT - Finally got a chance to get started. I will post any progress or discoveries made here - #6210 (comment)

[rnveach]

@dalayach No updates or activity. Anyone is free to help with a fix.

[davidalayachew]

I will do my best. I'm still in school, so forgive me if it takes forever.

EDIT - Discoveries and changes made will be posted here.

EDIT 1 - I've already mentioned before that simply disabling the check will do the job. Another option that seems to work is the following.

/** Blah. */
public class Issue_with_Checkstyle_Indentation
{
   /** Blah. */
   public Issue_with_Checkstyle_Indentation()
   {
   
      String no_problem = "";
      String problem = "";
      
      no_problem = // THIS SEEMS TO WORK BECAUSE OF THE = SIGN
         do_nothing(
            problem
               .concat("zyx"
               )
               .concat("255, 254, 253"
               )
         ); 
                     
      do_nothing( // I WOULD ASSUME THAT IS WHY THIS FAILS
         problem
            .concat("zyx"
            )
            .concat("255, 254, 253"
            )
         ); 
   
   }
   
   /** blah.
    *  @param something blah
    *  @return something blah
    */
   public String do_nothing(String something)
   { 
      return something;
   }

   /** BLAH. 
    * @param args blah
    */
   public static void main(String[] args)
   { Issue_with_Checkstyle_Indentation iwci = new Issue_with_Checkstyle_Indentation(); }

}

I know this doesn't help much, but on my way to understanding and simplifying the problem, I figured this would help other people out.

Again, it seems that whatever the problem is, it will likely go away as long as your chain of methods is changing the value of something via = sign.

This also means that whatever rule that allows checkstyle to ignore when there is an equal sign vs. where there is not is probably the same rule we are going to use to fix the problem.

EDIT 2 - I think I've identified the source. Still don't know WHAT the bug is, but I have a good idea where it is. If you download the source files, open up the source file folder and look at these files in this order (since the next files was called/extended by the file(s) before it)...

1. src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/IndentationCheck.java (method at line 314)
2. src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/PrimordialHandler.java (the whole class)
3. src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/AbstractExpressionHandler.java (look at methods at 113, 125, 263, 315, and 545 -- ESPECIALLY 545)

This should isolate the problem to only the relevant portions.

EDIT 3 - just to be SURE I identified the source, I used grep to find all files (within the indentation directory) that contained rparen (the crux of the issue, if I'm not mistaken). The only files that returned anything were...

1. src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/AbstractExpressionHandler.java
2. src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/MethodCallHandler.java
3. src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/TryHandler.java

Based off of this, it's safe to assume that we should look at these files too if we want to kill this bug for good. Not to say the other files contain the bug, but that if they did, it would likely be because they are implemented similarly to the way that resulted in the bug listed in OP. No guarantees though.

I should clarify though, the main focus is AbstractExpressionHandler.java. This edit is merely to say we likely won't be done after solving the issue in AbstractExpressionHandler.java.

[romani]

@dalayach , thanks a lot for desire to help, just do code changes and provide Pull request. We will guide you further.

[alabotski]

8.21 I have the some problems

[romani]

Fix is merged.
@alinkov , thanks a lot for your help.

[vilppuvuorinen]

The fix in 8.29 works only if the first line does not contain method chaining. I know, the examples are horrible.

Following snippets use <property name="lineWrappingIndentation" value="8"/>.

This case is fine:

Arrays.asList("a", "b", "c")
                .stream()
                .map(String::toUpperCase)
                .forEach(
                        System.out::println
                );

In this case we get flashbacks from 8.28:

Arrays.asList("a", "b", "c").stream()
                .map(String::toUpperCase)
                .forEach(
                        System.out::println
                );

Accepted formatting is:

Arrays.asList("a", "b", "c").stream()
                .map(String::toUpperCase)
                .forEach(
                        System.out::println
);

[romani]

@vilppuvuorinen, please open new issue with all details.