diff --git a/pmd-java/src/main/resources/rulesets/java/design.xml b/pmd-java/src/main/resources/rulesets/java/design.xml index 56ac9b6fce..be85041ad9 100644 --- a/pmd-java/src/main/resources/rulesets/java/design.xml +++ b/pmd-java/src/main/resources/rulesets/java/design.xml @@ -11,18 +11,16 @@ are suggested. + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#useutilityclass"> - 3 @@ -35,12 +33,11 @@ public class MaybeAUtility { - + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#simplifybooleanreturns"> Avoid unnecessary if-then-else statements when returning a boolean. The result of the conditional test can be returned instead. @@ -49,28 +46,26 @@ the conditional test can be returned instead. - + Avoid unnecessary comparisons in boolean expressions, they serve no purpose and impacts readability. @@ -99,11 +94,11 @@ public class Bar { + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#switchstmtsshouldhavedefault"> All switch statements should include a default option to catch any unspecified values. @@ -124,7 +119,7 @@ public void bar() { switch (x) { case 1: int j = 6; case 2: int j = 8; - // missing default: here + // missing default: here } } ]]> @@ -132,10 +127,10 @@ public void bar() { + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#avoiddeeplynestedifstmts"> Avoid creating deeply nested if-then statements since they are harder to read and error-prone to maintain. @@ -157,12 +152,11 @@ public class Foo { - - + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#avoidreassigningparameters"> Reassigning values to incoming parameters is not recommended. Use temporary local variables instead. @@ -178,11 +172,11 @@ public class Foo { - + 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 @@ -208,11 +202,11 @@ public class Foo { - + Calling overridable methods during construction poses a risk of invoking methods on an incompletely constructed object and can be difficult to debug. @@ -247,11 +241,11 @@ public class JuniorClass extends SeniorClass { - + 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 @@ -274,12 +268,12 @@ public class Outer { - + If a final field is assigned to a compile-time constant, it could be made static, thus saving overhead in each object at runtime. @@ -308,10 +302,10 @@ public class Foo { + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#closeresource"> Ensure that resources (like Connection, Statement, and ResultSet objects) are always closed after use. @@ -335,12 +329,12 @@ public class Bar { - + 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 @@ -359,21 +353,21 @@ confusing. - + By convention, the default label should be the last label in a switch statement. @@ -406,12 +400,12 @@ public class Foo { - + 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. @@ -445,12 +439,12 @@ public class Foo { - + 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 @@ -468,7 +462,6 @@ PrimarySuffix/Arguments/ArgumentList/Expression /PrimaryExpression/PrimaryPrefix/AllocationExpression /ArrayDimsAndInits/Expression/PrimaryExpression/PrimaryPrefix/Literal[@Image='0'] ] - ]]> @@ -488,12 +481,12 @@ Foo[] fooArray = foos.toArray(new Foo[foos.size()]); - + 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. @@ -517,12 +510,12 @@ boolean x = (y == Double.NaN); - + Tests for null should not use the equals() method. The '==' operator should be used instead. @@ -552,28 +545,25 @@ Tests for null should not use the equals() method. The '==' operator should be u - + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#confusingternary"> 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(); +`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 @@ -583,18 +573,18 @@ as "does the error case go first?" or "does the common case go first?". - + Avoid instantiating an object just to call getClass() on it; use the .class public member instead. @@ -615,21 +605,20 @@ Avoid instantiating an object just to call getClass() on it; use the .class publ - + Avoid idempotent operations - they have no effect. @@ -647,13 +636,12 @@ public class Foo { - + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#simpledateformatneedslocale"> Be sure to specify a Locale when creating SimpleDateFormat instances to ensure that locale-appropriate formatting is used. @@ -680,11 +668,11 @@ public class Foo { - + 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. @@ -706,12 +694,12 @@ public class Foo { - + When doing String.toLowerCase()/toUpperCase() conversions, use Locales to avoids problems with languages that have unusual conventions, i.e. Turkish. @@ -741,28 +729,29 @@ PrimarySuffix - + Do not use protected fields in final classes since they cannot be subclassed. Clarify your intent by using private or package access modifiers instead. @@ -790,11 +779,11 @@ public final class Bar { - + Identifies a possible unsafe usage of a static field. @@ -811,12 +800,12 @@ public class StaticField { - + A class that has private constructors and does not have any static methods or fields cannot be used. @@ -883,13 +872,12 @@ public class Foo { - - + 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 @@ -931,12 +919,12 @@ public class Foo { - + 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. @@ -982,13 +970,12 @@ public void bar(int status) { - - + 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. @@ -1001,10 +988,11 @@ one is chosen. The thread chosen is arbitrary; thus its usually safer to call n //StatementExpression/PrimaryExpression [PrimarySuffix/Arguments[@ArgumentCount = '0']] [ -PrimaryPrefix[./Name[@Image='notify' or ends-with(@Image,'.notify')] -or ../PrimarySuffix/@Image='notify' -or (./AllocationExpression and ../PrimarySuffix[@Image='notify']) -] + PrimaryPrefix[ + ./Name[@Image='notify' or ends-with(@Image,'.notify')] + or ../PrimarySuffix/@Image='notify' + or (./AllocationExpression and ../PrimarySuffix[@Image='notify']) + ] ] ]]> @@ -1022,12 +1010,12 @@ or (./AllocationExpression and ../PrimarySuffix[@Image='notify']) - + Each caught exception type should be handled in its own catch clause. @@ -1049,27 +1037,28 @@ Each caught exception type should be handled in its own catch clause. - + 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 @@ -1101,12 +1090,12 @@ public abstract class Foo { - + No need to check for null before an instanceof; the instanceof keyword returns false when given a null argument. @@ -1155,11 +1144,11 @@ class Foo { - + Use equals() to compare object references; avoid comparing them with ==. @@ -1176,13 +1165,12 @@ class Foo { - - + Position literals first in comparisons, if the second argument is null then NullPointerExceptions can be avoided, they will just return false. @@ -1193,20 +1181,15 @@ can be avoided, they will just return false. @@ -1223,13 +1206,12 @@ class Foo { - - + since="5.1" + message="Position literals first in String comparisons for EqualsIgnoreCase" + class="net.sourceforge.pmd.lang.rule.XPathRule" + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#positionliteralsfirstincaseinsensitivecomparisons"> Position literals first in comparisons, if the second argument is null then NullPointerExceptions can be avoided, they will just return false. @@ -1271,11 +1253,11 @@ class Foo { - + Avoid the creation of unnecessary local variables @@ -1292,11 +1274,11 @@ public class Foo { - + Non-thread safe singletons can result in bad state changes. Eliminate static singletons if possible by instantiating the object directly. Static @@ -1324,66 +1306,67 @@ public static Foo getFoo() { ]]> - - + + Some classes contain overloaded getInstance. The problem with overloaded getInstance methods is that the instance created using the overloaded method is not cached and so, - for each call and new objects will be created for every invocation. - -2 - + 2 + - - + + - - + + Some classes contain overloaded getInstance. The problem with overloaded getInstance methods is that the instance created using the overloaded method is not cached and so, - for each call and new objects will be created for every invocation. - -2 - + 2 + - - + + - + Uncommented Empty Method Body finds instances where a method body does not contain statements, but there is no comment. By explicitly commenting empty method bodies @@ -1408,12 +1391,12 @@ public void doSomething() { - + Uncommented Empty Constructor finds instances where a constructor does not contain statements, but there is no comment. By explicitly commenting empty @@ -1441,10 +1424,10 @@ public Foo() { + since="3.6" + message="Static DateFormatter objects should be accessed in a synchronized manner" + class="net.sourceforge.pmd.lang.java.rule.design.UnsynchronizedStaticDateFormatterRule" + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#unsynchronizedstaticdateformatter"> 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 @@ -1467,10 +1450,10 @@ public class Foo { + 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="${pmd.website.baseurl}/pmd_rules_java_design.html#preservestacktrace"> 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 @@ -1504,11 +1487,11 @@ public class Foo { - + 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. @@ -1517,30 +1500,30 @@ Comparing the value of size() to 0 does not convey intent as well as the isEmpty - + A class with only private constructors should be final, unless the private constructor is invoked by a inner class. @@ -1565,12 +1548,12 @@ public class Foo { //Should be final - + Empty or auto-generated 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. @@ -1600,7 +1583,7 @@ usage by developers who should be implementing their own versions in the concret - - - +]]> + + - - - + 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. - ]]> 3 - + 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 @@ -1668,40 +1649,38 @@ NullPointerExceptions. public class Example { // Not a good idea... public int[] badBehavior() { - // ... + // ... return null; } // Good behavior public String[] bonnePratique() { - //... - return new String[0]; + //... + return new String[0]; } } ]]> - + 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. - + 1 @@ -1709,21 +1688,21 @@ protected constructor in order to prevent instantiation than make the class misl - + -Switch statements are indended to be used to support complex branching behaviour. Using a switch for only a few +Switch statements are intended 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. @@ -1758,16 +1737,15 @@ public class Foo { - - + Use opposite operator instead of negating the whole expression with a logic complement operator. - + 3 @@ -1796,13 +1774,13 @@ public boolean bar(int a, int b) { - + 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 @@ -1833,23 +1811,23 @@ having to deal with the creation of an array. + 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.java.rule.design.FieldDeclarationsShouldBeAtStartOfClassRule" + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#fielddeclarationsshouldbeatstartofclass"> Fields should be declared at the top of the class, before any method declarations, constructors, initializers or inner classes. @@ -1872,17 +1850,20 @@ public class HelloWorldBean { - + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#godclass"> 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: +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. @@ -1890,12 +1871,12 @@ of Object-Oriented Systems. Springer, Berlin, 1 edition, October 2006. Page 80. 3 - + 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 @@ -1923,12 +1904,12 @@ public final class Foo { - + Avoid constants in interfaces. Interfaces should define types, constants are implementation details better placed in classes or enums. See Effective Java, item 19. @@ -1948,9 +1929,9 @@ better placed in classes or enums. See Effective Java, item 19. - + When accessing a private field / method from another class, the Java compiler will generate a accessor methods with package-private visibility. This adds overhead, and to the dex method count on Android. This situation can diff --git a/pmd-java/src/main/resources/rulesets/java/empty.xml b/pmd-java/src/main/resources/rulesets/java/empty.xml index 4001dd6375..cf5789214f 100644 --- a/pmd-java/src/main/resources/rulesets/java/empty.xml +++ b/pmd-java/src/main/resources/rulesets/java/empty.xml @@ -9,12 +9,12 @@ The Empty Code ruleset contains rules that find empty statements of any kind (em empty block statement, empty try or catch block,...). - + Empty Catch Block finds instances where an exception is caught, but nothing is done. In most circumstances, this swallows an exception which should either be acted on @@ -40,22 +40,22 @@ or reported. - + Empty If Statement finds instances where a condition is checked but nothing is done about it. @@ -83,13 +83,12 @@ public class Foo { - - + Empty While Statement finds all instances where a while statement does nothing. If it is a timing loop, then you should use Thread.sleep() for it; if it is @@ -108,21 +107,21 @@ a while loop that does a lot in the exit expression, rewrite it to make it clear - + Avoid empty try blocks - what's the point? @@ -139,23 +138,23 @@ Avoid empty try blocks - what's the point? - + Empty finally blocks serve no purpose and should be removed. @@ -172,25 +171,25 @@ Empty finally blocks serve no purpose and should be removed. - + Empty switch statements serve no purpose and should be removed. @@ -207,22 +206,22 @@ Empty switch statements serve no purpose and should be removed. - + Empty synchronized blocks serve no purpose and should be removed. @@ -239,23 +238,23 @@ Empty synchronized blocks serve no purpose and should be removed. - + An empty statement (or a semicolon by itself) that is not used as the sole body of a 'for' or 'while' loop is probably a bug. It could also be a double semicolon, which has no purpose @@ -292,12 +291,12 @@ public void doit() { - + Empty initializers serve no purpose and should be removed. @@ -329,10 +328,10 @@ public class Foo { since="5.0" message="Avoid empty block statements." class="net.sourceforge.pmd.lang.rule.XPathRule" - externalInfoUrl="${pmd.website.baseurl}/rules/java/empty.html#EmptyStatementBlock"> + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_empty.html#emptystatementblock"> Empty block statements serve no purpose and should be removed. - + 3 @@ -359,12 +358,12 @@ public class Foo { - + An empty static initializer serve no purpose and should be removed. @@ -381,9 +380,9 @@ An empty static initializer serve no purpose and should be removed. diff --git a/pmd-java/src/main/resources/rulesets/java/finalizers.xml b/pmd-java/src/main/resources/rulesets/java/finalizers.xml index 21c607b252..fe2ed9e068 100644 --- a/pmd-java/src/main/resources/rulesets/java/finalizers.xml +++ b/pmd-java/src/main/resources/rulesets/java/finalizers.xml @@ -9,12 +9,12 @@ These rules deal with different problems that can occur with finalizers. - + Empty finalize methods serve no purpose and should be removed. @@ -38,12 +38,12 @@ public class Foo { - + If the finalize() is implemented, it should do something besides just calling super.finalize(). @@ -66,18 +66,18 @@ If the finalize() is implemented, it should do something besides just calling su - + Methods named finalize() should not have parameters. It is confusing and most likely an attempt to overload Object.finalize(). It will not be called by the VM. @@ -96,20 +96,20 @@ overload Object.finalize(). It will not be called by the VM. - + If the finalize() is implemented, its last action should be to call super.finalize. @@ -117,7 +117,7 @@ If the finalize() is implemented, its last action should be to call super.finali - - + When overriding the finalize(), the new method should be set as protected. If made public, other classes may invoke it at inappropriate times. @@ -173,17 +173,17 @@ other classes may invoke it at inappropriate times. - + The method Object.finalize() is called by the garbage collector on an object when garbage collection determines that there are no more references to the object. It should not be invoked by application logic. @@ -192,8 +192,8 @@ that there are no more references to the object. It should not be invoked by app diff --git a/pmd-java/src/main/resources/rulesets/java/imports.xml b/pmd-java/src/main/resources/rulesets/java/imports.xml index c2a949ca3c..db98a90186 100644 --- a/pmd-java/src/main/resources/rulesets/java/imports.xml +++ b/pmd-java/src/main/resources/rulesets/java/imports.xml @@ -10,10 +10,10 @@ These rules deal with different problems that can occur with import statements. + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_imports.html#duplicateimports"> Duplicate or overlapping import statements should be avoided. @@ -28,23 +28,23 @@ public class Foo {} + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_imports.html#dontimportjavalang"> Avoid importing anything from the package 'java.lang'. These classes are automatically imported (JLS 7.5.3). 4 @@ -52,10 +52,10 @@ public class Foo {} + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_imports.html#unusedimports"> Avoid the use of unused import statements to prevent unwanted dependencies. @@ -69,77 +69,77 @@ public class Foo {} - + There is no need to import a type that lives in the same package. 3 +package foo; + +import foo.Buz; // no need for this +import foo.*; // or this + +public class Bar{} +]]> - - + If you overuse the static import feature, it can make your program unreadable and unmaintainable, polluting its namespace with all the static members you import. Readers of your code (including you, a few months after you wrote it) will not know which class a static member comes from (Sun 1.5 Language Guide). - ]]> - 3 - - - - + 3 + + + + $maximumStaticImports] - ]]> - - - + + + - - - - + + + + Import statements allow the use of non-fully qualified names. The use of a fully qualified name which is covered by an import statement is redundant. Consider using the non-fully qualified name. - ]]> - 4 - + 4 + - +]]> + diff --git a/pmd-java/src/main/resources/rulesets/java/j2ee.xml b/pmd-java/src/main/resources/rulesets/java/j2ee.xml index 3ae9891d8d..bc37c57844 100644 --- a/pmd-java/src/main/resources/rulesets/java/j2ee.xml +++ b/pmd-java/src/main/resources/rulesets/java/j2ee.xml @@ -10,11 +10,11 @@ Rules specific to the use of J2EE implementations. + language="java" + since="3.7" + message="In J2EE, getClassLoader() might not work as expected. Use Thread.currentThread().getContextClassLoader() instead." + class="net.sourceforge.pmd.lang.rule.XPathRule" + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_j2ee.html#useproperclassloader"> In J2EE, the getClassLoader() method might not work as expected. Use Thread.currentThread().getContextClassLoader() instead. @@ -32,19 +32,18 @@ Thread.currentThread().getContextClassLoader() instead. - + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_j2ee.html#mdbandsessionbeannamingconvention"> The EJB Specification states that any MessageDrivenBean or SessionBean should be suffixed by 'Bean'. @@ -70,20 +69,19 @@ The EJB Specification states that any MessageDrivenBean or SessionBean should be - + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_j2ee.html#remotesessioninterfacenamingconvention"> A Remote Home interface type of a Session EJB should be suffixed by 'Home'. @@ -109,20 +107,19 @@ A Remote Home interface type of a Session EJB should be suffixed by 'Home'. - + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_j2ee.html#localinterfacesessionnamingconvention"> The Local Interface of a Session EJB should be suffixed by 'Local'. @@ -148,20 +145,19 @@ The Local Interface of a Session EJB should be suffixed by 'Local'. - + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_j2ee.html#localhomenamingconvention"> The Local Home interface of a Session EJB should be suffixed by 'LocalHome'. @@ -187,20 +183,19 @@ The Local Home interface of a Session EJB should be suffixed by 'LocalHome'. - + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_j2ee.html#remoteinterfacenamingconvention"> Remote Interface of a Session EJB should not have a suffix. @@ -229,42 +224,41 @@ Remote Interface of a Session EJB should not have a suffix. - - + + Web applications should not call System.exit(), since only the web container or the application server should stop the JVM. This rule also checks for the equivalent call Runtime.getRuntime().exit(). - - 3 - - - + 3 + + + - - - - + + + - - + + According to the J2EE specification, an EJB should not have any static fields with write access. However, static read-only fields are allowed. This ensures proper behavior especially when instances are distributed by the container on several JREs. - - 3 - - - + 3 + + + - - - + + + - - - + The J2EE specification explicitly forbids the use of threads. - ]]> - - 3 - - - - - - - - - + 3 + + + + + + + - - + +// Neither this, +public class OtherThread implements Runnable { + // Nor this ... + public void methode() { + Runnable thread = new Thread(); thread.run(); + } +} +]]> + + diff --git a/pmd-java/src/main/resources/rulesets/java/javabeans.xml b/pmd-java/src/main/resources/rulesets/java/javabeans.xml index 00bbf34e69..6e7cf7c840 100644 --- a/pmd-java/src/main/resources/rulesets/java/javabeans.xml +++ b/pmd-java/src/main/resources/rulesets/java/javabeans.xml @@ -11,17 +11,17 @@ The JavaBeans Ruleset catches instances of bean rules not being followed. + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_javabeans.html#beanmembersshouldserialize"> If a class is a bean, or is referenced by a bean directly or indirectly it needs to be serializable. Member variables need to be marked as transient, static, or have accessor methods in the class. Marking variables as transient is the safest and easiest modification. Accessor methods should follow the Java naming conventions, i.e. for a variable named foo, getFoo() and setFoo() accessor methods should be provided. - 3 + 3 - + Serializable classes should provide a serialVersionUID field. @@ -73,13 +73,12 @@ and - - + + diff --git a/pmd-java/src/main/resources/rulesets/java/junit.xml b/pmd-java/src/main/resources/rulesets/java/junit.xml index 11d85d1779..8529e3975a 100644 --- a/pmd-java/src/main/resources/rulesets/java/junit.xml +++ b/pmd-java/src/main/resources/rulesets/java/junit.xml @@ -8,12 +8,12 @@ These rules deal with different problems that can occur with JUnit tests. - + The suite() method in a JUnit test needs to be both public and static. @@ -35,19 +35,19 @@ The suite() method in a JUnit test needs to be both public and static. import junit.framework.*; public class Foo extends TestCase { - public void suite() {} // oops, should be static - private static void suite() {} // oops, should be public + public void suite() {} // oops, should be static + private static void suite() {} // oops, should be public } ]]> + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_junit.html#junitspelling"> Some JUnit framework methods are easy to misspell. @@ -71,18 +71,18 @@ Some JUnit framework methods are easy to misspell. import junit.framework.*; public class Foo extends TestCase { - public void setup() {} // oops, should be setUp - public void TearDown() {} // oops, should be tearDown + public void setup() {} // oops, should be setUp + public void TearDown() {} // oops, should be tearDown } ]]> - + JUnit assertions should include an informative message - i.e., use the three-argument version of assertEquals(), not the two-argument version. @@ -91,22 +91,22 @@ assertEquals(), not the two-argument version. - + JUnit tests should include at least one assertion. This makes the tests more robust, and using assert with messages provide the developer a clearer idea of what the test does. @@ -126,11 +126,11 @@ public class Foo extends TestCase { - + Test classes end with the suffix Test. Having a non-test class with that name is not a good practice, since most people will assume it is a test case. Test classes have test methods named testXXX. @@ -150,12 +150,12 @@ public class CarTest { - + A JUnit test assertion with a boolean literal is unnecessary since it always will evaluate to the same thing. Consider using flow control (in case of assertTrue(false) or similar) or simply removing @@ -185,20 +185,20 @@ UnaryExpressionNotPlusMinus[@Image='!'] - + This rule detects JUnit assertions in object equality. These assertions should be made by more specific methods, like assertEquals. @@ -221,22 +221,22 @@ This rule detects JUnit assertions in object equality. These assertions should b - + This rule detects JUnit assertions in object references equality. These assertions should be made by more specific methods, like assertSame, assertNotSame. @@ -261,22 +261,22 @@ by more specific methods, like assertSame, assertNotSame. - + This rule detects JUnit assertions in object references equality. These assertions should be made by more specific methods, like assertNull, assertNotNull. @@ -300,35 +300,36 @@ more specific methods, like assertNull, assertNotNull. +public class FooTest extends TestCase { + void testCode() { + Object a = doSomething(); + assertTrue(a==null); // bad usage + assertNull(a); // good usage + assertTrue(a != null); // bad usage + assertNotNull(a); // good usage + } +} +]]> - + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_junit.html#simplifybooleanassertion"> Avoid negation in an assertTrue or assertFalse test. For example, rephrase: - assertTrue(!expr); - + assertTrue(!expr); + as: - assertFalse(expr); + assertFalse(expr); + 3 @@ -351,70 +352,68 @@ PrimaryExpression/PrimarySuffix/Arguments/ArgumentList - - + + + JUnit tests should not contain too many asserts. Many asserts are indicative of a complex test, for which it is harder to verify correctness. Consider breaking the test scenario into multiple, shorter test scenarios. Customize the maximum number of assertions used by this Rule to suit your needs. - - 3 - - - - - + 3 + + + + $maximumAsserts] -]]> - - - - + ]]> + + + - - - - + + + + + When asserting a value is the same as a literal or Boxed boolean, use assertTrue/assertFalse, instead of assertEquals. - - 3 - - - - + 3 + + + - - - - + ]]> + + + - - + + + diff --git a/pmd-java/src/main/resources/rulesets/java/logging-jakarta-commons.xml b/pmd-java/src/main/resources/rulesets/java/logging-jakarta-commons.xml index 5b3a6dde9e..a5e0eae2bf 100644 --- a/pmd-java/src/main/resources/rulesets/java/logging-jakarta-commons.xml +++ b/pmd-java/src/main/resources/rulesets/java/logging-jakarta-commons.xml @@ -8,19 +8,19 @@ The Jakarta Commons Logging ruleset contains a collection of rules that find questionable usages of that framework. - - + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_logging-jakarta-commons.html#usecorrectexceptionlogging"> + To make sure the full stacktrace is printed out, use the logging statement with two arguments: a String and a Throwable. - - 3 - - - + 3 + + + - - - + + + - + - - + + A logger should normally be defined private static final and be associated with the correct class. Private final Log log; is also allowed for rare cases where loggers need to be passed around, with the restriction that the logger needs to be passed into the constructor. - - 3 - - - - + 3 + + + - - - - - + ]]> + + + + - - + + - - - When log messages are composed by concatenating strings, the whole section should be guarded - by a isDebugEnabled() check to avoid performance and memory issues. - - 3 - - + +When log messages are composed by concatenating strings, the whole section should be guarded +by a isDebugEnabled() check to avoid performance and memory issues. + + 3 + + - - + + - - + + Whenever using a log level, one should check if the loglevel is actually enabled, or otherwise skip the associate String creation and manipulation. - - 2 - + + 2 + - - + + diff --git a/pmd-java/src/main/resources/rulesets/java/logging-java.xml b/pmd-java/src/main/resources/rulesets/java/logging-java.xml index 1d5bcb0539..c842516b61 100644 --- a/pmd-java/src/main/resources/rulesets/java/logging-java.xml +++ b/pmd-java/src/main/resources/rulesets/java/logging-java.xml @@ -9,16 +9,16 @@ The Java Logging ruleset contains a collection of rules that find questionable usages of the logger. - - + + Normally only one logger is used in each class. - - 2 - + + 2 + - - + + - + In most cases, the Logger reference can be declared as static and final. @@ -57,20 +57,20 @@ In most cases, the Logger reference can be declared as static and final. - - + + - + 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 @@ -101,81 +101,84 @@ class Foo{ } } ]]> - - + + - - + + Avoid printStackTrace(); use a logger call instead. - - 3 - - - + + 3 + + + - - - - + + + + - - + + - - + + Whenever using a log level, one should check if the loglevel is actually enabled, or otherwise skip the associate String creation and manipulation. - - 2 - + + 2 + - - + + - - + + Check for messages in slf4j loggers with non matching number of arguments and placeholders. - - 5 - - + 5 + + - - + + +