Skip to content

Commit 0cad100

Browse files
jensgerdesjensgerdes
committed
Refactored RuleSets
1 parent d70a15e commit 0cad100

File tree

2 files changed

+45
-118
lines changed

2 files changed

+45
-118
lines changed

‎sonar-pmd-plugin/src/main/java/org/sonar/plugins/pmd/xml/PmdRuleSets.java

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public static PmdRuleSet from(Reader configReader, ValidationMessages messages)
5555
* @return An instance of PmdRuleSet. The output may be empty but never null.
5656
*/
5757
public static PmdRuleSet from(ActiveRules activeRules, String repositoryKey) {
58-
return createQuietly(new ActiveRulesRuleSetFactory(activeRules, repositoryKey));
58+
return create(new ActiveRulesRuleSetFactory(activeRules, repositoryKey));
5959
}
6060

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

70-
private static PmdRuleSet createQuietly(RuleSetFactory factory) {
70+
private static PmdRuleSet create(RuleSetFactory factory) {
71+
return factory.create();
72+
}
73+
74+
private static PmdRuleSet createQuietly(XmlRuleSetFactory factory) {
75+
76+
final PmdRuleSet result = create(factory);
77+
7178
try {
72-
return factory.create();
73-
} finally {
74-
try {
75-
factory.close();
76-
} catch (IOException e) {
77-
LOG.warn("Failed to close the given resource.", e);
78-
}
79+
factory.close();
80+
} catch (IOException e) {
81+
LOG.warn("Failed to close the given resource.", e);
7982
}
83+
84+
return result;
8085
}
8186
}

‎sonar-pmd-plugin/src/test/java/org/sonar/plugins/pmd/xml/PmdRuleSetsTest.java

Lines changed: 30 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,15 @@
2222

2323
import java.io.IOException;
2424
import java.io.Reader;
25-
import java.net.URI;
26-
import java.net.URISyntaxException;
27-
import java.nio.charset.StandardCharsets;
28-
import java.nio.file.Files;
29-
import java.nio.file.Paths;
30-
import java.util.Optional;
3125

3226
import org.junit.jupiter.api.BeforeEach;
3327
import org.junit.jupiter.api.Test;
28+
import org.sonar.api.batch.rule.ActiveRules;
29+
import org.sonar.api.profiles.RulesProfile;
3430
import org.sonar.api.utils.ValidationMessages;
3531

3632
import static org.assertj.core.api.Assertions.assertThat;
33+
import static org.mockito.Mockito.mock;
3734

3835
class PmdRuleSetsTest {
3936

@@ -43,37 +40,17 @@ class PmdRuleSetsTest {
4340
void setup() {
4441
messages = ValidationMessages.create();
4542
}
46-
/*
47-
@Test
48-
void whenValidXmlGivenThenPmdRuleSetIsReturned() throws URISyntaxException, IOException {
4943

50-
// given
51-
final Reader reader = createReader("/org/sonar/plugins/pmd/simple.xml");
52-
53-
// when
54-
final PmdRuleSet result = PmdRuleSets.parse(reader, messages);
55-
56-
// then
57-
assertThat(result).isNotNull()
58-
.hasFieldOrPropertyWithValue("name", "Sonar")
59-
.hasFieldOrPropertyWithValue("description", "Sonar PMD rules")
60-
.satisfies(this::hasRuleCouplingBetweenObjects)
61-
.satisfies(this::hasRuleExcessiveImports)
62-
.satisfies(this::hasRuleUseNotifyAllInsteadOfNotify)
63-
.satisfies(this::hasRuleUseCollectionIsEmptyRule);
64-
65-
assertThat(result.getPmdRules()).hasSize(4);
66-
assertThatNoMessagesWritten();
67-
}
68-
*//*
6944
@Test
70-
void whenExceptionOccursWhileReadingThenEmptyRuleSetIsReturned() {
45+
void whenClosingTheResourceAfterParsingFailsThenReturnsResultQuietly() {
7146

7247
// given
73-
final Reader nullReader = null;
48+
final Reader mockReader = mock(Reader.class, invocationOnMock -> {
49+
throw new IOException();
50+
});
7451

7552
// when
76-
final PmdRuleSet result = PmdRuleSets.parse(nullReader, messages);
53+
final PmdRuleSet result = PmdRuleSets.from(mockReader, messages);
7754

7855
// then
7956
assertThat(result)
@@ -82,90 +59,35 @@ void whenExceptionOccursWhileReadingThenEmptyRuleSetIsReturned() {
8259
.element(0)
8360
.asList()
8461
.isEmpty();
62+
}
8563

86-
assertThat(messages.getErrors())
87-
.isNotEmpty();
88-
}*/
64+
@Test
65+
void whenActiveRulesGivenThenRuleSetIsReturned() {
8966

90-
private void assertThatNoMessagesWritten() {
91-
assertThat(messages.getInfos()).isEmpty();
92-
assertThat(messages.getWarnings()).isEmpty();
93-
assertThat(messages.getErrors()).isEmpty();
94-
}
67+
// given
68+
final ActiveRules mockedRules = mock(ActiveRules.class);
69+
final String anyRepoKey = "TEST";
9570

96-
private void hasRuleCouplingBetweenObjects(PmdRuleSet pmdRuleSet) {
97-
final Optional<PmdRule> couplingBetweenObjects = pmdRuleSet.getPmdRules()
98-
.stream()
99-
.filter(rule -> rule.getRef() != null)
100-
.filter(rule -> rule.getRef().endsWith("CouplingBetweenObjects"))
101-
.findFirst();
102-
103-
assertThat(couplingBetweenObjects).isPresent()
104-
.get()
105-
.hasFieldOrPropertyWithValue("priority", 2)
106-
.extracting("properties")
107-
.element(0)
108-
.asList()
109-
.element(0)
110-
.hasFieldOrPropertyWithValue("name", "threshold")
111-
.hasFieldOrPropertyWithValue("value", "20")
112-
.hasNoNullFieldsOrPropertiesExcept("cdataValue");
113-
}
71+
// when
72+
final PmdRuleSet result = PmdRuleSets.from(mockedRules, anyRepoKey);
11473

115-
private void hasRuleUseNotifyAllInsteadOfNotify(PmdRuleSet pmdRuleSet) {
116-
final Optional<PmdRule> useNotifyAllInsteadOfNotify = pmdRuleSet.getPmdRules()
117-
.stream()
118-
.filter(rule -> rule.getRef() != null)
119-
.filter(rule -> rule.getRef().endsWith("UseNotifyAllInsteadOfNotify"))
120-
.findFirst();
121-
122-
assertThat(useNotifyAllInsteadOfNotify).isPresent()
123-
.get()
124-
.hasFieldOrPropertyWithValue("priority", 4)
125-
.extracting("properties")
126-
.element(0)
127-
.asList()
128-
.isEmpty();
74+
// then
75+
assertThat(result)
76+
.isNotNull();
12977
}
13078

131-
private void hasRuleExcessiveImports(PmdRuleSet pmdRuleSet) {
132-
final Optional<PmdRule> excessiveImports = pmdRuleSet.getPmdRules()
133-
.stream()
134-
.filter(rule -> rule.getRef() != null)
135-
.filter(rule -> rule.getRef().endsWith("ExcessiveImports"))
136-
.findFirst();
137-
138-
assertThat(excessiveImports).isPresent()
139-
.get()
140-
.hasFieldOrPropertyWithValue("priority", null)
141-
.extracting("properties")
142-
.element(0)
143-
.asList()
144-
.element(0)
145-
.hasFieldOrPropertyWithValue("name", "minimum")
146-
.hasFieldOrPropertyWithValue("value", "30");
147-
}
79+
@Test
80+
void whenRulesProfileGivenThenRuleSetIsReturned() {
14881

149-
private void hasRuleUseCollectionIsEmptyRule(PmdRuleSet pmdRuleSet) {
150-
final Optional<PmdRule> couplingBetweenObjects = pmdRuleSet.getPmdRules()
151-
.stream()
152-
.filter(rule -> rule.getClazz() != null)
153-
.filter(rule -> rule.getClazz().endsWith("UseUtilityClassRule"))
154-
.findFirst();
155-
156-
assertThat(couplingBetweenObjects).isPresent()
157-
.get()
158-
.hasFieldOrPropertyWithValue("priority", 3)
159-
.extracting("properties")
160-
.element(0).asList()
161-
.isEmpty();
162-
}
82+
// given
83+
final RulesProfile mockedProfile = mock(RulesProfile.class);
84+
final String anyRepoKey = "TEST";
16385

164-
private Reader createReader(String path) throws URISyntaxException, IOException {
165-
final URI resource = PmdRuleSetsTest.class.getResource(path).toURI();
166-
return Files.newBufferedReader(
167-
Paths.get(resource),
168-
StandardCharsets.UTF_8
169-
);
86+
// when
87+
final PmdRuleSet result = PmdRuleSets.from(mockedProfile, anyRepoKey);
88+
89+
// then
90+
assertThat(result)
91+
.isNotNull();
17092
}
17193
}

0 commit comments

Comments
 (0)