From 1bf2a07bdcd8e56b485554687c8c3b6029dd7f7c Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 6 Jul 2012 15:25:10 +0200 Subject: [PATCH] SONAR-3432 Support rule property types in annotations --- .../java/org/sonar/check/RuleProperty.java | 9 ++ .../sonar/api/rules/AnnotationRuleParser.java | 32 ++++- .../java/org/sonar/api/rules/RuleParam.java | 2 +- .../api/rules/AnnotationRuleParserTest.java | 132 +++++++++++++----- 4 files changed, 133 insertions(+), 42 deletions(-) diff --git a/sonar-check-api/src/main/java/org/sonar/check/RuleProperty.java b/sonar-check-api/src/main/java/org/sonar/check/RuleProperty.java index efcf609eaba..44d1e91094a 100644 --- a/sonar-check-api/src/main/java/org/sonar/check/RuleProperty.java +++ b/sonar-check-api/src/main/java/org/sonar/check/RuleProperty.java @@ -46,4 +46,13 @@ public @interface RuleProperty { * Optional default value. */ String defaultValue() default ""; + + /** + * Optional type. + * See {@org.sonar.api.PropertyType} for possible values. + * If type is ommited, it is guessed from the type of the annotated field. + * + * @since 3.2 + */ + String type() default ""; } 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 f4fc4485638..1d9a20bd959 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,18 +19,19 @@ */ package org.sonar.api.rules; -import java.lang.reflect.Field; -import java.util.Collection; -import java.util.List; - +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Lists; import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.sonar.api.PropertyType; import org.sonar.api.ServerComponent; import org.sonar.api.utils.AnnotationUtils; import org.sonar.check.Check; -import com.google.common.collect.Lists; +import java.lang.reflect.Field; +import java.util.Collection; +import java.util.List; /** * @since 2.3 @@ -102,6 +103,15 @@ public final class AnnotationRuleParser implements ServerComponent { RuleParam param = rule.createParameter(fieldKey); param.setDescription(propertyAnnotation.description()); param.setDefaultValue(propertyAnnotation.defaultValue()); + if (!StringUtils.isBlank(propertyAnnotation.type())) { + try { + param.setType(PropertyType.valueOf(propertyAnnotation.type().trim()).name()); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("Invalid property type [" + propertyAnnotation.type() + "]", e); + } + } else { + param.setType(guessType(field.getType()).name()); + } } } @@ -113,4 +123,16 @@ public final class AnnotationRuleParser implements ServerComponent { param.setDescription(propertyAnnotation.description()); } } + + @VisibleForTesting + static PropertyType guessType(Class type) { + if ((type == Integer.class) || (type == int.class)) { + return PropertyType.INTEGER; + } else if ((type == Float.class) || (type == float.class)) { + return PropertyType.FLOAT; + } else if ((type == Boolean.class) || (type == boolean.class)) { + return PropertyType.BOOLEAN; + } + return PropertyType.STRING; + } } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/rules/RuleParam.java b/sonar-plugin-api/src/main/java/org/sonar/api/rules/RuleParam.java index aedafe0b7ff..9000666568a 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/rules/RuleParam.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/rules/RuleParam.java @@ -51,7 +51,7 @@ public class RuleParam { private String description; @Column(name = "param_type", updatable = true, nullable = true, length = 512) - private String type = "s"; + private String type = "STRING"; @Column(name = "default_value", updatable = true, nullable = true, length = 4000) private String defaultValue; 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 ab71282c48e..cafb678c217 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,66 +19,109 @@ */ package org.sonar.api.rules; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.nullValue; -import static org.junit.Assert.assertThat; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.api.PropertyType; +import org.sonar.check.IsoCategory; +import org.sonar.check.Priority; import java.util.Collections; import java.util.List; -import org.junit.Test; -import org.sonar.check.IsoCategory; -import org.sonar.check.Priority; +import static org.fest.assertions.Assertions.assertThat; public class AnnotationRuleParserTest { + @org.junit.Rule + public final ExpectedException exception = ExpectedException.none(); @Test public void ruleWithProperty() { List rules = parseAnnotatedClass(RuleWithProperty.class); - assertThat(rules.size(), is(1)); + assertThat(rules).hasSize(1); Rule rule = rules.get(0); - assertThat(rule.getKey(), is("foo")); - assertThat(rule.getName(), is("bar")); - assertThat(rule.getDescription(), is("Foo Bar")); - assertThat(rule.getSeverity(), is(RulePriority.BLOCKER)); - assertThat(rule.getParams().size(), is(1)); + assertThat(rule.getKey()).isEqualTo("foo"); + assertThat(rule.getName()).isEqualTo("bar"); + assertThat(rule.getDescription()).isEqualTo("Foo Bar"); + assertThat(rule.getSeverity()).isEqualTo(RulePriority.BLOCKER); + assertThat(rule.getParams()).hasSize(1); RuleParam prop = rule.getParam("property"); - assertThat(prop.getKey(), is("property")); - assertThat(prop.getDescription(), is("Ignore ?")); - assertThat(prop.getDefaultValue(), is("false")); + assertThat(prop.getKey()).isEqualTo("property"); + assertThat(prop.getDescription()).isEqualTo("Ignore ?"); + assertThat(prop.getDefaultValue()).isEqualTo("false"); + assertThat(prop.getType()).isEqualTo(PropertyType.STRING.name()); + } + + @Test + public void ruleWithIntegerProperty() { + List rules = parseAnnotatedClass(RuleWithIntegerProperty.class); + + RuleParam prop = rules.get(0).getParam("property"); + assertThat(prop.getDescription()).isEqualTo("Max"); + assertThat(prop.getDefaultValue()).isEqualTo("12"); + assertThat(prop.getType()).isEqualTo(PropertyType.INTEGER.name()); + } + + @Test + public void ruleWithTextProperty() { + List rules = parseAnnotatedClass(RuleWithTextProperty.class); + + RuleParam prop = rules.get(0).getParam("property"); + assertThat(prop.getDescription()).isEqualTo("text"); + assertThat(prop.getDefaultValue()).isEqualTo("Long text"); + assertThat(prop.getType()).isEqualTo(PropertyType.TEXT.name()); + } + + @Test + public void should_reject_invalid_prroperty_types() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("Invalid property type [INVALID]"); + + parseAnnotatedClass(RuleWithInvalidPropertyType.class); + } + + @Test + public void should_recognize_type() { + assertThat(AnnotationRuleParser.guessType(Integer.class)).isEqualTo(PropertyType.INTEGER); + assertThat(AnnotationRuleParser.guessType(int.class)).isEqualTo(PropertyType.INTEGER); + assertThat(AnnotationRuleParser.guessType(Float.class)).isEqualTo(PropertyType.FLOAT); + assertThat(AnnotationRuleParser.guessType(float.class)).isEqualTo(PropertyType.FLOAT); + assertThat(AnnotationRuleParser.guessType(Boolean.class)).isEqualTo(PropertyType.BOOLEAN); + assertThat(AnnotationRuleParser.guessType(boolean.class)).isEqualTo(PropertyType.BOOLEAN); + assertThat(AnnotationRuleParser.guessType(String.class)).isEqualTo(PropertyType.STRING); + assertThat(AnnotationRuleParser.guessType(Object.class)).isEqualTo(PropertyType.STRING); } @Test public void ruleWithoutNameNorDescription() { List rules = parseAnnotatedClass(RuleWithoutNameNorDescription.class); - assertThat(rules.size(), is(1)); + assertThat(rules).hasSize(1); Rule rule = rules.get(0); - assertThat(rule.getKey(), is("foo")); - assertThat(rule.getSeverity(), is(RulePriority.MAJOR)); - assertThat(rule.getName(), is(nullValue())); - assertThat(rule.getDescription(), is(nullValue())); + assertThat(rule.getKey()).isEqualTo("foo"); + assertThat(rule.getSeverity()).isEqualTo(RulePriority.MAJOR); + assertThat(rule.getName()).isNull(); + assertThat(rule.getDescription()).isNull(); } @Test public void ruleWithoutKey() { List rules = parseAnnotatedClass(RuleWithoutKey.class); - assertThat(rules.size(), is(1)); + assertThat(rules).hasSize(1); Rule rule = rules.get(0); - assertThat(rule.getKey(), is(RuleWithoutKey.class.getCanonicalName())); - assertThat(rule.getName(), is("foo")); - assertThat(rule.getDescription(), is(nullValue())); - assertThat(rule.getSeverity(), is(RulePriority.MAJOR)); + assertThat(rule.getKey()).isEqualTo(RuleWithoutKey.class.getCanonicalName()); + assertThat(rule.getName()).isEqualTo("foo"); + assertThat(rule.getDescription()).isNull(); + assertThat(rule.getSeverity()).isEqualTo(RulePriority.MAJOR); } @Test public void supportDeprecatedAnnotations() { List rules = parseAnnotatedClass(Check.class); - assertThat(rules.size(), is(1)); + assertThat(rules).hasSize(1); Rule rule = rules.get(0); - assertThat(rule.getKey(), is(Check.class.getCanonicalName())); - assertThat(rule.getName(), is(Check.class.getCanonicalName())); - assertThat(rule.getDescription(), is("Deprecated check")); - assertThat(rule.getSeverity(), is(RulePriority.BLOCKER)); + assertThat(rule.getKey()).isEqualTo(Check.class.getCanonicalName()); + assertThat(rule.getName()).isEqualTo(Check.class.getCanonicalName()); + assertThat(rule.getDescription()).isEqualTo("Deprecated check"); + assertThat(rule.getSeverity()).isEqualTo(RulePriority.BLOCKER); } private List parseAnnotatedClass(Class annotatedClass) { @@ -86,21 +129,38 @@ public class AnnotationRuleParserTest { } @org.sonar.check.Rule(name = "foo") - private class RuleWithoutKey { + static class RuleWithoutKey { } @org.sonar.check.Rule(key = "foo") - private class RuleWithoutNameNorDescription { + static class RuleWithoutNameNorDescription { } @org.sonar.check.Rule(key = "foo", name = "bar", description = "Foo Bar", priority = Priority.BLOCKER) - private class RuleWithProperty { + static class RuleWithProperty { @org.sonar.check.RuleProperty(description = "Ignore ?", defaultValue = "false") - String property; + public String property; } - @org.sonar.check.Check(description = "Deprecated check", priority = Priority.BLOCKER, isoCategory = IsoCategory.Maintainability) - private class Check { + @org.sonar.check.Rule(key = "foo", name = "bar", description = "Foo Bar", priority = Priority.BLOCKER) + static class RuleWithIntegerProperty { + @org.sonar.check.RuleProperty(description = "Max", defaultValue = "12") + public Integer property; } + @org.sonar.check.Rule(key = "foo", name = "bar", description = "Foo Bar", priority = Priority.BLOCKER) + static class RuleWithTextProperty { + @org.sonar.check.RuleProperty(description = "text", defaultValue = "Long text", type = "TEXT") + public String property; + } + + @org.sonar.check.Rule(key = "foo", name = "bar", description = "Foo Bar", priority = Priority.BLOCKER) + static class RuleWithInvalidPropertyType { + @org.sonar.check.RuleProperty(description = "text", defaultValue = "Long text", type = "INVALID") + public String property; + } + + @org.sonar.check.Check(description = "Deprecated check", priority = Priority.BLOCKER, isoCategory = IsoCategory.Maintainability) + static class Check { + } } -- 2.39.5