phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ProperLogger - make class-name configurable and requirement of static-final logger (deprecate LoggerIsNotStaticFinal)

Browse Source
This commit is contained in:
Ivo Smid
2019-04-22 14:55:15 +02:00
parent 601a6be306
commit ccf99a4dc3
4 changed files with 199 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files

82
pmd-java/src/main/resources/category/java/errorprone.xml
View File

@ -190,7 +190,7 @@ for (int i = 0; i < 10; i++) {
<description>
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.
Note that Oracle has declared Object.finalize() as deprecated since JDK 9.
</description>
<priority>3</priority>
@ -211,7 +211,7 @@ void foo() {
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#avoidcatchingnpe">
<description>
Code should never throw NullPointerExceptions under normal circumstances. A catch block may hide the
Code should never throw NullPointerExceptions under normal circumstances. A catch block may hide the
original error, causing other, more subtle problems later on.
</description>
<priority>3</priority>
@ -245,7 +245,7 @@ public class Foo {
class="net.sourceforge.pmd.lang.java.rule.errorprone.AvoidCatchingThrowableRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#avoidcatchingthrowable">
<description>
Catching Throwable errors is not recommended since its scope is very broad. It includes runtime issues such as
Catching Throwable errors is not recommended since its scope is very broad. It includes runtime issues such as
OutOfMemoryError that should be exposed and managed separately.
</description>
<priority>3</priority>
@ -372,8 +372,8 @@ public class A {
class="net.sourceforge.pmd.lang.java.rule.errorprone.AvoidFieldNameMatchingMethodNameRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#avoidfieldnamematchingmethodname">
<description>
It can be confusing to have a field name with the same name as a method. While this is permitted,
having information (field) and actions (method) is not clear naming. Developers versed in
It can be confusing to have a field name with the same name as a method. While this is permitted,
having information (field) and actions (method) is not clear naming. Developers versed in
Smalltalk often prefer this approach as the methods denote accessor methods.
</description>
<priority>3</priority>
@ -634,9 +634,9 @@ boolean x = (y == Double.NaN);
class="net.sourceforge.pmd.lang.java.rule.errorprone.BeanMembersShouldSerializeRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#beanmembersshouldserialize">
<description>
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
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.
</description>
<priority>3</priority>
@ -1324,7 +1324,7 @@ public class MyActivity extends Activity {
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#donotthrowexceptioninfinally">
<description>
Throwing exceptions within a 'finally' block is confusing since they may mask other exceptions
Throwing exceptions within a 'finally' block is confusing since they may mask other exceptions
or code defects.
Note: This is a PMD implementation of the Lint4j rule "A throw in a finally block"
</description>
@ -1415,8 +1415,8 @@ public class Count {
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#emptycatchblock">
<description>
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
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
or reported.
</description>
<priority>3</priority>
@ -1611,7 +1611,7 @@ public class Foo {
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#emptystatementnotinloop">
<description>
An empty statement (or a semicolon by itself) that is not used as the sole body of a 'for'
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
and should be removed.
</description>
@ -1742,7 +1742,7 @@ public class Foo {
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#emptywhilestmt">
<description>
Empty While Statement finds all instances where a while statement does nothing.
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
a while loop that does a lot in the exit expression, rewrite it to make it clearer.
</description>
@ -1900,7 +1900,7 @@ protected void finalize() {
<description>
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.
Note that Oracle has declared Object.finalize() as deprecated since JDK 9.
</description>
<priority>3</priority>
@ -1932,9 +1932,9 @@ public class Foo {
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#finalizeshouldbeprotected">
<description>
When overriding the finalize(), the new method should be set as protected. If made public,
When overriding the finalize(), the new method should be set as protected. If made public,
other classes may invoke it at inappropriate times.
Note that Oracle has declared Object.finalize() as deprecated since JDK 9.
</description>
<priority>3</priority>
@ -2180,9 +2180,12 @@ public class Foo extends TestCase {
since="2.0"
message="The Logger variable declaration does not contain the static and final modifiers"
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#loggerisnotstaticfinal">
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#loggerisnotstaticfinal"
deprecated="true">
<description>
In most cases, the Logger reference can be declared as static and final.
This rule is deprecated. The rule is replaced by {% rule java/errorprone/ProperLogger %}.
</description>
<priority>2</priority>
<properties>
@ -2625,7 +2628,8 @@ class Foo{
since="3.3"
message="Logger should be defined private static final and have the correct class"
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#properlogger">
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#properlogger"
typeResolution="true">
<description>
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,
@ -2633,23 +2637,37 @@ with the restriction that the logger needs to be passed into the constructor.
</description>
<priority>3</priority>
<properties>
<property name="version" value="2.0"/>
<property name="xpath">
<value>
<![CDATA[
//ClassOrInterfaceBodyDeclaration[FieldDeclaration//ClassOrInterfaceType[@Image='Log']
and
not(FieldDeclaration[@Final='true'][@Static='true'][@Private='true'][.//VariableDeclaratorId[@Image=$staticLoggerName]]
and
//ArgumentList//ClassOrInterfaceType[@Image = ancestor::ClassOrInterfaceDeclaration/@Image or @Image = ancestor::EnumDeclaration/@Image])
and
not(FieldDeclaration[@Final='true'][@Private='true'][.//VariableDeclaratorId[@Image='log']]
[count(.//VariableInitializer)=0]
[ancestor::ClassOrInterfaceBody//StatementExpression[.//PrimaryExpression/descendant::*[@Image='log']][count(.//AllocationExpression)=0]]
)]
//ClassOrInterfaceBodyDeclaration[
FieldDeclaration//ClassOrInterfaceType[pmd-java:typeIs($loggerClass)]
and not (
(: check logger name :)
(
FieldDeclaration[$requireStaticFinalLogger][@Final=true()][@Static=true()][.//VariableDeclaratorId[@Image=$loggerName or @Image=$staticLoggerName]]
|
FieldDeclaration[$requireStaticFinalLogger = false()][.//VariableDeclaratorId[@Image=$loggerName or @Image=$staticLoggerName]]
)
and
(: in place initialization with method argument pointing to current class (even for Enums) :)
.//ArgumentList//ClassOrInterfaceType[@Image = ancestor::ClassOrInterfaceDeclaration/@Image or @Image = ancestor::EnumDeclaration/@Image]
)
and not (
(: final logger initialized inside constructor :)
FieldDeclaration[@Final=true()][@Private=true()][.//VariableDeclaratorId[@Image=$loggerName or @Image=$staticLoggerName]]
[count(.//VariableInitializer)=0]
[ancestor::ClassOrInterfaceBody//StatementExpression[.//PrimaryExpression/descendant::*[@Image=$loggerName or @Image=$staticLoggerName]][count(.//AllocationExpression)=0]]
)
]
]]>
</value>
</property>
<property name="staticLoggerName" type="String" description="Name of the static Logger variable" value="LOG"/>
<property name="staticLoggerName" type="String" description="deprecated!(Use 'loggerName' property) Name of the static Logger variable" value="LOG"/>
<property name="loggerName" type="String" description="Name of the Logger variable" value="LOG"/>
<property name="loggerClass" type="String" description="Class name of the logger" value="Log"/>
<property name="requireStaticFinalLogger" type="Boolean" description="Static final logger is required" value="false"/>
</properties>
<example>
<![CDATA[
@ -2896,7 +2914,7 @@ new StringBuffer() // 16
new StringBuffer(6) // 6
new StringBuffer("hello world") // 11 + 16 = 27
new StringBuffer('A') // chr(A) = 65
new StringBuffer("A") // 1 + 16 = 17
new StringBuffer("A") // 1 + 16 = 17
new StringBuilder() // 16
new StringBuilder(6) // 6
@ -3044,7 +3062,7 @@ public void foo() {
class="net.sourceforge.pmd.lang.java.rule.errorprone.TestClassWithoutTestCasesRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#testclasswithouttestcases">
<description>
Test classes end with the suffix Test. Having a non-test class with that name is not a good practice,
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.
</description>
<priority>3</priority>
@ -3415,7 +3433,7 @@ class Foo {
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#useproperclassloader">
<description>
In J2EE, the getClassLoader() method might not work as expected. Use
In J2EE, the getClassLoader() method might not work as expected. Use
Thread.currentThread().getContextClassLoader() instead.
</description>
<priority>3</priority>

101
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/ProperLogger.xml
View File

@ -29,6 +29,7 @@ public class Foo {
<description><![CDATA[
Ok, special case
]]></description>
<rule-property name="loggerName">log</rule-property>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
@ -55,18 +56,19 @@ public enum Foo {
<description><![CDATA[
bug 1626232, a separate variable initialization should not confuse the rule
]]></description>
<rule-property name="loggerName">log</rule-property>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
private String bar = "";
private final Log log;
Foo(Log log) {
this.log = log;
Foo(Log log1) {
this.log = log1;
}
}
]]> </code>
</test-code>
<test-code>
<test-code>
<description><![CDATA[
Enums shoud be accepted for declaration
]]></description>
@ -87,6 +89,99 @@ bug 1626232, extra loggers with different names are not allowed by default (use
public class Foo {
private static final Log LOG = LogFactory.getLog(Foo.class);
private static final Log LOG2 = LogFactory.getLog(Foo.class + ".foo");
}
]]> </code>
</test-code>
<test-code>
<description><![CDATA[
Custom logger-class with invalid logger variable name
]]></description>
<rule-property name="loggerClass">MyLog</rule-property>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
private static final MyLog INVALID_LOG_NAME = LogFactory.getLog(Foo.class);
}
]]> </code>
</test-code>
<test-code>
<description><![CDATA[
Public logger
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public static final Log LOG = LogFactory.getLog(Foo.class);
}
]]> </code>
</test-code>
<test-code>
<description><![CDATA[
Public logger when static final required
]]></description>
<rule-property name="requireStaticFinalLogger">true</rule-property>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public static final Log LOG = LogFactory.getLog(Foo.class);
}
]]> </code>
</test-code>
<test-code>
<description><![CDATA[
Logger initialized as String constant from class name
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
Log LOG = LogFactory.getLog(Foo.class.getName());
}
]]> </code>
</test-code>
<test-code>
<description><![CDATA[
Package-protected logger when static final is not required
]]></description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
Log LOG = LogFactory.getLog(Foo.class);
}
]]> </code>
</test-code>
<test-code>
<description><![CDATA[
Package-protected logger when static final is required
]]></description>
<rule-property name="requireStaticFinalLogger">true</rule-property>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
Log LOG = LogFactory.getLog(Foo.class);
}
]]> </code>
</test-code>
<test-code>
<description><![CDATA[
Check type resolution of logger
]]></description>
<rule-property name="loggerClass">org.slf4j.Logger</rule-property>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
org.slf4j.Logger LOG = org.slf4j.LoggerFactory.getLogger(Foo.class);
}
]]> </code>
</test-code>
<test-code>
<description><![CDATA[
Check type resolution of logger with invalid logger specified
]]></description>
<rule-property name="loggerClass">org.slf4j.Logger</rule-property>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
org.slf4j.Logger LOG = org.slf4j.LoggerFactory.getLogger(Bar.class);
}
]]> </code>
</test-code>