Merge pull request #29 from rxin/style

Scala style checker & style fixes

.travis.yml

@@ -2,4 +2,6 @@
  scala:
    - "2.10.3"
  jdk:
-   - oraclejdk7
+   - oraclejdk7
+ script:
+   - "GIT_AUTHOR_NAME=\"Michael Armbrust\" GIT_AUTHOR_EMAIL=\"michael@databricks.com\" GIT_COMMITTER_NAME=\"Michael Armbrust\" GIT_COMMITTER_EMAIL=\"michael@databricks.com\" sbt ++$TRAVIS_SCALA_VERSION 'set scalacOptions += \"-Xfatal-warnings\"' test:compile scalastyle test ghpages-push-site"

build.sbt

@@ -31,6 +31,8 @@ libraryDependencies ++= Seq(
  "org.apache.hive" % "hive-serde" % "0.12.0",
   "com.typesafe" %% "scalalogging-slf4j" % "1.0.1")
 
+org.scalastyle.sbt.ScalastylePlugin.Settings
+
 // Multiple queries rely on the TestShark singleton.  See comments there for more details.
 parallelExecution in Test := false
 

project/plugins.sbt

@@ -31,4 +31,10 @@ addSbtPlugin("com.typesafe.sbt" % "sbt-ghpages" % "0.5.2")
 
 addSbtPlugin("com.typesafe.sbt" % "sbt-site" % "0.7.1")
 
-addSbtPlugin("com.eed3si9n" % "sbt-assembly" % "0.9.2")
+addSbtPlugin("com.eed3si9n" % "sbt-assembly" % "0.9.2")
+
+
+addSbtPlugin("org.scalastyle" %% "scalastyle-sbt-plugin" % "0.3.2")
+
+// For scalastyle
+resolvers += "sonatype-releases" at "https://oss.sonatype.org/content/repositories/releases/"

scalastyle-config.xml

@@ -0,0 +1,125 @@
+<!-- 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: -->
+<!-- // scalastyle:off -->
+<!-- ... -->
+<!-- // naughty stuff -->
+<!-- ... -->
+<!-- // scalastyle:on -->
+
+<scalastyle>
+ <name>Scalastyle standard configuration</name>
+ <check level="error" class="org.scalastyle.file.FileTabChecker" enabled="true"></check>
+ <!-- <check level="error" class="org.scalastyle.file.FileLengthChecker" enabled="true"> -->
+ <!--  <parameters> -->
+ <!--   <parameter name="maxFileLength"><![CDATA[800]]></parameter> -->
+ <!--  </parameters> -->
+ <!-- </check> -->
+<!--  <check level="error" class="org.scalastyle.file.HeaderMatchesChecker" enabled="true">
+  <parameters>
+   <parameter name="header"><![CDATA[/*
+ * Copyright (C) 2012 The Regents of The University California.
+ * 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.
+ * 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.
+ */]]></parameter>
+  </parameters>
+ </check> -->
+ <check level="error" class="org.scalastyle.scalariform.SpacesAfterPlusChecker" enabled="true"></check>
+ <check level="error" class="org.scalastyle.file.WhitespaceEndOfLineChecker" enabled="true"></check>
+ <check level="error" class="org.scalastyle.scalariform.SpacesBeforePlusChecker" enabled="true"></check>
+ <check level="error" class="org.scalastyle.file.FileLineLengthChecker" enabled="true">
+  <parameters>
+   <parameter name="maxLineLength"><![CDATA[100]]></parameter>
+   <parameter name="tabSize"><![CDATA[2]]></parameter>
+  </parameters>
+ </check>
+ <check level="error" class="org.scalastyle.scalariform.ClassNamesChecker" enabled="true">
+  <parameters>
+   <parameter name="regex"><![CDATA[[A-Z][A-Za-z]*]]></parameter>
+  </parameters>
+ </check>
+ <check level="error" class="org.scalastyle.scalariform.ObjectNamesChecker" enabled="true">
+  <parameters>
+   <parameter name="regex"><![CDATA[[A-Z][A-Za-z]*]]></parameter>
+  </parameters>
+ </check>
+ <check level="error" class="org.scalastyle.scalariform.PackageObjectNamesChecker" enabled="true">
+  <parameters>
+   <parameter name="regex"><![CDATA[^[a-z][A-Za-z]*$]]></parameter>
+  </parameters>
+ </check>
+ <check level="error" class="org.scalastyle.scalariform.EqualsHashCodeChecker" enabled="true"></check>
+ <!-- <check level="error" class="org.scalastyle.scalariform.IllegalImportsChecker" enabled="true"> -->
+ <!--  <parameters> -->
+ <!--   <parameter name="illegalImports"><![CDATA[sun._,java.awt._]]></parameter> -->
+ <!--  </parameters> -->
+ <!-- </check> -->
+ <check level="error" class="org.scalastyle.scalariform.ParameterNumberChecker" enabled="true">
+  <parameters>
+   <parameter name="maxParameters"><![CDATA[8]]></parameter>
+  </parameters>
+ </check>
+ <!-- <check level="error" class="org.scalastyle.scalariform.MagicNumberChecker" enabled="true"> -->
+ <!--  <parameters> -->
+ <!--   <parameter name="ignore"><![CDATA[-1,0,1,2,3]]></parameter> -->
+ <!--  </parameters> -->
+ <!-- </check> -->
+ <check level="error" class="org.scalastyle.scalariform.NoWhitespaceBeforeLeftBracketChecker" enabled="true"></check>
+ <check level="error" class="org.scalastyle.scalariform.NoWhitespaceAfterLeftBracketChecker" enabled="true"></check>
+ <!-- <check level="error" class="org.scalastyle.scalariform.ReturnChecker" enabled="true"></check> -->
+ <!-- <check level="error" class="org.scalastyle.scalariform.NullChecker" enabled="true"></check> -->
+ <!-- <check level="error" class="org.scalastyle.scalariform.NoCloneChecker" enabled="true"></check> -->
+ <!-- <check level="error" class="org.scalastyle.scalariform.NoFinalizeChecker" enabled="true"></check> -->
+ <!-- <check level="error" class="org.scalastyle.scalariform.CovariantEqualsChecker" enabled="true"></check> -->
+ <!-- <check level="error" class="org.scalastyle.scalariform.StructuralTypeChecker" enabled="true"></check> -->
+ <!-- <check level="error" class="org.scalastyle.file.RegexChecker" enabled="true"> -->
+ <!--  <parameters> -->
+ <!--   <parameter name="regex"><![CDATA[println]]></parameter> -->
+ <!--  </parameters> -->
+ <!-- </check> -->
+ <!-- <check level="error" class="org.scalastyle.scalariform.NumberOfTypesChecker" enabled="true"> -->
+ <!--  <parameters> -->
+ <!--   <parameter name="maxTypes"><![CDATA[30]]></parameter> -->
+ <!--  </parameters> -->
+ <!-- </check> -->
+ <!-- <check level="error" class="org.scalastyle.scalariform.CyclomaticComplexityChecker" enabled="true"> -->
+ <!--  <parameters> -->
+ <!--   <parameter name="maximum"><![CDATA[10]]></parameter> -->
+ <!--  </parameters> -->
+ <!-- </check> -->
+ <check level="error" class="org.scalastyle.scalariform.UppercaseLChecker" enabled="true"></check>
+ <check level="error" class="org.scalastyle.scalariform.SimplifyBooleanExpressionChecker" enabled="true"></check>
+ <check level="error" class="org.scalastyle.scalariform.IfBraceChecker" enabled="true">
+  <parameters>
+   <parameter name="singleLineAllowed"><![CDATA[true]]></parameter>
+   <parameter name="doubleLineAllowed"><![CDATA[true]]></parameter>
+  </parameters>
+ </check>
+ <!-- <check level="error" class="org.scalastyle.scalariform.MethodLengthChecker" enabled="true"> -->
+ <!--  <parameters> -->
+ <!--   <parameter name="maxLength"><![CDATA[50]]></parameter> -->
+ <!--  </parameters> -->
+ <!-- </check> -->
+ <!-- <check level="error" class="org.scalastyle.scalariform.MethodNamesChecker" enabled="true"> -->
+ <!--  <parameters> -->
+ <!--   <parameter name="regex"><![CDATA[^[a-z][A-Za-z0-9]*$]]></parameter> -->
+ <!--  </parameters> -->
+ <!-- </check> -->
+ <!-- <check level="error" class="org.scalastyle.scalariform.NumberOfMethodsInTypeChecker" enabled="true"> -->
+ <!--  <parameters> -->
+ <!--   <parameter name="maxMethods"><![CDATA[30]]></parameter> -->
+ <!--  </parameters> -->
+ <!-- </check> -->
+ <!-- <check level="error" class="org.scalastyle.scalariform.PublicMethodsHaveTypeChecker" enabled="true"></check> -->
+ <check level="error" class="org.scalastyle.file.NewLineAtEofChecker" enabled="true"></check>
+ <check level="error" class="org.scalastyle.file.NoNewLineAtEofChecker" enabled="false"></check>
+</scalastyle>

src/main/scala/catalyst/analysis/HiveTypeCoercion.scala

@@ -201,7 +201,8 @@ trait HiveTypeCoercion {
       // No need to change Equals operators as that actually makes sense for boolean types.
       case e: Equals => e
       // Otherwise turn them to Byte types so that there exists and ordering.
-      case p: BinaryComparison if p.left.dataType == BooleanType && p.right.dataType == BooleanType =>
+      case p: BinaryComparison
+          if p.left.dataType == BooleanType && p.right.dataType == BooleanType =>
         p.makeCopy(Array(Cast(p.left, ByteType), Cast(p.right, ByteType)))
     }
   }

src/main/scala/catalyst/analysis/package.scala

@@ -5,4 +5,4 @@ package catalyst
  * Analysis consists of translating [[UnresolvedAttribute]]s and [[UnresolvedRelation]]s
  * into fully typed objects using information in a schema [[Catalog]].
  */
-package object analysis
+package object analysis

src/main/scala/catalyst/dsl.scala → src/main/scala/catalyst/dsl/package.scala

@@ -9,7 +9,7 @@ import plans.logical._
 import types._
 
 /**
- * A collection of implicit conversions that create a DSL for easily constructing catalyst data structures.
+ * A collection of implicit conversions that create a DSL for constructing catalyst data structures.
  *
  * {{{
  *  scala> import catalyst.dsl._
@@ -35,22 +35,22 @@ import types._
  * }}}
  */
 package object dsl {
-  abstract protected trait ImplicitOperators {
+  protected trait ImplicitOperators {
     def expr: Expression
 
-    def +(other: Expression) = Add(expr, other)
-    def -(other: Expression) = Subtract(expr, other)
-    def *(other: Expression) = Multiply(expr, other)
-    def /(other: Expression) = Divide(expr, other)
+    def + (other: Expression) = Add(expr, other)
+    def - (other: Expression) = Subtract(expr, other)
+    def * (other: Expression) = Multiply(expr, other)
+    def / (other: Expression) = Divide(expr, other)
 
-    def &&(other: Expression) = And(expr, other)
-    def ||(other: Expression) = Or(expr, other)
+    def && (other: Expression) = And(expr, other)
+    def || (other: Expression) = Or(expr, other)
 
-    def <(other: Expression) = LessThan(expr, other)
-    def <=(other: Expression) = LessThanOrEqual(expr, other)
-    def >(other: Expression) = GreaterThan(expr, other)
-    def >=(other: Expression) = GreaterThanOrEqual(expr, other)
-    def ===(other: Expression) = Equals(expr, other)
+    def < (other: Expression) = LessThan(expr, other)
+    def <= (other: Expression) = LessThanOrEqual(expr, other)
+    def > (other: Expression) = GreaterThan(expr, other)
+    def >= (other: Expression) = GreaterThanOrEqual(expr, other)
+    def === (other: Expression) = Equals(expr, other)
 
     def asc = SortOrder(expr, Ascending)
     def desc = SortOrder(expr, Descending)
@@ -79,9 +79,10 @@ package object dsl {
     def attr = analysis.UnresolvedAttribute(s)
 
     /** Creates a new typed attributes of type int */
-    def int = AttributeReference(s, IntegerType, false)()
+    def int = AttributeReference(s, IntegerType, nullable = false)()
+
     /** Creates a new typed attributes of type string */
-    def string = AttributeReference(s, StringType, false)()
+    def string = AttributeReference(s, StringType, nullable = false)()
   }
 
   implicit class DslAttribute(a: AttributeReference) {
@@ -94,18 +95,27 @@ package object dsl {
 
   implicit class DslLogicalPlan(plan: LogicalPlan) {
     def select(exprs: NamedExpression*) = Project(exprs, plan)
+
     def where(condition: Expression) = Filter(condition, plan)
-    def join(otherPlan: LogicalPlan, joinType: JoinType = Inner, condition: Option[Expression] = None) =
+
+    def join(
+        otherPlan: LogicalPlan,
+        joinType: JoinType = Inner,
+        condition: Option[Expression] = None) =
       Join(plan, otherPlan, joinType, condition)
+
     def orderBy(sortExprs: SortOrder*) = Sort(sortExprs, plan)
+
     def groupBy(groupingExprs: Expression*)(aggregateExprs: Expression*) = {
       val aliasedExprs = aggregateExprs.map {
         case ne: NamedExpression => ne
         case e => Alias(e, e.toString)()
       }
       Aggregate(groupingExprs, aliasedExprs, plan)
     }
+
     def subquery(alias: Symbol) = Subquery(alias.name, plan)
+
     def unionAll(otherPlan: LogicalPlan) = Union(plan, otherPlan)
 
     def filter[T1](arg1: Symbol)(udf: (T1) => Boolean) =
@@ -122,4 +132,4 @@ package object dsl {
 
     def analyze = analysis.SimpleAnalyzer(plan)
   }
-}
+}

src/main/scala/catalyst/errors/package.scala

@@ -3,7 +3,7 @@ package catalyst
 import trees._
 
 /**
- * Functions for attaching and retrieving trees that are associated with errors in a catalyst optimizer.
+ * Functions for attaching and retrieving trees that are associated with errors.
  */
 package object errors {
 
@@ -31,4 +31,4 @@ package object errors {
    * the stack of exceptions of type `TreeType` is returned.
    */
   def getTree[TreeType <: TreeNode[_]](f: => Unit): TreeType = ??? // TODO: Implement
-}
+}

src/main/scala/catalyst/examples/SchemaRddExample.scala

@@ -39,4 +39,4 @@ object SchemaRddExample {
     val filtered2 = testLogs.filter( _.date match { case dateRegEx(_,day,_) => day.toInt == 1 } )
     filtered2.toRdd.collect.foreach(println)
   }
-}
+}

src/main/scala/catalyst/examples/ViewsExample.scala

@@ -12,9 +12,12 @@ import execution.TestShark._ // For .toRdd execution using locally running test
 object ViewsExample {
   def main(args: Array[String]): Unit = {
     // Create a list of named views that can be substituted into logical plans.
-    // In this example the views read from local, in-memory relations with schema (a INT, b STRING) and (c INT, d STRING)
-    // respectively. loadData returns a copy of that relation with the specified tuples appended to the Rdd.
-    // The .select uses the DSL to add a projection on top of the relation that returns only the column "a".
+    // In this example the views read from local, in-memory relations with schema
+    // (a INT, b STRING) and (c INT, d STRING) respectively.
+    //
+    // loadData returns a copy of that relation with the specified tuples appended to the Rdd.
+    // The .select uses the DSL to add a projection on top of the relation that returns only
+    // the column "a".
     val views = Map(
       "view1" -> LocalRelation('a.int, 'b.string).loadData(("a", 1) :: ("b", 2) :: Nil).select('a),
       "view2" -> LocalRelation('c.int, 'd.string).loadData(("c", 1) :: ("d", 2) :: Nil)
@@ -35,6 +38,6 @@ object ViewsExample {
 
     println(s"With relations:\n$withRelations ")
     println(s"Analyzed:\n${withRelations.analyze}") // Print with all references resolved.
-    println(s"Answer: ${withRelations.toRdd.collect.toSeq}")
+    println(s"Answer: ${withRelations.toRdd.collect().toSeq}")
   }
-}
+}

src/main/scala/catalyst/execution/FunctionRegistry.scala

@@ -6,8 +6,10 @@ import scala.collection.JavaConversions._
 import org.apache.hadoop.hive.ql.exec.{FunctionInfo, FunctionRegistry}
 import org.apache.hadoop.hive.ql.udf.generic.GenericUDF
 import org.apache.hadoop.hive.ql.exec.UDF
-import org.apache.hadoop.hive.serde2.objectinspector.primitive.{AbstractPrimitiveJavaObjectInspector, PrimitiveObjectInspectorFactory}
-import org.apache.hadoop.io._
+import org.apache.hadoop.hive.serde2.{io => hiveIo}
+import org.apache.hadoop.hive.serde2.objectinspector.primitive.AbstractPrimitiveJavaObjectInspector
+import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory
+import org.apache.hadoop.{io => hadoopIo}
 
 import expressions._
 import types._
@@ -38,15 +40,15 @@ object HiveFunctionRegistry extends analysis.FunctionRegistry {
   }
 
   def javaClassToDataType(clz: Class[_]): DataType = clz match {
-    case c: Class[_] if c == classOf[DoubleWritable] => DoubleType
-    case c: Class[_] if c == classOf[org.apache.hadoop.hive.serde2.io.DoubleWritable] => DoubleType
-    case c: Class[_] if c == classOf[org.apache.hadoop.hive.serde2.io.HiveDecimalWritable] => DecimalType
-    case c: Class[_] if c == classOf[org.apache.hadoop.hive.serde2.io.ByteWritable] => ByteType
-    case c: Class[_] if c == classOf[org.apache.hadoop.hive.serde2.io.ShortWritable] => ShortType
-    case c: Class[_] if c == classOf[Text] => StringType
-    case c: Class[_] if c == classOf[org.apache.hadoop.io.IntWritable] => IntegerType
-    case c: Class[_] if c == classOf[org.apache.hadoop.io.LongWritable] => LongType
-    case c: Class[_] if c == classOf[org.apache.hadoop.io.FloatWritable] => FloatType
+    case c: Class[_] if c == classOf[hadoopIo.DoubleWritable] => DoubleType
+    case c: Class[_] if c == classOf[hiveIo.DoubleWritable] => DoubleType
+    case c: Class[_] if c == classOf[hiveIo.HiveDecimalWritable] => DecimalType
+    case c: Class[_] if c == classOf[hiveIo.ByteWritable] => ByteType
+    case c: Class[_] if c == classOf[hiveIo.ShortWritable] => ShortType
+    case c: Class[_] if c == classOf[hadoopIo.Text] => StringType
+    case c: Class[_] if c == classOf[hadoopIo.IntWritable] => IntegerType
+    case c: Class[_] if c == classOf[hadoopIo.LongWritable] => LongType
+    case c: Class[_] if c == classOf[hadoopIo.FloatWritable] => FloatType
     case c: Class[_] if c == classOf[java.lang.String] => StringType
     case c: Class[_] if c == java.lang.Short.TYPE => ShortType
     case c: Class[_] if c == java.lang.Integer.TYPE => ShortType
@@ -82,14 +84,14 @@ abstract class HiveUdf extends Expression with ImplementedUdf with Logging {
 
   def unwrap(a: Any): Any = a match {
     case null => null
-    case i: IntWritable => i.get
-    case t: Text => t.toString
-    case l: LongWritable => l.get
-    case d: DoubleWritable => d.get()
-    case d: org.apache.hadoop.hive.serde2.io.DoubleWritable => d.get
-    case s: org.apache.hadoop.hive.serde2.io.ShortWritable => s.get
-    case b: BooleanWritable => b.get()
-    case b: org.apache.hadoop.hive.serde2.io.ByteWritable => b.get
+    case i: hadoopIo.IntWritable => i.get
+    case t: hadoopIo.Text => t.toString
+    case l: hadoopIo.LongWritable => l.get
+    case d: hadoopIo.DoubleWritable => d.get()
+    case d: hiveIo.DoubleWritable => d.get
+    case s: hiveIo.ShortWritable => s.get
+    case b: hadoopIo.BooleanWritable => b.get()
+    case b: hiveIo.ByteWritable => b.get
     case list: java.util.List[_] => list.map(unwrap)
     case p: java.lang.Short => p
     case p: java.lang.Long => p
@@ -174,7 +176,7 @@ case class HiveGenericUdf(
   }
 
   def wrap(a: Any): Any = a match {
-    case s: String => new Text(s)
+    case s: String => new hadoopIo.Text(s)
     case i: Int => i: java.lang.Integer
     case b: Boolean => b: java.lang.Boolean
     case d: Double => d: java.lang.Double
@@ -191,4 +193,4 @@ case class HiveGenericUdf(
     }.toArray
     unwrap(instance.evaluate(args))
   }
-}
+}