]> source.dussan.org Git - sonarqube.git/commitdiff
Checkstyle: fix export/import of duplicated rules (for example RegexpSingleline)
authorsimonbrandhof <simon.brandhof@gmail.com>
Fri, 8 Oct 2010 10:18:38 +0000 (10:18 +0000)
committersimonbrandhof <simon.brandhof@gmail.com>
Fri, 8 Oct 2010 10:18:38 +0000 (10:18 +0000)
plugins/sonar-checkstyle-plugin/src/main/java/org/sonar/plugins/checkstyle/CheckstyleProfileExporter.java
plugins/sonar-checkstyle-plugin/src/main/java/org/sonar/plugins/checkstyle/CheckstyleProfileImporter.java
plugins/sonar-checkstyle-plugin/src/test/java/org/sonar/plugins/checkstyle/CheckstyleProfileExporterTest.java
plugins/sonar-checkstyle-plugin/src/test/java/org/sonar/plugins/checkstyle/CheckstyleProfileImporterTest.java
plugins/sonar-checkstyle-plugin/src/test/resources/org/sonar/plugins/checkstyle/CheckstyleProfileExporterTest/addTheIdPropertyWhenManyInstancesWithTheSameConfigKey.xml
plugins/sonar-checkstyle-plugin/src/test/resources/org/sonar/plugins/checkstyle/CheckstyleProfileExporterTest/exportParameters.xml
plugins/sonar-checkstyle-plugin/src/test/resources/org/sonar/plugins/checkstyle/CheckstyleProfileExporterTest/singleCheckstyleRulesToExport.xml
plugins/sonar-checkstyle-plugin/src/test/resources/org/sonar/plugins/checkstyle/CheckstyleProfileImporterTest/idProperty.xml [deleted file]
plugins/sonar-checkstyle-plugin/src/test/resources/org/sonar/plugins/checkstyle/CheckstyleProfileImporterTest/idPropertyShouldBeTheRuleKey.xml [new file with mode: 0644]
plugins/sonar-checkstyle-plugin/src/test/resources/org/sonar/plugins/checkstyle/CheckstyleProfileImporterTest/shouldUseTheIdPropertyToFindRule.xml [new file with mode: 0644]

index 96b8acfe714858a9cf8d4bee20fdaeb445139454..3be0002210620d6bde7b1a7da4ad6680060c155d 100644 (file)
@@ -29,7 +29,6 @@ import org.sonar.api.profiles.ProfileExporter;
 import org.sonar.api.profiles.RulesProfile;
 import org.sonar.api.resources.Java;
 import org.sonar.api.rules.ActiveRule;
-import org.sonar.api.rules.ActiveRuleParam;
 import org.sonar.api.rules.RuleParam;
 import org.sonar.api.utils.SonarException;
 
@@ -94,7 +93,7 @@ public class CheckstyleProfileExporter extends ProfileExporter {
       if (!isInTreeWalker(configKey)) {
         List<ActiveRule> activeRules = activeRulesByConfigKey.get(configKey);
         for (ActiveRule activeRule : activeRules) {
-          appendModule(writer, activeRule, activeRules.size() > 1);
+          appendModule(writer, activeRule);
         }
       }
     }
@@ -107,7 +106,7 @@ public class CheckstyleProfileExporter extends ProfileExporter {
       if (isInTreeWalker(configKey)) {
         List<ActiveRule> activeRules = activeRulesByConfigKey.get(configKey);
         for (ActiveRule activeRule : activeRules) {
-          appendModule(writer, activeRule, activeRules.size() > 1);
+          appendModule(writer, activeRule);
         }
       }
     }
@@ -132,12 +131,12 @@ public class CheckstyleProfileExporter extends ProfileExporter {
     return result;
   }
 
-  private void appendModule(Writer writer, ActiveRule activeRule, boolean manyInstances) throws IOException {
+  private void appendModule(Writer writer, ActiveRule activeRule) throws IOException {
     String moduleName = StringUtils.substringAfterLast(activeRule.getConfigKey(), "/");
     writer.append("<module name=\"");
     StringEscapeUtils.escapeXml(writer, moduleName);
     writer.append("\">");
-    if (manyInstances) {
+    if (activeRule.getRule().getParent() != null) {
       appendModuleProperty(writer, "id", activeRule.getRuleKey());
     }
     appendModuleProperty(writer, "severity", CheckstyleSeverityUtils.toSeverity(activeRule.getPriority()));
@@ -164,5 +163,5 @@ public class CheckstyleProfileExporter extends ProfileExporter {
     }
   }
 
-  
+
 }
index 891f6c7d91da25527fd071f4bf6c027d1a39c19f..1766e0fb79e91a789a787f23500d678a451aec64 100644 (file)
@@ -19,6 +19,7 @@
  */
 package org.sonar.plugins.checkstyle;
 
+import com.google.common.collect.Maps;
 import org.apache.commons.lang.StringUtils;
 import org.codehaus.stax2.XMLInputFactory2;
 import org.codehaus.staxmate.SMInputFactory;
@@ -36,6 +37,7 @@ import org.sonar.api.utils.ValidationMessages;
 import javax.xml.stream.XMLInputFactory;
 import javax.xml.stream.XMLStreamException;
 import java.io.Reader;
+import java.util.Map;
 
 public class CheckstyleProfileImporter extends ProfileImporter {
 
@@ -86,22 +88,12 @@ public class CheckstyleProfileImporter extends ProfileImporter {
   }
 
   private void processModule(RulesProfile profile, String path, SMInputCursor moduleCursor, ValidationMessages messages) throws XMLStreamException {
-    String configKey = moduleCursor.getAttrValue("name");
-    if (isFilter(configKey)) {
-      messages.addWarningText("Checkstyle filters are not imported: " + configKey);
+    String moduleName = moduleCursor.getAttrValue("name");
+    if (isFilter(moduleName)) {
+      messages.addWarningText("Checkstyle filters are not imported: " + moduleName);
 
-    } else if (isIgnored(configKey)) {
-      // ignore !
-
-    } else {
-      Rule rule = ruleFinder.find(RuleQuery.create().withRepositoryKey(CheckstyleConstants.REPOSITORY_KEY).withConfigKey(path + configKey));
-      if (rule == null) {
-        messages.addWarningText("Checkstyle rule not found: " + path + configKey);
-        
-      } else {
-        ActiveRule activeRule = profile.activateRule(rule, null);
-        processProperties(moduleCursor, messages, activeRule);
-      }
+    } else if (!isIgnored(moduleName)) {
+      processRule(profile, path, moduleName, moduleCursor, messages);
     }
   }
 
@@ -116,24 +108,50 @@ public class CheckstyleProfileImporter extends ProfileImporter {
         StringUtils.equals(configKey, "SuppressWithNearbyCommentFilter");
   }
 
-  private void processProperties(SMInputCursor moduleCursor, ValidationMessages messages, ActiveRule activeRule) throws XMLStreamException {
+  private void processRule(RulesProfile profile, String path, String moduleName, SMInputCursor moduleCursor, ValidationMessages messages) throws XMLStreamException {
+    Map<String, String> properties = processProps(moduleCursor);
+
+    Rule rule;
+    String id = properties.get("id");
+    String warning;
+    if (StringUtils.isNotBlank(id)) {
+      rule = ruleFinder.find(RuleQuery.create().withRepositoryKey(CheckstyleConstants.REPOSITORY_KEY).withKey(id));
+      warning = "Checkstyle rule with key '" + id + "' not found";
+
+    } else {
+      String configKey = path + moduleName;
+      rule = ruleFinder.find(RuleQuery.create().withRepositoryKey(CheckstyleConstants.REPOSITORY_KEY).withConfigKey(configKey));
+      warning = "Checkstyle rule with config key '" + configKey + "' not found";
+    }
+
+    if (rule == null) {
+      messages.addWarningText(warning);
+
+    } else {
+      ActiveRule activeRule = profile.activateRule(rule, null);
+      activateProperties(activeRule, properties);
+    }
+  }
+
+  private Map<String, String> processProps(SMInputCursor moduleCursor) throws XMLStreamException {
+    Map<String, String> props = Maps.newHashMap();
     SMInputCursor propertyCursor = moduleCursor.childElementCursor("property");
     while (propertyCursor.getNext() != null) {
-      processProperty(activeRule, propertyCursor, messages);
+      String key = propertyCursor.getAttrValue("name");
+      String value = propertyCursor.getAttrValue("value");
+      props.put(key, value);
     }
+    return props;
   }
 
-  private void processProperty(ActiveRule activeRule, SMInputCursor propertyCursor, ValidationMessages messages) throws XMLStreamException {
-    String key = propertyCursor.getAttrValue("name");
-    String value = propertyCursor.getAttrValue("value");
-    if (StringUtils.equals("id", key)) {
-      messages.addWarningText("The property 'id' is not supported in the Checkstyle rule: " + activeRule.getConfigKey());
-
-    } else if (StringUtils.equals("severity", key)) {
-      activeRule.setPriority(CheckstyleSeverityUtils.fromSeverity(value));
+  private void activateProperties(ActiveRule activeRule, Map<String, String> properties) {
+    for (Map.Entry<String, String> property : properties.entrySet()) {
+      if (StringUtils.equals("severity", property.getKey())) {
+        activeRule.setPriority(CheckstyleSeverityUtils.fromSeverity(property.getValue()));
 
-    } else {
-      activeRule.setParameter(key, value);
+      } else if (!StringUtils.equals("id", property.getKey())) {
+        activeRule.setParameter(property.getKey(), property.getValue());
+      }
     }
   }
 }
index 88010b7a9190ad51359914699f8d50cc67f1f1f7..f8727211127067121cb23a08e3fd290211b26d7c 100644 (file)
@@ -85,7 +85,7 @@ public class CheckstyleProfileExporterTest {
   public void addTheIdPropertyWhenManyInstancesWithTheSameConfigKey() throws IOException, SAXException {
     RulesProfile profile = RulesProfile.create("sonar way", "java");
     Rule rule1 = Rule.create("checkstyle", "com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck", "Javadoc").setConfigKey("Checker/JavadocPackage");
-    Rule rule2 = Rule.create("checkstyle", "com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck_12345", "Javadoc").setConfigKey("Checker/JavadocPackage");
+    Rule rule2 = Rule.create("checkstyle", "com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck_12345", "Javadoc").setConfigKey("Checker/JavadocPackage").setParent(rule1);
 
     profile.activateRule(rule1, RulePriority.MAJOR);
     profile.activateRule(rule2, RulePriority.CRITICAL);
index 8ad1a8c6b54dc1c68e7480792a2c8e1e655fa964..8a50acae1c9f417a6431d98736c92c75d293af1b 100644 (file)
@@ -19,6 +19,7 @@
  */
 package org.sonar.plugins.checkstyle;
 
+import org.apache.commons.lang.StringUtils;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.invocation.InvocationOnMock;
@@ -33,11 +34,8 @@ import java.io.StringReader;
 
 import static org.hamcrest.core.Is.is;
 import static org.hamcrest.core.IsNull.nullValue;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertThat;
+import static org.junit.Assert.*;
 import static org.mockito.Matchers.anyObject;
-import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -98,20 +96,29 @@ public class CheckstyleProfileImporterTest {
     Reader reader = new StringReader(TestUtils.getResourceContent("/org/sonar/plugins/checkstyle/CheckstyleProfileImporterTest/simple.xml"));
     RulesProfile profile = importer.importProfile(reader, messages);
 
-    ActiveRule activeRule= profile.getActiveRuleByConfigKey("checkstyle", "Checker/TreeWalker/EqualsHashCode");
+    ActiveRule activeRule = profile.getActiveRuleByConfigKey("checkstyle", "Checker/TreeWalker/EqualsHashCode");
     assertThat(activeRule.getPriority(), is(RulePriority.BLOCKER)); // reuse the rule default priority
   }
 
   @Test
-  public void idPropertyIsNotSupported() {
-    Reader reader = new StringReader(TestUtils.getResourceContent("/org/sonar/plugins/checkstyle/CheckstyleProfileImporterTest/idProperty.xml"));
+  public void idPropertyShouldBeTheRuleKey() {
+    Reader reader = new StringReader(TestUtils.getResourceContent("/org/sonar/plugins/checkstyle/CheckstyleProfileImporterTest/idPropertyShouldBeTheRuleKey.xml"));
     RulesProfile profile = importer.importProfile(reader, messages);
 
-    ActiveRule check = profile.getActiveRuleByConfigKey("checkstyle", "Checker/JavadocPackage");
-    assertThat(check.getParameter("id"), nullValue());
+    assertNull(profile.getActiveRuleByConfigKey("checkstyle", "Checker/JavadocPackage"));
     assertThat(messages.getWarnings().size(), is(1));
   }
 
+  @Test
+  public void shouldUseTheIdPropertyToFindRule() {
+    Reader reader = new StringReader(TestUtils.getResourceContent("/org/sonar/plugins/checkstyle/CheckstyleProfileImporterTest/shouldUseTheIdPropertyToFindRule.xml"));
+    RulesProfile profile = importer.importProfile(reader, messages);
+
+    assertNotNull(profile.getActiveRuleByConfigKey("checkstyle", "Checker/JavadocPackage"));
+    assertThat(profile.getActiveRuleByConfigKey("checkstyle", "Checker/JavadocPackage").getRule().getKey(), is("com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck_12345"));
+    assertThat(messages.getWarnings().size(), is(0));
+  }
+
   @Test
   public void testUnvalidXML() {
     Reader reader = new StringReader("not xml");
@@ -132,21 +139,28 @@ public class CheckstyleProfileImporterTest {
 
   private RuleFinder newRuleFinder() {
     RuleFinder ruleFinder = mock(RuleFinder.class);
-    when(ruleFinder.find((RuleQuery)anyObject())).thenAnswer(new Answer<Rule>() {
+    when(ruleFinder.find((RuleQuery) anyObject())).thenAnswer(new Answer<Rule>() {
       public Rule answer(InvocationOnMock iom) throws Throwable {
-        RuleQuery query = (RuleQuery)iom.getArguments()[0];
+        RuleQuery query = (RuleQuery) iom.getArguments()[0];
         Rule rule = null;
-        if (query.getConfigKey().equals("Checker/JavadocPackage")) {
+        if (StringUtils.equals(query.getConfigKey(), "Checker/JavadocPackage")) {
           rule = Rule.create(query.getRepositoryKey(), "com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck", "Javadoc Package")
               .setConfigKey("Checker/JavadocPackage")
               .setPriority(RulePriority.MAJOR);
           rule.createParameter("format");
           rule.createParameter("ignore");
 
-        } else if (query.getConfigKey().equals("Checker/TreeWalker/EqualsHashCode")) {
+        } else if (StringUtils.equals(query.getConfigKey(), "Checker/TreeWalker/EqualsHashCode")) {
           rule = Rule.create(query.getRepositoryKey(), "com.puppycrawl.tools.checkstyle.checks.coding.EqualsHashCodeCheck", "Equals HashCode")
               .setConfigKey("Checker/TreeWalker/EqualsHashCode")
               .setPriority(RulePriority.BLOCKER);
+
+        } else if (StringUtils.equals(query.getKey(), "com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck_12345")) {
+          rule = Rule.create(query.getRepositoryKey(), "com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck_12345", "Javadoc Package")
+              .setConfigKey("Checker/JavadocPackage")
+              .setPriority(RulePriority.MAJOR);
+          rule.createParameter("format");
+          rule.createParameter("ignore");
         }
         return rule;
       }
index 89e580029f9070191b55f23567638e309aada639..7b6fcef7cc9e8137f80bddd449de0d9278a631df 100644 (file)
@@ -3,7 +3,6 @@
 <module name="Checker">
     <module name="SuppressionCommentFilter"/> 
     <module name="JavadocPackage">
-        <property name="id" value="com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck" />
         <property name="severity" value="warning" />
     </module>
     <module name="JavadocPackage">
index 461ad4ef4a1d435276eda3b1c704db39f38519ba..07537f2bad95cf402aef8906fa769f1500f7d535 100644 (file)
@@ -1,12 +1,12 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!-- Generated by Sonar -->
 <module name="Checker">
-    <module name="SuppressionCommentFilter"/> 
-    <module name="JavadocPackage">
-        <property name="severity" value="warning" />
-        <property name="format" value="abcde" />
-    </module>
-    <module name="TreeWalker">
-        <module name="FileContentsHolder"/>
-    </module>
+  <module name="SuppressionCommentFilter"/>
+  <module name="JavadocPackage">
+    <property name="severity" value="warning"/>
+    <property name="format" value="abcde"/>
+  </module>
+  <module name="TreeWalker">
+    <module name="FileContentsHolder"/>
+  </module>
 </module>
index 906d20f139c1f91fcef13785d900cee7fa3d00b5..e55f99cad0ec67ac232293cdcc3e2bb5045cea98 100644 (file)
@@ -1,17 +1,17 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!-- Generated by Sonar -->
 <module name="Checker">
-    <module name="SuppressionCommentFilter"/>
-    <module name="JavadocPackage">
-        <property name="severity" value="warning"/>
+  <module name="SuppressionCommentFilter"/>
+  <module name="JavadocPackage">
+    <property name="severity" value="warning"/>
+  </module>
+  <module name="TreeWalker">
+    <module name="FileContentsHolder"/>
+    <module name="LocalFinalVariableName">
+      <property name="severity" value="info"/>
     </module>
-    <module name="TreeWalker">
-        <module name="FileContentsHolder"/>
-        <module name="LocalFinalVariableName">
-            <property name="severity" value="info"/>
-        </module>
-        <module name="LineLength">
-            <property name="severity" value="error"/>
-        </module>
+    <module name="LineLength">
+      <property name="severity" value="error"/>
     </module>
+  </module>
 </module>
diff --git a/plugins/sonar-checkstyle-plugin/src/test/resources/org/sonar/plugins/checkstyle/CheckstyleProfileImporterTest/idProperty.xml b/plugins/sonar-checkstyle-plugin/src/test/resources/org/sonar/plugins/checkstyle/CheckstyleProfileImporterTest/idProperty.xml
deleted file mode 100644 (file)
index d8dec5d..0000000
+++ /dev/null
@@ -1,11 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.2//EN" "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
-<!-- Generated by Sonar -->
-<module name="Checker">
-    <module name="JavadocPackage">
-        <property name="id" value="javadoc" />      
-        <property name="severity" value="error" />
-        <property name="format" value="abcde" />
-        <property name="ignore" value="true" />
-    </module>
-</module>
diff --git a/plugins/sonar-checkstyle-plugin/src/test/resources/org/sonar/plugins/checkstyle/CheckstyleProfileImporterTest/idPropertyShouldBeTheRuleKey.xml b/plugins/sonar-checkstyle-plugin/src/test/resources/org/sonar/plugins/checkstyle/CheckstyleProfileImporterTest/idPropertyShouldBeTheRuleKey.xml
new file mode 100644 (file)
index 0000000..d8dec5d
--- /dev/null
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.2//EN" "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
+<!-- Generated by Sonar -->
+<module name="Checker">
+    <module name="JavadocPackage">
+        <property name="id" value="javadoc" />      
+        <property name="severity" value="error" />
+        <property name="format" value="abcde" />
+        <property name="ignore" value="true" />
+    </module>
+</module>
diff --git a/plugins/sonar-checkstyle-plugin/src/test/resources/org/sonar/plugins/checkstyle/CheckstyleProfileImporterTest/shouldUseTheIdPropertyToFindRule.xml b/plugins/sonar-checkstyle-plugin/src/test/resources/org/sonar/plugins/checkstyle/CheckstyleProfileImporterTest/shouldUseTheIdPropertyToFindRule.xml
new file mode 100644 (file)
index 0000000..97fdf2f
--- /dev/null
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.2//EN" "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">
+<!-- Generated by Sonar -->
+<module name="Checker">
+    <module name="JavadocPackage">
+        <property name="id" value="com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocPackageCheck_12345" />      
+        <property name="severity" value="error" />
+        <property name="format" value="abcde" />
+        <property name="ignore" value="true" />
+    </module>
+</module>