Update Static Analysis

.baseline/checkstyle/checkstyle.xml

@@ -8,6 +8,10 @@
     <property name="severity" value="error"/>
 
     <module name="FileTabCharacter"/> <!-- Java Style Guide: Whitespace characters -->
+    <module name="LineLength"> <!-- Java Style Guide: No line-wrapping -->
+        <property name="max" value="120"/>
+        <property name="ignorePattern" value="^package.*|^import.*|a href|href|http://|https://|ftp://"/>
+    </module>
     <module name="NewlineAtEndOfFile"> <!-- Java Style Guide: Line ending: LF -->
         <property name="lineSeparator" value="lf"/>
     </module>
@@ -212,10 +216,6 @@
         </module>
         <module name="InnerAssignment"/> <!-- Java Coding Guidelines: Inner assignments: Not used -->
         <module name="LeftCurly"/> <!-- Java Style Guide: Nonempty blocks: K & R style -->
-        <module name="LineLength"> <!-- Java Style Guide: No line-wrapping -->
-            <property name="max" value="120"/>
-            <property name="ignorePattern" value="^package.*|^import.*|a href|href|http://|https://|ftp://"/>
-        </module>
         <module name="MemberName"> <!-- Java Style Guide: Non-constant field names -->
             <property name="format" value="^[a-z][a-zA-Z0-9]+$"/>
             <message key="name.invalidPattern" value="Member name ''{0}'' must match pattern ''{1}''."/>
@@ -375,6 +375,10 @@
             <property name="format" value="serial"/>
         </module>
         <module name="SuppressWarningsHolder" />  <!-- Required for SuppressWarningsFilter -->
+        <module name="TodoComment"> <!-- Using TodoComment as a CommentRegex to find comments without a leading space-->
+            <property name="format" value="^\w"/>
+            <message key="todo.match" value="There must be whitespace at the beginning of all comments."/>
+        </module>
         <module name="TypeName"> <!-- Java Style Guide: Class names -->
             <message key="name.invalidPattern" value="Type name ''{0}'' must match pattern ''{1}''."/>
         </module>
@@ -409,12 +413,11 @@
         <module name="JavadocMethod"> <!-- Java Style Guide: Where Javadoc is used -->
             <property name="scope" value="public"/>
             <property name="allowMissingParamTags" value="true"/>
-            <property name="allowMissingThrowsTags" value="true"/>
             <property name="allowMissingReturnTag" value="true"/>
-            <property name="minLineCount" value="99999999"/>
             <property name="allowedAnnotations" value="Override, Test"/>
-            <property name="allowThrowsTagsForSubclasses" value="true"/>
+            <property name="validateThrows" value="false"/>
         </module>
+        <module name="JavadocMissingWhitespaceAfterAsterisk"/>
         <module name="JavadocStyle"> <!-- Java Style Guide: Javadoc -->
             <property name="checkFirstSentence" value="false"/>
         </module>

api/src/main/java/org/apache/iceberg/Files.java

@@ -34,7 +34,8 @@
 
 public class Files {
 
-  private Files() {}
+  private Files() {
+  }
 
   public static OutputFile localOutput(File file) {
     return new LocalOutputFile(file);

api/src/main/java/org/apache/iceberg/expressions/ExpressionVisitors.java

@@ -27,7 +27,8 @@
  */
 public class ExpressionVisitors {
 
-  private ExpressionVisitors() {}
+  private ExpressionVisitors() {
+  }
 
   public abstract static class ExpressionVisitor<R> {
     public R alwaysTrue() {

api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java

@@ -34,7 +34,8 @@
 
 class ProjectionUtil {
 
-  private ProjectionUtil() {}
+  private ProjectionUtil() {
+  }
 
   static <T> UnboundPredicate<T> truncateInteger(
       String name, BoundLiteralPredicate<Integer> pred, Transform<Integer, T> transform) {

api/src/main/java/org/apache/iceberg/transforms/TransformUtil.java

@@ -30,7 +30,8 @@
 
 class TransformUtil {
 
-  private TransformUtil() {}
+  private TransformUtil() {
+  }
 
   private static final OffsetDateTime EPOCH = Instant.ofEpochSecond(0).atOffset(ZoneOffset.UTC);
   private static final int EPOCH_YEAR = EPOCH.getYear();

api/src/main/java/org/apache/iceberg/types/Comparators.java

@@ -30,7 +30,8 @@
 
 public class Comparators {
 
-  private Comparators() {}
+  private Comparators() {
+  }
 
   private static final ImmutableMap<Type.PrimitiveType, Comparator<?>> COMPARATORS = ImmutableMap
       .<Type.PrimitiveType, Comparator<?>>builder()

api/src/main/java/org/apache/iceberg/types/Conversions.java

@@ -36,7 +36,8 @@
 
 public class Conversions {
 
-  private Conversions() {}
+  private Conversions() {
+  }
 
   private static final String HIVE_NULL = "__HIVE_DEFAULT_PARTITION__";
 

api/src/main/java/org/apache/iceberg/types/TypeUtil.java

@@ -37,7 +37,8 @@
 
 public class TypeUtil {
 
-  private TypeUtil() {}
+  private TypeUtil() {
+  }
 
   public static Schema select(Schema schema, Set<Integer> fieldIds) {
     Preconditions.checkNotNull(schema, "Schema cannot be null");

api/src/main/java/org/apache/iceberg/types/Types.java

@@ -36,7 +36,8 @@
 
 public class Types {
 
-  private Types() {}
+  private Types() {
+  }
 
   private static final ImmutableMap<String, PrimitiveType> TYPES = ImmutableMap
       .<String, PrimitiveType>builder()

api/src/test/java/org/apache/iceberg/AssertHelpers.java

@@ -24,7 +24,8 @@
 
 public class AssertHelpers {
 
-  private AssertHelpers() {}
+  private AssertHelpers() {
+  }
 
   /**
    * A convenience method to avoid a large number of @Test(expected=...) tests

api/src/test/java/org/apache/iceberg/TestHelpers.java

@@ -37,7 +37,8 @@
 
 public class TestHelpers {
 
-  private TestHelpers() {}
+  private TestHelpers() {
+  }
 
   public static <T> T assertAndUnwrap(Expression expr, Class<T> expected) {
     Assert.assertTrue("Expression should have expected type: " + expected,

arrow/src/main/java/org/apache/iceberg/arrow/ArrowSchemaUtil.java

@@ -42,7 +42,8 @@ public class ArrowSchemaUtil {
   private static final String ORIGINAL_TYPE = "originalType";
   private static final String MAP_TYPE = "mapType";
 
-  private ArrowSchemaUtil() { }
+  private ArrowSchemaUtil() {
+  }
 
   /**
    * Convert Iceberg schema to Arrow Schema.

arrow/src/main/java/org/apache/iceberg/arrow/vectorized/VectorizedArrowReader.java

@@ -186,7 +186,7 @@ private void allocateFieldVector(boolean dictionaryEncodedVector) {
           case UTF8:
           case BSON:
             this.vec = arrowField.createVector(rootAlloc);
-            //TODO: Possibly use the uncompressed page size info to set the initial capacity
+            // TODO: Possibly use the uncompressed page size info to set the initial capacity
             vec.setInitialCapacity(batchSize * AVERAGE_VARIABLE_WIDTH_RECORD_SIZE);
             vec.allocateNewSafe();
             this.readType = ReadType.VARCHAR;
@@ -262,7 +262,7 @@ private void allocateFieldVector(boolean dictionaryEncodedVector) {
             break;
           case BINARY:
             this.vec = arrowField.createVector(rootAlloc);
-            //TODO: Possibly use the uncompressed page size info to set the initial capacity
+            // TODO: Possibly use the uncompressed page size info to set the initial capacity
             vec.setInitialCapacity(batchSize * AVERAGE_VARIABLE_WIDTH_RECORD_SIZE);
             vec.allocateNewSafe();
             this.readType = ReadType.VARBINARY;
@@ -347,7 +347,8 @@ public String toString() {
     }
 
     @Override
-    public void setBatchSize(int batchSize) {}
+    public void setBatchSize(int batchSize) {
+    }
   }
 
   /**
@@ -377,8 +378,8 @@ public String toString() {
     }
 
     @Override
-    public void setBatchSize(int batchSize) {}
-
+    public void setBatchSize(int batchSize) {
+    }
   }
 
 }

baseline.gradle

@@ -61,7 +61,7 @@ subprojects {
 
   pluginManager.withPlugin('com.palantir.baseline-error-prone') {
     tasks.withType(JavaCompile).configureEach {
-      options.errorprone.errorproneArgs += [
+      options.errorprone.errorproneArgs.addAll (
           // error-prone is slow, don't run on tests
           '-XepExcludedPaths:.*/test/.*',
           // specific to Palantir
@@ -71,11 +71,12 @@ subprojects {
           // subclasses are not equal
           '-Xep:EqualsGetClass:OFF',
           // patterns that are allowed
-          '-Xep:SwitchStatementDefaultCase:OFF',
           '-Xep:MissingCasesInEnumSwitch:OFF',
+          //Added because it errors out compile, but we need to figure out if we want it
+          '-Xep:StrictUnusedVariable:OFF',
           '-Xep:TypeParameterShadowing:OFF',
           '-Xep:TypeParameterUnusedInFormals:OFF',
-        ]
+      )
     }
   }
 }

build.gradle

@@ -28,15 +28,15 @@ buildscript {
   }
   dependencies {
     classpath 'com.github.jengelman.gradle.plugins:shadow:5.0.0'
-    classpath 'com.palantir.baseline:gradle-baseline-java:0.55.0'
+    classpath 'com.palantir.baseline:gradle-baseline-java:3.36.2'
     classpath 'com.diffplug.spotless:spotless-plugin-gradle:3.14.0'
     classpath 'gradle.plugin.org.inferred:gradle-processors:2.1.0'
     classpath 'me.champeau.gradle:jmh-gradle-plugin:0.4.8'
   }
 }
 
 plugins {
-  id 'com.palantir.git-version' version '0.9.1'
+  id 'com.palantir.git-version' version '0.12.3'
   id 'nebula.dependency-recommender' version '9.0.2'
 }
 

bundled-guava/src/main/java/org/apache/iceberg/GuavaClasses.java

@@ -51,7 +51,7 @@
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
 
-//inspired in part by https://github.com/apache/avro/blob/release-1.8.2/lang/java/guava/src/main/java/org/apache/avro/GuavaClasses.java
+// inspired in part by https://github.com/apache/avro/blob/release-1.8.2/lang/java/guava/src/main/java/org/apache/avro/GuavaClasses.java
 public class GuavaClasses {
 
   /*

common/src/main/java/org/apache/iceberg/common/DynClasses.java

@@ -25,7 +25,8 @@
 
 public class DynClasses {
 
-  private DynClasses() {}
+  private DynClasses() {
+  }
 
   public static Builder builder() {
     return new Builder();

common/src/main/java/org/apache/iceberg/common/DynConstructors.java

@@ -35,7 +35,8 @@
  */
 public class DynConstructors {
 
-  private DynConstructors() {}
+  private DynConstructors() {
+  }
 
   public static class Ctor<C> extends DynMethods.UnboundMethod {
     private final Constructor<C> ctor;

common/src/main/java/org/apache/iceberg/common/DynFields.java

@@ -34,7 +34,8 @@
 
 public class DynFields {
 
-  private DynFields() {}
+  private DynFields() {
+  }
 
   /**
    * Convenience wrapper class around {@link java.lang.reflect.Field}.

common/src/main/java/org/apache/iceberg/common/DynMethods.java

@@ -34,7 +34,8 @@
  */
 public class DynMethods {
 
-  private DynMethods() {}
+  private DynMethods() {
+  }
 
   /**
    * Convenience wrapper class around {@link java.lang.reflect.Method}.

core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java

@@ -51,7 +51,8 @@ public abstract class BaseMetastoreTableOperations implements TableOperations {
   private boolean shouldRefresh = true;
   private int version = -1;
 
-  protected BaseMetastoreTableOperations() { }
+  protected BaseMetastoreTableOperations() {
+  }
 
   @Override
   public TableMetadata current() {

core/src/main/java/org/apache/iceberg/BaseRewriteManifests.java

@@ -207,8 +207,8 @@ private void keepActiveManifests(List<ManifestFile> currentManifests) {
     // keep any existing manifests as-is that were not processed
     keptManifests.clear();
     currentManifests.stream()
-      .filter(manifest -> !rewrittenManifests.contains(manifest) && !deletedManifests.contains(manifest))
-      .forEach(manifest -> keptManifests.add(manifest));
+        .filter(manifest -> !rewrittenManifests.contains(manifest) && !deletedManifests.contains(manifest))
+        .forEach(manifest -> keptManifests.add(manifest));
   }
 
   private void reset() {

core/src/main/java/org/apache/iceberg/DataFiles.java

@@ -35,7 +35,8 @@
 
 public class DataFiles {
 
-  private DataFiles() {}
+  private DataFiles() {
+  }
 
   static PartitionData newPartitionData(PartitionSpec spec) {
     return new PartitionData(spec.partitionType());

core/src/main/java/org/apache/iceberg/IncrementalDataTableScan.java

@@ -68,7 +68,7 @@ public TableScan appendsAfter(long newFromSnapshotId) {
 
   @Override
   public CloseableIterable<FileScanTask> planFiles() {
-    //TODO publish an incremental appends scan event
+    // TODO publish an incremental appends scan event
     List<Snapshot> snapshots = snapshotsWithin(table(),
         context().fromSnapshotId(), context().toSnapshotId());
     Set<Long> snapshotIds = Sets.newHashSet(Iterables.transform(snapshots, Snapshot::snapshotId));

core/src/main/java/org/apache/iceberg/InheritableMetadataFactory.java

@@ -25,7 +25,8 @@ class InheritableMetadataFactory {
 
   private static final InheritableMetadata EMPTY = new EmptyInheritableMetadata();
 
-  private InheritableMetadataFactory() {}
+  private InheritableMetadataFactory() {
+  }
 
   static InheritableMetadata empty() {
     return EMPTY;
@@ -84,7 +85,8 @@ public <F extends ContentFile<F>> ManifestEntry<F> apply(ManifestEntry<F> manife
 
   static class EmptyInheritableMetadata implements InheritableMetadata {
 
-    private EmptyInheritableMetadata() {}
+    private EmptyInheritableMetadata() {
+    }
 
     @Override
     public <F extends ContentFile<F>> ManifestEntry<F> apply(ManifestEntry<F> manifestEntry) {

core/src/main/java/org/apache/iceberg/LocationProviders.java

@@ -32,7 +32,8 @@
 
 public class LocationProviders {
 
-  private LocationProviders() {}
+  private LocationProviders() {
+  }
 
   public static LocationProvider locationsFor(String location, Map<String, String> properties) {
     if (PropertyUtil.propertyAsBoolean(properties,

core/src/main/java/org/apache/iceberg/MetricsConfig.java

@@ -36,7 +36,8 @@ public class MetricsConfig implements Serializable {
   private Map<String, MetricsMode> columnModes = Maps.newHashMap();
   private MetricsMode defaultMode;
 
-  private MetricsConfig() {}
+  private MetricsConfig() {
+  }
 
   public static MetricsConfig getDefault() {
     MetricsConfig spec = new MetricsConfig();

core/src/main/java/org/apache/iceberg/MetricsModes.java

@@ -35,7 +35,8 @@ public class MetricsModes {
 
   private static final Pattern TRUNCATE = Pattern.compile("truncate\\((\\d+)\\)");
 
-  private MetricsModes() {}
+  private MetricsModes() {
+  }
 
   public static MetricsMode fromString(String mode) {
     if ("none".equalsIgnoreCase(mode)) {
@@ -55,7 +56,8 @@ public static MetricsMode fromString(String mode) {
     throw new IllegalArgumentException("Invalid metrics mode: " + mode);
   }
 
-  public interface MetricsMode extends Serializable {}
+  public interface MetricsMode extends Serializable {
+  }
 
   /**
    * Under this mode, value_counts, null_value_counts, lower_bounds, upper_bounds are not persisted.

core/src/main/java/org/apache/iceberg/SchemaParser.java

@@ -36,7 +36,8 @@
 
 public class SchemaParser {
 
-  private SchemaParser() {}
+  private SchemaParser() {
+  }
 
   private static final String TYPE = "type";
   private static final String STRUCT = "struct";