Skip to content

Commit

Permalink
Refactored RuleSets
Browse files Browse the repository at this point in the history
  • Loading branch information
@jensgerdes
jensgerdes committed Feb 13, 2019
1 parent d70a15e commit 0cad100
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 118 deletions.
25 changes: 15 additions & 10 deletions sonar-pmd-plugin/src/main/java/org/sonar/plugins/pmd/xml/PmdRuleSets.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public static PmdRuleSet from(Reader configReader, ValidationMessages messages)
* @return An instance of PmdRuleSet. The output may be empty but never null.
*/
public static PmdRuleSet from(ActiveRules activeRules, String repositoryKey) {
return createQuietly(new ActiveRulesRuleSetFactory(activeRules, repositoryKey));
return create(new ActiveRulesRuleSetFactory(activeRules, repositoryKey));
}

/**
Expand All @@ -64,18 +64,23 @@ public static PmdRuleSet from(ActiveRules activeRules, String repositoryKey) {
* @return An instance of PmdRuleSet. The output may be empty but never null.
*/
public static PmdRuleSet from(RulesProfile rulesProfile, String repositoryKey) {
return createQuietly(new RulesProfileRuleSetFactory(rulesProfile, repositoryKey));
return create(new RulesProfileRuleSetFactory(rulesProfile, repositoryKey));
}

private static PmdRuleSet createQuietly(RuleSetFactory factory) {
private static PmdRuleSet create(RuleSetFactory factory) {
return factory.create();
}

private static PmdRuleSet createQuietly(XmlRuleSetFactory factory) {

final PmdRuleSet result = create(factory);

try {
return factory.create();
} finally {
try {
factory.close();
} catch (IOException e) {
LOG.warn("Failed to close the given resource.", e);
}
factory.close();
} catch (IOException e) {
LOG.warn("Failed to close the given resource.", e);
}

return result;
}
}
138 changes: 30 additions & 108 deletions sonar-pmd-plugin/src/test/java/org/sonar/plugins/pmd/xml/PmdRuleSetsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,15 @@

import java.io.IOException;
import java.io.Reader;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.Optional;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.sonar.api.batch.rule.ActiveRules;
import org.sonar.api.profiles.RulesProfile;
import org.sonar.api.utils.ValidationMessages;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;

class PmdRuleSetsTest {

Expand All @@ -43,37 +40,17 @@ class PmdRuleSetsTest {
void setup() {
messages = ValidationMessages.create();
}
/*
@Test
void whenValidXmlGivenThenPmdRuleSetIsReturned() throws URISyntaxException, IOException {

// given
final Reader reader = createReader("/org/sonar/plugins/pmd/simple.xml");
// when
final PmdRuleSet result = PmdRuleSets.parse(reader, messages);
// then
assertThat(result).isNotNull()
.hasFieldOrPropertyWithValue("name", "Sonar")
.hasFieldOrPropertyWithValue("description", "Sonar PMD rules")
.satisfies(this::hasRuleCouplingBetweenObjects)
.satisfies(this::hasRuleExcessiveImports)
.satisfies(this::hasRuleUseNotifyAllInsteadOfNotify)
.satisfies(this::hasRuleUseCollectionIsEmptyRule);
assertThat(result.getPmdRules()).hasSize(4);
assertThatNoMessagesWritten();
}
*//*
@Test
void whenExceptionOccursWhileReadingThenEmptyRuleSetIsReturned() {
void whenClosingTheResourceAfterParsingFailsThenReturnsResultQuietly() {

// given
final Reader nullReader = null;
final Reader mockReader = mock(Reader.class, invocationOnMock -> {
throw new IOException();
});

// when
final PmdRuleSet result = PmdRuleSets.parse(nullReader, messages);
final PmdRuleSet result = PmdRuleSets.from(mockReader, messages);

// then
assertThat(result)
Expand All @@ -82,90 +59,35 @@ void whenExceptionOccursWhileReadingThenEmptyRuleSetIsReturned() {
.element(0)
.asList()
.isEmpty();
}

assertThat(messages.getErrors())
.isNotEmpty();
}*/
@Test
void whenActiveRulesGivenThenRuleSetIsReturned() {

private void assertThatNoMessagesWritten() {
assertThat(messages.getInfos()).isEmpty();
assertThat(messages.getWarnings()).isEmpty();
assertThat(messages.getErrors()).isEmpty();
}
// given
final ActiveRules mockedRules = mock(ActiveRules.class);
final String anyRepoKey = "TEST";

private void hasRuleCouplingBetweenObjects(PmdRuleSet pmdRuleSet) {
final Optional<PmdRule> couplingBetweenObjects = pmdRuleSet.getPmdRules()
.stream()
.filter(rule -> rule.getRef() != null)
.filter(rule -> rule.getRef().endsWith("CouplingBetweenObjects"))
.findFirst();

assertThat(couplingBetweenObjects).isPresent()
.get()
.hasFieldOrPropertyWithValue("priority", 2)
.extracting("properties")
.element(0)
.asList()
.element(0)
.hasFieldOrPropertyWithValue("name", "threshold")
.hasFieldOrPropertyWithValue("value", "20")
.hasNoNullFieldsOrPropertiesExcept("cdataValue");
}
// when
final PmdRuleSet result = PmdRuleSets.from(mockedRules, anyRepoKey);

private void hasRuleUseNotifyAllInsteadOfNotify(PmdRuleSet pmdRuleSet) {
final Optional<PmdRule> useNotifyAllInsteadOfNotify = pmdRuleSet.getPmdRules()
.stream()
.filter(rule -> rule.getRef() != null)
.filter(rule -> rule.getRef().endsWith("UseNotifyAllInsteadOfNotify"))
.findFirst();

assertThat(useNotifyAllInsteadOfNotify).isPresent()
.get()
.hasFieldOrPropertyWithValue("priority", 4)
.extracting("properties")
.element(0)
.asList()
.isEmpty();
// then
assertThat(result)
.isNotNull();
}

private void hasRuleExcessiveImports(PmdRuleSet pmdRuleSet) {
final Optional<PmdRule> excessiveImports = pmdRuleSet.getPmdRules()
.stream()
.filter(rule -> rule.getRef() != null)
.filter(rule -> rule.getRef().endsWith("ExcessiveImports"))
.findFirst();

assertThat(excessiveImports).isPresent()
.get()
.hasFieldOrPropertyWithValue("priority", null)
.extracting("properties")
.element(0)
.asList()
.element(0)
.hasFieldOrPropertyWithValue("name", "minimum")
.hasFieldOrPropertyWithValue("value", "30");
}
@Test
void whenRulesProfileGivenThenRuleSetIsReturned() {

private void hasRuleUseCollectionIsEmptyRule(PmdRuleSet pmdRuleSet) {
final Optional<PmdRule> couplingBetweenObjects = pmdRuleSet.getPmdRules()
.stream()
.filter(rule -> rule.getClazz() != null)
.filter(rule -> rule.getClazz().endsWith("UseUtilityClassRule"))
.findFirst();

assertThat(couplingBetweenObjects).isPresent()
.get()
.hasFieldOrPropertyWithValue("priority", 3)
.extracting("properties")
.element(0).asList()
.isEmpty();
}
// given
final RulesProfile mockedProfile = mock(RulesProfile.class);
final String anyRepoKey = "TEST";

private Reader createReader(String path) throws URISyntaxException, IOException {
final URI resource = PmdRuleSetsTest.class.getResource(path).toURI();
return Files.newBufferedReader(
Paths.get(resource),
StandardCharsets.UTF_8
);
// when
final PmdRuleSet result = PmdRuleSets.from(mockedProfile, anyRepoKey);

// then
assertThat(result)
.isNotNull();
}
}

0 comments on commit 0cad100

Please sign in to comment.