[compiler] apply scalafix to all scala sources (#14156)

After this, there are only 150ish warnings remaining, which shouldn't be too hard to fix by hand. Most are unused locals. Scalafix can delete unused locals, but I disable that, because there were too many cases where it left the rhs unnecessarily, e.g. ``` ... val idx = Symbol(genUID()) ... ``` rewrites to ``` ... Symbol(genUID()) ... ``` I'd rather just leave those as errors to be fixed manually. This is intended to replace #14103, which ended up mixing manual changes to fix warnings with scalafix changes.
hail-is · Jan 19, 2024 · 68a51df · 68a51df
1 parent b10274c
commit 68a51df
Show file tree

Hide file tree

Showing 448 changed files with 2,625 additions and 3,277 deletions.
diff --git a/hail/.scalafix.conf b/hail/.scalafix.conf
@@ -3,6 +3,7 @@ rules = [
   NoValInForComprehension
   ProcedureSyntax
   OrganizeImports
+  RemoveUnused
 ]
 
 OrganizeImports {
@@ -19,3 +20,11 @@ OrganizeImports {
   importsOrder = SymbolsFirst
   removeUnused = true
 }
+
+RemoveUnused {
+  imports = true
+  locals = false
+  privates = false
+  patternvars = true
+  params = true
+}
diff --git a/hail/.scalafmt.conf b/hail/.scalafmt.conf
@@ -39,19 +39,11 @@ align {
 assumeStandardLibraryStripMargin = true
 
 rewrite {
-  rules = [RedundantBraces, SortModifiers, PreferCurlyFors, Imports]
+  rules = [RedundantBraces, SortModifiers, PreferCurlyFors]
   redundantBraces.generalExpressions = true
   redundantBraces.ifElseExpressions = false
   redundantBraces.stringInterpolation = true
   redundantBraces.maxBreaks = 4
-  imports {
-    sort = scalastyle
-    groups = [
-      ["is\\.hail\\..*"],
-      ["org\\.json4s\\..*"],
-      ["java\\..*", "scala\\..*"]
-    ]
-  }
   trailingCommas.style = multiple
 }
 

diff --git a/hail/build.gradle b/hail/build.gradle
@@ -54,7 +54,7 @@ project.ext {
     assert(scalaMajorVersion == "2.12")
 }
 
-compileScala {
+tasks.withType(ScalaCompile) {
     options.compilerArgs <<
         "-Xlint:all" <<
         "-Werror" <<
@@ -66,21 +66,14 @@ compileScala {
         "-target:jvm-1.8",
         "-feature",
         "-Xno-patmat-analysis",
-        "-Xfatal-warnings",
-        "-Xlint:_",
+        "-Xlint",
         "-deprecation",
         "-unchecked",
-        "-Xlint:-infer-any",
-        "-Xlint:-unsound-match"
+        "-Ywarn-unused:_,-explicits,-implicits",
+        "-Wconf:cat=unused-locals:w,cat=unused:info,any:w",
+        "-Ypartial-unification",
     ]
 
-    if (scalaMajorVersion == "2.12") {
-        scalaCompileOptions.additionalParameters +=
-            [ "-Xlint:-unused"
-            , "-Ypartial-unification"
-            ]
-    }
-
     scalaCompileOptions.forkOptions.with {
         jvmArgs = ["-Xms512M",
                    "-Xmx4096M",

diff --git a/hail/src/main/scala/is/hail/HailContext.scala b/hail/src/main/scala/is/hail/HailContext.scala
@@ -2,25 +2,23 @@ package is.hail
 
 import is.hail.backend.Backend
 import is.hail.backend.spark.SparkBackend
-import is.hail.expr.ir.BaseIR
 import is.hail.expr.ir.functions.IRFunctionRegistry
 import is.hail.io.fs.FS
 import is.hail.io.vcf._
 import is.hail.types.virtual._
 import is.hail.utils._
 
-import org.json4s.Extraction
-import org.json4s.jackson.JsonMethods
+import scala.reflect.ClassTag
 
 import java.io.InputStream
 import java.util.Properties
-import scala.collection.mutable
-import scala.reflect.ClassTag
 
-import org.apache.log4j.{ConsoleAppender, LogManager, PatternLayout, PropertyConfigurator}
+import org.apache.log4j.{LogManager, PropertyConfigurator}
 import org.apache.spark._
 import org.apache.spark.executor.InputMetrics
 import org.apache.spark.rdd.RDD
+import org.json4s.Extraction
+import org.json4s.jackson.JsonMethods
 
 case class FilePartition(index: Int, file: String) extends Partition
 
@@ -82,10 +80,10 @@ object HailContext {
       // new-style version: MAJOR.MINOR.SECURITY (started in JRE 9)
       /* see:
        * https://docs.oracle.com/javase/9/migrate/toc.htm#JSMIG-GUID-3A71ECEF-5FC5-46FE-9BA9-88CBFCE828CB */
-      case javaVersion("1", major, minor) =>
+      case javaVersion("1", major, _) =>
         if (major.toInt < 8)
           fatal(s"Hail requires Java 1.8, found $versionString")
-      case javaVersion(major, minor, security) =>
+      case javaVersion(major, _, _) =>
         if (major.toInt != 11)
           fatal(s"Hail requires Java 8 or 11, found $versionString")
       case _ =>
@@ -197,7 +195,7 @@ class HailContext private (
       .groupBy(_.source.file)
   }
 
-  def grepPrint(fs: FS, regex: String, files: Seq[String], maxLines: Int) {
+  def grepPrint(fs: FS, regex: String, files: Seq[String], maxLines: Int): Unit = {
     fileAndLineCounts(fs, regex, files, maxLines).foreach { case (file, lines) =>
       info(s"$file: ${lines.length} ${plural(lines.length, "match", "matches")}:")
       lines.map(_.value).foreach { line =>

diff --git a/hail/src/main/scala/is/hail/HailFeatureFlags.scala b/hail/src/main/scala/is/hail/HailFeatureFlags.scala
@@ -3,10 +3,10 @@ package is.hail
 import is.hail.backend.ExecutionCache
 import is.hail.utils._
 
-import org.json4s.JsonAST.{JArray, JObject, JString}
-
 import scala.collection.mutable
 
+import org.json4s.JsonAST.{JArray, JObject, JString}
+
 object HailFeatureFlags {
   val defaults = Map[String, (String, String)](
     // Must match __flags_env_vars_and_defaults in hail/backend/backend.py

diff --git a/hail/src/main/scala/is/hail/annotations/BroadcastValue.scala b/hail/src/main/scala/is/hail/annotations/BroadcastValue.scala
@@ -9,7 +9,7 @@ import is.hail.types.virtual.{TBaseStruct, TStruct}
 import is.hail.utils.{formatSpace, log, ArrayOfByteArrayOutputStream}
 import is.hail.utils.prettyPrint.ArrayOfByteArrayInputStream
 
-import java.io.{ByteArrayInputStream, ByteArrayOutputStream, InputStream}
+import java.io.InputStream
 
 import org.apache.spark.sql.Row
 

diff --git a/hail/src/main/scala/is/hail/annotations/ChunkCache.scala b/hail/src/main/scala/is/hail/annotations/ChunkCache.scala
@@ -2,9 +2,10 @@ package is.hail.annotations
 
 import is.hail.expr.ir.LongArrayBuilder
 
+import scala.collection.mutable
+
 import java.util.TreeMap
 import java.util.function.BiConsumer
-import scala.collection.mutable
 
 /** ChunkCache minimizes calls to free and allocate by holding onto chunks when they are no longer
   * in use. When a chunk is needed, the cache is searched. If the size requested is less than a

diff --git a/hail/src/main/scala/is/hail/annotations/OrderedRVIterator.scala b/hail/src/main/scala/is/hail/annotations/OrderedRVIterator.scala
@@ -79,7 +79,7 @@ case class OrderedRVIterator(
 
       var isValid: Boolean = true
 
-      def setValue() {
+      def setValue(): Unit = {
         if (left.isValid) {
           while (buffer.nonEmpty && mixedOrd(left.value, buffer.head) > 0)
             buffer.dequeue()
@@ -95,7 +95,7 @@ case class OrderedRVIterator(
         }
       }
 
-      def advance() {
+      def advance(): Unit = {
         left.advance()
         setValue()
       }

diff --git a/hail/src/main/scala/is/hail/annotations/Region.scala b/hail/src/main/scala/is/hail/annotations/Region.scala
@@ -1,13 +1,10 @@
 package is.hail.annotations
 
 import is.hail.asm4s
-import is.hail.asm4s.{coerce, Code}
-import is.hail.backend.HailTaskContext
+import is.hail.asm4s.Code
 import is.hail.types.physical._
 import is.hail.utils._
 
-import org.apache.spark.TaskContext
-
 object Region {
   type Size = Int
   val REGULAR: Size = 0
@@ -89,22 +86,21 @@ object Region {
     (loadByte(b) & (1 << (bitOff & 7).toInt)) != 0
   }
 
-  def setBit(byteOff: Long, bitOff: Long) {
+  def setBit(byteOff: Long, bitOff: Long): Unit = {
     val b = byteOff + (bitOff >> 3)
     storeByte(b, (loadByte(b) | (1 << (bitOff & 7).toInt)).toByte)
   }
 
-  def clearBit(byteOff: Long, bitOff: Long) {
+  def clearBit(byteOff: Long, bitOff: Long): Unit = {
     val b = byteOff + (bitOff >> 3)
     storeByte(b, (loadByte(b) & ~(1 << (bitOff & 7).toInt)).toByte)
   }
 
-  def storeBit(byteOff: Long, bitOff: Long, b: Boolean) {
+  def storeBit(byteOff: Long, bitOff: Long, b: Boolean): Unit =
     if (b)
       setBit(byteOff, bitOff)
     else
       clearBit(byteOff, bitOff)
-  }
 
   def loadInt(addr: Code[Long]): Code[Int] =
     Code.invokeScalaObject1[Long, Int](Region.getClass, "loadInt", addr)
@@ -313,7 +309,7 @@ object Region {
     v.result()
   }
 
-  def visit(t: PType, off: Long, v: ValueVisitor) {
+  def visit(t: PType, off: Long, v: ValueVisitor): Unit = {
     t match {
       case _: PBoolean => v.visitBoolean(Region.loadBoolean(off))
       case _: PInt32 => v.visitInt32(Region.loadInt(off))

diff --git a/hail/src/main/scala/is/hail/annotations/RegionMemory.scala b/hail/src/main/scala/is/hail/annotations/RegionMemory.scala
@@ -1,7 +1,6 @@
 package is.hail.annotations
 
-import is.hail.expr.ir.{AnyRefArrayBuilder, LongArrayBuilder, LongMissingArrayBuilder}
-import is.hail.types.physical.{PCanonicalNDArray, PNDArray}
+import is.hail.expr.ir.{AnyRefArrayBuilder, LongArrayBuilder}
 import is.hail.utils._
 
 final class RegionMemory(pool: RegionPool) extends AutoCloseable {

diff --git a/hail/src/main/scala/is/hail/annotations/RegionPool.scala b/hail/src/main/scala/is/hail/annotations/RegionPool.scala
@@ -3,10 +3,6 @@ package is.hail.annotations
 import is.hail.expr.ir.LongArrayBuilder
 import is.hail.utils._
 
-import java.util.TreeMap
-import java.util.function.BiConsumer
-import scala.collection.mutable
-
 object RegionPool {
 
   def apply(strictMemoryCheck: Boolean = false): RegionPool = {

diff --git a/hail/src/main/scala/is/hail/annotations/RegionValue.scala b/hail/src/main/scala/is/hail/annotations/RegionValue.scala
@@ -3,7 +3,6 @@ package is.hail.annotations
 import is.hail.asm4s.HailClassLoader
 import is.hail.io._
 import is.hail.types.physical.PType
-import is.hail.utils.{using, RestartableByteArrayInputStream}
 
 import java.io._
 
@@ -52,18 +51,16 @@ final class RegionValue(
 ) extends UnKryoSerializable {
   def getOffset: Long = offset
 
-  def set(newRegion: Region, newOffset: Long) {
+  def set(newRegion: Region, newOffset: Long): Unit = {
     region = newRegion
     offset = newOffset
   }
 
-  def setRegion(newRegion: Region) {
+  def setRegion(newRegion: Region): Unit =
     region = newRegion
-  }
 
-  def setOffset(newOffset: Long) {
+  def setOffset(newOffset: Long): Unit =
     offset = newOffset
-  }
 
   def pretty(t: PType): String = Region.pretty(t, offset)