From 41c79688c9760806fb15cf2a6b97cf98d852f18c Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Thu, 25 Feb 2016 23:03:07 +0100 Subject: [PATCH] SONAR-7332 Update RuleDefinition API to handle "Type" field --- .../DeprecatedRulesDefinitionLoaderTest.java | 4 +- .../server/rule/RuleTagsToTypeConverter.java | 50 +++++++++++++++++ .../api/server/rule/RulesDefinition.java | 46 +++++++++++++++- .../server/rule/RulesDefinitionXmlLoader.java | 24 ++++---- .../rule/RuleTagsToTypeConverterTest.java | 55 +++++++++++++++++++ .../RulesDefinitionAnnotationLoaderTest.java | 4 +- .../api/server/rule/RulesDefinitionTest.java | 26 +++++++++ .../rule/RulesDefinitionXmlLoaderTest.java | 4 +- .../RulesDefinitionXmlLoaderTest/rules.xml | 5 +- 9 files changed, 199 insertions(+), 19 deletions(-) create mode 100644 sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RuleTagsToTypeConverter.java create mode 100644 sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RuleTagsToTypeConverterTest.java diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/DeprecatedRulesDefinitionLoaderTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/DeprecatedRulesDefinitionLoaderTest.java index 7203a2dda42..83610b10433 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/DeprecatedRulesDefinitionLoaderTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/DeprecatedRulesDefinitionLoaderTest.java @@ -74,7 +74,7 @@ public class DeprecatedRulesDefinitionLoaderTest { rule.setConfigKey("Checker/TreeWalker/ConstantName"); rule.setSeverity(RulePriority.BLOCKER); rule.setStatus(Rule.STATUS_BETA); - rule.setTags(new String[] {"style", "security"}); + rule.setTags(new String[] {"style", "clumsy"}); rule.createParameter("format").setDescription("Regular expression").setDefaultValue("A-Z").setType("REGULAR_EXPRESSION"); return Arrays.asList(rule); } @@ -114,7 +114,7 @@ public class DeprecatedRulesDefinitionLoaderTest { assertThat(rule.severity()).isEqualTo(Severity.BLOCKER); assertThat(rule.internalKey()).isEqualTo("Checker/TreeWalker/ConstantName"); assertThat(rule.status()).isEqualTo(RuleStatus.BETA); - assertThat(rule.tags()).containsOnly("style", "security"); + assertThat(rule.tags()).containsOnly("style", "clumsy"); assertThat(rule.params()).hasSize(1); RulesDefinition.Param param = rule.param("format"); assertThat(param).isNotNull(); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RuleTagsToTypeConverter.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RuleTagsToTypeConverter.java new file mode 100644 index 00000000000..daf4c44f34e --- /dev/null +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RuleTagsToTypeConverter.java @@ -0,0 +1,50 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * 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 02110-1301, USA. + */ +package org.sonar.api.server.rule; + +import com.google.common.collect.ImmutableSet; +import java.util.Collection; +import java.util.Set; + +/** + * @see org.sonar.api.server.rule.RulesDefinition.NewRule#setType(RulesDefinition.Type) + * @since 5.5 + */ +class RuleTagsToTypeConverter { + + public static final String TAG_BUG = "bug"; + public static final String TAG_SECURITY = "security"; + static final Set RESERVED_TAGS = ImmutableSet.of(TAG_BUG, TAG_SECURITY); + + + private RuleTagsToTypeConverter() { + // only statics + } + + static RulesDefinition.Type convert(Collection tags) { + if (tags.contains(TAG_BUG)) { + return RulesDefinition.Type.BUG; + } + if (tags.contains(TAG_SECURITY)) { + return RulesDefinition.Type.VULNERABILITY; + } + return RulesDefinition.Type.CODE_SMELL; + } +} diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java index f714a336be7..cc5efce850b 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java @@ -46,6 +46,7 @@ import org.sonar.api.server.ServerSide; import org.sonar.api.server.debt.DebtRemediationFunction; import org.sonar.api.utils.log.Loggers; +import static com.google.common.base.Objects.firstNonNull; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; import static java.lang.String.format; @@ -80,6 +81,9 @@ import static org.apache.commons.lang.StringUtils.trimToNull; * // default severity when the rule is activated on a Quality profile. Default value is MAJOR. * .setSeverity(Severity.MINOR); * + * // optional type for SonarQube Quality Model. Default is RulesDefinition.Type.CODE_SMELL. + * .setType(RulesDefinition.Type.BUG) + * * x1Rule * .setDebtRemediationFunction(x1Rule.debtRemediationFunctions().linearWithOffset("1h", "30min")); * @@ -146,6 +150,13 @@ import static org.apache.commons.lang.StringUtils.trimToNull; @ExtensionPoint public interface RulesDefinition { + /** + * Rule type according to SonarQube Quality Model + * @since 5.5 + */ + enum Type { + CODE_SMELL, BUG, VULNERABILITY + } /** * Default sub-characteristics of technical debt model. See http://www.sqale.org * @deprecated in 5.5. SQALE Quality Model is replaced by SonarQube Quality Model. @@ -656,6 +667,7 @@ public interface RulesDefinition { class NewRule { private final String repoKey; private final String key; + private Type type; private String name; private String htmlDescription; private String markdownDescription; @@ -698,6 +710,26 @@ public interface RulesDefinition { return this; } + /** + * The type as defined by the SonarQube Quality Model. + *

+ * When a plugin does not define rule type, then it is deduced from + * tags: + *

    + *
  • if the rule has the "bug" tag then type is {@link Type#BUG}
    • + *
  • if the rule has the "security" tag then type is {@link Type#VULNERABILITY}
    • + *
  • if the rule has both tags "bug" and "security", then type is {@link Type#BUG}
    • + *
  • default type is {@link Type#CODE_SMELL}
    • + *
+ * Finally the "bug" and "security" tags are considered as reserved. They + * are silently removed from the final state of definition. + * @since 5.5 + */ + public NewRule setType(Type t) { + this.type = t; + return this; + } + /** * The optional description, in HTML format, has no max length. It's exclusive with markdown description * (see {@link #setMarkdownDescription(String)}) @@ -767,6 +799,7 @@ public interface RulesDefinition { * @see org.sonar.api.server.rule.RulesDefinition.SubCharacteristics for constant values * @deprecated in 5.5. SQALE Quality Model is replaced by SonarQube Quality Model. This method does nothing. * See https://jira.sonarsource.com/browse/MMF-184 + * @see #setType(Type) */ public NewRule setDebtSubCharacteristic(@Nullable String s) { return this; @@ -871,6 +904,7 @@ public interface RulesDefinition { private final String repoKey; private final String key; private final String name; + private final Type type; private final String htmlDescription; private final String markdownDescription; private final String internalKey; @@ -895,7 +929,8 @@ public interface RulesDefinition { this.status = newRule.status; this.debtRemediationFunction = newRule.debtRemediationFunction; this.effortToFixDescription = newRule.effortToFixDescription; - this.tags = ImmutableSortedSet.copyOf(newRule.tags); + this.type = (newRule.type == null ? RuleTagsToTypeConverter.convert(newRule.tags) : newRule.type); + this.tags = ImmutableSortedSet.copyOf(Sets.difference(newRule.tags, RuleTagsToTypeConverter.RESERVED_TAGS)); ImmutableMap.Builder paramsBuilder = ImmutableMap.builder(); for (NewParam newParam : newRule.paramsByKey.values()) { paramsBuilder.put(newParam.key, new Param(newParam)); @@ -915,6 +950,14 @@ public interface RulesDefinition { return name; } + /** + * @since 5.5 + * @see NewRule#setType(Type) + */ + public Type type() { + return type; + } + public String severity() { return severity; } @@ -940,6 +983,7 @@ public interface RulesDefinition { /** * @deprecated in 5.5. SQALE Quality Model is replaced by SonarQube Quality Model. {@code null} is * always returned. See https://jira.sonarsource.com/browse/MMF-184 + * @see #type() */ @CheckForNull @Deprecated diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinitionXmlLoader.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinitionXmlLoader.java index ddf1a64e350..f767cb5e0a1 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinitionXmlLoader.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinitionXmlLoader.java @@ -100,9 +100,12 @@ import static org.apache.commons.lang.StringUtils.trim; * <!-- Status displayed in rules console. Possible values are BETA, READY (default), DEPRECATED. --> * <status>BETA</status> * + * <!-- Type as defined by the SonarQube Quality Model. Possible values are CODE_SMELL (default), BUG and VULNERABILITY.--> + * <type>BUG</type> + * * <!-- Optional tags. See org.sonar.api.server.rule.RuleTagFormat. The maximal length of all tags is 4000 characters. --> - * <tag>style</tag> - * <tag>security</tag> + * <tag>misra</tag> + * <tag>multi-threading</tag> * * <!-- Optional parameters --> * <param> @@ -117,14 +120,7 @@ import static org.apache.commons.lang.StringUtils.trim; * <param> * <key>another-param</key> * </param> - * - * <!-- SQALE debt - key of sub-characteristic --> - * <!-- See {@link org.sonar.api.server.rule.RulesDefinition.SubCharacteristics} for core supported values. - * Any other values can be used. If sub-characteristic does not exist at runtime in the SQALE model, - * then the rule is created without any sub-characteristic. --> - * <!-- Since 5.3 --> - * <debtSubCharacteristic>MODULARITY</debtSubCharacteristic> - * + ** * <!-- SQALE debt - type of debt remediation function --> * <!-- See enum {@link org.sonar.api.server.debt.DebtRemediationFunction.Type} for supported values --> * <!-- Since 5.3 --> @@ -164,7 +160,6 @@ import static org.apache.commons.lang.StringUtils.trim; * <tag>cwe</tag> * <tag>security</tag> * <tag>user-experience</tag> - * <debtSubCharacteristic>SECURITY_FEATURES</debtSubCharacteristic> * <debtRemediationFunction>CONSTANT_ISSUE</debtRemediationFunction> * <debtRemediationFunctionOffset>10min</debtRemediationFunctionOffset> * </rule> @@ -239,6 +234,7 @@ public class RulesDefinitionXmlLoader { String descriptionFormat = DescriptionFormat.HTML.name(); String internalKey = null; String severity = Severity.defaultSeverity(); + String type = null; RuleStatus status = RuleStatus.defaultStatus(); boolean template = false; String effortToFixDescription = null; @@ -265,6 +261,9 @@ public class RulesDefinitionXmlLoader { if (equalsIgnoreCase("name", nodeName)) { name = nodeValue(cursor); + } else if (equalsIgnoreCase("type", nodeName)) { + type = nodeValue(cursor); + } else if (equalsIgnoreCase("description", nodeName)) { description = nodeValue(cursor); @@ -326,6 +325,9 @@ public class RulesDefinitionXmlLoader { .setTemplate(template) .setStatus(status) .setEffortToFixDescription(effortToFixDescription); + if (type != null) { + rule.setType(RulesDefinition.Type.valueOf(type)); + } fillDescription(rule, descriptionFormat, description); fillRemediationFunction(rule, debtRemediationFunction, debtRemediationFunctionOffset, debtRemediationFunctionCoeff); fillParams(rule, params); diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RuleTagsToTypeConverterTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RuleTagsToTypeConverterTest.java new file mode 100644 index 00000000000..7ef71532a2a --- /dev/null +++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RuleTagsToTypeConverterTest.java @@ -0,0 +1,55 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * 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 02110-1301, USA. + */ +package org.sonar.api.server.rule; + +import java.util.Collections; +import org.junit.Test; +import org.sonar.test.TestUtils; + +import static java.util.Arrays.asList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.api.server.rule.RuleTagsToTypeConverter.convert; + +public class RuleTagsToTypeConverterTest { + + @Test + public void type_is_bug_if_has_tag_bug() { + assertThat(convert(asList("misra", "bug"))).isEqualTo(RulesDefinition.Type.BUG); + // "bug" has priority on "security" + assertThat(convert(asList("security", "bug"))).isEqualTo(RulesDefinition.Type.BUG); + } + + @Test + public void type_is_vulnerability_if_has_tag_security() { + assertThat(convert(asList("misra", "security"))).isEqualTo(RulesDefinition.Type.VULNERABILITY); + } + + @Test + public void default_is_code_smell() { + assertThat(convert(asList("clumsy", "spring"))).isEqualTo(RulesDefinition.Type.CODE_SMELL); + assertThat(convert(Collections.emptyList())).isEqualTo(RulesDefinition.Type.CODE_SMELL); + } + + @Test + public void only_statics() { + assertThat(TestUtils.hasOnlyPrivateConstructors(RuleTagsToTypeConverter.class)).isTrue(); + + } +} diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionAnnotationLoaderTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionAnnotationLoaderTest.java index 6c8a7d1538e..24c35c11399 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionAnnotationLoaderTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionAnnotationLoaderTest.java @@ -128,7 +128,7 @@ public class RulesDefinitionAnnotationLoaderTest { RulesDefinition.Repository repository = load(RuleWithTags.class); assertThat(repository.rules()).hasSize(1); RulesDefinition.Rule rule = repository.rules().get(0); - assertThat(rule.tags()).containsOnly("style", "security"); + assertThat(rule.tags()).containsOnly("misra", "clumsy"); } @Test @@ -185,7 +185,7 @@ public class RulesDefinitionAnnotationLoaderTest { public String property; } - @org.sonar.check.Rule(key = "foo", name = "bar", description = "Bar", tags = {"style", "security"}) + @org.sonar.check.Rule(key = "foo", name = "bar", description = "Bar", tags = {"misra", "clumsy"}) static class RuleWithTags { } } diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionTest.java index 23aa50f6679..c8b90f23eac 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionTest.java @@ -410,4 +410,30 @@ public class RulesDefinitionTest { RulesDefinition.Rule rule = context.repository("findbugs").rule("NPE"); assertThat(rule.debtSubCharacteristic()).isNull(); } + + @Test + public void type_is_defined() { + RulesDefinition.NewRepository newRepository = context.createRepository("findbugs", "java"); + newRepository.createRule("NPE").setName("NPE").setHtmlDescription("desc") + .setType(RulesDefinition.Type.VULNERABILITY).setTags("bug", "misra"); + newRepository.done(); + + RulesDefinition.Rule rule = context.repository("findbugs").rule("NPE"); + // type VULNERABILITY is kept even if the tag "bug" is present + assertThat(rule.type()).isEqualTo(RulesDefinition.Type.VULNERABILITY); + // tag "bug" is reserved and removed. + assertThat(rule.tags()).containsOnly("misra"); + } + + @Test + public void guess_type_from_tags_if_type_is_missing() { + RulesDefinition.NewRepository newRepository = context.createRepository("findbugs", "java"); + newRepository.createRule("NPE").setName("NPE").setHtmlDescription("desc").setTags("bug", "misra"); + newRepository.done(); + + RulesDefinition.Rule rule = context.repository("findbugs").rule("NPE"); + assertThat(rule.type()).isEqualTo(RulesDefinition.Type.BUG); + // tag "bug" is reserved and removed + assertThat(rule.tags()).containsOnly("misra"); + } } diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionXmlLoaderTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionXmlLoaderTest.java index b941436390c..6809d6aa750 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionXmlLoaderTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionXmlLoaderTest.java @@ -54,7 +54,8 @@ public class RulesDefinitionXmlLoaderTest { assertThat(rule.template()).isTrue(); assertThat(rule.status()).isEqualTo(RuleStatus.BETA); assertThat(rule.internalKey()).isEqualTo("Checker/TreeWalker/LocalVariableName"); - assertThat(rule.tags()).containsOnly("style", "security"); + assertThat(rule.type()).isEqualTo(RulesDefinition.Type.BUG); + assertThat(rule.tags()).containsOnly("misra", "spring"); assertThat(rule.params()).hasSize(2); RulesDefinition.Param ignore = rule.param("ignore"); @@ -69,6 +70,7 @@ public class RulesDefinitionXmlLoaderTest { assertThat(rule.params()).isEmpty(); assertThat(rule.status()).isEqualTo(RuleStatus.READY); assertThat(rule.severity()).isEqualTo(Severity.MAJOR); + assertThat(rule.type()).isEqualTo(RulesDefinition.Type.CODE_SMELL); } @Test diff --git a/sonar-plugin-api/src/test/resources/org/sonar/api/server/rule/RulesDefinitionXmlLoaderTest/rules.xml b/sonar-plugin-api/src/test/resources/org/sonar/api/server/rule/RulesDefinitionXmlLoaderTest/rules.xml index 28ea7019c6e..05c3084cbac 100644 --- a/sonar-plugin-api/src/test/resources/org/sonar/api/server/rule/RulesDefinitionXmlLoaderTest/rules.xml +++ b/sonar-plugin-api/src/test/resources/org/sonar/api/server/rule/RulesDefinitionXmlLoaderTest/rules.xml @@ -10,8 +10,9 @@ BLOCKER MULTIPLE BETA - style - security + BUG + misra + spring tokens -- 2.39.5