Merge pull request ncredinburgh#3 from MichaelZinsmaier/PR-TemplateSu…

…pport

Template support for sonar-scalastyle

README.md

@@ -2,7 +2,10 @@
 
 # Sonar Scalastyle Plugin
 
-Wraps up Scalastyle as a SonarQube plugin.
+Wraps up Scalastyle as a SonarQube plugin. Currently scalastyle provides 60 different checks. They  
+are represented as 38 rules without parameters and 22 templates in SonarQube. Templates allow to instantiate
+the same check multiple times with different values. For example different regular expression rules with different
+severity levels.
 
 http://www.scalastyle.org/
 

pom.xml

@@ -12,7 +12,7 @@
     <groupId>com.ncredinburgh</groupId>
     <artifactId>sonar-scalastyle</artifactId>
     <packaging>sonar-plugin</packaging>
-    <version>0.0.2-SNAPSHOT</version>
+    <version>0.0.3-SNAPSHOT</version>
 
     <name>Sonar Scalastyle Plugin</name>
     <description>Enables analysis of Scala projects with Scalastyle.</description>

src/main/scala/com/ncredinburgh/sonar/scalastyle/Constants.scala

@@ -23,4 +23,7 @@ object Constants {
   val RepositoryKey = "Scalastyle"
   val RepositoryName = "Scalastyle Rules"
   val ProfileName = "Scalastyle"
+
+  /** the class of the checker that should be executed by the sonar rule */
+  val ClazzParam = "scalastyle-checker"
 }

src/main/scala/com/ncredinburgh/sonar/scalastyle/ScalastyleQualityProfile.scala

@@ -24,7 +24,9 @@ import org.sonar.api.rules.{RuleFinder, ActiveRule}
 import org.sonar.api.utils.ValidationMessages
 import org.scalastyle.ScalastyleError
 import scala.xml.XML
-
+import collection.JavaConversions._
+import org.sonar.api.rules.RuleQuery
+import org.sonar.api.rules.Rule
 
 /**
  * This class creates the default "Scalastyle" quality profile from Scalastyle's default_config.xml
@@ -37,30 +39,54 @@ class ScalastyleQualityProfile(ruleFinder: RuleFinder) extends ProfileDefinition
   override def createProfile(validation: ValidationMessages): RulesProfile = {
     val profile = RulesProfile.create(Constants.ProfileName, Constants.ScalaKey)
     val enabledRules = defaultConfigRules filter (x => (x \ "@enabled").text.equals("true"))
-    val defaultKeys = enabledRules map (x => (x \ "@class").text)
+    val defaultRuleClasses = enabledRules map (x => (x \ "@class").text)
 
-    val defaultRules = defaultKeys map {
-      case ruleKey =>
-        val rule = Option(ruleFinder.findByKey(Constants.RepositoryKey, ruleKey))
-        if (rule.isEmpty) validation.addWarningText(
-          s"Rule $ruleKey not found in ${Constants.RepositoryKey} repository! Rule won't be activated.")
-        rule
-    }
+    // currently findAll is buggy (sonar 4.5-5.1 https://jira.sonarsource.com/browse/SONAR-6390)
+    // will still work but won't add all possible rule to the default profile
+    val query = RuleQuery.create().withRepositoryKey(Constants.RepositoryKey)
+    val repoRules = ruleFinder.findAll(query)
+
+    for {clazz <- defaultRuleClasses} {
+      val ruleOption = repoRules.find(clazzMatch(_, clazz))
 
-    val activeRules = defaultRules.flatten.map(rule => profile.activateRule(rule, rule.getSeverity))
-    activeRules.foreach(setParameters)
+      ruleOption match {
+        case None => validation.addWarningText(s"Rule for $clazz not found in ${Constants.RepositoryKey} repository! Rule won't be activated.")
+        case Some(rule) => {
+          if (!rule.isTemplate()) {
+            val activated = profile.activateRule(rule, rule.getSeverity)
+            setParameters(activated, clazz)
+          }
+        }
+      }
+    }
 
     profile
   }
 
-  def setParameters(activeRule: ActiveRule) {
-    defaultConfigRules.find(x => (x \ "@class").text.equals(activeRule.getRuleKey) ) match {
+  def setParameters(activeRule: ActiveRule, clazz: String) {
+    // set parameters
+    defaultConfigRules.find(x => (x \ "@class").text.equals(clazz)) match {
       case Some(rule) => {
-        val params = (rule \ "parameters" \ "parameter").map(n => ((n \ "@name").text, n.text )).toMap
+        val params = (rule \ "parameters" \ "parameter").map(n => ((n \ "@name").text, n.text)).toMap
         params foreach { case (key, value) => activeRule.setParameter(key, value) }
       }
       case _ => log.warn("Default rule with key " + activeRule.getRuleKey + " could not found in default_config.xml")
     }
+
+    // set synthetic parameter
+    activeRule.setParameter(Constants.ClazzParam, clazz)
+  }
+
+  private def clazzMatch(rule: Rule, clazz: String): Boolean = {
+    Option(rule.getParam(Constants.ClazzParam)) match {
+      case Some(param) => {
+        param.getDefaultValue.equals(clazz)
+      }         
+      case None => {
+        log.warn(s"Could not find required parameter ${Constants.ClazzParam}, rule for $clazz cannot be activated")
+        false
+      }
+    }
   }
 
   private def xmlFromClassPath(s: String) = XML.load(classOf[ScalastyleError].getResourceAsStream(s))

src/main/scala/com/ncredinburgh/sonar/scalastyle/ScalastyleRepository.scala

@@ -20,7 +20,19 @@ package com.ncredinburgh.sonar.scalastyle
 
 import org.sonar.api.rule.Severity
 import org.sonar.api.server.rule.RulesDefinition
+import org.sonar.api.server.rule.RuleParamType
+import org.slf4j.LoggerFactory
+import org.sonar.api.server.rule.RulesDefinition.NewRepository
+import com.ncredinburgh.sonar.scalastyle.ScalastyleRepository.getStandardKey
+import scala.annotation.tailrec
 
+object ScalastyleRepository {
+
+  def getStandardKey(clazz: String) = {
+    val simpleClazz = clazz.reverse.takeWhile(_ != '.').reverse
+    s"scalastyle_${simpleClazz}"
+  }
+}
 
 /**
  * Scalastyle rules repository - creates a rule for each checker shipped with Scalastyle based
@@ -32,25 +44,63 @@ class ScalastyleRepository extends RulesDefinition {
     val repository = context
       .createRepository(Constants.RepositoryKey, Constants.ScalaKey)
       .setName(Constants.RepositoryName)
-    ScalastyleResources.allDefinedRules map {
-      case resRule =>
-        val rule = repository.createRule(resRule.clazz)
-        rule.setName(ScalastyleResources.label(resRule.id))
-        rule.setHtmlDescription(resRule.description)
-        // currently all rules comes with "warning" default level so we can treat with major severity
-        rule.setSeverity(Severity.MAJOR)
-
-        resRule.params map {
-          case param =>
-            rule
-              .createParam(param.name)
-              .setDefaultValue(param.defaultVal)
-              .setType(param.`type`)
-              .setDescription(param.desc)
+
+    ScalastyleResources.allDefinedRules foreach {
+      repoRule =>
+        {
+          val ruleKey = determineFreeRuleKey(repoRule.clazz, repository)
+
+          // define the rule
+          val rule = repository.createRule(ruleKey)
+          rule.setName(ScalastyleResources.label(repoRule.id))
+          rule.setHtmlDescription(repoRule.description)
+
+          // currently all rules comes with "warning" default level so we can treat with major severity
+          rule.setSeverity(Severity.MAJOR)
+
+          // add parameters
+          repoRule.params foreach {
+            param =>
+              {
+                rule
+                  .createParam(param.name)
+                  .setDefaultValue(param.defaultVal)
+                  .setType(param.`type`)
+                  .setDescription(param.desc)
+              }
+          }
+
+          // add synthetic parameter as reference to the class
+          rule.createParam(Constants.ClazzParam)
+            .setDefaultValue(repoRule.clazz)
+            .setType(RuleParamType.STRING)
+            .setDescription("Scalastyle checker that validates the rule.")
+
+          // if a rule has at least one real parameter make it a template    
+          rule.setTemplate(repoRule.params.size > 0)
+
         }
     }
 
     repository.done()
   }
 
+  /**
+   * determines a free rule key in the repo, in case the key scalastyle-<simple class name> is already
+   *  in use the name scalastyle_<simple class name>_<i> is tried i = 1, 2, ....
+   */
+  private def determineFreeRuleKey(clazz: String, repo: NewRepository): String = {
+    @tailrec
+    def getFreeRuleKey(key: String, count: Int, repo: NewRepository): String = {
+      val testee = if (count == 0) key else "$key_$count"
+      if (repo.rule(testee) == null) {
+        testee
+      } else {
+        getFreeRuleKey(key, (count + 1), repo)
+      }
+    }
+
+    getFreeRuleKey(getStandardKey(clazz), 0, repo)
+  }
+
 }

src/main/scala/com/ncredinburgh/sonar/scalastyle/ScalastyleRunner.scala

@@ -62,7 +62,12 @@ class ScalastyleRunner(rp: RulesProfile) {
   }
 
   private def ruleToChecker(activeRule: ActiveRule): ConfigurationChecker = {
-    val params = activeRule.getActiveRuleParams.map(p => (p.getKey, p.getValue)).toMap
-    ConfigurationChecker(activeRule.getRuleKey, ErrorLevel, true, params, None, None)
+    val sonarParams = activeRule.getActiveRuleParams.map(p => (p.getKey, p.getValue)).toMap
+
+    val checkerParams = sonarParams.filterNot(keyVal => keyVal._1 == Constants.ClazzParam)
+    val className = sonarParams(Constants.ClazzParam)
+    val sonarKey = activeRule.getRuleKey
+
+    ConfigurationChecker(className, ErrorLevel, true, sonarParams, None, Some(sonarKey))
   }
 }

src/main/scala/com/ncredinburgh/sonar/scalastyle/ScalastyleSensor.scala

@@ -100,7 +100,7 @@ class ScalastyleSensor(resourcePerspectives: ResourcePerspectives,
 
   private def findSonarRuleForError(error: StyleError[FileSpec]): Rule = {
     val key = Constants.RepositoryKey
-    val errorKey = error.clazz.getName
+    val errorKey = error.key // == scalastyle ConfigurationChecker.customId 
     log.debug("Looking for sonar rule for " + errorKey)
     ruleFinder.find(RuleQuery.create.withKey(errorKey).withRepositoryKey(key))
   }

src/test/scala/com/ncredinburgh/sonar/scalastyle/ScalastyleQualityProfileSpec.scala → src/test/scala/com/ncredinburgh/sonar/scalastyle/ScalastyleAdaptedQualityProfileSpec.scala

@@ -18,7 +18,7 @@
  */
 package com.ncredinburgh.sonar.scalastyle
 
-import com.ncredinburgh.sonar.scalastyle.testUtils.TestRuleFinder
+import com.ncredinburgh.sonar.scalastyle.testUtils.TestRuleFinderWithTemplates
 import org.junit.runner.RunWith
 import org.scalatest._
 import org.scalatest.junit.JUnitRunner
@@ -29,15 +29,18 @@ import org.sonar.api.utils.ValidationMessages
 import scala.collection.JavaConversions._
 
 /**
- * Tests ScalastyleQualityProfile
+ * Tests an adapted ScalastyleQualityProfile, assuming the user instantiated all templates once
  */
 @RunWith(classOf[JUnitRunner])
 class ScalastyleQualityProfileSpec extends FlatSpec with Matchers with MockitoSugar {
   trait Fixture {
     val validationMessages = ValidationMessages.create
-    val testee = new ScalastyleQualityProfile(TestRuleFinder)
+    val testee = new ScalastyleQualityProfile(TestRuleFinderWithTemplates)
   }
 
+  val rulesCount = 37
+  val parametersCount = 19
+
   "a scalastyle quality profile" should "create a default profile" in new Fixture {
     val rulesProfile = testee.createProfile(validationMessages)
 
@@ -47,23 +50,21 @@ class ScalastyleQualityProfileSpec extends FlatSpec with Matchers with MockitoSu
   }
 
   "the default quality profile" should "have all the rules in default config" in new Fixture {
-    val rulesCount = 37
-
     val rulesProfile = testee.createProfile(validationMessages)
 
     rulesProfile.getActiveRules.size shouldBe rulesCount
   }
 
   it should "have all the parameters in default config" in new Fixture {
-    val parametersCount = 19
+    val totalParameters = parametersCount + (rulesCount * 1) 
 
     val rulesProfile = testee.createProfile(validationMessages)
 
-    rulesProfile.getActiveRules.flatMap(_.getActiveRuleParams).size shouldBe parametersCount
+    rulesProfile.getActiveRules.flatMap(_.getActiveRuleParams).size shouldBe totalParameters
   }
 
   it should "have correct values for parameters" in new Fixture {
-    val ruleKey = "org.scalastyle.scalariform.NumberOfMethodsInTypeChecker"
+    val ruleKey = "scalastyle_NumberOfMethodsInTypeChecker"
 
     val rulesProfile = testee.createProfile(validationMessages)
     val rule = rulesProfile.getActiveRule(Constants.RepositoryKey, ruleKey)

src/test/scala/com/ncredinburgh/sonar/scalastyle/ScalastyleDefaultQualityProfileSpec.scala

@@ -0,0 +1,64 @@
+/*
+ * Sonar Scalastyle Plugin
+ * Copyright (C) 2014 All contributors
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+ */
+package com.ncredinburgh.sonar.scalastyle
+
+import com.ncredinburgh.sonar.scalastyle.testUtils.TestRuleFinderWithTemplates
+import org.junit.runner.RunWith
+import org.scalatest._
+import org.scalatest.junit.JUnitRunner
+import org.scalatest.mock.MockitoSugar
+import org.sonar.api.profiles.RulesProfile
+import org.sonar.api.utils.ValidationMessages
+import scala.collection.JavaConversions._
+import com.ncredinburgh.sonar.scalastyle.testUtils.TestRuleFinder
+
+/**
+ * Tests the default ScalastyleQualityProfile, only rules without parameters, no templates
+ */
+@RunWith(classOf[JUnitRunner])
+class ScalastyleDefaultQualityProfileSpec extends FlatSpec with Matchers with MockitoSugar {
+  trait Fixture {
+    val validationMessages = ValidationMessages.create
+    val testee = new ScalastyleQualityProfile(TestRuleFinder)
+  }
+
+  val rulesCount = 19 // rules without templates
+
+  "a scalastyle quality profile" should "create a default profile" in new Fixture {
+    val rulesProfile = testee.createProfile(validationMessages)
+
+    rulesProfile.getClass shouldEqual classOf[RulesProfile]
+    rulesProfile.getName shouldEqual Constants.ProfileName
+    rulesProfile.getLanguage shouldEqual Constants.ScalaKey
+  }
+
+  "the default quality profile" should "have all the rules in default config" in new Fixture {
+    val rulesProfile = testee.createProfile(validationMessages)
+
+    rulesProfile.getActiveRules.size shouldBe rulesCount
+  }
+
+  it should "have all the parameters in default config" in new Fixture {
+    val totalParameters = (rulesCount * 1) 
+
+    val rulesProfile = testee.createProfile(validationMessages)
+
+    rulesProfile.getActiveRules.flatMap(_.getActiveRuleParams).size shouldBe totalParameters
+  }
+}