You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by wf...@apache.org on 2014/05/27 20:25:10 UTC
[2/2] git commit: Enable some additional PMD rules,
and fix sources to satisfy them.
Enable some additional PMD rules, and fix sources to satisfy them.
Reviewed at https://reviews.apache.org/r/21849/
Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/a8fa267f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/a8fa267f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/a8fa267f
Branch: refs/heads/master
Commit: a8fa267f03e2fa36039eb014884858074f7b7575
Parents: 10da38a
Author: Bill Farner <wf...@apache.org>
Authored: Tue May 27 11:25:02 2014 -0700
Committer: Bill Farner <wf...@apache.org>
Committed: Tue May 27 11:25:02 2014 -0700
----------------------------------------------------------------------
build.gradle | 25 +-
config/pmd/design.xml | 1907 ++++++++++++++++++
config/pmd/logging-java.xml | 183 ++
.../apache/aurora/auth/SessionValidator.java | 2 +-
.../org/apache/aurora/scheduler/Driver.java | 2 +-
.../apache/aurora/scheduler/DriverFactory.java | 8 +-
.../aurora/scheduler/MesosSchedulerImpl.java | 27 +-
.../aurora/scheduler/MesosTaskFactory.java | 4 +-
.../aurora/scheduler/SchedulerLifecycle.java | 5 +-
.../aurora/scheduler/async/OfferQueue.java | 18 +-
.../aurora/scheduler/async/TaskGroups.java | 2 -
.../apache/aurora/scheduler/base/AsyncUtil.java | 24 +-
.../aurora/scheduler/base/CommandUtil.java | 2 +-
.../org/apache/aurora/scheduler/base/Query.java | 4 +-
.../configuration/ConfigurationManager.java | 2 +-
.../scheduler/configuration/Resources.java | 2 +-
.../configuration/SanitizedConfiguration.java | 22 +
.../scheduler/cron/quartz/AuroraCronJob.java | 2 +-
.../scheduler/cron/quartz/CronLifecycle.java | 5 +-
.../scheduler/cron/quartz/CronModule.java | 4 +-
.../aurora/scheduler/events/PubsubEvent.java | 18 +-
.../scheduler/filter/SchedulingFilter.java | 2 +-
.../scheduler/http/JerseyTemplateServlet.java | 6 +-
.../aurora/scheduler/http/LeaderRedirect.java | 4 +-
.../apache/aurora/scheduler/http/Quotas.java | 2 +-
.../apache/aurora/scheduler/http/Slaves.java | 2 +-
.../aurora/scheduler/http/StructDump.java | 6 +-
.../local/IsolatedSchedulerModule.java | 26 +-
.../scheduler/quota/QuotaCheckResult.java | 6 +-
.../aurora/scheduler/state/LockManagerImpl.java | 10 +-
.../scheduler/state/SchedulerCoreImpl.java | 16 +-
.../scheduler/storage/AttributeStore.java | 5 +-
.../aurora/scheduler/storage/JobStore.java | 2 +-
.../aurora/scheduler/storage/LockStore.java | 2 +-
.../aurora/scheduler/storage/QuotaStore.java | 2 +-
.../scheduler/storage/ReadWriteLockManager.java | 12 +-
.../scheduler/storage/SchedulerStore.java | 2 +-
.../aurora/scheduler/storage/Storage.java | 2 +-
.../scheduler/storage/StorageBackfill.java | 6 +-
.../aurora/scheduler/storage/TaskStore.java | 4 +-
.../scheduler/storage/backup/BackupModule.java | 10 +-
.../scheduler/storage/backup/Recovery.java | 2 +-
.../scheduler/storage/log/LogManager.java | 4 +-
.../scheduler/storage/log/LogStorage.java | 8 +-
.../storage/log/testing/LogOpMatcher.java | 4 +-
.../storage/mem/MemAttributeStore.java | 6 +-
.../scheduler/storage/mem/MemJobStore.java | 6 +-
.../thrift/SchedulerThriftInterface.java | 180 +-
.../thrift/aop/FeatureToggleInterceptor.java | 6 +-
49 files changed, 2361 insertions(+), 250 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/build.gradle
----------------------------------------------------------------------
diff --git a/build.gradle b/build.gradle
index 0dbaa61..646cdd2 100644
--- a/build.gradle
+++ b/build.gradle
@@ -264,7 +264,30 @@ tasks.withType(FindBugs) {
}
pmd {
- sourceSets = [ sourceSets.main ]
+ sourceSets = [sourceSets.main]
+ // PMD rule set names match XML files stored in the PMD jar. For example, with 5.11 you can
+ // see all the rules included with:
+ // $ find ~/.gradle -name pmd-5.1.1.jar | xargs zipinfo -1 | egrep java/.*.xml | head -n 5
+ // rulesets/java/clone.xml
+ // rulesets/java/basic.xml
+ // rulesets/java/strings.xml
+ // rulesets/java/sunsecure.xml
+ // rulesets/java/codesize.xml
+ // The format is straightforward: 'java-basic' refers to rulesets/java/basic.xml.
+ ruleSets = [
+ 'java-basic',
+ 'java-braces',
+ // TODO(wfarner): Circle back to (at least partially) re-enable the rules below.
+ //'java-design',
+ 'java-empty',
+ //'java-imports',
+ 'java-junit',
+ //'java-logging-java',
+ //'java-naming',
+ 'java-typeresolution',
+ 'java-unnecessary',
+ 'java-unusedcode']
+ ruleSetFiles = fileTree('config/pmd/')
toolVersion = '5.1.1'
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/config/pmd/design.xml
----------------------------------------------------------------------
diff --git a/config/pmd/design.xml b/config/pmd/design.xml
new file mode 100644
index 0000000..fb86e0a
--- /dev/null
+++ b/config/pmd/design.xml
@@ -0,0 +1,1907 @@
+<?xml version="1.0"?>
+<!--
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this work except in compliance with the License.
+You may obtain a copy of the License in the LICENSE file, or at:
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+<!--
+This is a modified file from the PMD project. Copying and editing ruleset
+configurations is the approach used to modify a ruleset behavior, such as
+disabling individul rules.
+-->
+
+<ruleset name="Design"
+ xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
+
+ <description>
+The Design ruleset contains rules that flag suboptimal code implementations. Alternate approaches
+are suggested.
+ </description>
+
+ <rule name="UseUtilityClass"
+ since="0.3"
+ message="All methods are static. Consider using a utility class instead. Alternatively, you could add a private constructor or make the class abstract to silence this warning."
+ class="net.sourceforge.pmd.lang.java.rule.design.UseUtilityClassRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UseUtilityClass">
+ <description>
+ <![CDATA[
+For classes that only have static methods, consider making them utility classes.
+Note that this doesn't apply to abstract classes, since their subclasses may
+well include non-static methods. Also, if you want this class to be a utility class,
+remember to add a private constructor to prevent instantiation.
+(Note, that this use was known before PMD 5.1.0 as UseSingleton).
+ ]]>
+ </description>
+ <priority>3</priority>
+ <example>
+<![CDATA[
+public class MaybeAUtility {
+ public static void foo() {}
+ public static void bar() {}
+}
+]]>
+ </example>
+ </rule>
+
+
+ <rule name="SimplifyBooleanReturns"
+ since="0.9"
+ message="Avoid unnecessary if..then..else statements when returning booleans"
+ class="net.sourceforge.pmd.lang.java.rule.design.SimplifyBooleanReturnsRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#SimplifyBooleanReturns">
+ <description>
+Avoid unnecessary if-then-else statements when returning a boolean. The result of
+the conditional test can be returned instead.
+ </description>
+ <priority>3</priority>
+ <example>
+<![CDATA[
+public boolean isBarEqualTo(int x) {
+
+ if (bar == x) { // this bit of code...
+ return true;
+ } else {
+ return false;
+ }
+}
+
+public boolean isBarEqualTo(int x) {
+
+ return bar == x; // can be replaced with this
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="SimplifyBooleanExpressions"
+ language="java"
+ since="1.05"
+ message="Avoid unnecessary comparisons in boolean expressions"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#SimplifyBooleanExpressions">
+ <description>
+Avoid unnecessary comparisons in boolean expressions, they serve no purpose and impacts readability.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+<![CDATA[
+//EqualityExpression/PrimaryExpression
+ /PrimaryPrefix/Literal/BooleanLiteral
+]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class Bar {
+ // can be simplified to
+ // bar = isFoo();
+ private boolean bar = (isFoo() == true);
+
+ public isFoo() { return false;}
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="SwitchStmtsShouldHaveDefault"
+ language="java"
+ since="1.0"
+ message="Switch statements should have a default label"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#SwitchStmtsShouldHaveDefault">
+ <description>
+All switch statements should include a default option to catch any unspecified values.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//SwitchStatement[not(SwitchLabel[@Default='true'])]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+public void bar() {
+ int x = 2;
+ switch (x) {
+ case 1: int j = 6;
+ case 2: int j = 8;
+ // missing default: here
+ }
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="AvoidDeeplyNestedIfStmts"
+ since="1.0"
+ message="Deeply nested if..then statements are hard to read"
+ class="net.sourceforge.pmd.lang.java.rule.design.AvoidDeeplyNestedIfStmtsRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AvoidDeeplyNestedIfStmts">
+ <description>
+Avoid creating deeply nested if-then statements since they are harder to read and error-prone to maintain.
+ </description>
+ <priority>3</priority>
+ <example>
+<![CDATA[
+public class Foo {
+ public void bar(int x, int y, int z) {
+ if (x>y) {
+ if (y>z) {
+ if (z==x) {
+ // !! too deep
+ }
+ }
+ }
+ }
+}
+]]>
+ </example>
+ </rule>
+
+
+ <rule name="AvoidReassigningParameters"
+ since="1.0"
+ message="Avoid reassigning parameters such as ''{0}''"
+ class="net.sourceforge.pmd.lang.java.rule.design.AvoidReassigningParametersRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AvoidReassigningParameters">
+ <description>
+Reassigning values to incoming parameters is not recommended. Use temporary local variables instead.
+ </description>
+ <priority>2</priority>
+ <example>
+<![CDATA[
+public class Foo {
+ private void foo(String bar) {
+ bar = "something else";
+ }
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="SwitchDensity"
+ since="1.02"
+ message="A high ratio of statements to labels in a switch statement. Consider refactoring."
+ class="net.sourceforge.pmd.lang.java.rule.design.SwitchDensityRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#SwitchDensity">
+ <description>
+A high ratio of statements to labels in a switch statement implies that the switch statement
+is overloaded. Consider moving the statements into new methods or creating subclasses based
+on the switch variable.
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+public class Foo {
+ public void bar(int x) {
+ switch (x) {
+ case 1: {
+ // lots of statements
+ break;
+ } case 2: {
+ // lots of statements
+ break;
+ }
+ }
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="ConstructorCallsOverridableMethod"
+ since="1.04"
+ message="Overridable {0} called during object construction"
+ class="net.sourceforge.pmd.lang.java.rule.design.ConstructorCallsOverridableMethodRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#ConstructorCallsOverridableMethod">
+ <description>
+Calling overridable methods during construction poses a risk of invoking methods on an incompletely
+constructed object and can be difficult to debug.
+It may leave the sub-class unable to construct its superclass or forced to replicate the construction
+process completely within itself, losing the ability to call super(). If the default constructor
+contains a call to an overridable method, the subclass may be completely uninstantiable. Note that
+this includes method calls throughout the control flow graph - i.e., if a constructor Foo() calls a
+private method bar() that calls a public method buz(), this denotes a problem.
+ </description>
+ <priority>1</priority>
+ <example>
+ <![CDATA[
+public class SeniorClass {
+ public SeniorClass(){
+ toString(); //may throw NullPointerException if overridden
+ }
+ public String toString(){
+ return "IAmSeniorClass";
+ }
+}
+public class JuniorClass extends SeniorClass {
+ private String name;
+ public JuniorClass(){
+ super(); //Automatic call leads to NullPointerException
+ name = "JuniorClass";
+ }
+ public String toString(){
+ return name.toUpperCase();
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="AccessorClassGeneration"
+ since="1.04"
+ message="Avoid instantiation through private constructors from outside of the constructor's class."
+ class="net.sourceforge.pmd.lang.java.rule.design.AccessorClassGenerationRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AccessorClassGeneration">
+ <description>
+Instantiation by way of private constructors from outside of the constructor's class often causes the
+generation of an accessor. A factory method, or non-privatization of the constructor can eliminate this
+situation. The generated class file is actually an interface. It gives the accessing class the ability
+to invoke a new hidden package scope constructor that takes the interface as a supplementary parameter.
+This turns a private constructor effectively into one with package scope, and is challenging to discern.
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+public class Outer {
+ void method(){
+ Inner ic = new Inner();//Causes generation of accessor class
+ }
+ public class Inner {
+ private Inner(){}
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="FinalFieldCouldBeStatic"
+ language="java"
+ since="1.1"
+ message="This final field could be made static"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#FinalFieldCouldBeStatic">
+ <description>
+If a final field is assigned to a compile-time constant, it could be made static, thus saving overhead
+in each object at runtime.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//FieldDeclaration
+ [@Final='true' and @Static='false']
+ [not (../../../../ClassOrInterfaceDeclaration[@Interface='true'])]
+ /VariableDeclarator/VariableInitializer/Expression
+ /PrimaryExpression[not(PrimarySuffix)]/PrimaryPrefix/Literal
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class Foo {
+ public final int BAR = 42; // this could be static and save some space
+}
+ ]]>
+ </example>
+ </rule>
+
+
+ <rule name="CloseResource"
+ since="1.2.2"
+ message="Ensure that resources like this {0} object are closed after use"
+ class="net.sourceforge.pmd.lang.java.rule.design.CloseResourceRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#CloseResource">
+ <description>
+Ensure that resources (like Connection, Statement, and ResultSet objects) are always closed after use.
+ </description>
+ <priority>3</priority>
+ <example>
+<![CDATA[
+public class Bar {
+ public void foo() {
+ Connection c = pool.getConnection();
+ try {
+ // do stuff
+ } catch (SQLException ex) {
+ // handle exception
+ } finally {
+ // oops, should close the connection using 'close'!
+ // c.close();
+ }
+ }
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="NonStaticInitializer"
+ language="java"
+ since="1.5"
+ message="Non-static initializers are confusing"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#NonStaticInitializer">
+ <description>
+A non-static initializer block will be called any time a constructor is invoked (just prior to
+invoking the constructor). While this is a valid language construct, it is rarely used and is
+confusing.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+<![CDATA[
+//Initializer[@Static='false']
+]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class MyClass {
+ // this block gets run before any call to a constructor
+ {
+ System.out.println("I am about to construct myself");
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="DefaultLabelNotLastInSwitchStmt"
+ language="java"
+ since="1.5"
+ message="The default label should be the last label in a switch statement"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#DefaultLabelNotLastInSwitchStmt">
+ <description>
+By convention, the default label should be the last label in a switch statement.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+<![CDATA[
+//SwitchStatement
+ [not(SwitchLabel[position() = last()][@Default='true'])]
+ [SwitchLabel[@Default='true']]
+]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class Foo {
+ void bar(int a) {
+ switch (a) {
+ case 1: // do something
+ break;
+ default: // the default case should be last, by convention
+ break;
+ case 2:
+ break;
+ }
+ }
+} ]]>
+ </example>
+ </rule>
+
+ <rule name="NonCaseLabelInSwitchStatement"
+ language="java"
+ since="1.5"
+ message="A non-case label was present in a switch statement"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#NonCaseLabelInSwitchStatement">
+ <description>
+A non-case label (e.g. a named break/continue label) was present in a switch statement.
+This legal, but confusing. It is easy to mix up the case labels and the non-case labels.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//SwitchStatement//BlockStatement/Statement/LabeledStatement
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class Foo {
+ void bar(int a) {
+ switch (a) {
+ case 1:
+ // do something
+ break;
+ mylabel: // this is legal, but confusing!
+ break;
+ default:
+ break;
+ }
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="OptimizableToArrayCall"
+ language="java"
+ since="1.8"
+ message="This call to Collection.toArray() may be optimizable"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#OptimizableToArrayCall">
+ <description>
+Calls to a collection's toArray() method should specify target arrays sized to match the size of the
+collection. Initial arrays that are too small are discarded in favour of new ones that have to be created
+that are the proper size.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//PrimaryExpression
+[PrimaryPrefix/Name[ends-with(@Image, 'toArray')]]
+[
+PrimarySuffix/Arguments/ArgumentList/Expression
+ /PrimaryExpression/PrimaryPrefix/AllocationExpression
+ /ArrayDimsAndInits/Expression/PrimaryExpression/PrimaryPrefix/Literal[@Image='0']
+]
+
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+List foos = getFoos();
+
+ // inefficient, the array will be discarded
+Foo[] fooArray = foos.toArray(new Foo[0]);
+
+ // much better; this one sizes the destination array,
+ // avoiding of a new one via reflection
+Foo[] fooArray = foos.toArray(new Foo[foos.size()]);
+ ]]>
+ </example>
+ </rule>
+
+
+ <rule name="BadComparison"
+ language="java"
+ since="1.8"
+ message="Avoid equality comparisons with Double.NaN"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#BadComparison">
+ <description>
+Avoid equality comparisons with Double.NaN. Due to the implicit lack of representation
+precision when comparing floating point numbers these are likely to cause logic errors.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//EqualityExpression[@Image='==']
+ /PrimaryExpression/PrimaryPrefix
+ /Name[@Image='Double.NaN' or @Image='Float.NaN']
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+boolean x = (y == Double.NaN);
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="EqualsNull"
+ language="java"
+ since="1.9"
+ message="Avoid using equals() to compare against null"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#EqualsNull">
+ <description>
+Tests for null should not use the equals() method. The '==' operator should be used instead.
+ </description>
+ <priority>1</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//PrimaryExpression
+ [
+ PrimaryPrefix[Name[ends-with(@Image, 'equals')]]
+ [following-sibling::node()/Arguments/ArgumentList[count(Expression)=1]
+ /Expression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]
+
+ or
+
+ PrimarySuffix[ends-with(@Image, 'equals')]
+ [following-sibling::node()/Arguments/ArgumentList[count(Expression)=1]
+ /Expression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral]
+
+ ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+String x = "foo";
+
+if (x.equals(null)) { // bad form
+ doSomething();
+ }
+
+if (x == null) { // preferred
+ doSomething();
+ }
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="ConfusingTernary"
+ since="1.9"
+ message="Avoid if (x != y) ..; else ..;"
+ class="net.sourceforge.pmd.lang.java.rule.design.ConfusingTernaryRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#ConfusingTernary">
+ <description>
+Avoid negation within an "if" expression with an "else" clause. For example, rephrase:
+
+ if (x != y) diff(); else same();
+as:
+ if (x == y) same(); else diff();
+
+Most "if (x != y)" cases without an "else" are often return cases, so consistent use of this
+rule makes the code easier to read. Also, this resolves trivial ordering problems, such
+as "does the error case go first?" or "does the common case go first?".
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+boolean bar(int x, int y) {
+ return (x != y) ? diff : same;
+ }
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="InstantiationToGetClass"
+ language="java"
+ since="2.0"
+ message="Avoid instantiating an object just to call getClass() on it; use the .class public member instead"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#InstantiationToGetClass">
+ <description>
+Avoid instantiating an object just to call getClass() on it; use the .class public member instead.
+ </description>
+ <priority>4</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//PrimarySuffix
+ [@Image='getClass']
+ [parent::PrimaryExpression
+ [PrimaryPrefix/AllocationExpression]
+ [count(PrimarySuffix) = 2]
+ ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+ // replace this
+Class c = new String().getClass();
+
+ // with this:
+Class c = String.class;
+
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="IdempotentOperations"
+ since="2.0"
+ message="Avoid idempotent operations (like assigning a variable to itself)."
+ class="net.sourceforge.pmd.lang.java.rule.design.IdempotentOperationsRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#IdempotentOperations">
+ <description>
+Avoid idempotent operations - they have no effect.
+ </description>
+ <priority>3</priority>
+
+ <example>
+ <![CDATA[
+public class Foo {
+ public void bar() {
+ int x = 2;
+ x = x;
+ }
+}
+ ]]>
+ </example>
+ </rule>
+<!--
+We are not currently focusing on localization.
+ <rule
+ name="SimpleDateFormatNeedsLocale"
+ language="java"
+ since="2.0"
+ message="When instantiating a SimpleDateFormat object, specify a Locale"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#SimpleDateFormatNeedsLocale">
+ <description>
+Be sure to specify a Locale when creating SimpleDateFormat instances to ensure that locale-appropriate
+formatting is used.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+<![CDATA[
+//AllocationExpression
+ [ClassOrInterfaceType[@Image='SimpleDateFormat']]
+ [Arguments[@ArgumentCount=1]]
+]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class Foo {
+ // Should specify Locale.US (or whatever)
+ private SimpleDateFormat sdf = new SimpleDateFormat("pattern");
+}
+ ]]>
+ </example>
+ </rule>
+-->
+
+ <rule name="ImmutableField"
+ since="2.0"
+ message="Private field ''{0}'' could be made final; it is only initialized in the declaration or constructor."
+ class="net.sourceforge.pmd.lang.java.rule.design.ImmutableFieldRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#ImmutableField">
+ <description>
+Identifies private fields whose values never change once they are initialized either in the declaration
+of the field or by a constructor. This helps in converting existing classes to becoming immutable ones.
+ </description>
+ <priority>3</priority>
+
+ <example>
+ <![CDATA[
+public class Foo {
+ private int x; // could be final
+ public Foo() {
+ x = 7;
+ }
+ public void foo() {
+ int a = x + 2;
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+<!--
+We're not currently focusing on localization.
+ <rule name="UseLocaleWithCaseConversions"
+ language="java"
+ since="2.0"
+ message="When doing a String.toLowerCase()/toUpperCase() call, use a Locale"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UseLocaleWithCaseConversions">
+ <description>
+When doing String.toLowerCase()/toUpperCase() conversions, use Locales to avoids problems with languages that
+have unusual conventions, i.e. Turkish.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//PrimaryExpression
+[PrimaryPrefix/Name
+ [ends-with(@Image, 'toLowerCase') or ends-with(@Image,
+'toUpperCase')]
+ ]
+[PrimarySuffix[position() = 1]/Arguments[@ArgumentCount=0]]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+class Foo {
+ // BAD
+ if (x.toLowerCase().equals("list"))...
+ /*
+ This will not match "LIST" when in Turkish locale
+ The above could be
+ if (x.toLowerCase(Locale.US).equals("list")) ...
+ or simply
+ if (x.equalsIgnoreCase("list")) ...
+ */
+ // GOOD
+ String z = a.toLowerCase(Locale.EN);
+}
+ ]]>
+ </example>
+ </rule>
+-->
+ <rule name="AvoidProtectedFieldInFinalClass"
+ language="java"
+ since="2.1"
+ message="Avoid protected fields in a final class. Change to private or package access."
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AvoidProtectedFieldInFinalClass">
+ <description>
+Do not use protected fields in final classes since they cannot be subclassed.
+Clarify your intent by using private or package access modifiers instead.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+<![CDATA[
+//ClassOrInterfaceDeclaration[@Final='true']
+/ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration
+/FieldDeclaration[@Protected='true']
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+public final class Bar {
+ private int x;
+ protected int y; // bar cannot be subclassed, so is y really private or package visible?
+ Bar() {}
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="AssignmentToNonFinalStatic"
+ since="2.2"
+ message="Possible unsafe assignment to a non-final static field in a constructor."
+ class="net.sourceforge.pmd.lang.java.rule.design.AssignmentToNonFinalStaticRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AssignmentToNonFinalStatic">
+ <description>
+Identifies a possible unsafe usage of a static field.
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+public class StaticField {
+ static int x;
+ public FinalFields(int y) {
+ x = y; // unsafe
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="MissingStaticMethodInNonInstantiatableClass"
+ language="java"
+ since="3.0"
+ message="Class cannot be instantiated and does not provide any static methods or fields"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#MissingStaticMethodInNonInstantiatableClass">
+ <description>
+A class that has private constructors and does not have any static methods or fields cannot be used.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//ClassOrInterfaceDeclaration[@Nested='false']
+[
+ (
+ count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration)>0
+ and
+ count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration) = count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[@Private='true'])
+ )
+ and
+ count(.//MethodDeclaration[@Static='true'])=0
+ and
+ count(.//FieldDeclaration[@Private='false'][@Static='true'])=0
+ and
+ count(.//ClassOrInterfaceDeclaration[@Nested='true']
+ [@Public='true']
+ [@Static='true']
+ [count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[@Public='true']) > 0]
+ [count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/MethodDeclaration
+ [@Public='true']
+ [./ResultType/Type/ReferenceType/ClassOrInterfaceType
+ [@Image = //ClassOrInterfaceDeclaration[@Nested='false']/@Image]
+ ]
+ ) > 0]
+ ) = 0
+]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+// This class is unusable, since it cannot be
+// instantiated (private constructor),
+// and no static method can be called.
+
+public class Foo {
+ private Foo() {}
+ void foo() {}
+}
+
+]]>
+ </example>
+ </rule>
+
+<!--
+We deliberately use method-level synchronization for simplicity in many cases.
+ <rule name="AvoidSynchronizedAtMethodLevel"
+ language="java"
+ since="3.0"
+ message="Use block level rather than method level synchronization"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AvoidSynchronizedAtMethodLevel">
+ <description>
+Method-level synchronization can cause problems when new code is added to the method.
+Block-level synchronization helps to ensure that only the code that needs synchronization
+gets it.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//MethodDeclaration[@Synchronized='true']
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+public class Foo {
+ // Try to avoid this:
+ synchronized void foo() {
+ }
+ // Prefer this:
+ void bar() {
+ synchronized(this) {
+ }
+ }
+
+ // Try to avoid this for static methods:
+ static synchronized void fooStatic() {
+ }
+
+ // Prefer this:
+ static void barStatic() {
+ synchronized(Foo.class) {
+ }
+ }
+}
+]]>
+ </example>
+ </rule>
+-->
+
+ <rule name="MissingBreakInSwitch"
+ language="java"
+ since="3.0"
+ message="A switch statement does not contain a break"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#MissingBreakInSwitch">
+ <description>
+Switch statements without break or return statements for each case option
+may indicate problematic behaviour. Empty cases are ignored as these indicate an intentional fall-through.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//SwitchStatement
+[(count(.//BreakStatement)
+ + count(BlockStatement/Statement/ReturnStatement)
+ + count(BlockStatement/Statement/ThrowStatement)
+ + count(SwitchLabel[name(following-sibling::node()) = 'SwitchLabel'])
+ + count(SwitchLabel[count(following-sibling::node()) = 0 or count(child::node()) = 0])
+ < count (SwitchLabel))]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+public void bar(int status) {
+ switch(status) {
+ case CANCELLED:
+ doCancelled();
+ // break; hm, should this be commented out?
+ case NEW:
+ doNew();
+ // is this really a fall-through?
+ case REMOVED:
+ doRemoved();
+ // what happens if you add another case after this one?
+ case OTHER: // empty case - this is interpreted as an intentional fall-through
+ case ERROR:
+ doErrorHandling();
+ break;
+ }
+}
+]]>
+ </example>
+ </rule>
+
+
+ <rule name="UseNotifyAllInsteadOfNotify"
+ language="java"
+ since="3.0"
+ message="Call Thread.notifyAll() rather than Thread.notify()"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UseNotifyAllInsteadOfNotify">
+ <description>
+Thread.notify() awakens a thread monitoring the object. If more than one thread is monitoring, then only
+one is chosen. The thread chosen is arbitrary; thus its usually safer to call notifyAll() instead.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//StatementExpression/PrimaryExpression
+[count(PrimarySuffix/Arguments/ArgumentList) = 0]
+[
+PrimaryPrefix[./Name[@Image='notify' or ends-with(@Image,'.notify')]
+or ../PrimarySuffix/@Image='notify'
+or (./AllocationExpression and ../PrimarySuffix[@Image='notify'])
+]
+]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+ void bar() {
+ x.notify();
+ // If many threads are monitoring x, only one (and you won't know which) will be notified.
+ // use instead:
+ x.notifyAll();
+ }
+]]>
+ </example>
+ </rule>
+
+ <rule name="AvoidInstanceofChecksInCatchClause"
+ language="java"
+ since="3.0"
+ message="An instanceof check is being performed on the caught exception. Create a separate catch clause for this exception type."
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AvoidInstanceofChecksInCatchClause">
+ <description>
+Each caught exception type should be handled in its own catch clause.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//CatchStatement/FormalParameter
+ /following-sibling::Block//InstanceOfExpression/PrimaryExpression/PrimaryPrefix
+ /Name[
+ @Image = ./ancestor::Block/preceding-sibling::FormalParameter
+ /VariableDeclaratorId/@Image
+ ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+try { // Avoid this
+ // do something
+} catch (Exception ee) {
+ if (ee instanceof IOException) {
+ cleanup();
+ }
+}
+try { // Prefer this:
+ // do something
+} catch (IOException ee) {
+ cleanup();
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="AbstractClassWithoutAbstractMethod"
+ language="java"
+ since="3.0"
+ message="This abstract class does not have any abstract methods"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AbstractClassWithoutAbstractMethod">
+ <description>
+The abstract class does not contain any abstract methods. An abstract class suggests
+an incomplete implementation, which is to be completed by subclasses implementing the
+abstract methods. If the class is intended to be used as a base class only (not to be instantiated
+directly) a protected constructor can be provided prevent direct instantiation.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value><![CDATA[
+//ClassOrInterfaceDeclaration
+ [@Abstract='true'
+ and count( .//MethodDeclaration[@Abstract='true'] )=0 ]
+ [count(ImplementsList)=0]
+ [count(.//ExtendsList)=0]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+public abstract class Foo {
+ void int method1() { ... }
+ void int method2() { ... }
+ // consider using abstract methods or removing
+ // the abstract modifier and adding protected constructors
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="SimplifyConditional"
+ language="java"
+ since="3.1"
+ message="No need to check for null before an instanceof"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#SimplifyConditional">
+ <description>
+No need to check for null before an instanceof; the instanceof keyword returns false when given a null argument.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//Expression
+ [ConditionalOrExpression
+ [EqualityExpression[@Image='==']
+ //NullLiteral
+ and
+ UnaryExpressionNotPlusMinus
+ [@Image='!']//InstanceOfExpression[PrimaryExpression
+ //Name/@Image = ancestor::ConditionalOrExpression/EqualityExpression
+ /PrimaryExpression/PrimaryPrefix/Name/@Image]
+ and
+ (count(UnaryExpressionNotPlusMinus) + 1 = count(*))
+ ]
+or
+ConditionalAndExpression
+ [EqualityExpression[@Image='!=']//NullLiteral
+ and
+InstanceOfExpression
+ [PrimaryExpression[count(PrimarySuffix[@ArrayDereference='true'])=0]
+ //Name[not(contains(@Image,'.'))]/@Image = ancestor::ConditionalAndExpression
+ /EqualityExpression/PrimaryExpression/PrimaryPrefix/Name/@Image]
+ and
+(count(InstanceOfExpression) + 1 = count(*))
+ ]
+]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+class Foo {
+ void bar(Object x) {
+ if (x != null && x instanceof Bar) {
+ // just drop the "x != null" check
+ }
+ }
+} ]]>
+ </example>
+ </rule>
+
+<rule name="CompareObjectsWithEquals"
+ since="3.2"
+ message="Use equals() to compare object references."
+ class="net.sourceforge.pmd.lang.java.rule.design.CompareObjectsWithEqualsRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#CompareObjectsWithEquals">
+ <description>
+Use equals() to compare object references; avoid comparing them with ==.
+ </description>
+ <priority>3</priority>
+ <example>
+<![CDATA[
+class Foo {
+ boolean bar(String a, String b) {
+ return a == b;
+ }
+}
+
+]]>
+ </example>
+</rule>
+
+
+<rule name="PositionLiteralsFirstInComparisons"
+ language="java"
+ since="3.3"
+ message="Position literals first in String comparisons"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#PositionLiteralsFirstInComparisons">
+ <description>
+Position literals first in comparisons, if the second argument is null then NullPointerExceptions
+can be avoided, they will just return false.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//PrimaryExpression[
+ PrimaryPrefix[Name
+ [
+ (ends-with(@Image, '.equals'))
+ ]
+ ]
+ [
+ (../PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Literal)
+ and
+ ( count(../PrimarySuffix/Arguments/ArgumentList/Expression) = 1 )
+ ]
+]
+[not(ancestor::Expression/ConditionalAndExpression//EqualityExpression[@Image='!=']//NullLiteral)]
+[not(ancestor::Expression/ConditionalOrExpression//EqualityExpression[@Image='==']//NullLiteral)]
+
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+class Foo {
+ boolean bar(String x) {
+ return x.equals("2"); // should be "2".equals(x)
+ }
+}
+
+]]>
+ </example>
+</rule>
+
+
+<rule name="PositionLiteralsFirstInCaseInsensitiveComparisons"
+ language="java"
+ since="5.1"
+ message="Position literals first in String comparisons for EqualsIgnoreCase"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#PositionLiteralsFirstInCaseInsensitiveComparisons">
+ <description>
+Position literals first in comparisons, if the second argument is null then NullPointerExceptions
+can be avoided, they will just return false.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//PrimaryExpression[
+ PrimaryPrefix[Name
+ [
+ (ends-with(@Image, '.equalsIgnoreCase'))
+ ]
+ ]
+ [
+ (../PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Literal)
+ and
+ ( count(../PrimarySuffix/Arguments/ArgumentList/Expression) = 1 )
+ ]
+]
+[not(ancestor::Expression/ConditionalAndExpression//EqualityExpression[@Image='!=']//NullLiteral)]
+[not(ancestor::Expression/ConditionalOrExpression//EqualityExpression[@Image='==']//NullLiteral)]
+
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+class Foo {
+ boolean bar(String x) {
+ return x.equalsIgnoreCase("2"); // should be "2".equalsIgnoreCase(x)
+ }
+}
+
+]]>
+ </example>
+</rule>
+
+
+ <rule name="UnnecessaryLocalBeforeReturn"
+ since="3.3"
+ message="Consider simply returning the value vs storing it in local variable ''{0}''"
+ class="net.sourceforge.pmd.lang.java.rule.design.UnnecessaryLocalBeforeReturnRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UnnecessaryLocalBeforeReturn">
+ <description>
+Avoid the creation of unnecessary local variables
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+public class Foo {
+ public int foo() {
+ int x = doSomething();
+ return x; // instead, just 'return doSomething();'
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="NonThreadSafeSingleton"
+ since="3.4"
+ message="Singleton is not thread safe"
+ class="net.sourceforge.pmd.lang.java.rule.design.NonThreadSafeSingletonRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#NonThreadSafeSingleton">
+ <description>
+Non-thread safe singletons can result in bad state changes. Eliminate
+static singletons if possible by instantiating the object directly. Static
+singletons are usually not needed as only a single instance exists anyway.
+Other possible fixes are to synchronize the entire method or to use an
+initialize-on-demand holder class (do not use the double-check idiom).
+
+See Effective Java, item 48.
+ </description>
+ <priority>3</priority>
+ <example><![CDATA[
+private static Foo foo = null;
+
+//multiple simultaneous callers may see partially initialized objects
+public static Foo getFoo() {
+ if (foo==null)
+ foo = new Foo();
+ return foo;
+}
+ ]]></example>
+ </rule>
+
+
+
+ <rule name="UncommentedEmptyMethod"
+ language="java"
+ since="3.4"
+ message="Document empty method"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UncommentedEmptyMethod">
+ <description>
+Uncommented Empty Method finds instances where a method does not contain
+statements, but there is no comment. By explicitly commenting empty methods
+it is easier to distinguish between intentional (commented) and unintentional
+empty methods.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//MethodDeclaration/Block[count(BlockStatement) = 0 and @containsComment = 'false']
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public void doSomething() {
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="UncommentedEmptyConstructor"
+ language="java"
+ since="3.4"
+ message="Document empty constructor"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UncommentedEmptyConstructor">
+ <description>
+Uncommented Empty Constructor finds instances where a constructor does not
+contain statements, but there is no comment. By explicitly commenting empty
+constructors it is easier to distinguish between intentional (commented)
+and unintentional empty constructors.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//ConstructorDeclaration[@Private='false'][count(BlockStatement) = 0 and ($ignoreExplicitConstructorInvocation = 'true' or not(ExplicitConstructorInvocation)) and @containsComment = 'false']
+ ]]>
+ </value>
+ </property>
+ <property name="ignoreExplicitConstructorInvocation" type="Boolean" description="Ignore explicit constructor invocation when deciding whether constructor is empty or not" value="false"/>
+ </properties>
+ <example>
+ <![CDATA[
+public Foo() {
+ // This constructor is intentionally empty. Nothing special is needed here.
+}
+ ]]>
+ </example>
+ </rule>
+
+<rule name="AvoidConstantsInterface"
+ language="java"
+ since="3.5"
+ message="An Interface should be used only to model a behaviour; consider converting this to a class."
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AvoidConstantsInterface">
+ <description>
+An interface should be used only to characterize the external behaviour of an
+implementing class: using an interface as a container of constants is a poor
+usage pattern and not recommended.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//ClassOrInterfaceDeclaration[@Interface="true"]
+ [
+ count(.//MethodDeclaration)=0
+ and
+ count(.//FieldDeclaration)>0
+ ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public interface ConstantsInterface {
+ public static final int CONSTANT1=0;
+ public static final String CONSTANT2="1";
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="UnsynchronizedStaticDateFormatter"
+ since="3.6"
+ message="Static DateFormatter objects should be accessed in a synchronized manner"
+ class="net.sourceforge.pmd.lang.java.rule.design.UnsynchronizedStaticDateFormatterRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UnsynchronizedStaticDateFormatter">
+ <description>
+SimpleDateFormat instances are not synchronized. Sun recommends using separate format instances
+for each thread. If multiple threads must access a static formatter, the formatter must be
+synchronized either on method or block level.
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+public class Foo {
+ private static final SimpleDateFormat sdf = new SimpleDateFormat();
+ void bar() {
+ sdf.format(); // poor, no thread-safety
+ }
+ synchronized void foo() {
+ sdf.format(); // preferred
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="PreserveStackTrace"
+ since="3.7"
+ message="New exception is thrown in catch block, original stack trace may be lost"
+ class="net.sourceforge.pmd.lang.java.rule.design.PreserveStackTraceRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#PreserveStackTrace">
+ <description>
+Throwing a new exception from a catch block without passing the original exception into the
+new exception will cause the original stack trace to be lost making it difficult to debug
+effectively.
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+public class Foo {
+ void good() {
+ try{
+ Integer.parseInt("a");
+ } catch (Exception e) {
+ throw new Exception(e); // first possibility to create exception chain
+ }
+ try {
+ Integer.parseInt("a");
+ } catch (Exception e) {
+ throw (IllegalStateException)new IllegalStateException().initCause(e); // second possibility to create exception chain.
+ }
+ }
+ void bad() {
+ try{
+ Integer.parseInt("a");
+ } catch (Exception e) {
+ throw new Exception(e.getMessage());
+ }
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="UseCollectionIsEmpty"
+ since="3.9"
+ message="Substitute calls to size() == 0 (or size() != 0) with calls to isEmpty()"
+ class="net.sourceforge.pmd.lang.java.rule.design.UseCollectionIsEmptyRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UseCollectionIsEmpty">
+ <description>
+The isEmpty() method on java.util.Collection is provided to determine if a collection has any elements.
+Comparing the value of size() to 0 does not convey intent as well as the isEmpty() method.
+ </description>
+ <priority>3</priority>
+ <example>
+ <![CDATA[
+public class Foo {
+ void good() {
+ List foo = getList();
+ if (foo.isEmpty()) {
+ // blah
+ }
+ }
+
+ void bad() {
+ List foo = getList();
+ if (foo.size() == 0) {
+ // blah
+ }
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="ClassWithOnlyPrivateConstructorsShouldBeFinal"
+ language="java"
+ since="4.1"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ message="A class which only has private constructors should be final"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#ClassWithOnlyPrivateConstructorsShouldBeFinal">
+ <description>
+A class with only private constructors should be final, unless the private constructor
+is invoked by a inner class.
+ </description>
+ <priority>1</priority>
+ <properties>
+ <property name="xpath">
+ <value><![CDATA[
+TypeDeclaration[count(../TypeDeclaration) = 1]/ClassOrInterfaceDeclaration
+[@Final = 'false']
+[count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[@Private = 'true']) >= 1 ]
+[count(./ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/ConstructorDeclaration[(@Public = 'true') or (@Protected = 'true') or (@PackagePrivate = 'true')]) = 0 ]
+[not(.//ClassOrInterfaceDeclaration)]
+ ]]></value>
+ </property>
+ </properties>
+ <example><![CDATA[
+public class Foo { //Should be final
+ private Foo() { }
+}
+ ]]></example>
+ </rule>
+
+
+ <rule name="EmptyMethodInAbstractClassShouldBeAbstract"
+ language="java"
+ since="4.1"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ message="An empty method in an abstract class should be abstract instead"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#EmptyMethodInAbstractClassShouldBeAbstract">
+ <description>
+Empty methods in an abstract class should be tagged as abstract. This helps to remove their inapproprate
+usage by developers who should be implementing their own versions in the concrete subclasses.
+ </description>
+ <priority>1</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+ //ClassOrInterfaceDeclaration[@Abstract = 'true']
+ /ClassOrInterfaceBody
+ /ClassOrInterfaceBodyDeclaration
+ /MethodDeclaration[@Abstract = 'false' and @Native = 'false']
+ [
+ ( boolean(./Block[count(./BlockStatement) = 1]/BlockStatement/Statement/ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral) = 'true' )
+ or
+ ( boolean(./Block[count(./BlockStatement) = 1]/BlockStatement/Statement/ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal[@Image = '0']) = 'true' )
+ or
+ ( boolean(./Block[count(./BlockStatement) = 1]/BlockStatement/Statement/ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal[string-length(@Image) = 2]) = 'true' )
+ or
+ (./Block[count(./BlockStatement) = 1]/BlockStatement/Statement/EmptyStatement)
+ or
+ ( count (./Block/*) = 0 )
+ ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public abstract class ShouldBeAbstract {
+ public Object couldBeAbstract() {
+ // Should be abstract method ?
+ return null;
+ }
+
+ public void couldBeAbstract() {
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="SingularField"
+ since="3.1"
+ message="Perhaps ''{0}'' could be replaced by a local variable."
+ class="net.sourceforge.pmd.lang.java.rule.design.SingularFieldRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#SingularField">
+ <description>
+ <![CDATA[
+Fields whose scopes are limited to just single methods do not rely on the containing
+object to provide them to other methods. They may be better implemented as local variables
+within those methods.
+ ]]>
+ </description>
+ <priority>3</priority>
+ <example><![CDATA[
+public class Foo {
+ private int x; // no reason to exist at the Foo instance level
+ public void foo(int y) {
+ x = y + 5;
+ return x;
+ }
+}
+ ]]></example>
+ </rule>
+
+ <rule name="ReturnEmptyArrayRatherThanNull"
+ language="java"
+ since="4.2"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ message="Return an empty array rather than 'null'."
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#ReturnEmptyArrayRatherThanNull">
+ <description>
+For any method that returns an array, it is a better to return an empty array rather than a
+null reference. This removes the need for null checking all results and avoids inadvertent
+NullPointerExceptions.
+ </description>
+ <priority>1</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+ //MethodDeclaration
+ [
+ (./ResultType/Type[@Array='true'])
+ and
+ (./Block/BlockStatement/Statement/ReturnStatement/Expression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral)
+ ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example><![CDATA[
+public class Example {
+ // Not a good idea...
+ public int[] badBehavior() {
+ // ...
+ return null;
+ }
+
+ // Good behavior
+ public String[] bonnePratique() {
+ //...
+ return new String[0];
+ }
+}
+ ]]></example>
+ </rule>
+
+ <rule name="AbstractClassWithoutAnyMethod"
+ language="java"
+ since="4.2"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ message="No abstract method which means that the keyword is most likely used to prevent instantiation. Use a private or protected constructor instead."
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AbstractClassWithoutAnyMethod">
+ <description>
+If an abstract class does not provides any methods, it may be acting as a simple data container
+that is not meant to be instantiated. In this case, it is probably better to use a private or
+protected constructor in order to prevent instantiation than make the class misleadingly abstract.
+ </description>
+ <priority>1</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//ClassOrInterfaceDeclaration[
+ (@Abstract = 'true')
+ and
+ (count(//MethodDeclaration) + count(//ConstructorDeclaration) = 0)
+]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class abstract Example {
+ String field;
+ int otherField;
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="TooFewBranchesForASwitchStatement"
+ language="java"
+ since="4.2"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ message="A switch with less than three branches is inefficient, use a 'if statement' instead."
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#TooFewBranchesForASwitchStatement">
+ <description>
+Switch statements are indended to be used to support complex branching behaviour. Using a switch for only a few
+cases is ill-advised, since switches are not as easy to understand as if-then statements. In these cases use the
+if-then statement to increase code readability.
+ </description>
+ <priority>1</priority>
+ <properties>
+ <property name="minimumNumberCaseForASwitch" type="Integer" description="Minimum number of branches for a switch" min="1" max="100" value="3"/>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//SwitchStatement[
+ (count(.//SwitchLabel) < $minimumNumberCaseForASwitch)
+]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+// With a minimumNumberCaseForASwitch of 3
+public class Foo {
+ public void bar() {
+ switch (condition) {
+ case ONE:
+ instruction;
+ break;
+ default:
+ break; // not enough for a 'switch' stmt, a simple 'if' stmt would have been more appropriate
+ }
+ }
+}
+ ]]>
+ </example>
+ </rule>
+
+
+ <rule name="LogicInversion"
+ language="java"
+ since="5.0"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ message="Use opposite operator instead of the logic complement operator."
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#LogicInversion">
+ <description>
+Use opposite operator instead of negating the whole expression with a logic complement operator.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//UnaryExpressionNotPlusMinus[@Image='!']/PrimaryExpression/PrimaryPrefix/Expression[EqualityExpression or RelationalExpression]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public boolean bar(int a, int b) {
+
+ if (!(a == b)) // use !=
+ return false;
+
+ if (!(a < b)) // use >=
+ return false;
+
+ return true;
+}
+ ]]>
+ </example>
+ </rule>
+
+ <rule name="UseVarargs"
+ language="java"
+ minimumLanguageVersion="1.5"
+ since="5.0"
+ message="Consider using varargs for methods or constructors which take an array the last parameter."
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#UseVarargs">
+ <description>
+Java 5 introduced the varargs parameter declaration for methods and constructors. This syntactic
+sugar provides flexibility for users of these methods and constructors, allowing them to avoid
+having to deal with the creation of an array.
+</description>
+ <priority>4</priority>
+ <properties>
+ <property name="xpath">
+ <value><![CDATA[
+//FormalParameters/FormalParameter[position()=last() and @Array='true' and @Varargs='false']
+ ]]></value>
+ </property>
+ </properties>
+ <example><![CDATA[
+public class Foo {
+ public void foo(String s, Object[] args) {
+ // Do something here...
+ }
+
+ public void bar(String s, Object... args) {
+ // Ahh, varargs tastes much better...
+ }
+}
+ ]]></example>
+ </rule>
+<!--
+We don't follow this practice, as we often prefer to keep like constants closest to where they are
+used.
+ <rule name="FieldDeclarationsShouldBeAtStartOfClass"
+ language="java"
+ since="5.0"
+ message="Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes."
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#FieldDeclarationsShouldBeAtStartOfClass">
+ <description>
+Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration/FieldDeclaration
+ [count(../preceding-sibling::ClassOrInterfaceBodyDeclaration/child::*[1]
+ [name() != 'FieldDeclaration' and name() != 'Annotation' and
+ (name() != 'EnumDeclaration' or $ignoreEnumDeclarations = 'false')]) > 0]
+ ]]>
+ </value>
+ </property>
+ <property name="ignoreEnumDeclarations" description="Ignore Enum Declarations that precede fields." type="Boolean" value="true"/>
+ </properties>
+ <example>
+ <![CDATA[
+public class HelloWorldBean {
+
+ // Field declared before methods / inner classes - OK
+ private String _thing;
+
+ public String getMessage() {
+ return "Hello World!";
+ }
+
+ // Field declared after methods / inner classes - avoid this
+ private String _fieldInWrongLocation;
+}
+ ]]>
+ </example>
+ </rule>
+-->
+<!--
+Unfortunately we have several large classes that trip this rule.
+TODO(wfarner): Break apart god classes.
+ <rule name="GodClass"
+ language="java"
+ since="5.0"
+ message="Possible God class"
+ class="net.sourceforge.pmd.lang.java.rule.design.GodClassRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#GodClass">
+ <description>
+The God Class rule detects the God Class design flaw using metrics. God classes do too many things,
+are very big and overly complex. They should be split apart to be more object-oriented.
+The rule uses the detection strategy described in "Object-Oriented Metrics in Practice".
+The violations are reported against the entire class. See also the references:
+Michele Lanza and Radu Marinescu. Object-Oriented Metrics in Practice:
+Using Software Metrics to Characterize, Evaluate, and Improve the Design
+of Object-Oriented Systems. Springer, Berlin, 1 edition, October 2006. Page 80.
+ </description>
+ <priority>3</priority>
+ </rule>
+-->
+ <rule name="AvoidProtectedMethodInFinalClassNotExtending"
+ language="java"
+ since="5.1"
+ message="Avoid protected methods in a final class that doesn't extend anything other than Object. Change to private or package access."
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/design.html#AvoidProtectedMethodInFinalClassNotExtending">
+ <description>
+Do not use protected methods in most final classes since they cannot be subclassed. This should
+only be allowed in final classes that extend other classes with protected methods (whose
+visibility cannot be reduced). Clarify your intent by using private or package access modifiers instead.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+<![CDATA[
+//ClassOrInterfaceDeclaration[@Final='true' and not(ExtendsList)]
+/ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration
+/MethodDeclaration[@Protected='true']
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+public final class Foo {
+ private int bar() {}
+ protected int baz() {} // Foo cannot be subclassed, and doesn't extend anything, so is baz() really private or package visible?
+}
+ ]]>
+ </example>
+ </rule>
+
+</ruleset>
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/config/pmd/logging-java.xml
----------------------------------------------------------------------
diff --git a/config/pmd/logging-java.xml b/config/pmd/logging-java.xml
new file mode 100644
index 0000000..e4ec6ad
--- /dev/null
+++ b/config/pmd/logging-java.xml
@@ -0,0 +1,183 @@
+<?xml version="1.0"?>
+<!--
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this work except in compliance with the License.
+You may obtain a copy of the License in the LICENSE file, or at:
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+<!--
+This is a modified file from the PMD project. Copying and editing ruleset
+configurations is the approach used to modify a ruleset behavior, such as
+disabling individul rules.
+-->
+
+<ruleset name="Java Logging"
+ xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
+
+ <description>
+The Java Logging ruleset contains a collection of rules that find questionable usages of the logger.
+ </description>
+
+ <rule name="MoreThanOneLogger"
+ since="2.0"
+ message="Class contains more than one logger."
+ class="net.sourceforge.pmd.lang.java.rule.logging.MoreThanOneLoggerRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/logging-java.html#MoreThanOneLogger">
+ <description>
+Normally only one logger is used in each class.
+ </description>
+ <priority>2</priority>
+ <example>
+ <![CDATA[
+public class Foo {
+ Logger log = Logger.getLogger(Foo.class.getName());
+ // It is very rare to see two loggers on a class, normally
+ // log information is multiplexed by levels
+ Logger log2= Logger.getLogger(Foo.class.getName());
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="LoggerIsNotStaticFinal"
+ language="java"
+ since="2.0"
+ message="The Logger variable declaration does not contain the static and final modifiers"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/logging-java.html#LoggerIsNotStaticFinal">
+ <description>
+In most cases, the Logger reference can be declared as static and final.
+ </description>
+ <priority>2</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//VariableDeclarator
+ [parent::FieldDeclaration]
+ [../Type/ReferenceType
+ /ClassOrInterfaceType[@Image='Logger']
+ and
+ (..[@Final='false'] or ..[@Static = 'false'] ) ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+public class Foo{
+ Logger log = Logger.getLogger(Foo.class.getName()); // not recommended
+
+ static final Logger log = Logger.getLogger(Foo.class.getName()); // preferred approach
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="SystemPrintln"
+ language="java"
+ since="2.1"
+ message="System.out.print is used"
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/logging-java.html#SystemPrintln">
+ <description>
+References to System.(out|err).print are usually intended for debugging purposes and can remain in
+the codebase even in production code. By using a logger one can enable/disable this behaviour at
+will (and by priority) and avoid clogging the Standard out log.
+ </description>
+ <priority>2</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+ <![CDATA[
+//Name[
+ starts-with(@Image, 'System.out.print')
+ or
+ starts-with(@Image, 'System.err.print')
+ ]
+ ]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+ <![CDATA[
+class Foo{
+ Logger log = Logger.getLogger(Foo.class.getName());
+ public void testA () {
+ System.out.println("Entering test");
+ // Better use this
+ log.fine("Entering test");
+ }
+}
+]]>
+ </example>
+ </rule>
+
+ <rule name="AvoidPrintStackTrace"
+ language="java"
+ since="3.2"
+ message="Avoid printStackTrace(); use a logger call instead."
+ class="net.sourceforge.pmd.lang.rule.XPathRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/logging-java.html#AvoidPrintStackTrace">
+ <description>
+Avoid printStackTrace(); use a logger call instead.
+ </description>
+ <priority>3</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+<![CDATA[
+//PrimaryExpression
+ [PrimaryPrefix/Name[contains(@Image,'printStackTrace')]]
+ [PrimarySuffix[not(boolean(Arguments/ArgumentList/Expression))]]
+]]>
+ </value>
+ </property>
+ </properties>
+ <example>
+<![CDATA[
+class Foo {
+ void bar() {
+ try {
+ // do something
+ } catch (Exception e) {
+ e.printStackTrace();
+ }
+ }
+}
+]]>
+ </example>
+ </rule>
+<!--
+In practice, we don't have high enough log volume for this rule to be useful.
+
+ <rule name="GuardLogStatementJavaUtil"
+ language="java"
+ since="5.1.0"
+ message="There is log block not surrounded by if"
+ class="net.sourceforge.pmd.lang.java.rule.logging.GuardLogStatementJavaUtilRule"
+ externalInfoUrl="http://pmd.sourceforge.net/pmd-5.1.1/rules/java/logging-java.html#GuardLogStatementJavaUtil">
+ <description>
+Whenever using a log level, one should check if the loglevel is actually enabled, or
+otherwise skip the associate String creation and manipulation.
+ </description>
+ <priority>2</priority>
+ <example>
+ <![CDATA[
+ // Add this for performance
+ if (log.isLoggable(Level.FINE)) { ...
+ log.fine("This happens");
+]]>
+ </example>
+ </rule>
+-->
+</ruleset>
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/auth/SessionValidator.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/auth/SessionValidator.java b/src/main/java/org/apache/aurora/auth/SessionValidator.java
index 6b0f423..eeebb78 100644
--- a/src/main/java/org/apache/aurora/auth/SessionValidator.java
+++ b/src/main/java/org/apache/aurora/auth/SessionValidator.java
@@ -60,7 +60,7 @@ public interface SessionValidator {
/**
* Thrown when authentication is not successful.
*/
- public static class AuthFailedException extends Exception {
+ class AuthFailedException extends Exception {
public AuthFailedException(String msg) {
super(msg);
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/Driver.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/Driver.java b/src/main/java/org/apache/aurora/scheduler/Driver.java
index 15e90d4..ffedfc3 100644
--- a/src/main/java/org/apache/aurora/scheduler/Driver.java
+++ b/src/main/java/org/apache/aurora/scheduler/Driver.java
@@ -76,7 +76,7 @@ public interface Driver {
/**
* Mesos driver implementation.
*/
- static class DriverImpl implements SettableDriver {
+ class DriverImpl implements SettableDriver {
private static final Logger LOG = Logger.getLogger(Driver.class.getName());
/**
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/DriverFactory.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/DriverFactory.java b/src/main/java/org/apache/aurora/scheduler/DriverFactory.java
index 549ef11..0f40112 100644
--- a/src/main/java/org/apache/aurora/scheduler/DriverFactory.java
+++ b/src/main/java/org/apache/aurora/scheduler/DriverFactory.java
@@ -48,7 +48,7 @@ import org.apache.mesos.SchedulerDriver;
*/
public interface DriverFactory extends Function<String, SchedulerDriver> {
- static class DriverFactoryImpl implements DriverFactory {
+ class DriverFactoryImpl implements DriverFactory {
private static final Logger LOG = Logger.getLogger(DriverFactory.class.getName());
@NotNull
@@ -136,11 +136,11 @@ public interface DriverFactory extends Function<String, SchedulerDriver> {
.setCheckpoint(REQUIRE_SLAVE_CHECKPOINT.get())
.setFailoverTimeout(FRAMEWORK_FAILOVER_TIMEOUT.get().as(Time.SECONDS));
- if (frameworkId != null) {
+ if (frameworkId == null) {
+ LOG.warning("Did not find a persisted framework ID, connecting as a new framework.");
+ } else {
LOG.info("Found persisted framework ID: " + frameworkId);
frameworkInfo.setId(FrameworkID.newBuilder().setValue(frameworkId));
- } else {
- LOG.warning("Did not find a persisted framework ID, connecting as a new framework.");
}
if (FRAMEWORK_AUTHENTICATION_FILE.hasAppliedValue()) {
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java b/src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java
index f05b05a..2b97198 100644
--- a/src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java
@@ -30,6 +30,7 @@ import org.apache.aurora.GuiceUtils.AllowUnchecked;
import org.apache.aurora.codec.ThriftBinaryCodec;
import org.apache.aurora.gen.ScheduleStatus;
import org.apache.aurora.gen.comm.SchedulerMessage;
+import org.apache.aurora.gen.comm.SchedulerMessage._Fields;
import org.apache.aurora.scheduler.base.Conversions;
import org.apache.aurora.scheduler.base.SchedulerException;
import org.apache.aurora.scheduler.events.EventSink;
@@ -180,7 +181,7 @@ class MesosSchedulerImpl implements Scheduler {
@Override
public void statusUpdate(SchedulerDriver driver, TaskStatus status) {
String info = status.hasData() ? status.getData().toStringUtf8() : null;
- String infoMsg = info != null ? " with info " + info : "";
+ String infoMsg = info == null ? "" : " with info " + info;
String coreMsg = status.hasMessage() ? " with core message " + status.getMessage() : "";
LOG.info("Received status update for task " + status.getTaskId().getValue()
+ " in state " + status.getState() + infoMsg + coreMsg);
@@ -236,20 +237,16 @@ class MesosSchedulerImpl implements Scheduler {
return;
}
- switch (schedulerMsg.getSetField()) {
- case DELETED_TASKS:
- for (String taskId : schedulerMsg.getDeletedTasks().getTaskIds()) {
- stateManager.changeState(
- taskId,
- Optional.<ScheduleStatus>absent(),
- ScheduleStatus.SANDBOX_DELETED,
- Optional.of("Sandbox disk space reclaimed."));
- }
- break;
-
- default:
- LOG.warning("Received unhandled scheduler message type: " + schedulerMsg.getSetField());
- break;
+ if (schedulerMsg.getSetField() == _Fields.DELETED_TASKS) {
+ for (String taskId : schedulerMsg.getDeletedTasks().getTaskIds()) {
+ stateManager.changeState(
+ taskId,
+ Optional.<ScheduleStatus>absent(),
+ ScheduleStatus.SANDBOX_DELETED,
+ Optional.of("Sandbox disk space reclaimed."));
+ }
+ } else {
+ LOG.warning("Received unhandled scheduler message type: " + schedulerMsg.getSetField());
}
} catch (ThriftBinaryCodec.CodingException e) {
LOG.log(Level.SEVERE, "Failed to decode framework message.", e);
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java b/src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java
index 06a12a1..f48ca51 100644
--- a/src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java
+++ b/src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java
@@ -61,7 +61,7 @@ public interface MesosTaskFactory {
*/
TaskInfo createFrom(IAssignedTask task, SlaveID slaveId) throws SchedulerException;
- static class ExecutorConfig {
+ class ExecutorConfig {
private final String executorPath;
public ExecutorConfig(String executorPath) {
@@ -73,7 +73,7 @@ public interface MesosTaskFactory {
}
}
- static class MesosTaskFactoryImpl implements MesosTaskFactory {
+ class MesosTaskFactoryImpl implements MesosTaskFactory {
private static final Logger LOG = Logger.getLogger(MesosTaskFactoryImpl.class.getName());
private static final String EXECUTOR_PREFIX = "thermos-";
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java b/src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java
index 5c0f09f..48b4404 100644
--- a/src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java
+++ b/src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java
@@ -147,10 +147,7 @@ public class SchedulerLifecycle implements EventSubscriber {
private final LeadingOptions leadingOptions;
private final ScheduledExecutorService executorService;
- private DefaultDelayedActions(
- LeadingOptions leadingOptions,
- ScheduledExecutorService executorService) {
-
+ DefaultDelayedActions(LeadingOptions leadingOptions, ScheduledExecutorService executorService) {
this.leadingOptions = checkNotNull(leadingOptions);
this.executorService = checkNotNull(executorService);
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java b/src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java
index e2492de..cf22e9c 100644
--- a/src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java
+++ b/src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java
@@ -104,13 +104,13 @@ public interface OfferQueue extends EventSubscriber {
* The delay is calculated for each offer that is received, so the return delay may be
* fixed or variable.
*/
- public interface OfferReturnDelay extends Supplier<Amount<Integer, Time>> {
+ interface OfferReturnDelay extends Supplier<Amount<Integer, Time>> {
}
/**
* Thrown when there was an unexpected failure trying to launch a task.
*/
- static class LaunchException extends Exception {
+ class LaunchException extends Exception {
LaunchException(String msg) {
super(msg);
}
@@ -151,7 +151,13 @@ public interface OfferQueue extends EventSubscriber {
// There's also a chance that we return an offer for compaction ~simultaneously with the
// same-host offer being canceled/returned. This is also fine.
Optional<HostOffer> sameSlave = hostOffers.get(offer.getSlaveId());
- if (!sameSlave.isPresent()) {
+ if (sameSlave.isPresent()) {
+ // If there are existing offers for the slave, decline all of them so the master can
+ // compact all of those offers into a single offer and send them back.
+ LOG.info("Returning offers for " + offer.getSlaveId().getValue() + " for compaction.");
+ decline(offer.getId());
+ removeAndDecline(sameSlave.get().offer.getId());
+ } else {
hostOffers.add(new HostOffer(offer, maintenance.getMode(offer.getHostname())));
executor.schedule(
new Runnable() {
@@ -162,12 +168,6 @@ public interface OfferQueue extends EventSubscriber {
},
returnDelay.get().as(Time.MILLISECONDS),
TimeUnit.MILLISECONDS);
- } else {
- // If there are existing offers for the slave, decline all of them so the master can
- // compact all of those offers into a single offer and send them back.
- LOG.info("Returning offers for " + offer.getSlaveId().getValue() + " for compaction.");
- decline(offer.getId());
- removeAndDecline(sameSlave.get().offer.getId());
}
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java b/src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java
index 17ddfc9..8a8e6e2 100644
--- a/src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java
+++ b/src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java
@@ -34,7 +34,6 @@ import com.twitter.common.quantity.Amount;
import com.twitter.common.quantity.Time;
import com.twitter.common.stats.Stats;
import com.twitter.common.util.BackoffStrategy;
-import com.twitter.common.util.Clock;
import com.twitter.common.util.concurrent.ExecutorServiceShutdown;
import org.apache.aurora.scheduler.base.AsyncUtil;
@@ -87,7 +86,6 @@ public class TaskGroups implements EventSubscriber {
ShutdownRegistry shutdownRegistry,
TaskGroupsSettings settings,
TaskScheduler taskScheduler,
- Clock clock,
RescheduleCalculator rescheduleCalculator) {
this(