Add Errorprone and fix errors

pom.xml

@@ -38,6 +38,8 @@
     <exec-maven.version>3.1.1</exec-maven.version>
     <build-helper-maven-plugin.version>3.5.0</build-helper-maven-plugin.version>
     <auto-value.version>1.10.4</auto-value.version>
+    <error-prone.version>2.24.1</error-prone.version>
+    <maven-checkstyle.version>3.3.1</maven-checkstyle.version>
   </properties>
   <dependencies>
     <dependency>
@@ -108,14 +110,27 @@
     </pluginManagement>
     <plugins>
       <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-compiler-plugin</artifactId>
         <configuration>
+          <showWarnings>true</showWarnings>
+          <failOnWarning>true</failOnWarning>
+          <compilerArgs>
+            <arg>-Xlint:-options</arg>
+            <arg>-XDcompilePolicy=simple</arg>
+            <arg>-Xplugin:ErrorProne -XepExcludedPaths:${project.build.directory}/generated-sources/.* -XepDisableWarningsInGeneratedCode</arg>
+          </compilerArgs>
           <annotationProcessorPaths>
             <path>
               <groupId>com.google.auto.value</groupId>
               <artifactId>auto-value</artifactId>
               <version>${auto-value.version}</version>
             </path>
+            <path>
+              <groupId>com.google.errorprone</groupId>
+              <artifactId>error_prone_core</artifactId>
+              <version>${error-prone.version}</version>
+            </path>
           </annotationProcessorPaths>
         </configuration>
       </plugin>

src/main/java/com/google/cloud/solutions/spannerddl/diff/ASTTreeUtils.java

@@ -22,6 +22,7 @@
 import com.google.cloud.solutions.spannerddl.parser.Token;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Locale;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
@@ -88,7 +89,7 @@ public static String tokensToString(
     Token t = firstToken;
     while (t != lastToken) {
       String tok = t.toString();
-      sb.append(isReservedWord(tok) && upperCaseReserved ? tok.toUpperCase() : tok);
+      sb.append(isReservedWord(tok) && upperCaseReserved ? tok.toUpperCase(Locale.ROOT) : tok);
 
       if (t.next != null
           && !t.next.toString().equals(",")
@@ -100,7 +101,7 @@ public static String tokensToString(
     }
     // append last token
     String tok = t.toString();
-    sb.append(isReservedWord(tok) && upperCaseReserved ? tok.toUpperCase() : tok);
+    sb.append(isReservedWord(tok) && upperCaseReserved ? tok.toUpperCase(Locale.ROOT) : tok);
     return sb.toString();
   }
 

src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java

@@ -39,6 +39,7 @@
 import com.google.cloud.solutions.spannerddl.parser.SimpleNode;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
+import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -341,10 +342,10 @@ static List<String> generateAlterTableStatements(
     }
 
     if (left.getInterleaveClause().isPresent()
-        && !(left.getInterleaveClause()
+        && !left.getInterleaveClause()
             .get()
             .getParentTableName()
-            .equals(right.getInterleaveClause().get().getParentTableName()))) {
+            .equals(right.getInterleaveClause().get().getParentTableName())) {
       throw new DdlDiffException(
           "Cannot change interleaved parent of table " + left.getTableName());
     }
@@ -356,10 +357,10 @@ static List<String> generateAlterTableStatements(
 
     // On delete changed
     if (left.getInterleaveClause().isPresent()
-        && !(left.getInterleaveClause()
+        && !left.getInterleaveClause()
             .get()
             .getOnDelete()
-            .equals(right.getInterleaveClause().get().getOnDelete()))) {
+            .equals(right.getInterleaveClause().get().getOnDelete())) {
       alterStatements.add(
           "ALTER TABLE "
               + left.getTableName()
@@ -391,9 +392,7 @@ static List<String> generateAlterTableStatements(
   }
 
   private static void addColumnDiffs(
-      String tableName,
-      ArrayList<String> alterStatements,
-      ValueDifference<ASTcolumn_def> columnDiff)
+      String tableName, List<String> alterStatements, ValueDifference<ASTcolumn_def> columnDiff)
       throws DdlDiffException {
 
     // check for compatible type changes.
@@ -599,8 +598,8 @@ private static String getDatabaseNameFromAlterDatabase(List<ASTddl_statement> st
   @VisibleForTesting
   static List<ASTddl_statement> parseDDL(String original) throws DdlDiffException {
     // Remove "--" comments and split by ";"
-    String[] statements = original.replaceAll("--.*(\n|$)", "").split(";");
-    ArrayList<ASTddl_statement> ddlStatements = new ArrayList<>(statements.length);
+    List<String> statements = Splitter.on(';').splitToList(original.replaceAll("--.*(\n|$)", ""));
+    ArrayList<ASTddl_statement> ddlStatements = new ArrayList<>(statements.size());
 
     for (String statement : statements) {
       statement = statement.trim();
@@ -612,40 +611,38 @@ static List<ASTddl_statement> parseDDL(String original) throws DdlDiffException
         int statementType = ddlStatement.jjtGetChild(0).getId();
 
         switch (statementType) {
-          case (DdlParserTreeConstants.JJTALTER_TABLE_STATEMENT):
-            {
-              ASTalter_table_statement alterTableStatement =
-                  (ASTalter_table_statement) ddlStatement.jjtGetChild(0);
-              // child 0 = table name
-              // child 1 = alter statement. Only ASTforeign_key is supported
-              if (!(alterTableStatement.jjtGetChild(1) instanceof ASTforeign_key)
-                  && !(alterTableStatement.jjtGetChild(1) instanceof ASTcheck_constraint)
-                  && !(alterTableStatement.jjtGetChild(1) instanceof ASTadd_row_deletion_policy)) {
-                throw new IllegalArgumentException(
-                    "Unsupported statement:\n"
-                        + statement
-                        + "\n"
-                        + "Can only create diffs from 'CREATE TABLE, CREATE INDEX and 'ALTER TABLE"
-                        + " table_name ADD [constraint|row deletion policy]' DDL statements");
-              }
-              if (alterTableStatement.jjtGetChild(1) instanceof ASTforeign_key
-                  && ((ASTforeign_key) alterTableStatement.jjtGetChild(1))
-                      .getName()
-                      .equals(ASTcreate_table_statement.ANONYMOUS_NAME)) {
-                throw new IllegalArgumentException(
-                    "Unsupported statement:\n"
-                        + statement
-                        + "\nCan not create diffs when anonymous constraints are used.");
-              }
-              if (alterTableStatement.jjtGetChild(1) instanceof ASTcheck_constraint
-                  && ((ASTcheck_constraint) alterTableStatement.jjtGetChild(1))
-                      .getName()
-                      .equals(ASTcreate_table_statement.ANONYMOUS_NAME)) {
-                throw new IllegalArgumentException(
-                    "Unsupported statement:\n"
-                        + statement
-                        + "\nCan not create diffs when anonymous constraints are used.");
-              }
+          case DdlParserTreeConstants.JJTALTER_TABLE_STATEMENT:
+            ASTalter_table_statement alterTableStatement =
+                (ASTalter_table_statement) ddlStatement.jjtGetChild(0);
+            // child 0 = table name
+            // child 1 = alter statement. Only ASTforeign_key is supported
+            if (!(alterTableStatement.jjtGetChild(1) instanceof ASTforeign_key)
+                && !(alterTableStatement.jjtGetChild(1) instanceof ASTcheck_constraint)
+                && !(alterTableStatement.jjtGetChild(1) instanceof ASTadd_row_deletion_policy)) {
+              throw new IllegalArgumentException(
+                  "Unsupported statement:\n"
+                      + statement
+                      + "\n"
+                      + "Can only create diffs from 'CREATE TABLE, CREATE INDEX and 'ALTER TABLE"
+                      + " table_name ADD [constraint|row deletion policy]' DDL statements");
+            }
+            if (alterTableStatement.jjtGetChild(1) instanceof ASTforeign_key
+                && ((ASTforeign_key) alterTableStatement.jjtGetChild(1))
+                    .getName()
+                    .equals(ASTcreate_table_statement.ANONYMOUS_NAME)) {
+              throw new IllegalArgumentException(
+                  "Unsupported statement:\n"
+                      + statement
+                      + "\nCan not create diffs when anonymous constraints are used.");
+            }
+            if (alterTableStatement.jjtGetChild(1) instanceof ASTcheck_constraint
+                && ((ASTcheck_constraint) alterTableStatement.jjtGetChild(1))
+                    .getName()
+                    .equals(ASTcreate_table_statement.ANONYMOUS_NAME)) {
+              throw new IllegalArgumentException(
+                  "Unsupported statement:\n"
+                      + statement
+                      + "\nCan not create diffs when anonymous constraints are used.");
             }
             break;
           case DdlParserTreeConstants.JJTCREATE_TABLE_STATEMENT:
@@ -703,8 +700,10 @@ public static void main(String[] args) {
       System.exit(0);
     } catch (IOException e) {
       System.err.println("Cannot read DDL file: " + e);
+      System.exit(1);
     } catch (DdlDiffException e) {
-      e.printStackTrace();
+      System.err.println("Failed to generate a diff: " + e.getMessage());
+      System.exit(1);
     }
   }
 }
@@ -835,15 +834,15 @@ static DatbaseDefinition create(List<ASTddl_statement> statements) {
         ImmutableMap.copyOf(alterDatabaseOptions));
   }
 
-  abstract Map<String, ASTcreate_table_statement> tablesInCreationOrder();
+  abstract ImmutableMap<String, ASTcreate_table_statement> tablesInCreationOrder();
 
-  abstract Map<String, ASTcreate_index_statement> indexes();
+  abstract ImmutableMap<String, ASTcreate_index_statement> indexes();
 
-  abstract Map<String, ConstraintWrapper> constraints();
+  abstract ImmutableMap<String, ConstraintWrapper> constraints();
 
-  abstract Map<String, ASTrow_deletion_policy_clause> ttls();
+  abstract ImmutableMap<String, ASTrow_deletion_policy_clause> ttls();
 
-  abstract Map<String, ASTcreate_change_stream_statement> changeStreams();
+  abstract ImmutableMap<String, ASTcreate_change_stream_statement> changeStreams();
 
-  abstract Map<String, String> alterDatabaseOptions();
+  abstract ImmutableMap<String, String> alterDatabaseOptions();
 }

src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffOptions.java

@@ -15,14 +15,17 @@
  */
 package com.google.cloud.solutions.spannerddl.diff;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
 import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableMap;
+import java.io.BufferedWriter;
 import java.io.File;
+import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
 import java.nio.file.InvalidPathException;
 import java.nio.file.Path;
-import java.util.Map;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.DefaultParser;
 import org.apache.commons.cli.HelpFormatter;
@@ -40,7 +43,7 @@ public abstract class DdlDiffOptions {
 
   public abstract Path outputDdlPath();
 
-  public abstract Map<String, Boolean> args();
+  public abstract ImmutableMap<String, Boolean> args();
 
   @VisibleForTesting
   static Options buildOptions() {
@@ -93,40 +96,40 @@ static Options buildOptions() {
   }
 
   static void printHelpAndExit(int exitStatus) {
-    try (PrintWriter pw = new PrintWriter(System.err)) {
-      new HelpFormatter()
-          .printHelp(
-              pw,
-              132,
-              "DdlDiff",
-              "Compares original and new DDL files and creates a DDL file with DROP, CREATE and"
-                  + " ALTER statements which convert the original Schema to the new Schema.\n"
-                  + "\n"
-                  + "Incompatible table changes (table hierarchy changes. column type changes) are"
-                  + " not supported and will cause this tool to fail.\n"
-                  + "\n"
-                  + "To prevent accidental data loss, and to make it easier to apply DDL changes,"
-                  + " DROP statements are not generated for removed tables, columns, indexes and"
-                  + " change streams. This can be overridden using the "
-                  + DdlDiff.ALLOW_DROP_STATEMENTS_OPT
-                  + " command line argument.\n"
-                  + "\n"
-                  + "By default, changes to indexes will also cause a failure. The "
-                  + DdlDiff.ALLOW_RECREATE_INDEXES_OPT
-                  + " command line option enables index changes by"
-                  + " generating statements to drop and recreate the index.\n"
-                  + "\n"
-                  + "By default, changes to foreign key constraints will also cause a failure. The "
-                  + DdlDiff.ALLOW_RECREATE_CONSTRAINTS_OPT
-                  + " command line option enables foreign key changes by"
-                  + " generating statements to drop and recreate the constraint.\n"
-                  + "\n",
-              buildOptions(),
-              1,
-              4,
-              "",
-              true);
-    }
+    PrintWriter pw = new PrintWriter(new BufferedWriter(new OutputStreamWriter(System.err, UTF_8)));
+    new HelpFormatter()
+        .printHelp(
+            pw,
+            132,
+            "DdlDiff",
+            "Compares original and new DDL files and creates a DDL file with DROP, CREATE and"
+                + " ALTER statements which convert the original Schema to the new Schema.\n"
+                + "\n"
+                + "Incompatible table changes (table hierarchy changes. column type changes) are"
+                + " not supported and will cause this tool to fail.\n"
+                + "\n"
+                + "To prevent accidental data loss, and to make it easier to apply DDL changes,"
+                + " DROP statements are not generated for removed tables, columns, indexes and"
+                + " change streams. This can be overridden using the "
+                + DdlDiff.ALLOW_DROP_STATEMENTS_OPT
+                + " command line argument.\n"
+                + "\n"
+                + "By default, changes to indexes will also cause a failure. The "
+                + DdlDiff.ALLOW_RECREATE_INDEXES_OPT
+                + " command line option enables index changes by"
+                + " generating statements to drop and recreate the index.\n"
+                + "\n"
+                + "By default, changes to foreign key constraints will also cause a failure. The "
+                + DdlDiff.ALLOW_RECREATE_CONSTRAINTS_OPT
+                + " command line option enables foreign key changes by"
+                + " generating statements to drop and recreate the constraint.\n"
+                + "\n",
+            buildOptions(),
+            1,
+            4,
+            "",
+            true);
+    pw.flush();
     System.exit(exitStatus);
   }
 
@@ -143,7 +146,7 @@ public static DdlDiffOptions parseCommandLine(String[] args) {
       Path outputDdlPath =
           new File(commandLine.getOptionValue(DdlDiff.OUTPUT_DDL_FILE_OPT)).toPath();
 
-      Map<String, Boolean> argsMap =
+      ImmutableMap<String, Boolean> argsMap =
           ImmutableMap.of(
               DdlDiff.ALLOW_RECREATE_INDEXES_OPT,
                   commandLine.hasOption(DdlDiff.ALLOW_RECREATE_INDEXES_OPT),

src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTalter_database_statement.java

@@ -43,4 +43,9 @@ public String toString() {
   public boolean equals(Object other) {
     return other instanceof ASTalter_table_statement && this.toString().equals(other.toString());
   }
+
+  @Override
+  public int hashCode() {
+    return toString().hashCode();
+  }
 }

src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTalter_table_statement.java

@@ -59,4 +59,9 @@ public String toString() {
     }
     return ret.toString();
   }
+
+  @Override
+  public int hashCode() {
+    return toString().hashCode();
+  }
 }

src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTchange_stream_for_clause.java

@@ -35,4 +35,9 @@ public String toString() {
     }
     return "FOR ALL";
   }
+
+  @Override
+  public int hashCode() {
+    return toString().hashCode();
+  }
 }

src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcheck_constraint.java

@@ -41,9 +41,10 @@ public String getExpression() {
     if (children[0] instanceof ASTconstraint_name) {
       child++;
     }
-    return (ASTTreeUtils.tokensToString((ASTcheck_constraint_expression) children[child], false));
+    return ASTTreeUtils.tokensToString((ASTcheck_constraint_expression) children[child], false);
   }
 
+  @Override
   public String toString() {
     return "CONSTRAINT " + getName() + " CHECK (" + getExpression() + ")";
   }
@@ -55,4 +56,9 @@ public boolean equals(Object other) {
     }
     return false;
   }
+
+  @Override
+  public int hashCode() {
+    return toString().hashCode();
+  }
 }