From: David Gageot <david@gageot.net>
Date: Fri, 6 Jul 2012 13:55:19 +0000 (+0200)
Subject: SONAR-3432 Check rule property values
X-Git-Tag: 3.2~186
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=9150ab1a5dccadea020d337f571d2f82543f9c12;p=sonarqube.git

SONAR-3432 Check rule property values

We need to keep the compatibility with old values like s,i,r...
---

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/rules/AnnotationRuleParser.java b/sonar-plugin-api/src/main/java/org/sonar/api/rules/AnnotationRuleParser.java
index 1d9a20bd959..df43d139657 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/rules/AnnotationRuleParser.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/rules/AnnotationRuleParser.java
@@ -19,6 +19,8 @@
  */
 package org.sonar.api.rules;
 
+import org.sonar.api.utils.SonarException;
+
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Lists;
 import org.apache.commons.lang.StringUtils;
@@ -107,7 +109,7 @@ public final class AnnotationRuleParser implements ServerComponent {
         try {
           param.setType(PropertyType.valueOf(propertyAnnotation.type().trim()).name());
         } catch (IllegalArgumentException e) {
-          throw new IllegalArgumentException("Invalid property type [" + propertyAnnotation.type() + "]", e);
+          throw new SonarException("Invalid property type [" + propertyAnnotation.type() + "]", e);
         }
       } else {
         param.setType(guessType(field.getType()).name());
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/rules/XMLRuleParser.java b/sonar-plugin-api/src/main/java/org/sonar/api/rules/XMLRuleParser.java
index 73091358c83..b4faa05c679 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/rules/XMLRuleParser.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/rules/XMLRuleParser.java
@@ -19,13 +19,16 @@
  */
 package org.sonar.api.rules;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.Maps;
+import com.google.common.io.Closeables;
 import org.apache.commons.io.FileUtils;
-import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.CharEncoding;
 import org.apache.commons.lang.StringUtils;
 import org.codehaus.staxmate.SMInputFactory;
 import org.codehaus.staxmate.in.SMHierarchicCursor;
 import org.codehaus.staxmate.in.SMInputCursor;
+import org.sonar.api.PropertyType;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.utils.SonarException;
 import org.sonar.check.Cardinality;
@@ -33,14 +36,20 @@ import org.sonar.check.Cardinality;
 import javax.xml.stream.XMLInputFactory;
 import javax.xml.stream.XMLStreamException;
 
-import java.io.*;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 
 /**
  * @since 2.3
  */
 public final class XMLRuleParser implements ServerComponent {
+  private final static Map<String, String> TYPE_MAP = typeMapWithDeprecatedValues();
 
   public List<Rule> parse(File file) {
     Reader reader = null;
@@ -52,7 +61,7 @@ public final class XMLRuleParser implements ServerComponent {
       throw new SonarException("Fail to load the file: " + file, e);
 
     } finally {
-      IOUtils.closeQuietly(reader);
+      Closeables.closeQuietly(reader);
     }
   }
 
@@ -69,7 +78,7 @@ public final class XMLRuleParser implements ServerComponent {
       throw new SonarException("Fail to load the xml stream", e);
 
     } finally {
-      IOUtils.closeQuietly(reader);
+      Closeables.closeQuietly(reader);
     }
   }
 
@@ -158,7 +167,7 @@ public final class XMLRuleParser implements ServerComponent {
     String typeAttribute = ruleC.getAttrValue("type");
     if (StringUtils.isNotBlank(typeAttribute)) {
       /* BACKWARD COMPATIBILITY WITH DEPRECATED FORMAT */
-      param.setType(StringUtils.trim(typeAttribute));
+      param.setType(type(StringUtils.trim(typeAttribute)));
     }
 
     SMInputCursor paramC = ruleC.childElementCursor();
@@ -172,7 +181,7 @@ public final class XMLRuleParser implements ServerComponent {
         param.setDescription(propText);
 
       } else if (StringUtils.equalsIgnoreCase("type", propNodeName)) {
-        param.setType(propText);
+        param.setType(type(propText));
 
       } else if (StringUtils.equalsIgnoreCase("defaultValue", propNodeName)) {
         param.setDefaultValue(propText);
@@ -182,4 +191,31 @@ public final class XMLRuleParser implements ServerComponent {
       throw new SonarException("Node <key> is missing in <param>");
     }
   }
+
+  private static Map<String, String> typeMapWithDeprecatedValues() {
+    Map<String, String> map = Maps.newHashMap();
+    map.put("i", PropertyType.INTEGER.name());
+    map.put("s", PropertyType.STRING.name());
+    map.put("b", PropertyType.BOOLEAN.name());
+    map.put("r", PropertyType.REGULAR_EXPRESSION.name());
+    map.put("s{}", "s{}");
+    map.put("i{}", "i{}");
+    for (PropertyType propertyType : PropertyType.values()) {
+      map.put(propertyType.name(), propertyType.name());
+    }
+    return map;
+  }
+
+  @VisibleForTesting
+  static String type(String type) {
+    String validType = TYPE_MAP.get(type);
+    if (null != validType) {
+      return validType;
+    }
+
+    if (type.matches(".\\[.+\\]")) {
+      return type;
+    }
+    throw new SonarException("Invalid property type [" + type + "]");
+  }
 }
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/rules/AnnotationRuleParserTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/rules/AnnotationRuleParserTest.java
index cafb678c217..2fa2b5c30de 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/rules/AnnotationRuleParserTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/rules/AnnotationRuleParserTest.java
@@ -19,6 +19,8 @@
  */
 package org.sonar.api.rules;
 
+import org.sonar.api.utils.SonarException;
+
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.sonar.api.PropertyType;
@@ -73,7 +75,7 @@ public class AnnotationRuleParserTest {
 
   @Test
   public void should_reject_invalid_prroperty_types() {
-    exception.expect(IllegalArgumentException.class);
+    exception.expect(SonarException.class);
     exception.expectMessage("Invalid property type [INVALID]");
 
     parseAnnotatedClass(RuleWithInvalidPropertyType.class);
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/rules/XMLRuleParserTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/rules/XMLRuleParserTest.java
index e72d7044dc4..ba04b6f7bb1 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/rules/XMLRuleParserTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/rules/XMLRuleParserTest.java
@@ -21,6 +21,7 @@ package org.sonar.api.rules;
 
 import org.hamcrest.core.Is;
 import org.junit.Test;
+import org.sonar.api.PropertyType;
 import org.sonar.api.utils.SonarException;
 import org.sonar.check.Cardinality;
 
@@ -28,6 +29,8 @@ import java.io.File;
 import java.io.StringReader;
 import java.util.List;
 
+import static com.google.common.collect.Iterables.getOnlyElement;
+import static org.fest.assertions.Assertions.assertThat;
 import static org.hamcrest.core.Is.is;
 import static org.hamcrest.core.IsNot.not;
 import static org.hamcrest.core.IsNull.nullValue;
@@ -76,6 +79,25 @@ public class XMLRuleParserTest {
     new XMLRuleParser().parse(new StringReader("<rules><rule><key>foo</key><name>Foo</name><param></param></rule></rules>"));
   }
 
+  @Test
+  public void should_read_rule_parameter_type() {
+    assertThat(typeOf("<rules><rule><key>foo</key><name>Foo</name><param><key>key</key><type>STRING</type></param></rule></rules>")).isEqualTo(PropertyType.STRING.name());
+    assertThat(typeOf("<rules><rule><key>foo</key><name>Foo</name><param><key>key</key><type>INTEGER</type></param></rule></rules>")).isEqualTo(PropertyType.INTEGER.name());
+    assertThat(typeOf("<rules><rule><key>foo</key><name>Foo</name><param><key>key</key><type>s</type></param></rule></rules>")).isEqualTo(PropertyType.STRING.name());
+    assertThat(typeOf("<rules><rule><key>foo</key><name>Foo</name><param><key>key</key><type>s{}</type></param></rule></rules>")).isEqualTo("s{}");
+    assertThat(typeOf("<rules><rule><key>foo</key><name>Foo</name><param><key>key</key><type>i{}</type></param></rule></rules>")).isEqualTo("i{}");
+    assertThat(typeOf("<rules><rule><key>foo</key><name>Foo</name><param><key>key</key><type>s[foo,bar]</type></param></rule></rules>")).isEqualTo("s[foo,bar]");
+  }
+
+  static String typeOf(String xml) {
+    return getOnlyElement(new XMLRuleParser().parse(new StringReader(xml))).getParam("key").getType();
+  }
+
+  @Test(expected = SonarException.class)
+  public void fail_on_invalid_rule_parameter_type() {
+    new XMLRuleParser().parse(new StringReader("<rules><rule><key>foo</key><name>Foo</name><param><key>key</key><type>INVALID</type></param></rule></rules>"));
+  }
+
   @Test
   public void testUtf8Encoding() {
     List<Rule> rules = new XMLRuleParser().parse(getClass().getResourceAsStream("/org/sonar/api/rules/XMLRuleParserTest/utf8.xml"));