Improve scalastyle rules to detect spaces (#1493)

Signed-off-by: Ahmed Hussein (amahussein) <[email protected]>

Contributes to #1158

This pull request includes several changes to the
`core/scalastyle-config.xml` file and updates to copyright years across
multiple Scala files. Additionally, it includes minor code cleanups.

### Updates to `scalastyle-config.xml`:

* Added new style checks:
- `SpacesAfterPlusChecker`, `SpacesBeforePlusChecker`,
`WhitespaceEndOfLineChecker`
- `DisallowSpaceBeforeTokenChecker`,
`SingleSpaceBetweenRParenAndLCurlyBrace`, `OmitBracesInCase`
- `NewLineAtEofChecker`, `SpaceAfterCommentStartChecker`,
`EnsureSingleSpaceBeforeTokenChecker`,
`EnsureSingleSpaceAfterTokenChecker`
  - `TokenChecker` for `println` statements
- Disabled checks: `ParameterNumberChecker`,
`PublicMethodsHaveTypeChecker`, `NonASCIICharacterChecker`

Author: amahussein

core/scalastyle-config.xml

@@ -1,5 +1,5 @@
 <!--
-  Copyright (c) 2023-2024, NVIDIA CORPORATION. All Rights Reserved.
+  Copyright (c) 2023-2025, NVIDIA CORPORATION. All Rights Reserved.
 
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
@@ -40,6 +40,12 @@ You can also disable only one rule, by specifying its rule id, as specified in:
 
     <check level="error" class="org.scalastyle.file.FileTabChecker" enabled="true"/>
 
+    <check level="error" class="org.scalastyle.scalariform.SpacesAfterPlusChecker" enabled="true"></check>
+
+    <check level="error" class="org.scalastyle.scalariform.SpacesBeforePlusChecker" enabled="true"></check>
+
+    <check level="error" class="org.scalastyle.file.WhitespaceEndOfLineChecker" enabled="true"></check>
+
     <check level="error" class="org.scalastyle.file.FileLineLengthChecker" enabled="true">
         <parameters>
             <parameter name="maxLineLength"><![CDATA[100]]></parameter>
@@ -58,6 +64,24 @@ You can also disable only one rule, by specifying its rule id, as specified in:
         </parameters>
     </check>
 
+    <check level="error" class="org.scalastyle.scalariform.DisallowSpaceBeforeTokenChecker" enabled="true">
+        <parameters>
+            <parameter name="tokens">COMMA</parameter>
+        </parameters>
+    </check>
+
+    <check customId="SingleSpaceBetweenRParenAndLCurlyBrace" level="error" class="org.scalastyle.file.RegexChecker" enabled="true">
+        <parameters><parameter name="regex">\)\{</parameter></parameters>
+        <customMessage><![CDATA[
+            Single Space between ')' and `{`.
+        ]]></customMessage>
+    </check>
+
+    <check customId="OmitBracesInCase" level="error" class="org.scalastyle.file.RegexChecker" enabled="true">
+        <parameters><parameter name="regex">case[^\n>]*=>\s*\{</parameter></parameters>
+        <customMessage>Omit braces in case clauses.</customMessage>
+    </check>
+
     <check level="error" class="org.scalastyle.scalariform.ClassNamesChecker" enabled="true">
         <parameters>
             <parameter name="regex"><![CDATA[[A-Z][A-Za-z]*]]></parameter>
@@ -92,10 +116,35 @@ You can also disable only one rule, by specifying its rule id, as specified in:
         </parameters>
     </check>
 
+    <check level="error" class="org.scalastyle.file.NewLineAtEofChecker" enabled="true"></check>
+
+    <check level="error" class="org.scalastyle.scalariform.SpaceAfterCommentStartChecker" enabled="true"></check>
+
+    <check level="error" class="org.scalastyle.scalariform.EnsureSingleSpaceBeforeTokenChecker" enabled="true">
+        <parameters>
+            <parameter name="tokens">ARROW, EQUALS, ELSE, TRY, CATCH, FINALLY, LARROW, RARROW</parameter>
+        </parameters>
+    </check>
+
+    <check level="error" class="org.scalastyle.scalariform.EnsureSingleSpaceAfterTokenChecker" enabled="true">
+        <parameters>
+            <parameter name="tokens">ARROW, EQUALS, COMMA, COLON, IF, ELSE, DO, WHILE, FOR, MATCH, TRY, CATCH, FINALLY, LARROW, RARROW</parameter>
+        </parameters>
+    </check>
+
     <!-- ??? usually shouldn't be checked into the code base. -->
     <check level="error" class="org.scalastyle.scalariform.NotImplementedErrorUsage"
            enabled="true"/>
 
+    <!-- Similar to Spark, all printlns need to be wrapped in '// scalastyle:off/on println' -->
+    <check customId="println" level="error" class="org.scalastyle.scalariform.TokenChecker" enabled="true">
+        <parameters><parameter name="regex">^println$</parameter></parameters>
+        <customMessage><![CDATA[Are you sure you want to println? If yes, wrap the code block with
+      // scalastyle:off println
+      println(...)
+      // scalastyle:on println]]></customMessage>
+    </check>
+
     <check customId="NoScalaDoc" level="error" class="org.scalastyle.file.RegexChecker"
            enabled="true">
         <parameters>
@@ -119,4 +168,12 @@ You can also disable only one rule, by specifying its rule id, as specified in:
     <!-- This project uses Javadoc rather than Scaladoc so scaladoc checks are disabled -->
     <check enabled="false" class="org.scalastyle.scalariform.ScalaDocChecker" level="warning"/>
 
+    <check customId="argcount" level="error" class="org.scalastyle.scalariform.ParameterNumberChecker" enabled="false">
+        <parameters><parameter name="maxParameters"><![CDATA[10]]></parameter></parameters>
+    </check>
+
+    <check level="error" class="org.scalastyle.scalariform.PublicMethodsHaveTypeChecker" enabled="false"></check>
+
+    <!-- Unit test uses ascii characters. So, we need to clean that up first -->
+    <check customId="nonascii" level="error" class="org.scalastyle.scalariform.NonASCIICharacterChecker" enabled="false"></check>
 </scalastyle>

core/src/main/scala/com/nvidia/spark/rapids/tool/AppSummaryInfoBaseProvider.scala

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024, NVIDIA CORPORATION.
+ * Copyright (c) 2024-2025, NVIDIA CORPORATION.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -40,7 +40,7 @@ class AppSummaryInfoBaseProvider extends AppInfoPropertyGetter
   override def getProperty(propKey: String): Option[String] = {
     if (propKey.startsWith(ToolUtils.PROPS_RAPIDS_KEY_PREFIX)) {
       getRapidsProperty(propKey)
-    } else if (propKey.startsWith("spark")){
+    } else if (propKey.startsWith("spark")) {
       getSparkProperty(propKey)
     } else {
       getSystemProperty(propKey)

core/src/main/scala/com/nvidia/spark/rapids/tool/Platform.scala

@@ -432,7 +432,7 @@ abstract class Platform(var gpuDevice: Option[GpuDevice],
   /**
    * Attempts to get the instance type based on the core and gpu requirements.
    */
-  def getInstanceByResources(cores:Int, numGpus: Int): Option[InstanceInfo] = None
+  def getInstanceByResources(cores: Int, numGpus: Int): Option[InstanceInfo] = None
 
   /**
    * Recommend a GPU Instance type to use for this application.
@@ -452,7 +452,7 @@ abstract class Platform(var gpuDevice: Option[GpuDevice],
       // we could put on a node.
       if (origClusterNumExecsPerNode == -1) {
         maxGpusSupported
-      }  else {
+      } else {
         origClusterNumExecsPerNode
       }
     } else {
@@ -611,7 +611,7 @@ class DatabricksAwsPlatform(gpuDevice: Option[GpuDevice],
     clusterProperties: Option[ClusterProperties])
   extends DatabricksPlatform(gpuDevice, clusterProperties)
   with Logging {
-  override val platformName: String =  PlatformNames.DATABRICKS_AWS
+  override val platformName: String = PlatformNames.DATABRICKS_AWS
   override val defaultGpuDevice: GpuDevice = A10GGpu
 
   override def getInstanceByResources(
@@ -665,7 +665,7 @@ class DatabricksAzurePlatform(gpuDevice: Option[GpuDevice],
 
 class DataprocPlatform(gpuDevice: Option[GpuDevice],
     clusterProperties: Option[ClusterProperties]) extends Platform(gpuDevice, clusterProperties) {
-  override val platformName: String =  PlatformNames.DATAPROC
+  override val platformName: String = PlatformNames.DATAPROC
   override val defaultGpuDevice: GpuDevice = T4Gpu
   override def isPlatformCSP: Boolean = true
   override def maxGpusSupported: Int = 4
@@ -694,21 +694,21 @@ class DataprocPlatform(gpuDevice: Option[GpuDevice],
 class DataprocServerlessPlatform(gpuDevice: Option[GpuDevice],
     clusterProperties: Option[ClusterProperties])
   extends DataprocPlatform(gpuDevice, clusterProperties) {
-  override val platformName: String =  PlatformNames.DATAPROC_SL
+  override val platformName: String = PlatformNames.DATAPROC_SL
   override val defaultGpuDevice: GpuDevice = L4Gpu
   override def isPlatformCSP: Boolean = true
 }
 
 class DataprocGkePlatform(gpuDevice: Option[GpuDevice],
     clusterProperties: Option[ClusterProperties])
   extends DataprocPlatform(gpuDevice, clusterProperties) {
-  override val platformName: String =  PlatformNames.DATAPROC_GKE
+  override val platformName: String = PlatformNames.DATAPROC_GKE
   override def isPlatformCSP: Boolean = true
 }
 
 class EmrPlatform(gpuDevice: Option[GpuDevice],
     clusterProperties: Option[ClusterProperties]) extends Platform(gpuDevice, clusterProperties) {
-  override val platformName: String =  PlatformNames.EMR
+  override val platformName: String = PlatformNames.EMR
   override val defaultGpuDevice: GpuDevice = A10GGpu
 
   override def isPlatformCSP: Boolean = true
@@ -755,7 +755,7 @@ class EmrPlatform(gpuDevice: Option[GpuDevice],
 
 class OnPremPlatform(gpuDevice: Option[GpuDevice],
     clusterProperties: Option[ClusterProperties]) extends Platform(gpuDevice, clusterProperties) {
-  override val platformName: String =  PlatformNames.ONPREM
+  override val platformName: String = PlatformNames.ONPREM
   // Note we don't have an speedup factor file for onprem l4's but we want auto tuner
   // to use L4.
   override val defaultGpuDevice: GpuDevice = L4Gpu

core/src/main/scala/com/nvidia/spark/rapids/tool/planparser/DeltaLakeHelper.scala

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024, NVIDIA CORPORATION.
+ * Copyright (c) 2024-2025, NVIDIA CORPORATION.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -55,7 +55,7 @@ class DLWriteWithFormatAndSchemaParser(node: SparkPlanGraphNode,
 object DeltaLakeHelper {
   // we look for the serdeLibrary which is part of the node description:
   // Serde Library: org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe
-  //private val serdeRegex = "Serde Library: ([\\w.]+)".r
+  // private val serdeRegex = "Serde Library: ([\\w.]+)".r
   private val serdeRegex = "Serde Library: ([\\w.]+)".r
   // We look for the schema in the node description. It is in the following format
   // Schema: root

core/src/main/scala/com/nvidia/spark/rapids/tool/planparser/ReadParser.scala

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022-2024, NVIDIA CORPORATION.
+ * Copyright (c) 2022-2025, NVIDIA CORPORATION.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -86,7 +86,7 @@ object ReadParser extends Logging {
   }
 
   // This tries to get just the field specified by tag in a string that
-  // may contain multiple fields.  It looks for a comma to delimit fields.
+  // may contain multiple fields. It looks for a comma to delimit fields.
   private def getFieldWithoutTag(str: String, tag: String): String = {
     val index = str.indexOf(tag)
     // remove the tag from the final string returned
@@ -187,7 +187,6 @@ object ReadParser extends Logging {
       }
       ReadMetaData(schema, location, fileFormat, tags = extractReadTags(node.desc))
     }
-
   }
 
   // For the read score we look at the read format and datatypes for each

core/src/main/scala/com/nvidia/spark/rapids/tool/planparser/SQLPlanParser.scala

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022-2024, NVIDIA CORPORATION.
+ * Copyright (c) 2022-2025, NVIDIA CORPORATION.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -67,7 +67,7 @@ object UnsupportedReasons extends Enumeration {
       case IS_UNSUPPORTED => "Unsupported"
       case CONTAINS_UNSUPPORTED_EXPR => "Contains unsupported expr"
       case UNSUPPORTED_IO_FORMAT => "Unsupported IO format"
-      case customReason @ _  => customReason.toString
+      case customReason @ _ => customReason.toString
     }
   }
 }
@@ -163,7 +163,7 @@ case class ExecInfo(
         OpActions.IgnoreNoPerf
       } else if (shouldIgnore) {
         OpActions.IgnorePerf
-      } else  {
+      } else {
         OpActions.Triage
       }
     }
@@ -296,7 +296,7 @@ object ExecInfo {
       children: Option[Seq[ExecInfo]], // only one level deep
       stages: Set[Int] = Set.empty,
       shouldRemove: Boolean = false,
-      unsupportedExecReason:String = "",
+      unsupportedExecReason: String = "",
       unsupportedExprs: Seq[UnsupportedExprOpRef] = Seq.empty,
       dataSet: Boolean = false,
       udf: Boolean = false,
@@ -660,7 +660,7 @@ object SQLPlanParser extends Logging {
     maxDuration
   }
 
-  private def ignoreExpression(expr:String): Boolean = {
+  private def ignoreExpression(expr: String): Boolean = {
     ignoreExpressions.contains(expr.toLowerCase)
   }
 
@@ -789,7 +789,7 @@ object SQLPlanParser extends Logging {
     parsedExpressions.toArray
   }
 
-  def parseWindowExpressions(exprStr:String): Array[String] = {
+  def parseWindowExpressions(exprStr: String): Array[String] = {
     val parsedExpressions = ArrayBuffer[String]()
     // [sum(cast(level#30 as bigint)) windowspecdefinition(device#29, id#28 ASC NULLS FIRST,
     // specifiedwindowframe(RangeFrame, unboundedpreceding$(), currentrow$())) AS sum#35L,

core/src/main/scala/com/nvidia/spark/rapids/tool/profiling/DriverLogProcessor.scala

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023-2024, NVIDIA CORPORATION.
+ * Copyright (c) 2023-2025, NVIDIA CORPORATION.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -64,7 +64,6 @@ class DriverLogProcessor(logPath: String)
   }
 }
 
-
 object BaseDriverLogInfoProvider {
   def noneDriverLog: BaseDriverLogInfoProvider = new BaseDriverLogInfoProvider()
-}
+}

core/src/main/scala/com/nvidia/spark/rapids/tool/profiling/GenerateTimeline.scala

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021-2024, NVIDIA CORPORATION.
+ * Copyright (c) 2021-2025, NVIDIA CORPORATION.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -35,7 +35,7 @@ class TimelineTaskInfo(val stageId: Int, val taskId: Long,
 
 class TimelineStageInfo(val stageId: Int,
     startTime: Long,
-    endTime:Long,
+    endTime: Long,
     val duration: Long) extends TimelineTiming(startTime, endTime)
 
 class TimelineJobInfo(val jobId: Int,
@@ -279,7 +279,7 @@ object GenerateTimeline {
       .flatMap(_.taskUpdatesMap.values).sum
 
     val semMetricsMs = app.accumManager.accumInfoMap.flatMap {
-        case (_,accumInfo: AccumInfo)
+        case (_, accumInfo: AccumInfo)
             if accumInfo.infoRef.name == AccumNameRef.NAMES_TABLE.get("gpuSemaphoreWait") =>
             Some(accumInfo.taskUpdatesMap.values.sum)
         case _ => None

core/src/main/scala/com/nvidia/spark/rapids/tool/profiling/HealthCheck.scala

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021-2024, NVIDIA CORPORATION.
+ * Copyright (c) 2021-2025, NVIDIA CORPORATION.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -47,7 +47,7 @@ class HealthCheck(apps: Seq[ApplicationInfo]) {
     ProfRemovedExecutorView.getRawView(apps)
   }
 
-  //Function to list all *possible* not-supported plan nodes if GPU Mode=on
+  // Function to list all *possible* not-supported plan nodes if GPU Mode=on
   def getPossibleUnsupportedSQLPlan: Seq[UnsupportedOpsProfileResult] = {
     val res = apps.flatMap { app =>
       app.planMetricProcessor.unsupportedSQLPlan.map { unsup =>

core/src/main/scala/com/nvidia/spark/rapids/tool/profiling/ProfileClassWarehouse.scala

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021-2024, NVIDIA CORPORATION.
+ * Copyright (c) 2021-2025, NVIDIA CORPORATION.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -203,7 +203,7 @@ class SQLExecutionInfoClass(
 }
 
 case class SQLAccumProfileResults(appIndex: Int, sqlID: Long, nodeID: Long,
-    nodeName: String, accumulatorId: Long, name: String, min: Long, median:Long,
+    nodeName: String, accumulatorId: Long, name: String, min: Long, median: Long,
     max: Long, total: Long, metricType: String, stageIds: String) extends ProfileResult {
   override val outputHeaders = Seq("appIndex", "sqlID", "nodeID", "nodeName", "accumulatorId",
     "name", "min", "median", "max", "total", "metricType", "stageIds")
@@ -310,7 +310,7 @@ case class AppInfoProfileResults(appIndex: Int, appName: String,
 
   override def convertToSeq: Seq[String] = {
     Seq(appIndex.toString, appName, appId.getOrElse(""),
-      sparkUser,  startTime.toString, endTimeToStr, durToStr,
+      sparkUser, startTime.toString, endTimeToStr, durToStr,
       durationStr, sparkRuntime.toString, sparkVersion, pluginEnabled.toString)
   }
   override def convertToCSVSeq: Seq[String] = {
@@ -1105,7 +1105,7 @@ case class WholeStageCodeGenResults(
   }
 }
 
-case class RecommendedPropertyResult(property: String, value: String){
+case class RecommendedPropertyResult(property: String, value: String) {
   override def toString: String = "--conf %s=%s".format(property, value)
 }