[GRIFFIN-312] Code Style Standardization

**What changes were proposed in this pull request?** This PR targets the following, - fix the various warnings during build and in source code, - perform code formatting as per a standard style, - fix scalastyle integration Using the [code format from spark code style can be automatically imposed on measure module. Link: https://github.com/apache/spark/blob/master/dev/.scalafmt.conf Since ScalaStyle targets scala source code only, it should be a part of the measure module only. Current misconfiguration is also suppressing the formatting errors Scalafmt is used for code formatting. **Does this PR introduce any user-facing change?** No **How was this patch tested?** Griffin test suite. Author: chitralverma <[email protected]> Closes #560 from chitralverma/code-style-standardization.
apache · Dec 25, 2019 · 7f718d3 · 7f718d3
1 parent ed47c2b
commit 7f718d3
Show file tree

Hide file tree

Showing 157 changed files with 2,891 additions and 2,573 deletions.
diff --git a/.scalafmt.conf b/.scalafmt.conf
@@ -0,0 +1,27 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+align = none
+align.openParenDefnSite = false
+align.openParenCallSite = false
+align.tokens = []
+optIn = {
+  configStyleArguments = false
+}
+danglingParentheses = false
+docstrings = JavaDoc
+maxColumn = 98
diff --git a/.travis.yml b/.travis.yml
@@ -42,5 +42,5 @@ before_script:
 script:
   - export MAVEN_SKIP_RC=true
   - export MAVEN_OPTS=" -Xmx1g -XX:ReservedCodeCacheSize=128m -Dorg.slf4j.simpleLogger.defaultLogLevel=warn -Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn"
-  - mvn -B clean scalastyle:check test -Dlogging.level.org.springframework=WARN
+  - mvn -B clean test -Dlogging.level.org.springframework=WARN
 
diff --git a/griffin-doc/dev/code-style.md b/griffin-doc/dev/code-style.md
@@ -18,9 +18,13 @@ under the License.
 -->
 
 # Apache Griffin Development Code Style Config Guide
-Griffin product consists of three main parts, they are developed and built on Angular 2/Java/Scala technique.
-So it's necessary to define a bunch of code style rules, all codes need to follow those rules and
-keep any codes submitted by various committers consistent.
+Apache Griffin consists of three main modules - Measure, Services and UI. These are developed using Scala, Java and Angular 2 respectively.
+
+Quoting the Databricks-Apache Spark Code Style Guide at [link](https://github.com/databricks/scala-style-guide) :
+> Code is written once by its author, but read and modified multiple times by lots of other engineers. As most bugs actually come from future modification of the code, we need to optimize our codebase for long-term, global readability and maintainability. The best way to achieve this is to write simple code.
+
+So it's necessary to define a bunch of code style rules, which are to be followed by all contributors and committers so as ot keep the code base consistent.
+
 
 ## Config Java Code Style
 We suggest developers use automatic tools to check code violation like [CheckStyle](https://github.com/checkstyle/checkstyle).<br>
@@ -309,8 +313,59 @@ We suggest developers use automatic tools to check code violation like [CheckSty
         ![idea check-run](../img/devguide/check-run-idea.png)
 
 
-## Config Scala Code Style
-to do
+## Scala Code Style Guide
+
+Since only Measure module is written in Scala, this guide applies to only the measure module. 
+For Scala code, Apache Griffin follows the official [Scala style guide](https://docs.scala-lang.org/style/) 
+and [Apache Spark Scala guide](https://github.com/databricks/scala-style-guide).
+
+#### Guide Update History
+| Version | Date       | Changes |
+|:-------:|:-----------|:--------|
+| 1       | 2019-12-24 | Initial version: Added Scala Code Style Guide |
+
+#### Configuration
+For code style checks Apache Griffin uses [Scalastyle](http://www.scalastyle.org/) and formatting is handled 
+by [Scalafmt](https://scalameta.org/scalafmt/).
+
+Configurations for both Scalastyle and Scalafmt are present in the root of Apache Griffin repository.
+ - Scalastyle Config: `scalastyle-config.xml`
+ - Scalafmt Config: `.scalafmt.conf`
+
+####  Formatting and Style Check
+
+Although automatic formatting and checks are built into the build steps of Apache Griffin, 
+to manually format Scala code and check for possible style violations, run the following commands.
+
+```
+## assuming current working directory is griffin root
+
+cd measure
+mvn clean verify -DskipTests
+```
+
+####  IntelliJ IDEA Setup
+
+Navigate to _Settings > Editor > Code Style > Scala_ and follow the screenshots below to ensure coding standards
+ in the IntelliJ IDEA.
+![scala_code_style_1](../img/devguide/scala-intellij-idea/1.png)
+![scala_code_style_2](../img/devguide/scala-intellij-idea/2.png)
+![scala_code_style_3](../img/devguide/scala-intellij-idea/3.png)
+![scala_code_style_4](../img/devguide/scala-intellij-idea/4.png)
+![scala_code_style_5](../img/devguide/scala-intellij-idea/5.png)
+![scala_code_style_6](../img/devguide/scala-intellij-idea/6.png)
+
+
+#### Important Notes
+1. [Guide Update History](#guide-update-history) section must be updated each time any section of 
+Scala Code Style Guide is edited.
+2. Formatting and code style checks must be ensured each time before submitting or updating a pull request as 
+violations will cause build failures.
+3. If you’re not sure about the right style for something, try to follow the style of the existing codebase. 
+Look at whether there are other examples in the code that use your feature. 
+
+In case of any queries or dilemmas feel free to reach out to the other contributors and committers 
+on the [[email protected]](mailto:[email protected]) list.
 
 ## Config Angular 2 Code Style
 to do
diff --git a/griffin-doc/img/devguide/scala-intellij-idea/1.png b/griffin-doc/img/devguide/scala-intellij-idea/1.png
diff --git a/griffin-doc/img/devguide/scala-intellij-idea/2.png b/griffin-doc/img/devguide/scala-intellij-idea/2.png
diff --git a/griffin-doc/img/devguide/scala-intellij-idea/3.png b/griffin-doc/img/devguide/scala-intellij-idea/3.png
diff --git a/griffin-doc/img/devguide/scala-intellij-idea/4.png b/griffin-doc/img/devguide/scala-intellij-idea/4.png
diff --git a/griffin-doc/img/devguide/scala-intellij-idea/5.png b/griffin-doc/img/devguide/scala-intellij-idea/5.png
diff --git a/griffin-doc/img/devguide/scala-intellij-idea/6.png b/griffin-doc/img/devguide/scala-intellij-idea/6.png
diff --git a/measure/pom.xml b/measure/pom.xml
@@ -17,7 +17,8 @@ KIND, either express or implied.  See the License for the
 specific language governing permissions and limitations
 under the License.
 -->
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
 
     <parent>
@@ -47,6 +48,9 @@ under the License.
         <mockito.version>1.10.19</mockito.version>
         <mysql.java.version>5.1.47</mysql.java.version>
         <cassandra.connector.version>2.4.1</cassandra.connector.version>
+        <scalastyle.version>1.0.0</scalastyle.version>
+        <scalafmt.parameters>--diff --test</scalafmt.parameters>
+        <scalafmt.skip>false</scalafmt.skip>
     </properties>
 
     <dependencies>
@@ -200,6 +204,8 @@ under the License.
     </dependencies>
 
     <build>
+        <sourceDirectory>src/main/scala</sourceDirectory>
+        <testSourceDirectory>src/test/scala</testSourceDirectory>
         <plugins>
             <plugin>
                 <groupId>net.alchim31.maven</groupId>
@@ -217,6 +223,11 @@ under the License.
                 </executions>
                 <configuration>
                     <scalaVersion>${scala.version}</scalaVersion>
+                    <args>
+                        <arg>-deprecation</arg>
+                        <arg>-feature</arg>
+                        <arg>-unchecked</arg>
+                    </args>
                 </configuration>
             </plugin>
             <plugin>
@@ -242,7 +253,8 @@ under the License.
                         </goals>
                         <configuration>
                             <transformers>
-                                <transformer implementation="org.apache.maven.plugins.shade.resource.ManifestResourceTransformer">
+                                <transformer
+                                        implementation="org.apache.maven.plugins.shade.resource.ManifestResourceTransformer">
                                     <mainClass>org.apache.griffin.measure.Application</mainClass>
                                 </transformer>
                             </transformers>
@@ -264,6 +276,51 @@ under the License.
                     </execution>
                 </executions>
             </plugin>
+            <plugin>
+                <groupId>org.antipathy</groupId>
+                <artifactId>mvn-scalafmt_${scala.binary.version}</artifactId>
+                <version>0.12_1.5.1</version>
+                <configuration>
+                    <parameters>${scalafmt.parameters}</parameters>
+                    <skip>${scalafmt.skip}</skip>
+                    <skipSources>${scalafmt.skip}</skipSources>
+                    <skipTestSources>${scalafmt.skip}</skipTestSources>
+                    <configLocation>${project.parent.basedir}/.scalafmt.conf</configLocation>
+                </configuration>
+                <executions>
+                    <execution>
+                        <phase>validate</phase>
+                        <goals>
+                            <goal>format</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
+            <plugin>
+                <groupId>org.scalastyle</groupId>
+                <artifactId>scalastyle-maven-plugin</artifactId>
+                <version>${scalastyle.version}</version>
+                <configuration>
+                    <verbose>false</verbose>
+                    <failOnViolation>true</failOnViolation>
+                    <includeTestSourceDirectory>false</includeTestSourceDirectory>
+                    <failOnWarning>false</failOnWarning>
+                    <sourceDirectory>${project.build.sourceDirectory}</sourceDirectory>
+                    <testSourceDirectory>${project.build.testSourceDirectory}</testSourceDirectory>
+                    <configLocation>${project.parent.basedir}/scalastyle-config.xml</configLocation>
+                    <outputFile>${basedir}/target/scalastyle-output.xml</outputFile>
+                    <inputEncoding>${project.build.sourceEncoding}</inputEncoding>
+                    <outputEncoding>${project.reporting.outputEncoding}</outputEncoding>
+                </configuration>
+                <executions>
+                    <execution>
+                        <phase>validate</phase>
+                        <goals>
+                            <goal>check</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
         </plugins>
     </build>
 </project>
diff --git a/measure/src/main/scala/org/apache/griffin/measure/Application.scala b/measure/src/main/scala/org/apache/griffin/measure/Application.scala
@@ -20,18 +20,22 @@ package org.apache.griffin.measure
 import scala.reflect.ClassTag
 import scala.util.{Failure, Success, Try}
 
-import org.apache.griffin.measure.configuration.dqdefinition.{DQConfig, EnvConfig, GriffinConfig, Param}
+import org.apache.griffin.measure.configuration.dqdefinition.{
+  DQConfig,
+  EnvConfig,
+  GriffinConfig,
+  Param
+}
 import org.apache.griffin.measure.configuration.dqdefinition.reader.ParamReaderFactory
 import org.apache.griffin.measure.configuration.enums.ProcessType
 import org.apache.griffin.measure.configuration.enums.ProcessType._
 import org.apache.griffin.measure.launch.DQApp
 import org.apache.griffin.measure.launch.batch.BatchDQApp
 import org.apache.griffin.measure.launch.streaming.StreamingDQApp
 
-
 /**
-  * application entrance
-  */
+ * application entrance
+ */
 object Application extends Loggable {
 
   def main(args: Array[String]): Unit = {
@@ -68,19 +72,19 @@ object Application extends Loggable {
       case BatchProcessType => BatchDQApp(allParam)
       case StreamingProcessType => StreamingDQApp(allParam)
       case _ =>
-        error(s"${procType} is unsupported process type!")
+        error(s"$procType is unsupported process type!")
         sys.exit(-4)
     }
 
-    startup
+    startup()
 
     // dq app init
     dqApp.init match {
       case Success(_) =>
         info("process init success")
       case Failure(ex) =>
         error(s"process init error: ${ex.getMessage}", ex)
-        shutdown
+        shutdown()
         sys.exit(-5)
     }
 
@@ -96,7 +100,7 @@ object Application extends Loggable {
         if (dqApp.retryable) {
           throw ex
         } else {
-          shutdown
+          shutdown()
           sys.exit(-5)
         }
     }
@@ -107,26 +111,24 @@ object Application extends Loggable {
         info("process end success")
       case Failure(ex) =>
         error(s"process end error: ${ex.getMessage}", ex)
-        shutdown
+        shutdown()
         sys.exit(-5)
     }
 
-    shutdown
+    shutdown()
 
     if (!success) {
       sys.exit(-5)
     }
   }
 
-  def readParamFile[T <: Param](file: String)(implicit m : ClassTag[T]): Try[T] = {
+  def readParamFile[T <: Param](file: String)(implicit m: ClassTag[T]): Try[T] = {
     val paramReader = ParamReaderFactory.getParamReader(file)
     paramReader.readConfig[T]
   }
 
-  private def startup(): Unit = {
-  }
+  private def startup(): Unit = {}
 
-  private def shutdown(): Unit = {
-  }
+  private def shutdown(): Unit = {}
 
 }
diff --git a/measure/src/main/scala/org/apache/griffin/measure/Loggable.scala b/measure/src/main/scala/org/apache/griffin/measure/Loggable.scala
@@ -24,9 +24,9 @@ trait Loggable {
 
   @transient private lazy val logger = Logger.getLogger(getClass)
 
-  @transient protected lazy val griffinLogger = Logger.getLogger("org.apache.griffin")
+  @transient protected lazy val griffinLogger: Logger = Logger.getLogger("org.apache.griffin")
 
-  def getGriffinLogLevel(): Level = {
+  def getGriffinLogLevel: Level = {
     var logger = griffinLogger
     while (logger != null && logger.getLevel == null) {
       logger = logger.getParent.asInstanceOf[Logger]