LeftCurlyCheck: An empty line should not be treated as a wrap

[romani]

from #6025 (comment)

https://checkstyle.org/config_blocks.html#LeftCurly
https://checkstyle.org/property_types.html#LeftCurlyOption

An empty line should not be treated as a wrap.
option=NLOW

an empty line implies wrap. More generally:

// option = NLOW
public class TestClass {

    public void testIf() {
      if (1 < 2)
      { // warn '{' at column 7 should be on the previous line
        
      }

      if (1 < 2)

      { // OK, blank line here
        
      }
    }

    public void testSwitch(int x) {
        switch (x) {
            case 0:
            { // warn '{' at column 13 should be on the previous line
            }
            case 0:

            { // OK, blank line here
                break;
            }
        }
    }
}

Also, a comment triggers a wrap:
openjdk10/make/src/classes/build/tools/cldrconverter/LDMLParseHandler.java.html#L267

        case "dayPeriodContext":
             // for FormatData
             // need to keep stand-alone and format, to allow for multiple inheritance in CLDR
             // for FormatData
             // need to keep stand-alone and format, to allow for multiple inheritance in CLDR
             {
                 String type = attributes.getValue("type");

spotbugs/spotbugs/src/main/java/edu/umd/cs/findbugs/OpcodeStack.java.html#L1562

            case Const.IF_ICMPGE:
 
             {
                 seenTransferOfControl = true;

[Gaurav-Punjabi]

I'm on it

[nrmancuso]

@romani just to be clear:

    public void testIf() {
        if (1 < 2)
        { // warn '{' at column 7 should be on the previous line

        }

        if (1 < 2)

        { // should be violation? (1)

        }
    }

    public void testSwitch(int x) {
        switch (x) {
            case 0:
            { // warn '{' at column 13 should be on the previous line
            }
            case 1:

            { // should be violation? (2)
                break;
            }

            case 2:
                // testing for wrap after comments
                // testing
                // testing
            { // should be violation? (3)
                break;
            }
        }
    }

All of these cases (1,2,3) should be violations, and not considered wrapped?

[romani]

// should be violation? (1)
// should be violation? (2)
// should be violation? (3)

no. because we have something (even empty line) between expression and {, we consider all after expression as continuation of expression.
If user did not mean it, he will put comments or empty lines inside {.

[nrmancuso]

➜  leftcurly git:(issue-6036-leftcurlycheck) ✗ javac InputLeftCurlyLineWrappingCommentsAndEmptyLines.java 
➜  leftcurly git:(issue-6036-leftcurlycheck) ✗ cat -n InputLeftCurlyLineWrappingCommentsAndEmptyLines.java 
     1  package com.puppycrawl.tools.checkstyle.checks.blocks.leftcurly;
     2  
     3  /*
     4   * Config:
     5   * LeftCurlyOption: NLOW
     6   */
     7  public class InputLeftCurlyLineWrappingCommentsAndEmptyLines {
     8  
     9      public void testIf() {
    10          if (1 < 2)
    11          { // warn '{' at column 7 should be on the previous line
    12  
    13          }
    14  
    15          if (1 < 2)
    16  
    17          { // should be violation? (1)
    18  
    19          }
    20      }
    21  
    22      public void testSwitch(int x) {
    23          switch (x) {
    24              case 0:
    25              { // warn '{' at column 13 should be on the previous line
    26              }
    27              case 1:
    28  
    29              { // should be violation? (2)
    30                  break;
    31              }
    32  
    33              case 2:
    34                  // testing for wrap after comments
    35                  // testing
    36                  // testing
    37              { // should be violation? (3)
    38                  break;
    39              }
    40          }
    41      }
    42  }

➜  leftcurly git:(issue-6036-leftcurlycheck) ✗ cat config.xml 
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
        "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
        "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>

    <module name="TreeWalker">
        <module name="LeftCurly">
            <property name="option" value="nlow"/>
        </module>
    </module>
</module>
➜  leftcurly git:(issue-6036-leftcurlycheck) ✗ java -jar /home/nick/IdeaProjects/checkstyle/target/checkstyle-8.38-SNAPSHOT-all.jar -c config.xml InputLeftCurlyLineWrappingCommentsAndEmptyLines.java
Starting audit...
[ERROR] InputLeftCurlyLineWrappingCommentsAndEmptyLines.java:11:9: '{' at column 9 should be on the previous line. [LeftCurly]
[ERROR] InputLeftCurlyLineWrappingCommentsAndEmptyLines.java:25:13: '{' at column 13 should be on the previous line. [LeftCurly]
Audit done.
Checkstyle ends with 2 errors.

(1), (2), (3) do not cause violations (latest master snapshot). Does this mean that there is no longer an issue, then?

[romani]

@pbludov ,
https://checkstyle.org/property_types.html#LeftCurlyOption

If the statement/expression/declaration connected to the brace spans multiple lines

so technically current behavior is correct, as extra empty line(or line with comment) is continuation of expression as curly brace is not yet detected.
Such cases are grey area ..... but I think what we have now is correct.

[pbludov]

@pbludov ,
so technically current behavior is correct, as extra empty line(or line with comment) is continuation of expression as curly brace is not yet detected.

I'm ok with current behaviour and ok to close this issue.