[CALCITE-3559] Drop HydromaticFileSetCheck, upgrade Checkstyle

[vlsi]

Checklist

• ClosingComment -- dropped (== // End file.java)
• NewLineAtEndOfFile -- Was covered with Spotless
• SplitLink -- migrated to checker.xml / Regexp
• OverrideSameLine -- migrated to Spotless
• OrphanP -- migrated to Spotless
• ParameterWithoutDescription -- migrated to checker.xml / Regexp
• BadJiraReference
• OpenParenthesesLevel
• JavadocTooLong -- almost covered with existing LineLength=100
• Tabs -- was covered with Checkstyle

[julianhyde]

Please don't commit this without more discussion.

HydromaticFileSetCheck checks things that checkstyle doesn't, and can't. If we remove it, we have less code style coverage.

[vlsi]

HydromaticFileSetCheck checks things that checkstyle doesn't, and can't. If we remove it, we have less code style coverage.

You are right.
The suggested direction is to keep away from Checkstyle as it distracts developers.

Almost all the checks can be implemented without HydromaticFileSetCheck, so it is natural to drop it.

We can even live with a reduced set of rules for a while.

[vlsi]

@julianhyde , as you can see in the issue description, this PR covers 7 of 10 checks of HydromaticFileSetCheck.

I guess we can live without BadJiraReference and JavadocTooLong for a while.

So the only missing bit here is OpenParenthesesLevel, then PR can be committed.

[vlsi]

Showcase.

Input file with formatting violations:

public class Babel {
  // This class is currently a place-holder. Javadoc gets upset
  // if there are no classes in babel/java/main.
  public void test() {
    String a = test("str\"ing()", string2());

    Object expr4 =
        gt(
            qw(42, 43), eq(gt(x,
                y), 2));

    Object expr = gt(eq(x, 2 + (3 - 1)
      ), 5);

    Object expr2 = "(" + gt(eq(x, (3 - 2) + 2
      ), 6);

    Object expr3 =
        gt(
            qw(), eq(gt(x,
                y), 2));

  }

  private int x = 41;
  private int y = 42;


  private int qw() {
    return 0;
  }

  private int qw(int a, int b) {
    return 0;
  }

  private int gt(int a, int b) {
    return 0;
  }

  private int eq(int a, int b) {
    return 0;
  }

  private String string2() {
    return null;
  }

  private String test(String a, String b) {
    return null;
  }
}

Before

HydromaticFileSetCheck output:

[ant:checkstyle] [ERROR] /Users/vladimirsitnikov/Documents/code/calcite/babel/src/main/java/org/apache/calcite/sql/babel/Babel.java:29: Open parentheses exceed closes by 2 or more [HydromaticFileSet]
[ant:checkstyle] [ERROR] /Users/vladimirsitnikov/Documents/code/calcite/babel/src/main/java/org/apache/calcite/sql/babel/Babel.java:40: Open parentheses exceed closes by 2 or more [HydromaticFileSet]

In case you wonder, line 29 is qw(42, 43), eq(gt(x,
line 40 is qw(), eq(gt(x,

The exception messages are quite obscure. It is kind of hard to understand what the rule wants me to do.

After

After the drop of HydromaticFileSetCheck, the build suggests the modifications.

> The following files had format violations:
      babel/src/main/java/org/apache/calcite/sql/babel/Babel.java
          @@ -26,18 +26,24 @@

           ····Object·expr4·=
           ········gt(
          -············qw(42,·43),·eq(gt(x,
          +············qw(42,·43),
          +············eq(
          +················gt(x,
           ················y),·2));

          -····Object·expr·=·gt(eq(x,·2·+·(3·-·1)
          +····Object·expr·=·gt(
          +········eq(x,·2·+·(3·-·1)
           ······),·5);

          -····Object·expr2·=·"("·+·gt(eq(x,·(3·-·2)·+·2
          +····Object·expr2·=·"("·+·gt(
          +········eq(x,·(3·-·2)·+·2
           ······),·6);

           ····Object·expr3·=
           ········gt(
          -············qw(),·eq(gt(x,
          +············qw(),
          +············eq(
          +················gt(x,
           ················y),·2));

           ··}
  Run 'gradlew spotlessApply' to fix these violations.

[hsyuan]

LGTM