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

[java] Update rule NonThreadSafeSingleton

Browse Source
This commit is contained in:
Andreas Dangel committed 2021-08-19 12:36:15 +02:00
1 parent 2242badb92
commit 72b7428578
4 files changed
+89 -77
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

.ci/files/all-java.xml
+1 -1
View File
@@ -281,7 +281,7 @@
<rule ref="category/java/multithreading.xml/DoNotUseThreads"/>
<rule ref="category/java/multithreading.xml/DontCallThreadRun"/>
<rule ref="category/java/multithreading.xml/DoubleCheckedLocking"/>
<!-- <rule ref="category/java/multithreading.xml/NonThreadSafeSingleton"/> -->
<rule ref="category/java/multithreading.xml/NonThreadSafeSingleton"/>
<!-- <rule ref="category/java/multithreading.xml/UnsynchronizedStaticFormatter"/> -->
<rule ref="category/java/multithreading.xml/UseConcurrentHashMap"/>
<rule ref="category/java/multithreading.xml/UseNotifyAllInsteadOfNotify"/>
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/multithreading/NonThreadSafeSingletonRule.java
+39 -52
View File
@@ -10,28 +10,24 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator;
import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr;
import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
import net.sourceforge.pmd.lang.java.ast.ASTAssignmentExpression;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTNullLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression;
import net.sourceforge.pmd.lang.java.ast.ASTSynchronizedStatement;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.ast.JModifier;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.java.symbols.JFieldSymbol;
import net.sourceforge.pmd.lang.java.symbols.JVariableSymbol;
import net.sourceforge.pmd.properties.PropertyDescriptor;
public class NonThreadSafeSingletonRule extends AbstractJavaRule {
private Map<String, ASTFieldDeclaration> fieldDecls = new HashMap<>();
private boolean checkNonStaticMethods = true;
private boolean checkNonStaticFields = true;
public class NonThreadSafeSingletonRule extends AbstractJavaRulechainRule {
private static final PropertyDescriptor<Boolean> CHECK_NON_STATIC_METHODS_DESCRIPTOR =
booleanProperty("checkNonStaticMethods")
@@ -42,70 +38,61 @@ public class NonThreadSafeSingletonRule extends AbstractJavaRule {
.desc("Check for non-static fields. Do not set this to true and checkNonStaticMethods to false.")
.defaultValue(false).build();
private Map<String, ASTFieldDeclaration> fieldDecls = new HashMap<>();
private boolean checkNonStaticMethods = true;
private boolean checkNonStaticFields = true;
public NonThreadSafeSingletonRule() {
super(ASTFieldDeclaration.class, ASTMethodDeclaration.class);
definePropertyDescriptor(CHECK_NON_STATIC_METHODS_DESCRIPTOR);
definePropertyDescriptor(CHECK_NON_STATIC_FIELDS_DESCRIPTOR);
}
@Override
public Object visit(ASTCompilationUnit node, Object data) {
public void start(RuleContext ctx) {
fieldDecls.clear();
checkNonStaticMethods = getProperty(CHECK_NON_STATIC_METHODS_DESCRIPTOR);
checkNonStaticFields = getProperty(CHECK_NON_STATIC_FIELDS_DESCRIPTOR);
return super.visit(node, data);
}
@Override
public Object visit(ASTFieldDeclaration node, Object data) {
if (checkNonStaticFields || node.isStatic()) {
fieldDecls.put(node.getVariableName(), node);
if (checkNonStaticFields || node.hasModifiers(JModifier.STATIC)) {
for (ASTVariableDeclaratorId varId : node.getVarIds()) {
fieldDecls.put(varId.getName(), node);
}
}
return super.visit(node, data);
return data;
}
@Override
public Object visit(ASTMethodDeclaration node, Object data) {
if (checkNonStaticMethods && !node.isStatic() || node.isSynchronized()) {
return super.visit(node, data);
if (checkNonStaticMethods && !node.hasModifiers(JModifier.STATIC) || node.hasModifiers(JModifier.SYNCHRONIZED)) {
return data;
}
List<ASTIfStatement> ifStatements = node.findDescendantsOfType(ASTIfStatement.class);
List<ASTIfStatement> ifStatements = node.descendants(ASTIfStatement.class).toList();
for (ASTIfStatement ifStatement : ifStatements) {
if (ifStatement.getFirstParentOfType(ASTSynchronizedStatement.class) == null) {
if (!ifStatement.hasDescendantOfType(ASTNullLiteral.class)) {
if (ifStatement.ancestors(ASTSynchronizedStatement.class).isEmpty()) {
if (ifStatement.getCondition().descendants(ASTNullLiteral.class).isEmpty()) {
continue;
}
ASTName n = ifStatement.getFirstDescendantOfType(ASTName.class);
if (n == null || !fieldDecls.containsKey(n.getImage())) {
ASTNamedReferenceExpr n = ifStatement.getCondition().descendants(ASTNamedReferenceExpr.class).first();
if (n == null || !fieldDecls.containsKey(n.getName())) {
continue;
}
List<ASTAssignmentOperator> assignments = ifStatement
.findDescendantsOfType(ASTAssignmentOperator.class);
List<ASTAssignmentExpression> assignments = ifStatement.descendants(ASTAssignmentExpression.class).toList();
boolean violation = false;
for (int ix = 0; ix < assignments.size(); ix++) {
ASTAssignmentOperator oper = assignments.get(ix);
if (!(oper.getParent() instanceof ASTStatementExpression)) {
continue;
}
ASTStatementExpression expr = (ASTStatementExpression) oper.getParent();
if (expr.getChild(0) instanceof ASTPrimaryExpression
&& ((ASTPrimaryExpression) expr.getChild(0)).getNumChildren() == 1
&& ((ASTPrimaryExpression) expr.getChild(0))
.getChild(0) instanceof ASTPrimaryPrefix) {
ASTPrimaryPrefix pp = (ASTPrimaryPrefix) ((ASTPrimaryExpression) expr.getChild(0))
.getChild(0);
String name = null;
if (pp.usesThisModifier()) {
ASTPrimarySuffix priSuf = expr.getFirstDescendantOfType(ASTPrimarySuffix.class);
name = priSuf.getImage();
} else {
ASTName astName = (ASTName) pp.getChild(0);
name = astName.getImage();
}
if (fieldDecls.containsKey(name)) {
violation = true;
for (ASTAssignmentExpression assignment : assignments) {
ASTAssignableExpr left = assignment.getLeftOperand();
if (left instanceof ASTNamedReferenceExpr) {
JVariableSymbol referencedSym = ((ASTNamedReferenceExpr) left).getReferencedSym();
if (referencedSym instanceof JFieldSymbol) {
String name = ((ASTNamedReferenceExpr) left).getName();
if (fieldDecls.containsKey(name)) {
violation = true;
}
}
}
}
@@ -114,6 +101,6 @@ public class NonThreadSafeSingletonRule extends AbstractJavaRule {
}
}
}
return super.visit(node, data);
return data;
}
}
pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/multithreading/NonThreadSafeSingletonTest.java
-1
View File
@@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.multithreading;
import net.sourceforge.pmd.testframework.PmdRuleTst;
@org.junit.Ignore("Rule has not been updated yet")
public class NonThreadSafeSingletonTest extends PmdRuleTst {
// no additional unit tests
}
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/multithreading/xml/NonThreadSafeSingleton.xml
+49 -23
View File
@@ -7,11 +7,15 @@
<test-code>
<description>failure case</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>7</expected-linenumbers>
<code><![CDATA[
import java.util.ArrayList;
import java.util.List;
public class Foo {
private static List buz;
public static List bar() {
if (buz == null) buz = new ArrayList();
private static List<Object> buz;
public static List<Object> bar() {
if (buz == null) buz = new ArrayList<>();
return buz;
}
}
@@ -22,10 +26,13 @@ public class Foo {
<description>OK, method is synchronized</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.ArrayList;
import java.util.List;
public class Foo {
private static List buz;
public static synchronized List bar() {
if (buz == null) buz = new ArrayList();
private static List<Object> buz;
public static synchronized List<Object> bar() {
if (buz == null) buz = new ArrayList<>();
return buz;
}
}
@@ -36,11 +43,14 @@ public class Foo {
<description>OK, in synchronized block</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.ArrayList;
import java.util.List;
public class Foo {
private static List buz;
public static List bar() {
private static List<Object> buz;
public static List<Object> bar() {
synchronized (baz) {
if (buz == null) buz = new ArrayList();
if (buz == null) buz = new ArrayList<>();
return buz;
}
}
@@ -52,10 +62,12 @@ public class Foo {
<description>OK, in returning non-static data</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.util.Locale;
public class Foo {
private static Locale locale;
public static List bar() {
if (locale==null) return Locale.getDefault();
public static Locale bar() {
if (locale == null) return Locale.getDefault();
return locale;
}
}
@@ -65,13 +77,17 @@ public class Foo {
<test-code>
<description>failure case, two if statements</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>8</expected-linenumbers>
<code><![CDATA[
import java.util.ArrayList;
import java.util.List;
public class Foo {
private static List buz;
private static List<Object> buz;
private static boolean b = false;
public static List bar(String foo) {
public static List<Object> bar(String foo) {
if (buz == null) {
buz = new ArrayList();
buz = new ArrayList<>();
if (foo == null) {
b = true;
}
@@ -85,12 +101,16 @@ public class Foo {
<test-code>
<description>failure case, compound if statement</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>7</expected-linenumbers>
<code><![CDATA[
import java.util.ArrayList;
import java.util.List;
public class Foo {
private static List list;
public static List bar(String param) {
private static List<Object> list;
public static List<Object> bar(String param) {
if (list == null || !param.equals("foo")) {
list = new ArrayList();
list = new ArrayList<>();
param = "x";
}
return list;
@@ -102,12 +122,16 @@ public class Foo {
<test-code>
<description>failure case 2</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>7</expected-linenumbers>
<code><![CDATA[
import java.util.Collections;
import java.util.List;
public class Foo {
private static List buz = null;
private static List bar() {
private static List<Object> buz = null;
private static List<Object> bar() {
if (buz == null) {
buz = Collections.get(Integer.MAX_SIZE);
buz = Collections.<Object> emptyList();
}
return buz;
}
@@ -134,15 +158,17 @@ public class A {
<test-code>
<description>#997 Rule NonThreadSafeSingleton gives analysis problem</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>7</expected-linenumbers>
<code><![CDATA[
public class A extends B {
public class B {}
class A extends B {
static B instance = null;
private boolean bar = false;
static void foo() {
if (instance == null ) {
if (instance == null) {
instance = new A();
((A)instance).bar=false;
((A)instance).bar = false;
}
}
}