From 0c44c1b6bd06f761f25590d95b9fafcb32244f11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sat, 18 Sep 2021 16:38:33 +0200 Subject: [PATCH] Update rule CheckSkipResult --- .ci/files/all-java.xml | 2 +- .../rule/errorprone/CheckSkipResultRule.java | 64 ++++++------------- .../rule/errorprone/CheckSkipResultTest.java | 1 - .../rule/errorprone/xml/CheckSkipResult.xml | 6 ++ 4 files changed, 25 insertions(+), 48 deletions(-) diff --git a/.ci/files/all-java.xml b/.ci/files/all-java.xml index 0a86a06f54..2828f57152 100644 --- a/.ci/files/all-java.xml +++ b/.ci/files/all-java.xml @@ -191,7 +191,7 @@ - + diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CheckSkipResultRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CheckSkipResultRule.java index 3b5710f780..b806f108ec 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CheckSkipResultRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/CheckSkipResultRule.java @@ -4,56 +4,28 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; -import java.io.InputStream; +import net.sourceforge.pmd.lang.java.ast.ASTExpressionStatement; +import net.sourceforge.pmd.lang.java.ast.ASTMethodCall; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; +import net.sourceforge.pmd.lang.java.types.InvocationMatcher; -import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.java.ast.ASTExpression; -import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; -import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; -import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression; -import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId; -import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; -import net.sourceforge.pmd.lang.java.symboltable.JavaNameOccurrence; -import net.sourceforge.pmd.lang.java.types.TypeTestUtil; -import net.sourceforge.pmd.lang.symboltable.NameOccurrence; +public class CheckSkipResultRule extends AbstractJavaRulechainRule { -public class CheckSkipResultRule extends AbstractJavaRule { + private static final InvocationMatcher SKIP_METHOD = InvocationMatcher.parse("java.io.InputStream#skip(_*)"); + + public CheckSkipResultRule() { + super(ASTMethodCall.class); + } @Override - public Object visit(ASTVariableDeclaratorId node, Object data) { - if (!TypeTestUtil.isA(InputStream.class, node.getTypeNode())) { - return data; + public Object visit(ASTMethodCall call, Object data) { + if (SKIP_METHOD.matchesCall(call) && !isResultUsed(call)) { + addViolation(data, call); } - for (NameOccurrence occ : node.getUsages()) { - JavaNameOccurrence jocc = (JavaNameOccurrence) occ; - NameOccurrence qualifier = jocc.getNameForWhichThisIsAQualifier(); - if (qualifier != null && "skip".equals(qualifier.getImage())) { - Node loc = jocc.getLocation(); - if (loc != null) { - ASTPrimaryExpression exp = loc.getFirstParentOfType(ASTPrimaryExpression.class); - while (exp != null) { - if (exp.getParent() instanceof ASTStatementExpression) { - // if exp is in a bare statement, - // the returned value is not used - addViolation(data, occ.getLocation()); - break; - } else if (exp.getParent() instanceof ASTExpression - && exp.getParent().getParent() instanceof ASTPrimaryPrefix) { - // if exp is enclosed in a pair of parenthesis - // let's have a look at the enclosing expression - // we'll see if it's in a bare statement - exp = exp.getFirstParentOfType(ASTPrimaryExpression.class); - } else { - // if exp is neither in a bare statement - // or between a pair of parentheses, - // it's in some other kind of statement - // or assignment so the returned value is used - break; - } - } - } - } - } - return data; + return null; + } + + private boolean isResultUsed(ASTMethodCall call) { + return !(call.getParent() instanceof ASTExpressionStatement); } } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/CheckSkipResultTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/CheckSkipResultTest.java index e096adbab3..03ff885411 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/CheckSkipResultTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/CheckSkipResultTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class CheckSkipResultTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CheckSkipResult.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CheckSkipResult.xml index 2c2bc31c5e..10a174ab36 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CheckSkipResult.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CheckSkipResult.xml @@ -9,6 +9,7 @@ 1 1 0 0 0