CommentsIndentation Check incorrect work with subsequent comments #3166

Closed
Vladlis opened this Issue May 10, 2016 · 2 comments

Comments

Projects
None yet
2 participants
@Vladlis
Member

Vladlis commented May 10, 2016

Config:

<module name = "Checker">
    <module name="TreeWalker">
         <module name="CommentsIndentation"/>
    </module>
</module>

1.
Input:

public class Test {
    void foo() {
        hashCode();
            // comment - line 4
          // comment - line 5
        hashCode();
    }
}

Result:

vlad@vlad-N61PC-M2S ~/cli $ java -jar checkstyle-6.18-all.jar -c my_check.xml Test.java
Starting audit...
[ERROR] /home/vlad/cli/Test.java:4: Comment has incorrect indentation level 12, expected is 10, indentation should be the same level as line 5. [CommentsIndentation]
[ERROR] /home/vlad/cli/Test.java:5: Comment has incorrect indentation level 10, expected is 8, indentation should be the same level as line 6. [CommentsIndentation]
Audit done.
Checkstyle ends with 2 errors.

Expected: in both cases indentation should be the same level as line 3 or line 6.

2.
Input:

public class Test {
    void foo() {
        hashCode();
            // comment - line 4
          // violation - line 5
    }
}

Result:

vlad@vlad-N61PC-M2S ~/cli $ java -jar checkstyle-6.18-all.jar -c my_check.xml Test.java
Starting audit...
[ERROR] /home/vlad/cli/Test.java:4: Comment has incorrect indentation level 12, expected is 10, indentation should be the same level as line 5. [CommentsIndentation]
Audit done.
Checkstyle ends with 1 errors.

Expected: in both cases indentation should be the same level as line 3.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 16, 2016

Member

Looks like we have problem with specially indented comments.
One day users will report this as a problem, is some cases it do make sense.

     private String actions; // Left null as long as possible, then
                             // created and re-used in the getAction function.

     // hostname part as it is passed
     private transient String hostname;

and

       String tableName = TABLENAME;// Request a table from
                               // an
                               // empty pool

and

      ConnectionFactoryBuilder builder = new ConnectionFactoryBuilder()
          .setOpTimeout(opTimeout)
          .setOpQueueMaxBlockTime(queueTimeout) // Cap the max time before anything times out
          .setFailureMode(FailureMode.Redistribute)
          .setShouldOptimize(true)              // When regions move lots of reads happen together
                                                // So combining them into single requests is nice.
          .setDaemon(true)                      // Don't keep threads around past the end of days.
          .setUseNagleAlgorithm(false)          // Ain't nobody got time for that
          .setReadBufferSize(HConstants.DEFAULT_BLOCKSIZE * 4 * 1024);  // 4 times larger than the
                                                                        // default block just in case
Member

romani commented Jun 16, 2016

Looks like we have problem with specially indented comments.
One day users will report this as a problem, is some cases it do make sense.

     private String actions; // Left null as long as possible, then
                             // created and re-used in the getAction function.

     // hostname part as it is passed
     private transient String hostname;

and

       String tableName = TABLENAME;// Request a table from
                               // an
                               // empty pool

and

      ConnectionFactoryBuilder builder = new ConnectionFactoryBuilder()
          .setOpTimeout(opTimeout)
          .setOpQueueMaxBlockTime(queueTimeout) // Cap the max time before anything times out
          .setFailureMode(FailureMode.Redistribute)
          .setShouldOptimize(true)              // When regions move lots of reads happen together
                                                // So combining them into single requests is nice.
          .setDaemon(true)                      // Don't keep threads around past the end of days.
          .setUseNagleAlgorithm(false)          // Ain't nobody got time for that
          .setReadBufferSize(HConstants.DEFAULT_BLOCKSIZE * 4 * 1024);  // 4 times larger than the
                                                                        // default block just in case

romani added a commit that referenced this issue Jun 16, 2016

@romani romani added the bug label Jun 16, 2016

@romani romani added this to the 7.0 milestone Jun 16, 2016

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 16, 2016

Member

fix is merged

Member

romani commented Jun 16, 2016

fix is merged

@romani romani closed this Jun 16, 2016

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