Extern config

README.md

@@ -17,7 +17,11 @@ Or, until published:
 
 Optionally, run `sbt console` in this project to see it in action.
 
-## Currently suported warnings
+## Currently supported warnings
+
+Note: for configuration documentation, unless otherwise stated, any "checkEnabled" configuration option
+is one of {"true", "false"} and any severity configuration option is one of {"warn", "error"}.
+Configuration documentation examples show the default settings that ship with this linter library.
 
 ### Unsafe `==`
 
@@ -26,13 +30,21 @@ Optionally, run `sbt console` in this project to see it in action.
                   Nil == None
                       ^
 
+    Configuration:
+        linter.eqeq.checkEnabled=true
+        linter.eqeq.severity=error
+
 ### Unsafe `contains`
 
     scala> List(1, 2, 3).contains("4")
     <console>:29: warning: SeqLike[Int].contains(java.lang.String) will probably return false.
                   List(1, 2, 3).contains("4")
                                 ^
 
+    Configuration:
+        linter.seq.contains.checkEnabled=true
+        linter.seq.contains.severity=warn
+
 
 ### Wildcard import from `scala.collection.JavaConversions`
 
@@ -41,25 +53,46 @@ Optionally, run `sbt console` in this project to see it in action.
            import scala.collection.JavaConversions._
                                    ^
 
+    Configuration:
+        None at present
+
 ### Any and all wildcard imports
 
     scala> import scala.collection.JavaConversions._
     <console>:10: warning: Wildcard imports should be avoided.
            import scala.reflect.generic._
 
 
+    Configuration:
+        linter.package.wildcard.whitelist.checkEnabled=true
+        linter.package.wildcard.whitelist.severity=warn
+        linter.package.wildcard.whitelist.packages=[]
+
 ### Calling `Option#get`
 
     scala> Option(1).get
     <console>:29: warning: Calling .get on Option will throw an exception if the Option is None.
                   Option(1).get
                             ^
 
+    Configuration:
+        linter.option.get.checkEnabled=true
+        linter.option.get.severity=warn
+
+## Configuring Linter
+
+Linter is configured with the com.typesafe.config library (https://github.com/typesafehub/config)
+
+In your application, you can override any of the configuration options from these locations (in priority-respecting order):
+* system properties
+* application.conf available on the classpath
+* application.json available on the classpath
+* application.properties available on the classpath
+
 ## Future Work
 
 * Add more warnings
-* Pick and choose which warnings you want
-* Choose whether they should be warnings or errors
+* Support "blacklist"ing imports and re-write the scala.collection.JavaCollections check with that mechanism
 
 ### Ideas for new warnings
 

build.sbt

@@ -1,12 +1,14 @@
 libraryDependencies <++= (scalaVersion) { (scalaVer) =>
   Seq(
     "org.scala-lang"           % "scala-compiler"  % scalaVer,
+    "com.typesafe"             % "config"          % "0.5.0",
     "org.scala-tools.testing" %% "specs"           % "1.6.9"  % "test" withSources(),
     "junit"                    % "junit"           % "4.8.2"  % "test" withSources(),
     "com.novocode"             % "junit-interface" % "0.7"    % "test"
   )
 }
 
-scalacOptions in console in Compile <+= (packageBin in Compile) map { pluginJar =>
-  "-Xplugin:"+pluginJar
+scalacOptions in console in Compile <++= (packageBin in Compile, managedClasspath in Compile) map { (pluginJar, cp) =>
+  val configJar = cp.map(_.data).find(_.toString.contains("config-0.5.0.jar"))
+  Seq("-Xplugin:"+pluginJar) ++ configJar.map("-Xplugin:"+_)
 }

src/main/resources/reference.conf

@@ -0,0 +1,62 @@
+#
+# HOCON-standard Linter configuration to control which Lint checks to run 
+# For syntax help, @see https://github.com/typesafehub/config
+#
+# Unless otherwise stated, any "checkEnabled" configuration is one of {"true", "false"}
+# Unless otherwise stated, any "severity" configuration is one of {"warn", "error"}
+#
+linter {
+
+    #
+    # EqualsEquals comparison check on objects of different types
+    #
+    eqeq {
+        checkEnabled=true
+        severity="error"
+    }
+
+    #
+    # Wildcard package import control - controls warnings related to importing foo.bar._
+    #
+    package.wildcard {
+
+        whitelist {
+            checkEnabled=true
+            severity="warn"
+
+            # Comma-separated list of packages to allow for wildcard importing
+            packages=[]
+        }
+
+        #  === TODO === respect me in the morning (these conf settings are ignored by the plugin)
+        blacklist {
+            # Set to true to check for a blacklist wildcard import, regardless of the "whitelist" setting
+            checkEnabled=true
+
+            # Comma-separated list of packages in the blacklist
+            blacklist=["scala.collections.JavaConversions"]
+        }
+    }
+
+    #
+    # Sequence/Collection related checks
+    #
+    seq {
+        # Checks for Seq.contains calls with different types
+        contains {
+            checkEnabled=true
+            severity=warn
+        }
+    }
+
+    #
+    # Option related checks (TODO - is there a broader classification that makes for better organization?)
+    #
+    option {
+        # Check for calling Option.get 
+        get {
+            checkEnabled=true
+            severity=warn
+        }
+    }
+}

src/main/scala/LinterConfig.scala

@@ -0,0 +1,27 @@
+package com.foursquare.lint
+
+import com.typesafe.config.ConfigFactory 
+
+/**
+ * <p>Configuration class for running (or not) different lint checks</p>
+ * <p>As a library, linter defaults to including the src/main/resources/reference.conf settings
+ * which are override-able by an including application.</p>
+ */
+object LinterConfig {
+
+  val config = ConfigFactory.load("linter")
+  config.checkValid(ConfigFactory.defaultReference(), "linter")
+
+  val eqeqCheckEnabled = config.getBoolean("linter.eqeq.checkEnabled")
+  val eqeqSeverity = LinterSeverity.withName(config.getString("linter.eqeq.severity"))
+
+  val packageWildcardWhitelistCheckEnabled = config.getBoolean("linter.package.wildcard.whitelist.checkEnabled")
+  val packageWildcardWhitelistPackages = config.getStringList("linter.package.wildcard.whitelist.packages")
+  val packageWildcardWhitelistSeverity = LinterSeverity.withName(config.getString("linter.package.wildcard.whitelist.severity"))
+
+  val seqContainsCheckEnabled = config.getBoolean("linter.seq.contains.checkEnabled")
+  val seqContainsSeverity = LinterSeverity.withName(config.getString("linter.seq.contains.severity"))
+
+  val optionGetCheckEnabled = config.getBoolean("linter.option.get.checkEnabled")
+  val optionGetSeverity = LinterSeverity.withName(config.getString("linter.option.get.severity"))
+}

src/main/scala/LinterPlugin.scala

@@ -26,7 +26,7 @@ class LinterPlugin(val global: Global) extends Plugin {
   val name = "linter"
   val description = ""
   val components = List[PluginComponent](LinterComponent)
-
+  
   private object LinterComponent extends PluginComponent {
     import global._
 
@@ -44,6 +44,8 @@ class LinterPlugin(val global: Global) extends Plugin {
 
     class LinterTraverser(unit: CompilationUnit) extends Traverser {
       import definitions.{AnyClass, ObjectClass, Object_==, OptionClass, SeqClass}
+      import LinterSeverity._
+      import LinterConfig._
 
       val JavaConversionsModule: Symbol = definitions.getModule("scala.collection.JavaConversions")
       val SeqLikeClass: Symbol = definitions.getClass("scala.collection.SeqLike")
@@ -66,33 +68,59 @@ class LinterPlugin(val global: Global) extends Plugin {
         selector.name == nme.WILDCARD && selector.renamePos == -1
       }
 
+      /**
+       * Apply a warning or an error to some position in the code being linted
+       */
+      def annotateUnit(pos: Position, message: String, severity: LinterSeverity.Value): Unit = severity match {
+        case WARN  => unit.warning(pos, message)
+        case ERROR => unit.error(pos, message)
+      }
+
       override def traverse(tree: Tree): Unit = tree match {
+        // eqeq check
         case Apply(eqeq @ Select(lhs, nme.EQ), List(rhs))
-            if methodImplements(eqeq.symbol, Object_==) && !(isSubtype(lhs, rhs) || isSubtype(rhs, lhs)) =>
+            if eqeqCheckEnabled 
+            && methodImplements(eqeq.symbol, Object_==) 
+            && !(isSubtype(lhs, rhs) || isSubtype(rhs, lhs)) =>
           val warnMsg = "Comparing with == on instances of different types (%s, %s) will probably return false."
-          unit.warning(eqeq.pos, warnMsg.format(lhs.tpe.widen, rhs.tpe.widen))
+          annotateUnit(eqeq.pos, warnMsg.format(lhs.tpe.widen, rhs.tpe.widen), eqeqSeverity)
 
+          // TODO - replace this with an implementation of the blacklist from our Config
         case Import(pkg, selectors)
             if pkg.symbol == JavaConversionsModule && selectors.exists(isGlobalImport) =>
           unit.warning(pkg.pos, "Conversions in scala.collection.JavaConversions._ are dangerous.")
 
+          // TODO - respect the whitelist exceptions
         case Import(pkg, selectors)
-            if selectors.exists(isGlobalImport) =>
-          unit.warning(pkg.pos, "Wildcard imports should be avoided.  Favor import selector clauses.")
+            if packageWildcardWhitelistCheckEnabled 
+            && selectors.exists(isGlobalImport) 
+
+            /* For a statement like "import java.util._", 
+               pkg.symbol = "util"
+               pkg.class = class scala.reflect.generic.Trees$Select
+               We need to figure out how to get the "ancestor symbol(s)" of pkg to do string matching on the whole package name
+
+             && !(packageWildcardWhitelistPackages.contains(pkg.symbol)) */ =>
+            println("The class of pkg is " + pkg.getClass)
+            annotateUnit(pkg.pos, "Wildcard imports should be avoided.  Favor import selector clauses.", packageWildcardWhitelistSeverity)
 
         case Apply(contains @ Select(seq, _), List(target))
-            if methodImplements(contains.symbol, SeqLikeContains) && !(target.tpe <:< SeqMemberType(seq.tpe)) =>
+            if seqContainsCheckEnabled 
+            && methodImplements(contains.symbol, SeqLikeContains) 
+            && !(target.tpe <:< SeqMemberType(seq.tpe)) =>
           val warnMsg = "SeqLike[%s].contains(%s) will probably return false."
-          unit.warning(contains.pos, warnMsg.format(SeqMemberType(seq.tpe), target.tpe.widen))
+          annotateUnit(contains.pos, warnMsg.format(SeqMemberType(seq.tpe), target.tpe.widen), seqContainsSeverity)
 
-        case get @ Select(_, nme.get) if methodImplements(get.symbol, OptionGet) =>
-          if (!get.pos.source.path.contains("src/test")) {
-            unit.warning(get.pos, "Calling .get on Option will throw an exception if the Option is None.")
-          }
+        case get @ Select(_, nme.get) 
+          if optionGetCheckEnabled
+          && methodImplements(get.symbol, OptionGet)
+          && !(get.pos.source.path.contains("src/test")) =>
+            annotateUnit(get.pos, "Calling .get on Option will throw an exception if the Option is None.", optionGetSeverity)
 
         case _ =>
           super.traverse(tree)
       }
     }
   }
 }
+

src/main/scala/LinterSeverity.scala

@@ -0,0 +1,15 @@
+package com.foursquare.lint
+
+/**
+ * Enumeration that models the different compile-time notifications Linter
+ * can alert the developer to based on configuration of the different
+ * types of lint checks.
+ * 
+ * These severities are a subset of the available notification functions on
+ * the @scala.tools.nsc.CompilationUnits$CompilationUnit
+ */
+object LinterSeverity extends Enumeration {
+  type LinterSeverity = Value
+  val WARN = Value("warn")
+  val ERROR = Value("error")
+}