[SPARK-22066][BUILD] Update checkstyle to 8.2, enable it, fix violations

assembly/pom.xml

@@ -187,7 +187,7 @@
           <plugin>
             <groupId>org.apache.maven.plugins</groupId>
             <artifactId>maven-assembly-plugin</artifactId>
-            <version>3.0.0</version>
+            <version>3.1.0</version>
             <executions>
               <execution>
                 <id>dist</id>

common/network-common/src/main/java/org/apache/spark/network/util/ByteUnit.java

@@ -17,12 +17,12 @@
 package org.apache.spark.network.util;
 
 public enum ByteUnit {
-  BYTE (1),
-  KiB (1024L),
-  MiB ((long) Math.pow(1024L, 2L)),
-  GiB ((long) Math.pow(1024L, 3L)),
-  TiB ((long) Math.pow(1024L, 4L)),
-  PiB ((long) Math.pow(1024L, 5L));
+  BYTE(1),
+  KiB(1024L),
+  MiB((long) Math.pow(1024L, 2L)),
+  GiB((long) Math.pow(1024L, 3L)),
+  TiB((long) Math.pow(1024L, 4L)),
+  PiB((long) Math.pow(1024L, 5L));
 
   ByteUnit(long multiplier) {
     this.multiplier = multiplier;

common/network-common/src/main/java/org/apache/spark/network/util/NettyMemoryMetrics.java

@@ -44,7 +44,7 @@ public class NettyMemoryMetrics implements MetricSet {
   private final String metricPrefix;
 
   @VisibleForTesting
-  final static Set<String> VERBOSE_METRICS = new HashSet<>();
+  static final Set<String> VERBOSE_METRICS = new HashSet<>();
   static {
     VERBOSE_METRICS.addAll(Arrays.asList(
       "numAllocations",

common/sketch/pom.xml

@@ -56,20 +56,17 @@
   <build>
     <outputDirectory>target/scala-${scala.binary.version}/classes</outputDirectory>
     <testOutputDirectory>target/scala-${scala.binary.version}/test-classes</testOutputDirectory>
-    <pluginManagement>
-      <plugins>
-        <plugin>
-          <groupId>net.alchim31.maven</groupId>
-          <artifactId>scala-maven-plugin</artifactId>
-          <version>3.2.2</version>
-          <configuration>
-            <javacArgs combine.children="append">
-              <!-- This option is needed to suppress warnings from sun.misc.Unsafe usage -->
-              <javacArg>-XDignore.symbol.file</javacArg>
-            </javacArgs>
-          </configuration>
-        </plugin>
-      </plugins>
-    </pluginManagement>
+    <plugins>
+      <plugin>
+        <groupId>net.alchim31.maven</groupId>
+        <artifactId>scala-maven-plugin</artifactId>
+        <configuration>
+          <javacArgs combine.children="append">
+            <!-- This option is needed to suppress warnings from sun.misc.Unsafe usage -->
+            <javacArg>-XDignore.symbol.file</javacArg>
+          </javacArgs>
+        </configuration>
+      </plugin>
+    </plugins>
   </build>
 </project>

common/unsafe/pom.xml

@@ -93,20 +93,17 @@
   <build>
     <outputDirectory>target/scala-${scala.binary.version}/classes</outputDirectory>
     <testOutputDirectory>target/scala-${scala.binary.version}/test-classes</testOutputDirectory>
-    <pluginManagement>
-      <plugins>
-        <plugin>
-          <groupId>net.alchim31.maven</groupId>
-          <artifactId>scala-maven-plugin</artifactId>
-          <version>3.2.2</version>
-          <configuration>
-            <javacArgs combine.children="append">
-              <!-- This option is needed to suppress warnings from sun.misc.Unsafe usage -->
-              <javacArg>-XDignore.symbol.file</javacArg>
-            </javacArgs>
-          </configuration>
-        </plugin>
-      </plugins>
-    </pluginManagement>
+    <plugins>
+      <plugin>
+        <groupId>net.alchim31.maven</groupId>
+        <artifactId>scala-maven-plugin</artifactId>
+        <configuration>
+          <javacArgs combine.children="append">
+            <!-- This option is needed to suppress warnings from sun.misc.Unsafe usage -->
+            <javacArg>-XDignore.symbol.file</javacArg>
+          </javacArgs>
+        </configuration>
+      </plugin>
+    </plugins>
   </build>
 </project>

common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java

@@ -513,9 +513,9 @@ public UTF8String trim() {
 
   /**
    * Based on the given trim string, trim this string starting from both ends
-   * This method searches for each character in the source string, removes the character if it is found
-   * in the trim string, stops at the first not found. It calls the trimLeft first, then trimRight.
-   * It returns a new string in which both ends trim characters have been removed.
+   * This method searches for each character in the source string, removes the character if it is
+   * found in the trim string, stops at the first not found. It calls the trimLeft first, then
+   * trimRight. It returns a new string in which both ends trim characters have been removed.
    * @param trimString the trim character string
    */
   public UTF8String trim(UTF8String trimString) {
@@ -540,8 +540,9 @@ public UTF8String trimLeft() {
 
   /**
    * Based on the given trim string, trim this string starting from left end
-   * This method searches each character in the source string starting from the left end, removes the character if it
-   * is in the trim string, stops at the first character which is not in the trim string, returns the new string.
+   * This method searches each character in the source string starting from the left end, removes
+   * the character if it is in the trim string, stops at the first character which is not in the
+   * trim string, returns the new string.
    * @param trimString the trim character string
    */
   public UTF8String trimLeft(UTF8String trimString) {
@@ -552,7 +553,8 @@ public UTF8String trimLeft(UTF8String trimString) {
     int trimIdx = 0;
 
     while (srchIdx < numBytes) {
-      UTF8String searchChar = copyUTF8String(srchIdx, srchIdx + numBytesForFirstByte(this.getByte(srchIdx)) - 1);
+      UTF8String searchChar = copyUTF8String(
+          srchIdx, srchIdx + numBytesForFirstByte(this.getByte(srchIdx)) - 1);
       int searchCharBytes = searchChar.numBytes;
       // try to find the matching for the searchChar in the trimString set
       if (trimString.find(searchChar, 0) >= 0) {
@@ -587,8 +589,9 @@ public UTF8String trimRight() {
 
   /**
    * Based on the given trim string, trim this string starting from right end
-   * This method searches each character in the source string starting from the right end, removes the character if it
-   * is in the trim string, stops at the first character which is not in the trim string, returns the new string.
+   * This method searches each character in the source string starting from the right end,
+   * removes the character if it is in the trim string, stops at the first character which is not
+   * in the trim string, returns the new string.
    * @param trimString the trim character string
    */
   public UTF8String trimRight(UTF8String trimString) {
@@ -608,11 +611,13 @@ public UTF8String trimRight(UTF8String trimString) {
       numChars ++;
     }
 
-    // index trimEnd points to the first no matching byte position from the right side of the source string.
+    // index trimEnd points to the first no matching byte position from the right side of
+    // the source string.
     int trimEnd = numBytes - 1;
     while (numChars > 0) {
-      UTF8String searchChar =
-        copyUTF8String(stringCharPos[numChars - 1], stringCharPos[numChars - 1] + stringCharLen[numChars - 1] - 1);
+      UTF8String searchChar = copyUTF8String(
+          stringCharPos[numChars - 1],
+          stringCharPos[numChars - 1] + stringCharLen[numChars - 1] - 1);
       if (trimString.find(searchChar, 0) >= 0) {
         trimEnd -= stringCharLen[numChars - 1];
       } else {

common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java

@@ -777,7 +777,8 @@ public void trimRightWithTrimString() {
     assertEquals(fromString("cc"), fromString("ccbaaaa").trimRight(fromString("ba")));
     assertEquals(fromString(""), fromString("aabbbbaaa").trimRight(fromString("ab")));
     assertEquals(fromString("  he"), fromString("  hello ").trimRight(fromString(" ol")));
-    assertEquals(fromString("oohell"), fromString("oohellooo../*&").trimRight(fromString("./,&%*o")));
+    assertEquals(fromString("oohell"),
+        fromString("oohellooo../*&").trimRight(fromString("./,&%*o")));
 
     assertEquals(EMPTY_UTF8, fromString("  ").trimRight(fromString(" ")));
 

core/pom.xml

@@ -483,6 +483,7 @@
           <plugin>
             <groupId>org.codehaus.mojo</groupId>
             <artifactId>exec-maven-plugin</artifactId>
+            <version>1.6.0</version>
             <executions>
               <execution>
                 <id>sparkr-pkg</id>

core/src/main/java/org/apache/spark/io/ReadAheadInputStream.java

@@ -32,11 +32,12 @@
 
 /**
  * {@link InputStream} implementation which asynchronously reads ahead from the underlying input
- * stream when specified amount of data has been read from the current buffer. It does it by maintaining
- * two buffer - active buffer and read ahead buffer. Active buffer contains data which should be returned
- * when a read() call is issued. The read ahead buffer is used to asynchronously read from the underlying
- * input stream and once the current active buffer is exhausted, we flip the two buffers so that we can
- * start reading from the read ahead buffer without being blocked in disk I/O.
+ * stream when specified amount of data has been read from the current buffer. It does it by
+ * maintaining two buffers - active buffer and read ahead buffer. Active buffer contains data
+ * which should be returned when a read() call is issued. The read ahead buffer is used to
+ * asynchronously read from the underlying input stream and once the current active buffer is
+ * exhausted, we flip the two buffers so that we can start reading from the read ahead buffer
+ * without being blocked in disk I/O.
  */
 public class ReadAheadInputStream extends InputStream {
 
@@ -83,7 +84,8 @@ public class ReadAheadInputStream extends InputStream {
 
   private final InputStream underlyingInputStream;
 
-  private final ExecutorService executorService = ThreadUtils.newDaemonSingleThreadExecutor("read-ahead");
+  private final ExecutorService executorService =
+      ThreadUtils.newDaemonSingleThreadExecutor("read-ahead");
 
   private final Condition asyncReadComplete = stateChangeLock.newCondition();
 
@@ -98,13 +100,14 @@ public class ReadAheadInputStream extends InputStream {
    * @param readAheadThresholdInBytes If the active buffer has less data than the read-ahead
    *                                  threshold, an async read is triggered.
    */
-  public ReadAheadInputStream(InputStream inputStream, int bufferSizeInBytes, int readAheadThresholdInBytes) {
+  public ReadAheadInputStream(
+      InputStream inputStream, int bufferSizeInBytes, int readAheadThresholdInBytes) {
     Preconditions.checkArgument(bufferSizeInBytes > 0,
         "bufferSizeInBytes should be greater than 0, but the value is " + bufferSizeInBytes);
     Preconditions.checkArgument(readAheadThresholdInBytes > 0 &&
             readAheadThresholdInBytes < bufferSizeInBytes,
-        "readAheadThresholdInBytes should be greater than 0 and less than bufferSizeInBytes, but the" +
-            "value is " + readAheadThresholdInBytes);
+        "readAheadThresholdInBytes should be greater than 0 and less than bufferSizeInBytes, " +
+            "but the value is " + readAheadThresholdInBytes);
     activeBuffer = ByteBuffer.allocate(bufferSizeInBytes);
     readAheadBuffer = ByteBuffer.allocate(bufferSizeInBytes);
     this.readAheadThresholdInBytes = readAheadThresholdInBytes;

core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeSorterSpillReader.java

@@ -76,9 +76,8 @@ public UnsafeSorterSpillReader(
         SparkEnv.get() == null ? 0.5 :
              SparkEnv.get().conf().getDouble("spark.unsafe.sorter.spill.read.ahead.fraction", 0.5);
 
-    final boolean readAheadEnabled =
-            SparkEnv.get() == null ? false :
-                    SparkEnv.get().conf().getBoolean("spark.unsafe.sorter.spill.read.ahead.enabled", true);
+    final boolean readAheadEnabled = SparkEnv.get() != null &&
+        SparkEnv.get().conf().getBoolean("spark.unsafe.sorter.spill.read.ahead.enabled", true);
 
     final InputStream bs =
         new NioBufferedFileInputStream(file, (int) bufferSizeBytes);

core/src/test/java/org/apache/spark/io/ReadAheadInputStreamSuite.java

@@ -28,6 +28,7 @@ public class ReadAheadInputStreamSuite extends GenericFileInputStreamSuite {
   @Before
   public void setUp() throws IOException {
     super.setUp();
-    inputStream = new ReadAheadInputStream(new NioBufferedFileInputStream(inputFile), 8 * 1024, 4 * 1024);
+    inputStream = new ReadAheadInputStream(
+        new NioBufferedFileInputStream(inputFile), 8 * 1024, 4 * 1024);
   }
 }

dev/checkstyle.xml

@@ -52,20 +52,6 @@
       <property name="file" value="dev/checkstyle-suppressions.xml"/>
     </module>
 
-    <!--
-    If you wish to turn off checking for a section of code, you can put a comment in the source
-    before and after the section, with the following syntax:
-
-      // checkstyle:off no.XXX (such as checkstyle.off: NoFinalizer)
-      ...  // stuff that breaks the styles
-      // checkstyle:on
-    -->
-    <module name="SuppressionCommentFilter">
-        <property name="offCommentFormat" value="checkstyle.off\: ([\w\|]+)"/>
-        <property name="onCommentFormat" value="checkstyle.on\: ([\w\|]+)"/>
-        <property name="checkFormat" value="$1"/>
-    </module>
-
     <!-- Checks for whitespace                               -->
     <!-- See http://checkstyle.sf.net/config_whitespace.html -->
     <module name="FileTabCharacter">
@@ -81,6 +67,19 @@
     <module name="NewlineAtEndOfFile"/>
 
     <module name="TreeWalker">
+        <!--
+        If you wish to turn off checking for a section of code, you can put a comment in the source
+        before and after the section, with the following syntax:
+
+          // checkstyle:off no.XXX (such as checkstyle.off: NoFinalizer)
+          ...  // stuff that breaks the styles
+          // checkstyle:on
+        -->
+        <module name="SuppressionCommentFilter">
+            <property name="offCommentFormat" value="checkstyle.off\: ([\w\|]+)"/>
+            <property name="onCommentFormat" value="checkstyle.on\: ([\w\|]+)"/>
+            <property name="checkFormat" value="$1"/>
+        </module>
         <module name="OuterTypeFilename"/>
         <module name="IllegalTokenText">
             <property name="tokens" value="STRING_LITERAL, CHAR_LITERAL"/>
@@ -182,6 +181,5 @@
         <module name="UnusedImports"/>
         <module name="RedundantImport"/>
         <module name="RedundantModifier"/>
-        <module name="FileContentsHolder"/>
     </module>
 </module>

dev/test-dependencies.sh

@@ -46,7 +46,7 @@ OLD_VERSION=$($MVN -q \
     -Dexec.executable="echo" \
     -Dexec.args='${project.version}' \
     --non-recursive \
-    org.codehaus.mojo:exec-maven-plugin:1.5.0:exec)
+    org.codehaus.mojo:exec-maven-plugin:1.6.0:exec)
 if [ $? != 0 ]; then
     echo -e "Error while getting version string from Maven:\n$OLD_VERSION"
     exit 1