@@ -88,7 +88,7 @@ public class IssueCreationDateCalculator extends IssueVisitor { | |||
} else { | |||
// Rule can't be inactive (see contract of IssueVisitor) | |||
ActiveRule activeRule = activeRulesHolder.get(issue.getRuleKey()).get(); | |||
if (activeRuleIsNew(activeRule, lastAnalysisOptional.get()) | |||
if (activeRuleIsNewOrChanged(activeRule, lastAnalysisOptional.get()) | |||
|| ruleImplementationChanged(activeRule.getRuleKey(), activeRule.getPluginKey(), lastAnalysisOptional.get())) { | |||
backdateIssue(component, issue); | |||
} | |||
@@ -127,9 +127,8 @@ public class IssueCreationDateCalculator extends IssueVisitor { | |||
return lastAnalysisDate < scannerPlugin.getUpdatedAt(); | |||
} | |||
private static boolean activeRuleIsNew(ActiveRule activeRule, Long lastAnalysisDate) { | |||
long ruleCreationDate = activeRule.getCreatedAt(); | |||
return lastAnalysisDate < ruleCreationDate; | |||
private static boolean activeRuleIsNewOrChanged(ActiveRule activeRule, Long lastAnalysisDate) { | |||
return lastAnalysisDate < activeRule.getUpdatedAt(); | |||
} | |||
private Optional<Date> getDateOfLatestChange(Component component, DefaultIssue issue) { |
@@ -33,13 +33,15 @@ public class ActiveRule { | |||
private final Map<String, String> params; | |||
private final long createdAt; | |||
private final String pluginKey; | |||
private final long updatedAt; | |||
public ActiveRule(RuleKey ruleKey, String severity, Map<String, String> params, long createdAt, @Nullable String pluginKey) { | |||
public ActiveRule(RuleKey ruleKey, String severity, Map<String, String> params, long createdAt, long updatedAt, @Nullable String pluginKey) { | |||
this.ruleKey = ruleKey; | |||
this.severity = severity; | |||
this.pluginKey = pluginKey; | |||
this.params = ImmutableMap.copyOf(params); | |||
this.createdAt = createdAt; | |||
this.updatedAt = updatedAt; | |||
} | |||
public RuleKey getRuleKey() { | |||
@@ -58,6 +60,10 @@ public class ActiveRule { | |||
return createdAt; | |||
} | |||
public long getUpdatedAt() { | |||
return updatedAt; | |||
} | |||
@CheckForNull | |||
public String getPluginKey() { | |||
return pluginKey; |
@@ -72,6 +72,6 @@ public class LoadQualityProfilesStep implements ComputationStep { | |||
private static ActiveRule convert(ScannerReport.ActiveRule input, Rule rule) { | |||
RuleKey key = RuleKey.of(input.getRuleRepository(), input.getRuleKey()); | |||
Map<String, String> params = new HashMap<>(input.getParamsByKeyMap()); | |||
return new ActiveRule(key, input.getSeverity().name(), params, input.getCreatedAt(), rule.getPluginKey()); | |||
return new ActiveRule(key, input.getSeverity().name(), params, input.getCreatedAt(), input.getUpdatedAt(), rule.getPluginKey()); | |||
} | |||
} |
@@ -208,6 +208,22 @@ public class IssueCreationDateCalculatorTest { | |||
assertChangeOfCreationDateTo(expectedDate); | |||
} | |||
@Test | |||
@UseDataProvider("backdatingDateCases") | |||
public void should_backdate_date_if_scm_is_available_and_rule_has_changed(BiConsumer<DefaultIssue, ScmInfo> configure, long expectedDate) { | |||
previousAnalysisWas(2000L); | |||
currentAnalysisIs(3000L); | |||
makeIssueNew(); | |||
configure.accept(issue, createMockScmInfo()); | |||
setRuleCreatedAt(1200L); | |||
setRuleUpdatedAt(2800L); | |||
run(); | |||
assertChangeOfCreationDateTo(expectedDate); | |||
} | |||
@Test | |||
@UseDataProvider("backdatingDateCases") | |||
public void should_backdate_date_if_scm_is_available_and_first_analysis(BiConsumer<DefaultIssue, ScmInfo> configure, long expectedDate) { | |||
@@ -418,6 +434,11 @@ public class IssueCreationDateCalculatorTest { | |||
private void setRuleCreatedAt(long createdAt) { | |||
when(activeRule.getCreatedAt()).thenReturn(createdAt); | |||
when(activeRule.getUpdatedAt()).thenReturn(createdAt); | |||
} | |||
private void setRuleUpdatedAt(long updateAt) { | |||
when(activeRule.getUpdatedAt()).thenReturn(updateAt); | |||
} | |||
private void rulePlugin(String pluginKey) { |
@@ -367,6 +367,6 @@ public class TrackerRawInputFactoryTest { | |||
} | |||
private void markRuleAsActive(RuleKey ruleKey) { | |||
activeRulesHolder.put(new ActiveRule(ruleKey, Severity.CRITICAL, emptyMap(), 1_000L, null)); | |||
activeRulesHolder.put(new ActiveRule(ruleKey, Severity.CRITICAL, emptyMap(), 1_000L, 1_000L, null)); | |||
} | |||
} |
@@ -77,7 +77,7 @@ public class CommentDensityRuleTest { | |||
@Test | |||
public void no_issues_if_enough_comments() { | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, ImmutableMap.of(CommonRuleKeys.INSUFFICIENT_COMMENT_DENSITY_PROPERTY, "25"), 1_000L, PLUGIN_KEY)); | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, ImmutableMap.of(CommonRuleKeys.INSUFFICIENT_COMMENT_DENSITY_PROPERTY, "25"), 1_000L, 1_000L, PLUGIN_KEY)); | |||
measureRepository.addRawMeasure(FILE.getReportAttributes().getRef(), CoreMetrics.COMMENT_LINES_DENSITY_KEY, Measure.newMeasureBuilder().create(90.0, 1)); | |||
DefaultIssue issue = underTest.processFile(FILE, PLUGIN_KEY); | |||
@@ -135,7 +135,7 @@ public class CommentDensityRuleTest { | |||
} | |||
private void prepareForIssue(String minDensity, ReportComponent file, double commentLineDensity, int commentLines, int ncloc) { | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, ImmutableMap.of(CommonRuleKeys.INSUFFICIENT_COMMENT_DENSITY_PROPERTY, minDensity), 1_000L, PLUGIN_KEY)); | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, ImmutableMap.of(CommonRuleKeys.INSUFFICIENT_COMMENT_DENSITY_PROPERTY, minDensity), 1_000L, 1_000L, PLUGIN_KEY)); | |||
measureRepository.addRawMeasure(file.getReportAttributes().getRef(), CoreMetrics.COMMENT_LINES_DENSITY_KEY, Measure.newMeasureBuilder().create(commentLineDensity, 1)); | |||
measureRepository.addRawMeasure(file.getReportAttributes().getRef(), CoreMetrics.COMMENT_LINES_KEY, Measure.newMeasureBuilder().create(commentLines)); | |||
measureRepository.addRawMeasure(file.getReportAttributes().getRef(), CoreMetrics.NCLOC_KEY, Measure.newMeasureBuilder().create(ncloc)); |
@@ -38,7 +38,7 @@ public class CommonRuleTest { | |||
@Test | |||
public void test_getMinDensityParam() { | |||
ActiveRule activeRule = new ActiveRule(RuleTesting.XOO_X1, Severity.MAJOR, ImmutableMap.of("minDensity", "30.5"), 1_000L, PLUGIN_KEY); | |||
ActiveRule activeRule = new ActiveRule(RuleTesting.XOO_X1, Severity.MAJOR, ImmutableMap.of("minDensity", "30.5"), 1_000L, 1_000L, PLUGIN_KEY); | |||
double minDensity = CommonRule.getMinDensityParam(activeRule, "minDensity"); | |||
assertThat(minDensity).isEqualTo(30.5); | |||
@@ -49,7 +49,7 @@ public class CommonRuleTest { | |||
thrown.expect(IllegalStateException.class); | |||
thrown.expectMessage("Required parameter [minDensity] is missing on rule [xoo:x1]"); | |||
ActiveRule activeRule = new ActiveRule(RuleTesting.XOO_X1, Severity.MAJOR, ImmutableMap.of(), 1_000L, PLUGIN_KEY); | |||
ActiveRule activeRule = new ActiveRule(RuleTesting.XOO_X1, Severity.MAJOR, ImmutableMap.of(), 1_000L, 1_000L, PLUGIN_KEY); | |||
CommonRule.getMinDensityParam(activeRule, "minDensity"); | |||
} | |||
@@ -58,7 +58,7 @@ public class CommonRuleTest { | |||
thrown.expect(IllegalStateException.class); | |||
thrown.expectMessage("Minimum density of rule [xoo:x1] is incorrect. Got [-30.5] but must be between 0 and 100."); | |||
ActiveRule activeRule = new ActiveRule(RuleTesting.XOO_X1, Severity.MAJOR, ImmutableMap.of("minDensity", "-30.5"), 1_000L, PLUGIN_KEY); | |||
ActiveRule activeRule = new ActiveRule(RuleTesting.XOO_X1, Severity.MAJOR, ImmutableMap.of("minDensity", "-30.5"), 1_000L, 1_000L, PLUGIN_KEY); | |||
CommonRule.getMinDensityParam(activeRule, "minDensity"); | |||
} | |||
@@ -67,7 +67,7 @@ public class CommonRuleTest { | |||
thrown.expect(IllegalStateException.class); | |||
thrown.expectMessage("Minimum density of rule [xoo:x1] is incorrect. Got [305] but must be between 0 and 100."); | |||
ActiveRule activeRule = new ActiveRule(RuleTesting.XOO_X1, Severity.MAJOR, ImmutableMap.of("minDensity", "305"), 1_000L, PLUGIN_KEY); | |||
ActiveRule activeRule = new ActiveRule(RuleTesting.XOO_X1, Severity.MAJOR, ImmutableMap.of("minDensity", "305"), 1_000L, 1_000L, PLUGIN_KEY); | |||
CommonRule.getMinDensityParam(activeRule, "minDensity"); | |||
} | |||
} |
@@ -86,7 +86,7 @@ public abstract class CoverageRuleTest { | |||
@Test | |||
public void no_issue_if_enough_coverage() { | |||
activeRuleHolder.put(new ActiveRule(getRuleKey(), Severity.CRITICAL, ImmutableMap.of(getMinPropertyKey(), "65"), 1_000L, PLUGIN_KEY)); | |||
activeRuleHolder.put(new ActiveRule(getRuleKey(), Severity.CRITICAL, ImmutableMap.of(getMinPropertyKey(), "65"), 1_000L, 1_000L, PLUGIN_KEY)); | |||
measureRepository.addRawMeasure(FILE.getReportAttributes().getRef(), getCoverageMetricKey(), Measure.newMeasureBuilder().create(90.0, 1)); | |||
DefaultIssue issue = underTest.processFile(FILE, "java"); | |||
@@ -96,7 +96,7 @@ public abstract class CoverageRuleTest { | |||
@Test | |||
public void issue_if_coverage_is_too_low() { | |||
activeRuleHolder.put(new ActiveRule(getRuleKey(), Severity.CRITICAL, ImmutableMap.of(getMinPropertyKey(), "65"), 1_000L, PLUGIN_KEY)); | |||
activeRuleHolder.put(new ActiveRule(getRuleKey(), Severity.CRITICAL, ImmutableMap.of(getMinPropertyKey(), "65"), 1_000L, 1_000L, PLUGIN_KEY)); | |||
measureRepository.addRawMeasure(FILE.getReportAttributes().getRef(), getCoverageMetricKey(), Measure.newMeasureBuilder().create(20.0, 1)); | |||
measureRepository.addRawMeasure(FILE.getReportAttributes().getRef(), getUncoveredMetricKey(), Measure.newMeasureBuilder().create(40)); | |||
measureRepository.addRawMeasure(FILE.getReportAttributes().getRef(), getToCoverMetricKey(), Measure.newMeasureBuilder().create(50)); | |||
@@ -114,7 +114,7 @@ public abstract class CoverageRuleTest { | |||
@Test | |||
public void no_issue_if_coverage_is_not_set() { | |||
activeRuleHolder.put(new ActiveRule(getRuleKey(), Severity.CRITICAL, ImmutableMap.of(getMinPropertyKey(), "65"), 1_000L, PLUGIN_KEY)); | |||
activeRuleHolder.put(new ActiveRule(getRuleKey(), Severity.CRITICAL, ImmutableMap.of(getMinPropertyKey(), "65"), 1_000L, 1_000L, PLUGIN_KEY)); | |||
DefaultIssue issue = underTest.processFile(FILE, "java"); | |||
@@ -66,7 +66,7 @@ public class DuplicatedBlockRuleTest { | |||
@Test | |||
public void no_issue_if_no_duplicated_blocks() { | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, PLUGIN_KEY)); | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, 1_000L, PLUGIN_KEY)); | |||
measureRepository.addRawMeasure(FILE.getReportAttributes().getRef(), CoreMetrics.DUPLICATED_BLOCKS_KEY, Measure.newMeasureBuilder().create(0)); | |||
DefaultIssue issue = underTest.processFile(FILE, "java"); | |||
@@ -76,7 +76,7 @@ public class DuplicatedBlockRuleTest { | |||
@Test | |||
public void issue_if_duplicated_blocks() { | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, PLUGIN_KEY)); | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, 1_000L, PLUGIN_KEY)); | |||
measureRepository.addRawMeasure(FILE.getReportAttributes().getRef(), CoreMetrics.DUPLICATED_BLOCKS_KEY, Measure.newMeasureBuilder().create(3)); | |||
DefaultIssue issue = underTest.processFile(FILE, "java"); |
@@ -67,7 +67,7 @@ public class SkippedTestRuleTest { | |||
@Test | |||
public void issue_if_skipped_tests() { | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, PLUGIN_KEY)); | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, 1_000L, PLUGIN_KEY)); | |||
measureRepository.addRawMeasure(FILE.getReportAttributes().getRef(), CoreMetrics.SKIPPED_TESTS_KEY, Measure.newMeasureBuilder().create(2)); | |||
DefaultIssue issue = underTest.processFile(FILE, "java"); | |||
@@ -80,7 +80,7 @@ public class SkippedTestRuleTest { | |||
@Test | |||
public void no_issues_if_zero_skipped_tests() { | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, PLUGIN_KEY)); | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, 1_000L, PLUGIN_KEY)); | |||
measureRepository.addRawMeasure(FILE.getReportAttributes().getRef(), CoreMetrics.SKIPPED_TESTS_KEY, Measure.newMeasureBuilder().create(0)); | |||
DefaultIssue issue = underTest.processFile(FILE, "java"); | |||
@@ -90,7 +90,7 @@ public class SkippedTestRuleTest { | |||
@Test | |||
public void no_issues_if_measure_is_absent() { | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, PLUGIN_KEY)); | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, 1_000L, PLUGIN_KEY)); | |||
DefaultIssue issue = underTest.processFile(FILE, "java"); | |||
@@ -69,7 +69,7 @@ public class TestErrorRuleTest { | |||
@Test | |||
public void issue_if_errors_or_failures() { | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, PLUGIN_KEY)); | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, 1_000L, PLUGIN_KEY)); | |||
measureRepository.addRawMeasure(FILE.getReportAttributes().getRef(), CoreMetrics.TEST_ERRORS_KEY, Measure.newMeasureBuilder().create(2)); | |||
measureRepository.addRawMeasure(FILE.getReportAttributes().getRef(), CoreMetrics.TEST_FAILURES_KEY, Measure.newMeasureBuilder().create(1)); | |||
@@ -83,7 +83,7 @@ public class TestErrorRuleTest { | |||
@Test | |||
public void no_issues_if_zero_errors_and_failures() { | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, PLUGIN_KEY)); | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, 1_000L, PLUGIN_KEY)); | |||
measureRepository.addRawMeasure(FILE.getReportAttributes().getRef(), CoreMetrics.TEST_ERRORS_KEY, Measure.newMeasureBuilder().create(0)); | |||
measureRepository.addRawMeasure(FILE.getReportAttributes().getRef(), CoreMetrics.TEST_FAILURES_KEY, Measure.newMeasureBuilder().create(0)); | |||
@@ -94,7 +94,7 @@ public class TestErrorRuleTest { | |||
@Test | |||
public void no_issues_if_test_measures_are_absent() { | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, PLUGIN_KEY)); | |||
activeRuleHolder.put(new ActiveRule(RULE_KEY, Severity.CRITICAL, Collections.emptyMap(), 1_000L, 1_000L, PLUGIN_KEY)); | |||
DefaultIssue issue = underTest.processFile(FILE, "java"); | |||
@@ -52,7 +52,7 @@ public class ActiveRulesHolderImplTest { | |||
@Test | |||
public void get_active_rule() { | |||
underTest.set(asList(new ActiveRule(RULE_KEY, Severity.BLOCKER, Collections.emptyMap(), SOME_DATE, PLUGIN_KEY))); | |||
underTest.set(asList(new ActiveRule(RULE_KEY, Severity.BLOCKER, Collections.emptyMap(), SOME_DATE, SOME_DATE, PLUGIN_KEY))); | |||
Optional<ActiveRule> activeRule = underTest.get(RULE_KEY); | |||
assertThat(activeRule.isPresent()).isTrue(); | |||
@@ -65,7 +65,7 @@ public class ActiveRulesHolderImplTest { | |||
thrown.expect(IllegalStateException.class); | |||
thrown.expectMessage("Active rules have already been initialized"); | |||
underTest.set(asList(new ActiveRule(RULE_KEY, Severity.BLOCKER, Collections.emptyMap(), 1_000L, PLUGIN_KEY))); | |||
underTest.set(asList(new ActiveRule(RULE_KEY, Severity.BLOCKER, Collections.emptyMap(), SOME_DATE, SOME_DATE, PLUGIN_KEY))); | |||
underTest.set(Collections.emptyList()); | |||
} | |||
@@ -84,7 +84,7 @@ public class ActiveRulesHolderImplTest { | |||
thrown.expectMessage("Active rule must not be declared multiple times: squid:S001"); | |||
underTest.set(asList( | |||
new ActiveRule(RULE_KEY, Severity.BLOCKER, Collections.emptyMap(), 1_000L, PLUGIN_KEY), | |||
new ActiveRule(RULE_KEY, Severity.MAJOR, Collections.emptyMap(), 1_000L, PLUGIN_KEY))); | |||
new ActiveRule(RULE_KEY, Severity.BLOCKER, Collections.emptyMap(), SOME_DATE, SOME_DATE, PLUGIN_KEY), | |||
new ActiveRule(RULE_KEY, Severity.MAJOR, Collections.emptyMap(), SOME_DATE, SOME_DATE, PLUGIN_KEY))); | |||
} | |||
} |
@@ -28,7 +28,7 @@ import static java.util.Collections.emptyMap; | |||
public class AlwaysActiveRulesHolderImpl implements ActiveRulesHolder { | |||
@Override | |||
public Optional<ActiveRule> get(RuleKey ruleKey) { | |||
return Optional.of(new ActiveRule(ruleKey, Severity.MAJOR, emptyMap(), 1_000L, null)); | |||
return Optional.of(new ActiveRule(ruleKey, Severity.MAJOR, emptyMap(), 1_000L, 1_000L, null)); | |||
} | |||
} |
@@ -19,7 +19,6 @@ | |||
*/ | |||
package org.sonar.ce.task.projectanalysis.step; | |||
import java.util.Optional; | |||
import org.assertj.core.data.MapEntry; | |||
import org.junit.Rule; | |||
import org.junit.Test; | |||
@@ -33,9 +32,11 @@ import org.sonar.ce.task.projectanalysis.qualityprofile.ActiveRulesHolderImpl; | |||
import org.sonar.ce.task.step.TestComputationStepContext; | |||
import org.sonar.scanner.protocol.Constants; | |||
import org.sonar.scanner.protocol.output.ScannerReport; | |||
import org.sonarqube.ws.Rules; | |||
import static java.util.Arrays.asList; | |||
import static org.assertj.core.api.Assertions.assertThat; | |||
import static org.assertj.core.api.Assertions.assertThatCode; | |||
import static org.sonar.db.rule.RuleTesting.XOO_X1; | |||
import static org.sonar.db.rule.RuleTesting.XOO_X2; | |||
@@ -58,8 +59,11 @@ public class LoadQualityProfilesStepTest { | |||
.setPluginKey("xoo"); | |||
ScannerReport.ActiveRule.Builder batch1 = ScannerReport.ActiveRule.newBuilder() | |||
.setRuleRepository(XOO_X1.repository()).setRuleKey(XOO_X1.rule()) | |||
.setSeverity(Constants.Severity.BLOCKER); | |||
.setRuleRepository(XOO_X1.repository()) | |||
.setRuleKey(XOO_X1.rule()) | |||
.setSeverity(Constants.Severity.BLOCKER) | |||
.setCreatedAt(1000L) | |||
.setUpdatedAt(1200L); | |||
batch1.getMutableParamsByKey().put("p1", "v1"); | |||
ScannerReport.ActiveRule.Builder batch2 = ScannerReport.ActiveRule.newBuilder() | |||
@@ -70,15 +74,19 @@ public class LoadQualityProfilesStepTest { | |||
assertThat(activeRulesHolder.getAll()).hasSize(2); | |||
Optional<ActiveRule> ar1 = activeRulesHolder.get(XOO_X1); | |||
assertThat(ar1.get().getSeverity()).isEqualTo(Severity.BLOCKER); | |||
assertThat(ar1.get().getParams()).containsExactly(MapEntry.entry("p1", "v1")); | |||
assertThat(ar1.get().getPluginKey()).isEqualTo("xoo"); | |||
Optional<ActiveRule> ar2 = activeRulesHolder.get(XOO_X2); | |||
assertThat(ar2.get().getSeverity()).isEqualTo(Severity.MAJOR); | |||
assertThat(ar2.get().getParams()).isEmpty(); | |||
assertThat(ar2.get().getPluginKey()).isEqualTo("xoo"); | |||
ActiveRule ar1 = activeRulesHolder.get(XOO_X1).get(); | |||
assertThat(ar1.getSeverity()).isEqualTo(Severity.BLOCKER); | |||
assertThat(ar1.getParams()).containsExactly(MapEntry.entry("p1", "v1")); | |||
assertThat(ar1.getPluginKey()).isEqualTo("xoo"); | |||
assertThat(ar1.getCreatedAt()).isEqualTo(1000L); | |||
assertThat(ar1.getUpdatedAt()).isEqualTo(1200L); | |||
ActiveRule ar2 = activeRulesHolder.get(XOO_X2).get(); | |||
assertThat(ar2.getSeverity()).isEqualTo(Severity.MAJOR); | |||
assertThat(ar2.getParams()).isEmpty(); | |||
assertThat(ar2.getPluginKey()).isEqualTo("xoo"); | |||
assertThat(ar1.getCreatedAt()).isEqualTo(1000L); | |||
assertThat(ar1.getUpdatedAt()).isEqualTo(1200L); | |||
} | |||
@Test |
@@ -167,6 +167,7 @@ public class ActiveRuleCompleter { | |||
builder.setInherit(inheritance != null ? inheritance : ActiveRuleInheritance.NONE.name()); | |||
builder.setSeverity(activeRule.getSeverityString()); | |||
builder.setCreatedAt(DateUtils.formatDateTime(activeRule.getCreatedAt())); | |||
builder.setUpdatedAt(DateUtils.formatDateTime(activeRule.getUpdatedAt())); | |||
Rules.Active.Param.Builder paramBuilder = Rules.Active.Param.newBuilder(); | |||
for (ActiveRuleParamDto parameter : parameters) { | |||
builder.addParams(paramBuilder.clear() |
@@ -44,6 +44,7 @@ public class RulesWsParameters { | |||
public static final String FIELD_REPO = "repo"; | |||
public static final String FIELD_NAME = "name"; | |||
public static final String FIELD_CREATED_AT = "createdAt"; | |||
public static final String FIELD_UPDATED_AT = "updatedAt"; | |||
public static final String FIELD_SEVERITY = "severity"; | |||
public static final String FIELD_STATUS = "status"; | |||
public static final String FIELD_INTERNAL_KEY = "internalKey"; | |||
@@ -96,7 +97,7 @@ public class RulesWsParameters { | |||
public static final String FIELD_PARAMS = "params"; | |||
public static final String FIELD_ACTIVES = "actives"; | |||
public static final Set<String> OPTIONAL_FIELDS = ImmutableSet.of(FIELD_REPO, FIELD_NAME, FIELD_CREATED_AT, FIELD_SEVERITY, FIELD_STATUS, FIELD_INTERNAL_KEY, | |||
public static final Set<String> OPTIONAL_FIELDS = ImmutableSet.of(FIELD_REPO, FIELD_NAME, FIELD_CREATED_AT, FIELD_UPDATED_AT, FIELD_SEVERITY, FIELD_STATUS, FIELD_INTERNAL_KEY, | |||
FIELD_IS_EXTERNAL, FIELD_IS_TEMPLATE, FIELD_TEMPLATE_KEY, FIELD_TAGS, FIELD_SYSTEM_TAGS, FIELD_LANGUAGE, FIELD_LANGUAGE_NAME, FIELD_HTML_DESCRIPTION, | |||
FIELD_MARKDOWN_DESCRIPTION, FIELD_NOTE_LOGIN, FIELD_MARKDOWN_NOTE, FIELD_HTML_NOTE, | |||
FIELD_DEFAULT_DEBT_REM_FUNCTION, FIELD_EFFORT_TO_FIX_DESCRIPTION, FIELD_DEBT_OVERLOADED, FIELD_DEBT_REM_FUNCTION, |
@@ -122,7 +122,8 @@ public class SearchAction implements RulesWsAction { | |||
.setChangelog(new Change("7.1", "The field 'scope' has been added to the response")) | |||
.setChangelog(new Change("7.1", "The field 'scope' has been added to the 'f' parameter")) | |||
.setChangelog(new Change("7.2", "The field 'isExternal' has been added to the response")) | |||
.setChangelog(new Change("7.2", "The field 'includeExternal' has been added to the 'f' parameter")); | |||
.setChangelog(new Change("7.2", "The field 'includeExternal' has been added to the 'f' parameter")) | |||
.setChangelog(new Change("7.5", "The field 'updatedAt' has been added to the 'f' parameter")); | |||
action.createParam(FACETS) | |||
.setDescription("Comma-separated list of the facets to be computed. No facet is computed by default.") |
@@ -8,6 +8,7 @@ | |||
"repo": "squid", | |||
"name": "Expressions should not be too complex", | |||
"createdAt": "2013-03-27T08:52:40+0100", | |||
"updatedAt": "2013-03-27T08:52:40+0100", | |||
"htmlDesc": "<p>\nThe complexity of an expression is defined by the number of <code>&&</code>, <code>||</code> and <code>condition ? ifTrue : ifFalse</code> operators it contains.\nA single expression's complexity should not become too high to keep the code readable.\n</p>\n\n<p>The following code, with a maximum complexity of 3:</p>\n\n<pre>\nif (condition1 && condition2 && condition3 && condition4) { /* ... */ } // Non-Compliant\n</pre>\n\n<p>could be refactored into something like:</p>\n\n<pre>\nif (relevantMethodName1() && relevantMethodName2()) { /* ... */ } // Compliant\n\n/* ... */\n\nprivate boolean relevantMethodName1() {\n return condition1 && condition2;\n}\n\nprivate boolean relevantMethodName2() {\n return condition3 && condition4;\n}\n</pre>", | |||
"severity": "MAJOR", | |||
"status": "READY", | |||
@@ -33,6 +34,7 @@ | |||
"repo": "squid", | |||
"name": "Avoid too complex class", | |||
"createdAt": "2013-03-27T08:52:40+0100", | |||
"updatedAt": "2013-03-27T08:52:40+0100", | |||
"htmlDesc": "<p>The Cyclomatic Complexity is measured by the number of (&&, ||)\n\toperators and (if, while, do, for, ?:, catch, switch, case, return,\n\tthrow) statements in the body of a class plus one for each constructor,\n\tmethod (but not getter/setter), static initializer, or instance\n\tinitializer in the class. The last return stament in method, if exists,\n\tis not taken into account.</p>\n<p>\n\tEven when the Cyclomatic Complexity of a class is very high, this\n\tcomplexity might be well distributed among all methods. Nevertheless,\n\tmost of the time, a very complex class is a class which breaks the <a\n\t\thref='http://en.wikipedia.org/wiki/Single_responsibility_principle'>Single\n\t\tResponsibility Principle</a> and which should be re-factored to be split\n\tin several classes.\n</p>", | |||
"severity": "MAJOR", | |||
"status": "READY", | |||
@@ -58,6 +60,7 @@ | |||
"repo": "squid", | |||
"name": "Methods should not be too complex", | |||
"createdAt": "2013-03-27T08:52:40+0100", | |||
"updatedAt": "2013-03-27T08:52:40+0100", | |||
"htmlDesc": "<p>The Cyclomatic Complexity is measured by the number of\n\t(&&, ||) operators and (if, while, do, for, ?:, catch, switch,\n\tcase, return, throw) statements in the body of a class plus one for\n\teach constructor, method (but not getter/setter), static initializer,\n\tor instance initializer in the class. The last return stament in\n\tmethod, if exists, is not taken into account.</p>\n<p>\n\tEven when the Cyclomatic Complexity of a class is very high, this\n\tcomplexity might be well distributed among all methods. Nevertheless,\n\tmost of the time, a very complex class is a class which breaks the <a\n\t\thref=\"http://en.wikipedia.org/wiki/Single_responsibility_principle\">Single\n\t\tResponsibility Principle</a> and which should be re-factored to be split\n\tin several classes.\n</p>", | |||
"severity": "MAJOR", | |||
"status": "READY", | |||
@@ -83,6 +86,7 @@ | |||
"repo": "squid", | |||
"name": "XPath rule", | |||
"createdAt": "2013-03-27T08:52:40+0100", | |||
"updatedAt": "2013-03-27T08:52:40+0100", | |||
"htmlDesc": "<p>\nThis rule allows to define some homemade Java rules with help of an XPath expression.\n</p>\n\n<p>\nIssues are created depending on the return value of the XPath expression. If the XPath expression returns:\n</p>\n<ul>\n <li>a single or list of AST nodes, then a line issue with the given message is created for each node</li>\n <li>a boolean, then a file issue with the given message is created only if the boolean is true</li>\n <li>anything else, no issue is created</li>\n</ul>\n\n<p>\nHere is an example of an XPath expression to log an issue on each if statement : //ifStatement\n</p>", | |||
"severity": "MAJOR", | |||
"status": "READY", | |||
@@ -116,6 +120,7 @@ | |||
"repo": "squid", | |||
"name": "firstOf() alternatives should be rules or token types", | |||
"createdAt": "2013-05-30T10:35:35+0200", | |||
"updatedAt": "2013-03-27T08:52:40+0100", | |||
"htmlDesc": "<p>\r\nThe tree produced by the <code>firstOf()</code> matcher is hard to work with from checks when alternatives are not named.\r\n</p>\r\n\r\n<p>\r\nConsider the following rule:\r\n</p>\r\n\r\n<pre>\r\nb.rule(COMPILATION_UNIT).is(\r\n b.firstOf( /* Non-Compliant */\r\n \"FOO\",\r\n \"BAR\"));\r\n</pre>\r\n\r\n<p>\r\nIf, from a check, one wants to forbid the usage of the \"BAR\" alternative,\r\nthe easiest option will be to verify that the value of the first token is \"BAR\",\r\ni.e. <code>\"BAR\".equals(compilationUnitNode.getTokenValue())</code>.\r\n</p>\r\n\r\n<p>\r\nThis is not maintainable, for at least two reasons:\r\n</p>\r\n\r\n<ul>\r\n <li>The grammar might evolve to also accept \"bar\" in lowercase, which will break <code>\"BAR\".equals(...)</code></li>\r\n <li>The grammar might evolve to optionally accept \"hello\" before the <code>firstOf()</code>, which will break <code>compilationUnitNode.getTokenValue()</code></li>\r\n</ul>\r\n\r\n<p>\r\nInstead, it is much better to rewrite the grammar as:\r\n</p>\r\n\r\n<pre>\r\nb.rule(COMPILATION_UNIT).is(\r\n firstOf( /* Compliant */\r\n FOO,\r\n BAR));\r\nb.rule(FOO).is(\"FOO\");\r\nb.rule(BAR).is(\"BAR\");\r\n</pre>\r\n\r\n<p>\r\nThe same check which forbids \"BAR\" would be written as: <code>compilationUnitNode.hasDirectChildren(BAR)</code>.\r\nThis allows both of the previous grammar evolutions to be made without impacting the check at all.\r\n</p>", | |||
"severity": "MAJOR", | |||
"status": "READY", |
@@ -71,4 +71,5 @@ public interface ActiveRule { | |||
*/ | |||
@CheckForNull | |||
String templateRuleKey(); | |||
} |
@@ -35,15 +35,12 @@ public class ActiveRulesBuilder { | |||
private final Map<RuleKey, NewActiveRule> map = new LinkedHashMap<>(); | |||
public NewActiveRule create(RuleKey ruleKey) { | |||
return new NewActiveRule(this, ruleKey); | |||
} | |||
void activate(NewActiveRule newActiveRule) { | |||
public ActiveRulesBuilder addRule(NewActiveRule newActiveRule) { | |||
if (map.containsKey(newActiveRule.ruleKey)) { | |||
throw new IllegalStateException(String.format("Rule '%s' is already activated", newActiveRule.ruleKey)); | |||
} | |||
map.put(newActiveRule.ruleKey, newActiveRule); | |||
return this; | |||
} | |||
public ActiveRules build() { |
@@ -35,6 +35,7 @@ public class DefaultActiveRule implements ActiveRule { | |||
private final String templateRuleKey; | |||
private final Map<String, String> params; | |||
private final long createdAt; | |||
private final long updatedAt; | |||
DefaultActiveRule(NewActiveRule newActiveRule) { | |||
this.severity = newActiveRule.severity; | |||
@@ -44,6 +45,7 @@ public class DefaultActiveRule implements ActiveRule { | |||
this.params = Collections.unmodifiableMap(new HashMap<>(newActiveRule.params)); | |||
this.language = newActiveRule.language; | |||
this.createdAt = newActiveRule.createdAt; | |||
this.updatedAt = newActiveRule.updatedAt; | |||
} | |||
@Override | |||
@@ -85,4 +87,8 @@ public class DefaultActiveRule implements ActiveRule { | |||
public long createdAt() { | |||
return createdAt; | |||
} | |||
public long updatedAt() { | |||
return updatedAt; | |||
} | |||
} |
@@ -22,6 +22,7 @@ package org.sonar.api.batch.rule.internal; | |||
import java.util.HashMap; | |||
import java.util.Map; | |||
import javax.annotation.Nullable; | |||
import javax.annotation.concurrent.Immutable; | |||
import org.apache.commons.lang.StringUtils; | |||
import org.sonar.api.rule.RuleKey; | |||
import org.sonar.api.rule.Severity; | |||
@@ -29,71 +30,93 @@ import org.sonar.api.rule.Severity; | |||
/** | |||
* @since 4.2 | |||
*/ | |||
@Immutable | |||
public class NewActiveRule { | |||
final RuleKey ruleKey; | |||
String name; | |||
String severity = Severity.defaultSeverity(); | |||
Map<String, String> params = new HashMap<>(); | |||
long createdAt; | |||
String internalKey; | |||
String language; | |||
String templateRuleKey; | |||
private final ActiveRulesBuilder builder; | |||
final String name; | |||
final String severity; | |||
final Map<String, String> params; | |||
final long createdAt; | |||
final long updatedAt; | |||
final String internalKey; | |||
final String language; | |||
final String templateRuleKey; | |||
NewActiveRule(ActiveRulesBuilder builder, RuleKey ruleKey) { | |||
this.builder = builder; | |||
this.ruleKey = ruleKey; | |||
NewActiveRule(Builder builder) { | |||
this.ruleKey = builder.ruleKey; | |||
this.name = builder.name; | |||
this.severity = builder.severity; | |||
this.params = builder.params; | |||
this.createdAt = builder.createdAt; | |||
this.updatedAt = builder.updatedAt; | |||
this.internalKey = builder.internalKey; | |||
this.language = builder.language; | |||
this.templateRuleKey = builder.templateRuleKey; | |||
} | |||
public NewActiveRule setName(String name) { | |||
this.name = name; | |||
return this; | |||
} | |||
public static class Builder { | |||
private RuleKey ruleKey; | |||
private String name; | |||
private String severity = Severity.defaultSeverity(); | |||
private Map<String, String> params = new HashMap<>(); | |||
private long createdAt; | |||
private long updatedAt; | |||
private String internalKey; | |||
private String language; | |||
private String templateRuleKey; | |||
public NewActiveRule setSeverity(@Nullable String severity) { | |||
this.severity = StringUtils.defaultIfBlank(severity, Severity.defaultSeverity()); | |||
return this; | |||
} | |||
public Builder setRuleKey(RuleKey ruleKey) { | |||
this.ruleKey = ruleKey; | |||
return this; | |||
} | |||
public NewActiveRule setInternalKey(@Nullable String internalKey) { | |||
this.internalKey = internalKey; | |||
return this; | |||
} | |||
public Builder setName(String name) { | |||
this.name = name; | |||
return this; | |||
} | |||
public NewActiveRule setTemplateRuleKey(@Nullable String templateRuleKey) { | |||
this.templateRuleKey = templateRuleKey; | |||
return this; | |||
} | |||
public Builder setSeverity(@Nullable String severity) { | |||
this.severity = StringUtils.defaultIfBlank(severity, Severity.defaultSeverity()); | |||
return this; | |||
} | |||
public NewActiveRule setLanguage(@Nullable String language) { | |||
this.language = language; | |||
return this; | |||
} | |||
public Builder setParam(String key, @Nullable String value) { | |||
// possible improvement : check that the param key exists in rule definition | |||
if (value == null) { | |||
params.remove(key); | |||
} else { | |||
params.put(key, value); | |||
} | |||
return this; | |||
} | |||
public NewActiveRule setParam(String key, @Nullable String value) { | |||
// possible improvement : check that the param key exists in rule definition | |||
if (value == null) { | |||
params.remove(key); | |||
} else { | |||
params.put(key, value); | |||
public Builder setCreatedAt(long createdAt) { | |||
this.createdAt = createdAt; | |||
return this; | |||
} | |||
return this; | |||
} | |||
public Map<String, String> params() { | |||
return params; | |||
} | |||
public Builder setUpdatedAt(long updatedAt) { | |||
this.updatedAt = updatedAt; | |||
return this; | |||
} | |||
public long getCreatedAt() { | |||
return createdAt; | |||
} | |||
public Builder setInternalKey(@Nullable String internalKey) { | |||
this.internalKey = internalKey; | |||
return this; | |||
} | |||
public void setCreatedAt(long createdAt) { | |||
this.createdAt = createdAt; | |||
} | |||
public Builder setLanguage(@Nullable String language) { | |||
this.language = language; | |||
return this; | |||
} | |||
public Builder setTemplateRuleKey(@Nullable String templateRuleKey) { | |||
this.templateRuleKey = templateRuleKey; | |||
return this; | |||
} | |||
public ActiveRulesBuilder activate() { | |||
builder.activate(this); | |||
return builder; | |||
public NewActiveRule build() { | |||
return new NewActiveRule(this); | |||
} | |||
} | |||
} |
@@ -22,6 +22,7 @@ package org.sonar.api.batch.rule; | |||
import org.junit.Test; | |||
import org.junit.rules.ExpectedException; | |||
import org.sonar.api.batch.rule.internal.ActiveRulesBuilder; | |||
import org.sonar.api.batch.rule.internal.NewActiveRule; | |||
import org.sonar.api.rule.RuleKey; | |||
import org.sonar.api.utils.SonarException; | |||
@@ -46,7 +47,10 @@ public class CheckFactoryTest { | |||
@Test | |||
public void class_name_as_check_key() { | |||
RuleKey ruleKey = RuleKey.of("squid", "org.sonar.api.batch.rule.CheckWithoutProperties"); | |||
builder.create(ruleKey).activate(); | |||
NewActiveRule rule = new NewActiveRule.Builder() | |||
.setRuleKey(ruleKey) | |||
.build(); | |||
builder.addRule(rule); | |||
CheckFactory checkFactory = new CheckFactory(builder.build()); | |||
Checks checks = checkFactory.create("squid").addAnnotatedChecks(CheckWithoutProperties.class); | |||
@@ -60,7 +64,11 @@ public class CheckFactoryTest { | |||
@Test | |||
public void param_as_string_field() { | |||
RuleKey ruleKey = RuleKey.of("squid", "org.sonar.api.batch.rule.CheckWithStringProperty"); | |||
builder.create(ruleKey).setParam("pattern", "foo").activate(); | |||
NewActiveRule rule = new NewActiveRule.Builder() | |||
.setRuleKey(ruleKey) | |||
.setParam("pattern", "foo") | |||
.build(); | |||
builder.addRule(rule); | |||
CheckFactory checkFactory = new CheckFactory(builder.build()); | |||
Checks checks = checkFactory.create("squid").addAnnotatedChecks(CheckWithStringProperty.class); | |||
@@ -77,7 +85,11 @@ public class CheckFactoryTest { | |||
thrown.expectMessage("The field 'unknown' does not exist or is not annotated with @RuleProperty in the class org.sonar.api.batch.rule.CheckWithStringProperty"); | |||
RuleKey ruleKey = RuleKey.of("squid", "org.sonar.api.batch.rule.CheckWithStringProperty"); | |||
builder.create(ruleKey).setParam("unknown", "foo").activate(); | |||
NewActiveRule rule = new NewActiveRule.Builder() | |||
.setRuleKey(ruleKey) | |||
.setParam("unknown", "foo") | |||
.build(); | |||
builder.addRule(rule); | |||
CheckFactory checkFactory = new CheckFactory(builder.build()); | |||
checkFactory.create("squid").addAnnotatedChecks(CheckWithStringProperty.class); | |||
@@ -86,7 +98,12 @@ public class CheckFactoryTest { | |||
@Test | |||
public void param_as_primitive_fields() { | |||
RuleKey ruleKey = RuleKey.of("squid", "org.sonar.api.batch.rule.CheckWithPrimitiveProperties"); | |||
builder.create(ruleKey).setParam("max", "300").setParam("ignore", "true").activate(); | |||
NewActiveRule rule = new NewActiveRule.Builder() | |||
.setRuleKey(ruleKey) | |||
.setParam("max", "300") | |||
.setParam("ignore", "true") | |||
.build(); | |||
builder.addRule(rule); | |||
CheckFactory checkFactory = new CheckFactory(builder.build()); | |||
Checks checks = checkFactory.create("squid").addAnnotatedChecks(CheckWithPrimitiveProperties.class); | |||
@@ -103,7 +120,11 @@ public class CheckFactoryTest { | |||
@Test | |||
public void param_as_inherited_field() { | |||
RuleKey ruleKey = RuleKey.of("squid", "org.sonar.api.batch.rule.CheckWithPrimitiveProperties"); | |||
builder.create(ruleKey).setParam("max", "300").activate(); | |||
NewActiveRule rule = new NewActiveRule.Builder() | |||
.setRuleKey(ruleKey) | |||
.setParam("max", "300") | |||
.build(); | |||
builder.addRule(rule); | |||
CheckFactory checkFactory = new CheckFactory(builder.build()); | |||
Checks checks = checkFactory.create("squid").addAnnotatedChecks(CheckWithPrimitiveProperties.class); | |||
@@ -116,7 +137,11 @@ public class CheckFactoryTest { | |||
@Test | |||
public void use_template_rule_key() { | |||
RuleKey ruleKey = RuleKey.of("squid", "S0001_123"); | |||
builder.create(ruleKey).setTemplateRuleKey("S0001").activate(); | |||
NewActiveRule rule = new NewActiveRule.Builder() | |||
.setRuleKey(ruleKey) | |||
.setTemplateRuleKey("S0001") | |||
.build(); | |||
builder.addRule(rule); | |||
CheckFactory checkFactory = new CheckFactory(builder.build()); | |||
Checks checks = checkFactory.create("squid").addAnnotatedChecks(CheckWithKey.class); | |||
@@ -133,7 +158,11 @@ public class CheckFactoryTest { | |||
thrown.expect(SonarException.class); | |||
RuleKey ruleKey = RuleKey.of("squid", "org.sonar.api.batch.rule.CheckWithUnsupportedPropertyType"); | |||
builder.create(ruleKey).setParam("max", "300").activate(); | |||
NewActiveRule rule = new NewActiveRule.Builder() | |||
.setRuleKey(ruleKey) | |||
.setParam("max", "300") | |||
.build(); | |||
builder.addRule(rule); | |||
CheckFactory checkFactory = new CheckFactory(builder.build()); | |||
checkFactory.create("squid").addAnnotatedChecks(CheckWithUnsupportedPropertyType.class); | |||
@@ -142,7 +171,11 @@ public class CheckFactoryTest { | |||
@Test | |||
public void override_field_key() { | |||
RuleKey ruleKey = RuleKey.of("squid", "org.sonar.api.batch.rule.CheckWithOverriddenPropertyKey"); | |||
builder.create(ruleKey).setParam("maximum", "300").activate(); | |||
NewActiveRule rule = new NewActiveRule.Builder() | |||
.setRuleKey(ruleKey) | |||
.setParam("maximum", "300") | |||
.build(); | |||
builder.addRule(rule); | |||
CheckFactory checkFactory = new CheckFactory(builder.build()); | |||
Checks checks = checkFactory.create("squid").addAnnotatedChecks(CheckWithOverriddenPropertyKey.class); | |||
@@ -158,7 +191,11 @@ public class CheckFactoryTest { | |||
@Test | |||
public void checks_as_objects() { | |||
RuleKey ruleKey = RuleKey.of("squid", "org.sonar.api.batch.rule.CheckWithStringProperty"); | |||
builder.create(ruleKey).setParam("pattern", "foo").activate(); | |||
NewActiveRule rule = new NewActiveRule.Builder() | |||
.setRuleKey(ruleKey) | |||
.setParam("pattern", "foo") | |||
.build(); | |||
builder.addRule(rule); | |||
CheckFactory checkFactory = new CheckFactory(builder.build()); | |||
CheckWithStringProperty check = new CheckWithStringProperty(); |
@@ -42,16 +42,24 @@ public class ActiveRulesBuilderTest { | |||
@Test | |||
public void build_rules() { | |||
ActiveRules activeRules = new ActiveRulesBuilder() | |||
.create(RuleKey.of("squid", "S0001")) | |||
NewActiveRule activeRule = new NewActiveRule.Builder() | |||
.setRuleKey(RuleKey.of("squid", "S0001")) | |||
.setName("My Rule") | |||
.setSeverity(Severity.CRITICAL) | |||
.setInternalKey("__S0001__") | |||
.setParam("min", "20") | |||
.activate() | |||
.build(); | |||
ActiveRules activeRules = new ActiveRulesBuilder() | |||
.addRule(activeRule) | |||
// most simple rule | |||
.create(RuleKey.of("squid", "S0002")).activate() | |||
.create(RuleKey.of("findbugs", "NPE")).setInternalKey(null).setSeverity(null).setParam("foo", null).activate() | |||
.addRule(new NewActiveRule.Builder().setRuleKey(RuleKey.of("squid", "S0002")).build()) | |||
.addRule(new NewActiveRule.Builder() | |||
.setRuleKey(RuleKey.of("findbugs", "NPE")) | |||
.setInternalKey(null) | |||
.setSeverity(null) | |||
.setParam("foo", null) | |||
.build()) | |||
.build(); | |||
assertThat(activeRules.findAll()).hasSize(3); | |||
@@ -83,11 +91,14 @@ public class ActiveRulesBuilderTest { | |||
@Test | |||
public void fail_to_add_twice_the_same_rule() { | |||
ActiveRulesBuilder builder = new ActiveRulesBuilder(); | |||
builder.create(RuleKey.of("squid", "S0001")).activate(); | |||
NewActiveRule rule = new NewActiveRule.Builder() | |||
.setRuleKey(RuleKey.of("squid", "S0001")) | |||
.build(); | |||
builder.addRule(rule); | |||
thrown.expect(IllegalStateException.class); | |||
thrown.expectMessage("Rule 'squid:S0001' is already activated"); | |||
builder.create(RuleKey.of("squid", "S0001")).activate(); | |||
builder.addRule(rule); | |||
} | |||
} |
@@ -0,0 +1,85 @@ | |||
/* | |||
* SonarQube | |||
* Copyright (C) 2009-2018 SonarSource SA | |||
* mailto:info 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.batch.rule.internal; | |||
import com.google.common.collect.ImmutableMap; | |||
import org.junit.Before; | |||
import org.junit.Test; | |||
import org.sonar.api.rule.RuleKey; | |||
import org.sonar.api.rule.Severity; | |||
import static org.assertj.core.api.Assertions.assertThat; | |||
public class NewActiveRuleTest { | |||
private NewActiveRule.Builder builder; | |||
@Before | |||
public void setBuilder() { | |||
builder = new NewActiveRule.Builder(); | |||
} | |||
@Test | |||
public void builder_should_set_every_param() { | |||
NewActiveRule rule = builder | |||
.setRuleKey(RuleKey.of("foo", "bar")) | |||
.setName("name") | |||
.setSeverity(Severity.CRITICAL) | |||
.setParam("key", "value") | |||
.setCreatedAt(1_000L) | |||
.setUpdatedAt(1_000L) | |||
.setInternalKey("internal_key") | |||
.setLanguage("language") | |||
.setTemplateRuleKey("templateRuleKey") | |||
.build(); | |||
assertThat(rule.ruleKey).isEqualTo(RuleKey.of("foo", "bar")); | |||
assertThat(rule.name).isEqualTo("name"); | |||
assertThat(rule.severity).isEqualTo(Severity.CRITICAL); | |||
assertThat(rule.params).isEqualTo(ImmutableMap.of("key", "value")); | |||
assertThat(rule.createdAt).isEqualTo(1_000L); | |||
assertThat(rule.updatedAt).isEqualTo(1_000L); | |||
assertThat(rule.internalKey).isEqualTo("internal_key"); | |||
assertThat(rule.language).isEqualTo("language"); | |||
assertThat(rule.templateRuleKey).isEqualTo("templateRuleKey"); | |||
} | |||
@Test | |||
public void severity_should_have_default_value() { | |||
NewActiveRule rule = builder.build(); | |||
assertThat(rule.severity).isEqualTo(Severity.defaultSeverity()); | |||
} | |||
@Test | |||
public void params_should_be_empty_map_if_no_params() { | |||
NewActiveRule rule = builder.build(); | |||
assertThat(rule.params).isEqualTo(ImmutableMap.of()); | |||
} | |||
@Test | |||
public void set_param_remove_param_if_value_is_null() { | |||
NewActiveRule rule = builder | |||
.setParam("foo", "bar") | |||
.setParam("removed", "value") | |||
.setParam("removed", null) | |||
.build(); | |||
assertThat(rule.params).isEqualTo(ImmutableMap.of("foo", "bar")); | |||
} | |||
} |
@@ -26,6 +26,7 @@ import org.junit.Rule; | |||
import org.junit.Test; | |||
import org.junit.rules.ExpectedException; | |||
import org.junit.rules.TemporaryFolder; | |||
import org.mockito.Mockito; | |||
import org.sonar.api.batch.bootstrap.ProjectDefinition; | |||
import org.sonar.api.batch.fs.InputFile; | |||
import org.sonar.api.batch.fs.internal.DefaultFileSystem; | |||
@@ -36,6 +37,7 @@ import org.sonar.api.batch.fs.internal.TestInputFileBuilder; | |||
import org.sonar.api.batch.rule.ActiveRules; | |||
import org.sonar.api.batch.rule.Severity; | |||
import org.sonar.api.batch.rule.internal.ActiveRulesBuilder; | |||
import org.sonar.api.batch.rule.internal.NewActiveRule; | |||
import org.sonar.api.batch.sensor.error.AnalysisError; | |||
import org.sonar.api.batch.sensor.error.NewAnalysisError; | |||
import org.sonar.api.batch.sensor.highlighting.TypeOfText; | |||
@@ -51,6 +53,8 @@ import org.sonar.api.rules.RuleType; | |||
import static org.assertj.core.api.Assertions.assertThat; | |||
import static org.assertj.core.api.Assertions.tuple; | |||
import static org.assertj.core.data.MapEntry.entry; | |||
import static org.mockito.Mockito.mock; | |||
import static org.mockito.Mockito.when; | |||
public class SensorContextTesterTest { | |||
@@ -79,7 +83,10 @@ public class SensorContextTesterTest { | |||
@Test | |||
public void testActiveRules() { | |||
ActiveRules activeRules = new ActiveRulesBuilder().create(RuleKey.of("repo", "rule")).activate().build(); | |||
NewActiveRule activeRule = new NewActiveRule.Builder() | |||
.setRuleKey(RuleKey.of("foo", "bar")) | |||
.build(); | |||
ActiveRules activeRules = new ActiveRulesBuilder().addRule(activeRule).build(); | |||
tester.setActiveRules(activeRules); | |||
assertThat(tester.activeRules().findAll()).hasSize(1); | |||
} |
@@ -46,6 +46,7 @@ public class ActiveRulesPublisher implements ReportPublisherStep { | |||
builder.setRuleKey(input.ruleKey().rule()); | |||
builder.setSeverity(Constants.Severity.valueOf(input.severity())); | |||
builder.setCreatedAt(input.createdAt()); | |||
builder.setUpdatedAt(input.updatedAt()); | |||
builder.getMutableParamsByKey().putAll(input.params()); | |||
return builder.build(); | |||
}).collect(toList())); |
@@ -71,27 +71,30 @@ public class ActiveRulesProvider extends ProviderAdapter { | |||
} | |||
private static ActiveRules transform(Collection<LoadedActiveRule> loadedRules) { | |||
ActiveRulesBuilder builder = new ActiveRulesBuilder(); | |||
ActiveRulesBuilder activeRulesBuilder = new ActiveRulesBuilder(); | |||
for (LoadedActiveRule activeRule : loadedRules) { | |||
NewActiveRule newActiveRule = builder.create(activeRule.getRuleKey()); | |||
newActiveRule.setName(activeRule.getName()); | |||
newActiveRule.setSeverity(activeRule.getSeverity()); | |||
newActiveRule.setCreatedAt(activeRule.getCreatedAt()); | |||
newActiveRule.setLanguage(activeRule.getLanguage()); | |||
newActiveRule.setInternalKey(activeRule.getInternalKey()); | |||
newActiveRule.setTemplateRuleKey(activeRule.getTemplateRuleKey()); | |||
NewActiveRule.Builder builder = new NewActiveRule.Builder(); | |||
builder | |||
.setRuleKey(activeRule.getRuleKey()) | |||
.setName(activeRule.getName()) | |||
.setSeverity(activeRule.getSeverity()) | |||
.setCreatedAt(activeRule.getCreatedAt()) | |||
.setUpdatedAt(activeRule.getUpdatedAt()) | |||
.setLanguage(activeRule.getLanguage()) | |||
.setInternalKey(activeRule.getInternalKey()) | |||
.setTemplateRuleKey(activeRule.getTemplateRuleKey()); | |||
// load parameters | |||
if (activeRule.getParams() != null) { | |||
for (Map.Entry<String, String> params : activeRule.getParams().entrySet()) { | |||
newActiveRule.setParam(params.getKey(), params.getValue()); | |||
builder.setParam(params.getKey(), params.getValue()); | |||
} | |||
} | |||
newActiveRule.activate(); | |||
activeRulesBuilder.addRule(builder.build()); | |||
} | |||
return builder.build(); | |||
return activeRulesBuilder.build(); | |||
} | |||
private static List<LoadedActiveRule> load(ActiveRulesLoader loader, String qProfileKey) { |
@@ -45,7 +45,7 @@ import static org.sonar.api.utils.DateUtils.dateToLong; | |||
import static org.sonar.api.utils.DateUtils.parseDateTime; | |||
public class DefaultActiveRulesLoader implements ActiveRulesLoader { | |||
private static final String RULES_SEARCH_URL = "/api/rules/search.protobuf?f=repo,name,severity,lang,internalKey,templateKey,params,actives,createdAt&activation=true"; | |||
private static final String RULES_SEARCH_URL = "/api/rules/search.protobuf?f=repo,name,severity,lang,internalKey,templateKey,params,actives,createdAt,updatedAt&activation=true"; | |||
private static final String RULES_SEARCH_NO_HOTSPOT_URL; | |||
static { | |||
@@ -127,6 +127,7 @@ public class DefaultActiveRulesLoader implements ActiveRulesLoader { | |||
loadedRule.setName(r.getName()); | |||
loadedRule.setSeverity(active.getSeverity()); | |||
loadedRule.setCreatedAt(dateToLong(parseDateTime(active.getCreatedAt()))); | |||
loadedRule.setUpdatedAt(dateToLong(parseDateTime(active.getUpdatedAt()))); | |||
loadedRule.setLanguage(r.getLang()); | |||
loadedRule.setInternalKey(r.getInternalKey()); | |||
if (r.hasTemplateKey()) { |
@@ -31,6 +31,7 @@ public class LoadedActiveRule { | |||
private String language; | |||
private Map<String, String> params; | |||
private long createdAt; | |||
private long updatedAt; | |||
private String templateRuleKey; | |||
private String internalKey; | |||
@@ -82,6 +83,14 @@ public class LoadedActiveRule { | |||
this.createdAt = createdAt; | |||
} | |||
public long getUpdatedAt() { | |||
return updatedAt; | |||
} | |||
public void setUpdatedAt(long updatedAt) { | |||
this.updatedAt = updatedAt; | |||
} | |||
@CheckForNull | |||
public String getTemplateRuleKey() { | |||
return templateRuleKey; |
@@ -27,6 +27,7 @@ import org.mockito.runners.MockitoJUnitRunner; | |||
import org.sonar.api.batch.fs.internal.DefaultInputFile; | |||
import org.sonar.api.batch.fs.internal.TestInputFileBuilder; | |||
import org.sonar.api.batch.rule.internal.ActiveRulesBuilder; | |||
import org.sonar.api.batch.rule.internal.NewActiveRule; | |||
import org.sonar.api.batch.rule.internal.RulesBuilder; | |||
import org.sonar.api.batch.sensor.issue.internal.DefaultExternalIssue; | |||
import org.sonar.api.batch.sensor.issue.internal.DefaultIssue; | |||
@@ -115,7 +116,7 @@ public class ModuleIssuesTest { | |||
@Test | |||
public void ignore_null_rule_of_active_rule() { | |||
ruleBuilder.add(SQUID_RULE_KEY).setName(SQUID_RULE_NAME); | |||
activeRulesBuilder.create(SQUID_RULE_KEY).activate(); | |||
activeRulesBuilder.addRule(new NewActiveRule.Builder().setRuleKey(SQUID_RULE_KEY).build()); | |||
initModuleIssues(); | |||
DefaultIssue issue = new DefaultIssue() | |||
@@ -130,7 +131,10 @@ public class ModuleIssuesTest { | |||
@Test | |||
public void add_issue_to_cache() { | |||
ruleBuilder.add(SQUID_RULE_KEY).setName(SQUID_RULE_NAME); | |||
activeRulesBuilder.create(SQUID_RULE_KEY).setSeverity(Severity.INFO).activate(); | |||
activeRulesBuilder.addRule(new NewActiveRule.Builder() | |||
.setRuleKey(SQUID_RULE_KEY) | |||
.setSeverity(Severity.INFO) | |||
.build()); | |||
initModuleIssues(); | |||
DefaultIssue issue = new DefaultIssue() | |||
@@ -169,7 +173,10 @@ public class ModuleIssuesTest { | |||
@Test | |||
public void use_severity_from_active_rule_if_no_severity_on_issue() { | |||
ruleBuilder.add(SQUID_RULE_KEY).setName(SQUID_RULE_NAME); | |||
activeRulesBuilder.create(SQUID_RULE_KEY).setSeverity(Severity.INFO).activate(); | |||
activeRulesBuilder.addRule(new NewActiveRule.Builder() | |||
.setRuleKey(SQUID_RULE_KEY) | |||
.setSeverity(Severity.INFO) | |||
.build()); | |||
initModuleIssues(); | |||
DefaultIssue issue = new DefaultIssue() | |||
@@ -186,7 +193,11 @@ public class ModuleIssuesTest { | |||
@Test | |||
public void use_rule_name_if_no_message() { | |||
ruleBuilder.add(SQUID_RULE_KEY).setName(SQUID_RULE_NAME); | |||
activeRulesBuilder.create(SQUID_RULE_KEY).setSeverity(Severity.INFO).setName(SQUID_RULE_NAME).activate(); | |||
activeRulesBuilder.addRule(new NewActiveRule.Builder() | |||
.setRuleKey(SQUID_RULE_KEY) | |||
.setSeverity(Severity.INFO) | |||
.setName(SQUID_RULE_NAME) | |||
.build()); | |||
initModuleIssues(); | |||
DefaultIssue issue = new DefaultIssue() | |||
@@ -205,7 +216,10 @@ public class ModuleIssuesTest { | |||
@Test | |||
public void filter_issue() { | |||
ruleBuilder.add(SQUID_RULE_KEY).setName(SQUID_RULE_NAME); | |||
activeRulesBuilder.create(SQUID_RULE_KEY).setSeverity(Severity.INFO).activate(); | |||
activeRulesBuilder.addRule(new NewActiveRule.Builder() | |||
.setRuleKey(SQUID_RULE_KEY) | |||
.setSeverity(Severity.INFO) | |||
.build()); | |||
initModuleIssues(); | |||
DefaultIssue issue = new DefaultIssue() |
@@ -24,7 +24,6 @@ import org.junit.Rule; | |||
import org.junit.Test; | |||
import org.junit.rules.TemporaryFolder; | |||
import org.sonar.api.batch.rule.ActiveRules; | |||
import org.sonar.api.batch.rule.internal.ActiveRulesBuilder; | |||
import org.sonar.api.batch.rule.internal.DefaultActiveRules; | |||
import org.sonar.api.batch.rule.internal.NewActiveRule; | |||
import org.sonar.api.rule.RuleKey; | |||
@@ -47,7 +46,13 @@ public class ActiveRulesPublisherTest { | |||
File outputDir = temp.newFolder(); | |||
ScannerReportWriter writer = new ScannerReportWriter(outputDir); | |||
NewActiveRule ar = new ActiveRulesBuilder().create(RuleKey.of("java", "S001")).setSeverity("BLOCKER").setParam("p1", "v1"); | |||
NewActiveRule ar = new NewActiveRule.Builder() | |||
.setRuleKey(RuleKey.of("java", "S001")) | |||
.setSeverity("BLOCKER") | |||
.setParam("p1", "v1") | |||
.setCreatedAt(1_000L) | |||
.setUpdatedAt(2_000L) | |||
.build(); | |||
ActiveRules activeRules = new DefaultActiveRules(singletonList(ar)); | |||
ActiveRulesPublisher underTest = new ActiveRulesPublisher(activeRules); | |||
@@ -59,6 +64,8 @@ public class ActiveRulesPublisherTest { | |||
assertThat(reportAr.getRuleRepository()).isEqualTo("java"); | |||
assertThat(reportAr.getRuleKey()).isEqualTo("S001"); | |||
assertThat(reportAr.getSeverity()).isEqualTo(Constants.Severity.BLOCKER); | |||
assertThat(reportAr.getCreatedAt()).isEqualTo(1_000L); | |||
assertThat(reportAr.getUpdatedAt()).isEqualTo(2_000L); | |||
assertThat(reportAr.getParamsByKeyMap()).hasSize(1); | |||
assertThat(reportAr.getParamsByKeyMap().entrySet().iterator().next().getKey()).isEqualTo("p1"); | |||
assertThat(reportAr.getParamsByKeyMap().entrySet().iterator().next().getValue()).isEqualTo("v1"); |
@@ -20,9 +20,11 @@ | |||
package org.sonar.scanner.rule; | |||
import com.google.common.collect.ImmutableList; | |||
import com.google.common.collect.ImmutableMap; | |||
import java.util.Date; | |||
import java.util.LinkedList; | |||
import java.util.List; | |||
import org.assertj.core.groups.Tuple; | |||
import org.junit.Before; | |||
import org.junit.Test; | |||
import org.mockito.Mock; | |||
@@ -77,6 +79,28 @@ public class ActiveRulesProviderTest { | |||
verifyNoMoreInteractions(loader); | |||
} | |||
@Test | |||
public void testParamsAreTransformed() { | |||
LoadedActiveRule r1 = mockRule("rule1"); | |||
LoadedActiveRule r2 = mockRule("rule2"); | |||
r2.setParams(ImmutableMap.of("foo1", "bar1", "foo2", "bar2")); | |||
List<LoadedActiveRule> qpRules = ImmutableList.of(r1, r2); | |||
when(loader.load(eq("qp"))).thenReturn(qpRules); | |||
ModuleQProfiles profiles = mockProfiles("qp"); | |||
ActiveRules activeRules = provider.provide(loader, profiles); | |||
assertThat(activeRules.findAll()).hasSize(2); | |||
assertThat(activeRules.findAll()).extracting("ruleKey", "params").containsOnly( | |||
Tuple.tuple(RuleKey.of("rule1", "rule1"), ImmutableMap.of()), | |||
Tuple.tuple(RuleKey.of("rule2", "rule2"), ImmutableMap.of("foo1", "bar1", "foo2", "bar2")) | |||
); | |||
verify(loader).load(eq("qp")); | |||
verifyNoMoreInteractions(loader); | |||
} | |||
private static ModuleQProfiles mockProfiles(String... keys) { | |||
List<QualityProfile> profiles = new LinkedList<>(); | |||
@@ -118,7 +118,7 @@ public class DefaultActiveRulesLoaderTest { | |||
} | |||
private String urlOfPage(int page, boolean noHotspots) { | |||
return "/api/rules/search.protobuf?f=repo,name,severity,lang,internalKey,templateKey,params,actives,createdAt&activation=true" | |||
return "/api/rules/search.protobuf?f=repo,name,severity,lang,internalKey,templateKey,params,actives,createdAt,updatedAt&activation=true" | |||
+ (noHotspots ? "&types=CODE_SMELL,BUG,VULNERABILITY" : "") + "&qprofile=c%2B-test_c%2B-values-17445&p=" + page | |||
+ "&ps=500"; | |||
} | |||
@@ -144,6 +144,7 @@ public class DefaultActiveRulesLoaderTest { | |||
Active.Builder activeBuilder = Active.newBuilder(); | |||
activeBuilder.setCreatedAt("2014-05-27T15:50:45+0100"); | |||
activeBuilder.setUpdatedAt("2014-05-27T15:50:45+0100"); | |||
if (EXAMPLE_KEY.equals(key)) { | |||
activeBuilder.addParams(Rules.Active.Param.newBuilder().setKey(FORMAT_KEY).setValue(FORMAT_VALUE)); | |||
activeBuilder.setSeverity(SEVERITY_VALUE); |
@@ -22,6 +22,7 @@ package org.sonar.scanner.rule; | |||
import java.util.Arrays; | |||
import org.junit.Test; | |||
import org.sonar.api.batch.rule.internal.ActiveRulesBuilder; | |||
import org.sonar.api.batch.rule.internal.NewActiveRule; | |||
import org.sonar.api.config.internal.MapSettings; | |||
import org.sonar.api.profiles.RulesProfile; | |||
import org.sonar.api.rule.RuleKey; | |||
@@ -77,7 +78,10 @@ public class RulesProfileProviderTest { | |||
QProfile qProfile = new QProfile("java-sw", "Sonar way", "java", null); | |||
when(qProfiles.findAll()).thenReturn(Arrays.asList(qProfile)); | |||
ActiveRulesBuilder activeRulesBuilder = new ActiveRulesBuilder(); | |||
activeRulesBuilder.create(RuleKey.of("java", "S001")).setTemplateRuleKey("T001").setLanguage("java").activate(); | |||
activeRulesBuilder.addRule(new NewActiveRule.Builder() | |||
.setRuleKey(RuleKey.of("java", "S001")) | |||
.setTemplateRuleKey("T001").setLanguage("java") | |||
.build()); | |||
RulesProfile profile = provider.provide(qProfiles, activeRulesBuilder.build(), settings.asConfig()); | |||
@@ -29,6 +29,7 @@ import org.sonar.api.batch.fs.internal.DefaultFileSystem; | |||
import org.sonar.api.batch.fs.internal.TestInputFileBuilder; | |||
import org.sonar.api.batch.rule.ActiveRules; | |||
import org.sonar.api.batch.rule.internal.ActiveRulesBuilder; | |||
import org.sonar.api.batch.rule.internal.NewActiveRule; | |||
import org.sonar.api.batch.sensor.internal.DefaultSensorDescriptor; | |||
import org.sonar.api.config.internal.MapSettings; | |||
import org.sonar.api.rule.RuleKey; | |||
@@ -106,18 +107,15 @@ public class SensorOptimizerTest { | |||
assertThat(optimizer.shouldExecute(descriptor)).isFalse(); | |||
ActiveRules activeRules = new ActiveRulesBuilder() | |||
.create(RuleKey.of("repo1", "foo")) | |||
.activate() | |||
.addRule(new NewActiveRule.Builder().setRuleKey(RuleKey.of("repo1", "foo")).build()) | |||
.build(); | |||
optimizer = new SensorOptimizer(fs, activeRules, settings.asConfig()); | |||
assertThat(optimizer.shouldExecute(descriptor)).isFalse(); | |||
activeRules = new ActiveRulesBuilder() | |||
.create(RuleKey.of("repo1", "foo")) | |||
.activate() | |||
.create(RuleKey.of("squid", "rule")) | |||
.activate() | |||
.addRule(new NewActiveRule.Builder().setRuleKey(RuleKey.of("repo1", "foo")).build()) | |||
.addRule(new NewActiveRule.Builder().setRuleKey(RuleKey.of("squid", "rule")).build()) | |||
.build(); | |||
optimizer = new SensorOptimizer(fs, activeRules, settings.asConfig()); | |||
assertThat(optimizer.shouldExecute(descriptor)).isTrue(); |
@@ -79,6 +79,7 @@ message ActiveRule { | |||
Severity severity = 3; | |||
map<string,string> params_by_key = 4; | |||
int64 createdAt = 5; | |||
int64 updatedAt = 6; | |||
} | |||
message ComponentLink { |
@@ -160,6 +160,7 @@ message Active { | |||
optional string unusedParent = 4; | |||
repeated Param params = 5; | |||
optional string createdAt = 6; | |||
optional string updatedAt = 7; | |||
message Param { | |||
optional string key = 1; |