Added rules and exclusions for checkstyle, findbugs and pmd
The checkstyle, findbugs and pmd reports will now run only on net.onrc... classes.
Added rulesets and suppressions to implement a first pass of reasonable rules to follow for ONOS sources.
Address Yuta's review comments
Make <id> tags in executions consistent
Fix header comment in ruleset and suppression
Add '_' as a valid character in rule names
Change-Id: I1045f751fd27b0d0e815329cc34f0a780c9b38af
Address Pavlin's comments on checkstyle rules.
Change-Id: I1045f751fd27b0d0e815329cc34f0a780c9b38af
diff --git a/conf/checkstyle/onos_suppressions.xml b/conf/checkstyle/onos_suppressions.xml
new file mode 100644
index 0000000..61341f3
--- /dev/null
+++ b/conf/checkstyle/onos_suppressions.xml
@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!DOCTYPE suppressions PUBLIC "-//Puppy Crawl//DTD Suppressions 1.1//EN" "http://www.puppycrawl.com/dtds/suppressions_1_1.dtd">
+
+
+
+
+<suppressions>
+ <!-- Suppressions for ONOS -->
+ <suppress files="com.tinkerpop.*" checks="[_a-zA-Z0-9]*"/>
+ <suppress files="edu.stanford.ramcloud.*" checks="[_a-zA-Z0-9]*"/>
+ <suppress files="net.floodlightcontroller.*" checks="[_a-zA-Z0-9]*"/>
+ <suppress files="org.openflow.*" checks="[_a-zA-Z0-9]*"/>
+
+ <suppress files=".*" checks="FinalParametersCheck"/>
+ <suppress files=".*" checks="MagicNumbersCheck"/>
+ <suppress files=".*" checks="DesignForExtensionCheck"/>
+ <suppress files=".*" checks="TodoCommentCheck"/>
+ <suppress files=".*" checks="AvoidInlineConditionalsCheck"/>
+ <suppress files=".*" checks="OperatorWrapCheck"/>
+</suppressions>
+
diff --git a/conf/checkstyle/sun_checks.xml b/conf/checkstyle/sun_checks.xml
index 51a7f8c..d09a367 100644
--- a/conf/checkstyle/sun_checks.xml
+++ b/conf/checkstyle/sun_checks.xml
@@ -3,6 +3,7 @@
"-//Puppy Crawl//DTD Check Configuration 1.3//EN"
"http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
+
<!--
Checkstyle configuration that checks the sun coding conventions from:
@@ -38,10 +39,11 @@
<property name="basedir" value="${basedir}"/>
-->
-
<!-- Checks that a package-info.java file exists for each package. -->
<!-- See http://checkstyle.sf.net/config_javadoc.html#JavadocPackage -->
- <module name="JavadocPackage"/>
+ <!-- ONOS does not currently supply package level Javadoc information
+ in package-info files -->
+ <!-- <module name="JavadocPackage"/> -->
<!-- Checks whether files end with a new line. -->
<!-- See http://checkstyle.sf.net/config_misc.html#NewlineAtEndOfFile -->
@@ -81,7 +83,11 @@
<!-- See http://checkstyle.sf.net/config_javadoc.html -->
<module name="JavadocMethod"/>
<module name="JavadocType"/>
- <module name="JavadocVariable"/>
+ <module name="JavadocVariable">
+ <!-- Suppress check for private member Javadocs.
+ Possibly revist fixing these. -->
+ <property name="scope" value="public"/>
+ </module>
<module name="JavadocStyle"/>
@@ -108,7 +114,11 @@
<!-- Checks for Size Violations. -->
<!-- See http://checkstyle.sf.net/config_sizes.html -->
- <module name="LineLength"/>
+ <module name="LineLength">
+ <!-- ONOS standard usage is 80 columns, but we allow up
+ to 120 to not break the build. -->
+ <property name="max" value="120" />
+ </module>
<module name="MethodLength"/>
<module name="ParameterNumber"/>
@@ -120,7 +130,8 @@
<module name="MethodParamPad"/>
<module name="NoWhitespaceAfter"/>
<module name="NoWhitespaceBefore"/>
- <module name="OperatorWrap"/>
+ <!-- Disabled for ONOS. Default rules specify undesired behavior for the '?' operator -->
+ <!-- <module name="OperatorWrap"/> -->
<module name="ParenPad"/>
<module name="TypecastParenPad"/>
<module name="WhitespaceAfter"/>
@@ -135,7 +146,11 @@
<!-- Checks for blocks. You know, those {}'s -->
<!-- See http://checkstyle.sf.net/config_blocks.html -->
- <module name="AvoidNestedBlocks"/>
+ <module name="AvoidNestedBlocks">
+ <!-- ONOS alows declarations inside of switch case blocks -->
+ <property name="allowInSwitchCase" value="true"/>
+ <property name="severity" value="warning"/>
+ </module>
<module name="EmptyBlock"/>
<module name="LeftCurly"/>
<module name="NeedBraces"/>
@@ -144,13 +159,16 @@
<!-- Checks for common coding problems -->
<!-- See http://checkstyle.sf.net/config_coding.html -->
- <module name="AvoidInlineConditionals"/>
+ <!-- ONOS allows conditional operators -->
+ <!-- <module name="AvoidInlineConditionals"/> -->
<module name="EmptyStatement"/>
<module name="EqualsHashCode"/>
<module name="HiddenField"/>
<module name="IllegalInstantiation"/>
<module name="InnerAssignment"/>
- <module name="MagicNumber"/>
+ <!-- Many violations of this rule present, revist in a
+ subsequent round of cleanups -->
+ <!-- <module name="MagicNumber"/> -->
<module name="MissingSwitchDefault"/>
<module name="RedundantThrows"/>
<module name="SimplifyBooleanExpression"/>
@@ -158,7 +176,9 @@
<!-- Checks for class design -->
<!-- See http://checkstyle.sf.net/config_design.html -->
- <module name="DesignForExtension"/>
+ <!-- ONOS produces many warnings of this type.
+ Fixing all of these is outside the scope of the current cleanup. -->
+ <!-- <module name="DesignForExtension"/> -->
<module name="FinalClass"/>
<module name="HideUtilityClassConstructor"/>
<module name="InterfaceIsType"/>
@@ -168,8 +188,11 @@
<!-- Miscellaneous other checks. -->
<!-- See http://checkstyle.sf.net/config_misc.html -->
<module name="ArrayTypeStyle"/>
- <module name="FinalParameters"/>
- <module name="TodoComment"/>
+ <!-- Many violations of this rule currently, too many to fix
+ in the current cleanup. -->
+ <!-- <module name="FinalParameters"/> -->
+ <!-- ONOS allows TODO markers in checked in source code -->
+ <!-- <module name="TodoComment"/> -->
<module name="UpperEll"/>
</module>
diff --git a/conf/findbugs/exclude.xml b/conf/findbugs/exclude.xml
index b6a240a..391ae2b 100644
--- a/conf/findbugs/exclude.xml
+++ b/conf/findbugs/exclude.xml
@@ -11,4 +11,16 @@
<Match>
<Class name="~com.tinkerpop.blueprints.impls.ramcloud.RamCloudGraphProtos(.*)?" />
</Match>
+ <Match>
+ <Class name="~.*com\.tinkerpop\..*"/>
+ </Match>
+ <Match>
+ <Class name="~.*edu\.stanford\..*"/>
+ </Match>
+ <Match>
+ <Class name="~.*net\.floodlightcontroller\..*"/>
+ </Match>
+ <Match>
+ <Class name="~.*org\.openflow\..*"/>
+ </Match>
</FindBugsFilter>
diff --git a/conf/pmd/onos_ruleset.xml b/conf/pmd/onos_ruleset.xml
new file mode 100644
index 0000000..f70b8c0
--- /dev/null
+++ b/conf/pmd/onos_ruleset.xml
@@ -0,0 +1,48 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ name="ONOS Rules"
+ xmlns="http://pmd.sf.net/ruleset/1.0.0"
+ xsi:noNamespaceSchemaLocation="http://pmd.sf.net/ruleset_xml_schema.xsd"
+ xsi:schemaLocation="http://pmd.sf.net/ruleset/1.0.0 http://pmd.sf.net/ruleset_xml_schema.xsd" >
+
+ <description>ONOS PMD rules</description>
+
+ <rule ref="rulesets/java/unnecessary.xml" >
+ <exclude name="UselessParentheses" />
+ </rule>
+ <rule ref="rulesets/java/basic.xml">
+ <exclude name="EmptyCatchBlock"/>
+ </rule>
+ <rule ref="rulesets/java/basic.xml/EmptyCatchBlock">
+ <properties>
+ <property name="allowCommentedBlocks" value="true"/>
+ </properties>
+ </rule>
+ <rule ref="rulesets/java/unusedcode.xml"/>
+ <rule ref="rulesets/java/imports.xml"/>
+ <rule ref="rulesets/java/optimizations.xml">
+ <exclude name="LocalVariableCouldBeFinal" />
+ <exclude name="MethodArgumentCouldBeFinal" />
+ <exclude name="AvoidInstantiatingObjectsInLoops" />
+ </rule>
+
+ <rule ref="rulesets/java/strings.xml">
+ <exclude name="AvoidDuplicateLiterals" />
+ </rule>
+ <rule ref="rulesets/java/braces.xml"/>
+ <rule ref="rulesets/java/naming.xml">
+ <exclude name="AvoidInstantiatingObjectsInLoops" />
+ <exclude name="ShortMethodName" />
+ <exclude name="ShortVariable" />
+ <exclude name="LongVariable" />
+ </rule>
+ <rule ref="rulesets/java/clone.xml"/>
+ <rule ref="rulesets/java/strictexception.xml"/>
+ <rule ref="rulesets/java/design.xml">
+ <exclude name="GodClass" />
+ </rule>
+ <rule ref="rulesets/java/coupling.xml">
+ <exclude name="LawOfDemeter" />
+ <exclude name="ExcessiveImports" />
+ </rule>
+</ruleset>
\ No newline at end of file
diff --git a/pom.xml b/pom.xml
index f3cb76a..77cadcc 100644
--- a/pom.xml
+++ b/pom.xml
@@ -46,7 +46,21 @@
<version>${checkstyle-plugin.version}</version>
<configuration>
<configLocation>conf/checkstyle/sun_checks.xml</configLocation>
+ <suppressionsLocation>
+ ${basedir}/conf/checkstyle/onos_suppressions.xml
+ </suppressionsLocation>
+ <failsOnError>false</failsOnError>
</configuration>
+ <executions>
+ <execution>
+ <id>validate-checkstyle</id>
+ <phase>compile</phase>
+ <goals>
+ <!-- Uncomment this goal to make the build fail on Checkstyle errors -->
+ <!--<goal>check</goal>-->
+ </goals>
+ </execution>
+ </executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
@@ -236,6 +250,42 @@
<effort>${findbugs.effort}</effort>
<excludeFilterFile>${findbugs.excludeFilterFile}</excludeFilterFile>
</configuration>
+ <executions>
+ <execution>
+ <id>validate-findbugs</id>
+ <phase>compile</phase>
+ <goals>
+ <!-- Uncomment this goal to make the build fail on findbugs errors -->
+ <!--<goal>check</goal>-->
+ </goals>
+ </execution>
+ </executions>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-pmd-plugin</artifactId>
+ <version>3.1</version>
+ <configuration>
+ <excludes>
+ <exclude>**/com/tinkerpop/**</exclude>
+ <exclude>**/edu/stanford/**</exclude>
+ <exclude>**/net/floodlightcontroller/**</exclude>
+ <exclude>**/org/openflow/**</exclude>
+ </excludes>
+ <rulesets>
+ <ruleset>${basedir}/conf/pmd/onos_ruleset.xml</ruleset>
+ </rulesets>
+ </configuration>
+ <executions>
+ <execution>
+ <id>validate-pmd</id>
+ <phase>compile</phase>
+ <goals>
+ <!-- Uncomment this goal to make the build fail on pmd errors -->
+ <!--<goal>check</goal>-->
+ </goals>
+ </execution>
+ </executions>
</plugin>
</plugins>
<pluginManagement>
@@ -326,6 +376,9 @@
- maven-pmd-plugin configuration in pom.xml
-->
<excludes>**/RCProtos.java,**/RamCloudGraphProtos.java</excludes>
+ <suppressionsLocation>
+ ${basedir}/conf/checkstyle/onos_suppressions.xml
+ </suppressionsLocation>
</configuration>
<reportSets>
<reportSet>
@@ -365,7 +418,14 @@
<excludes>
<exclude>**/RCProtos.java</exclude>
<exclude>**/RamCloudGraphProtos.java</exclude>
+ <exclude>**/com/tinkerpop/**</exclude>
+ <exclude>**/edu/stanford/**</exclude>
+ <exclude>**/net/floodlightcontroller/**</exclude>
+ <exclude>**/org/openflow/**</exclude>
</excludes>
+ <rulesets>
+ <ruleset>${basedir}/conf/pmd/onos_ruleset.xml</ruleset>
+ </rulesets>
</configuration>
</plugin>
<plugin>