Skip to content

Commit

Permalink
Issue #47: pmd is enforced on whole code
Browse files Browse the repository at this point in the history
  • Loading branch information
romani committed Dec 25, 2016
1 parent 222b93f commit f782939
Show file tree
Hide file tree
Showing 12 changed files with 88 additions and 49 deletions.
39 changes: 29 additions & 10 deletions checkstyle-sonar-plugin/config/pmd.xml
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,14 @@
<properties>
<property name="showClassesComplexity" value="false"/>
<property name="reportLevel" value="11"/>
</properties>
<!-- very big logic in test -->
<property name="violationSuppressXPath" value="//MethodDeclaration[@Name='validateSonarProperties' and ../../..[@Image='ChecksTest']]"/>
</properties>gnature
</rule>
<rule ref="rulesets/java/codesize.xml/NPathComplexity">
<properties>
<!-- very big logic in test -->
<property name="violationSuppressXPath" value="//MethodDeclaration[@Name='validateSonarProperties' and ../../..[@Image='ChecksTest']]"/>
</properties>
</rule>
<rule ref="rulesets/java/codesize.xml/TooManyFields">
Expand Down Expand Up @@ -145,11 +149,28 @@

<rule ref="rulesets/java/finalizers.xml"/>
<rule ref="rulesets/java/imports.xml"/>
<rule ref="rulesets/java/imports.xml/TooManyStaticImports">
<properties>
<property name="maximumStaticImports" value="10" />
</properties>
</rule>
<rule ref="rulesets/java/javabeans.xml">
<!-- too many false-positives -->
<exclude name="BeanMembersShouldSerialize"/>
</rule>
<rule ref="rulesets/java/junit.xml"/>

<rule ref="rulesets/java/junit.xml">
<!-- verify from another library is used -->
<exclude name="JUnitTestsShouldIncludeAssert" />
<!-- too much false positives -->
<exclude name="JUnitAssertionsShouldIncludeMessage"/>
</rule>
<rule ref="rulesets/java/junit.xml/JUnitTestContainsTooManyAsserts">
<properties>
<property name="maximumAsserts" value="15" />
</properties>
</rule>

<rule ref="rulesets/java/logging-jakarta-commons.xml">
<!-- slf4j does not need guards -->
<exclude name="GuardLogStatement"/>
Expand Down Expand Up @@ -204,18 +225,16 @@
<property name="violationSuppressXPath" value="//ClassOrInterfaceDeclaration[@Image='CheckstyleAuditListener' or @Image='CheckstyleExecutor']"/>
</properties>
</rule>
<rule ref="rulesets/java/strictexception.xml/AvoidThrowingRawExceptionTypes">
<rule ref="rulesets/java/strictexception.xml/SignatureDeclareThrowsException">
<properties>
<!-- not sure if this is good or bad -->
<property name="violationSuppressXPath" value="//MethodDeclaration[@Name='verifyTestConfigurationFiles' and ../../..[@Image='ChecksTest']]"/>
</properties>
</rule>

<rule ref="rulesets/java/strings.xml"/>
<rule ref="rulesets/java/strings.xml/AvoidDuplicateLiterals">
<properties>
<!-- Annotations like '@SuppressWarnings' don't need to be checked, it is better to keep their strings
connected to the annotation instead of separating. -->
<property name="skipAnnotations" value="true"/>
</properties>
<rule ref="rulesets/java/strings.xml">
<!-- too much false positived in tests -->
<exclude name="AvoidDuplicateLiterals" />
</rule>
<rule ref="rulesets/java/sunsecure.xml/MethodReturnsInternalArray">
<properties>
Expand Down
2 changes: 1 addition & 1 deletion checkstyle-sonar-plugin/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@
<skipEmptyReport>false</skipEmptyReport>
<failOnViolation>true</failOnViolation>
<printFailingErrors>true</printFailingErrors>
<!--<includeTests>true</includeTests>-->
<includeTests>true</includeTests>
<rulesets>
<ruleset>config/pmd.xml</ruleset>
</rulesets>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public void testUtilityMethods() {


@Test
public void add_error_test() throws Exception {
public void addErrorTest() {
Rule rule = setupRule("repo", "key");

Issuable issuable = setupIssuable();
Expand All @@ -101,14 +101,14 @@ public void add_error_test() throws Exception {
}

@Test
public void add_error_on_unknown_rule() throws Exception {
public void addErrorOnUnknownRule() {
Issuable issuable = setupIssuable();
addErrorToListener();
verifyZeroInteractions(issuable);
}

@Test
public void add_error_on_unknown_file() throws Exception {
public void addErrorOnUnknownFile() {
Rule rule = setupRule("repo", "key");
addErrorToListener();
verifyZeroInteractions(rule);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public void exportProfile(RulesProfile profile, Writer writer) {
try {
writer.write("<conf/>");
} catch (IOException e) {
throw new RuntimeException(e);
throw new IllegalStateException(e);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
public class CheckstyleConstantsTest {

@Test
public void private_constructor() throws Exception {
public void privateConstructor() throws ReflectiveOperationException {
Constructor<CheckstyleConstants> constructor = CheckstyleConstants.class.getDeclaredConstructor();
assertThat(constructor.isAccessible()).isFalse();
constructor.setAccessible(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@
import org.sonar.plugins.java.api.JavaResourceLocator;

import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.Charset;
import java.util.Arrays;
import java.util.Locale;
Expand All @@ -56,7 +58,7 @@ public class CheckstyleExecutorTest {
public ExpectedException thrown = ExpectedException.none();

@Test
public void execute() throws Exception {
public void execute() throws CheckstyleException {
CheckstyleConfiguration conf = mockConf();
CheckstyleAuditListener listener = mockListener();
CheckstyleExecutor executor = new CheckstyleExecutor(conf, listener, createJavaResourceLocator());
Expand All @@ -82,7 +84,7 @@ public void execute() throws Exception {
}

@Test
public void execute_exception() throws Exception {
public void executeException() throws CheckstyleException {
thrown.expect(IllegalStateException.class);
thrown.expectMessage("Can not execute Checkstyle");
CheckstyleConfiguration conf = mockConf();
Expand All @@ -91,7 +93,7 @@ public void execute_exception() throws Exception {
}

@Test
public void getURL_exception() throws Exception {
public void getURLException() throws URISyntaxException {
thrown.expect(IllegalStateException.class);
thrown.expectMessage("Fail to create the project classloader. Classpath element is invalid: htp://aa");
CheckstyleExecutor executor = new CheckstyleExecutor(null, null, createJavaResourceLocator());
Expand All @@ -105,7 +107,7 @@ private static JavaResourceLocator createJavaResourceLocator() {
}

@Test
public void canGenerateXMLReport_in_english() throws Exception {
public void canGenerateXMLReportInEnglish() throws CheckstyleException, IOException {
Locale initialLocale = Locale.getDefault();
Locale.setDefault(Locale.FRENCH);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public void importParameters() {
}

@Test
public void properties_should_be_inherited() {
public void propertiesShouldBeInherited() {
Reader reader = new StringReader(CheckstyleTestUtils.getResourceContent("/org/sonar/plugins/checkstyle/CheckstyleProfileImporterTest/inheritance_of_properties.xml"));
RulesProfile profile = importer.importProfile(reader, messages);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

public class CheckstyleRulesDefinitionTest {

List<String> NO_SQALE = ImmutableList.of(
private final List<String> NO_SQALE = ImmutableList.of(
"com.puppycrawl.tools.checkstyle.checks.TranslationCheck",
"com.puppycrawl.tools.checkstyle.checks.TodoCommentCheck",
"com.puppycrawl.tools.checkstyle.checks.regexp.RegexpSinglelineCheck",
Expand Down Expand Up @@ -72,20 +72,20 @@ public void test() {
.isNotNull();
}

if (!NO_SQALE.contains(rule.key())) {
if (NO_SQALE.contains(rule.key())) {
assertThat(rule.debtRemediationFunction())
.overridingErrorMessage("Sqale remediation function is not set for rule '" + rule.key())
.isNotNull();
.overridingErrorMessage("Sqale remediation function is set for rule '" + rule.key())
.isNull();
assertThat(rule.debtSubCharacteristic())
.overridingErrorMessage("Sqale characteristic is not set for rule '" + rule.key())
.isNotNull();
.overridingErrorMessage("Sqale characteristic is set for rule '" + rule.key())
.isNull();
} else {
assertThat(rule.debtRemediationFunction())
.overridingErrorMessage("Sqale remediation function is set for rule '" + rule.key())
.isNull();
.overridingErrorMessage("Sqale remediation function is not set for rule '" + rule.key())
.isNotNull();
assertThat(rule.debtSubCharacteristic())
.overridingErrorMessage("Sqale characteristic is set for rule '" + rule.key())
.isNull();
.overridingErrorMessage("Sqale characteristic is not set for rule '" + rule.key())
.isNotNull();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,19 @@ public class CheckstyleSensorTest {
private Project project = new Project("projectKey");

@Test
public void shouldExecuteOnProject_without_java_file_and_with_rule() throws Exception {
public void shouldExecuteOnProjectWithoutJavaFileAndWithRule() {
addOneActiveRule();
assertThat(sensor.shouldExecuteOnProject(project)).isFalse();
}

@Test
public void shouldExecuteOnProject_with_java_file_and_without_rule() throws Exception {
public void shouldExecuteOnProjectWithJavaFileAndWithoutRule() {
addOneJavaFile();
assertThat(sensor.shouldExecuteOnProject(project)).isFalse();
}

@Test
public void shouldExecuteOnProject_with_java_files_and_rules() throws Exception {
public void shouldExecuteOnProjectWithJavaFilesAndRules() {
addOneJavaFile();
addOneActiveRule();
assertThat(sensor.shouldExecuteOnProject(project)).isTrue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public void testFromSeverity() {
}

@Test
public void private_constructor() throws Exception {
public void privateConstructor() throws ReflectiveOperationException {
Constructor<CheckstyleSeverityUtils> constructor = CheckstyleSeverityUtils.class.getDeclaredConstructor();
assertThat(constructor.isAccessible()).isFalse();
constructor.setAccessible(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@

import static org.junit.Assert.assertTrue;

public class CheckstyleTestUtils {
public final class CheckstyleTestUtils {

private CheckstyleTestUtils() {
// no code
}

public static String getResourceContent(String path) {
try {
Expand Down
Loading

0 comments on commit f782939

Please sign in to comment.