hiifong/pmd
1
0
Fork 0
forked from phoedos/pmd
Code Pull Requests Activity

[java] Fix #5263 - UnnecessaryFullyQualifiedName FP with forward reference.

Browse Source
This commit is contained in:
Clément Fournier 2024-11-21 15:26:54 +01:00
parent 0d11f151bd
commit e63edf358e
No known key found for this signature in database
GPG Key ID: 6F9389D8E390BD4C
3 changed files with 128 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/internal/JavaAstUtils.java
View File

@ -32,6 +32,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr
import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.AccessType;
import net.sourceforge.pmd.lang.java.ast.ASTAssignmentExpression;
import net.sourceforge.pmd.lang.java.ast.ASTBlock;
import net.sourceforge.pmd.lang.java.ast.ASTBodyDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTBooleanLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTBreakStatement;
import net.sourceforge.pmd.lang.java.ast.ASTCastExpression;
@ -40,11 +41,13 @@ import net.sourceforge.pmd.lang.java.ast.ASTClassType;
import net.sourceforge.pmd.lang.java.ast.ASTCompactConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorCall;
import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTEnumConstant;
import net.sourceforge.pmd.lang.java.ast.ASTExecutableDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTExplicitConstructorInvocation;
import net.sourceforge.pmd.lang.java.ast.ASTExpression;
import net.sourceforge.pmd.lang.java.ast.ASTExpressionStatement;
import net.sourceforge.pmd.lang.java.ast.ASTFieldAccess;
import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTForStatement;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameters;
@ -806,10 +809,17 @@ public final class JavaAstUtils {
return false;
}
/**
* Return true if the node is in static context relative to the deepest enclosing class.
*/
public static boolean isInStaticCtx(JavaNode node) {
return node.ancestors()
return node.ancestorsOrSelf()
.map(NodeStream.asInstanceOf(ASTBodyDeclaration.class, ASTExplicitConstructorInvocation.class))
.take(1)
.any(it -> it instanceof ASTExecutableDeclaration && ((ASTExecutableDeclaration) it).isStatic()
|| it instanceof ASTFieldDeclaration && ((ASTFieldDeclaration) it).isStatic()
|| it instanceof ASTInitializer && ((ASTInitializer) it).isStatic()
|| it instanceof ASTEnumConstant
|| it instanceof ASTExplicitConstructorInvocation
);
}

46
pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryFullyQualifiedNameRule.java
View File

@ -13,11 +13,15 @@ import java.util.function.Function;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
import net.sourceforge.pmd.lang.java.ast.ASTBodyDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTClassType;
import net.sourceforge.pmd.lang.java.ast.ASTFieldAccess;
import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
import net.sourceforge.pmd.lang.java.ast.ASTTypeBody;
import net.sourceforge.pmd.lang.java.ast.ASTTypeExpression;
import net.sourceforge.pmd.lang.java.ast.ASTVariableId;
import net.sourceforge.pmd.lang.java.ast.JavaNode;
import net.sourceforge.pmd.lang.java.ast.internal.JavaAstUtils;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
import net.sourceforge.pmd.lang.java.symbols.JAccessibleElementSymbol;
import net.sourceforge.pmd.lang.java.symbols.JClassSymbol;
@ -94,7 +98,7 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRulechainRule
} else if (getProperty(REPORT_FIELDS) && opa instanceof ASTFieldAccess) {
ASTFieldAccess fieldAccess = (ASTFieldAccess) opa;
ScopeInfo reasonForFieldInScope = fieldMeansSame(fieldAccess);
if (reasonForFieldInScope != null) {
if (reasonForFieldInScope != null && !isForwardReference(fieldAccess)) {
String simpleName = formatMemberName(next, fieldAccess.getReferencedSym());
String reasonToString = unnecessaryReasonWrapper(reasonForFieldInScope);
String unnecessary = produceQualifier(deepest, next, true);
@ -252,4 +256,44 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRulechainRule
throw AssertionUtil.shouldNotReachHere("unknown constant ScopeInfo: " + scopeInfo);
}
}
/**
* Return true if removing the qualification from this field access
* would produce an "Illegal forward reference" compiler error. This
* would happen if the referenced field is defined after the reference,
* in the same class. Note that the java compiler uses definite assignment
* to find forward references. Here we over-approximate this, to avoid
* depending on the dataflow pass. We could fix this later though.
*
* @param fieldAccess A field access
*/
private static boolean isForwardReference(ASTFieldAccess fieldAccess) {
JFieldSymbol referencedSym = fieldAccess.getReferencedSym();
if (referencedSym == null || referencedSym.isUnresolved()) {
return false;
}
// The field must be declared in the same compilation unit
// to be a forward reference.
ASTVariableId fieldDecl = referencedSym.tryGetNode();
if (fieldDecl == null) {
return false;
}
ASTBodyDeclaration enclosing = fieldAccess.ancestors(ASTBodyDeclaration.class)
.first();
if (enclosing != null
&& enclosing.getParent().getParent() == fieldDecl.getEnclosingType()) {
// the access is made in the same class
if (JavaAstUtils.isInStaticCtx(fieldDecl)
&& !JavaAstUtils.isInStaticCtx(fieldAccess)) {
// field is static but access is non-static: no problem
return false;
}
// else compare position: if access is before definition, we have a problem
int declIndex = fieldDecl.ancestors().filter(it -> it.getParent() instanceof ASTTypeBody).firstOrThrow().getIndexInParent();
int accessIndex = enclosing.getIndexInParent();
return accessIndex <= declIndex;
}
return false;
}
}

72
pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryFullyQualifiedName.xml
View File

@ -1125,4 +1125,76 @@ public class Foo {
}
]]></code>
</test-code>
<test-code>
<description>#5263 FP when forward reference is illegal</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public enum EnumX {
VALUE_X(EnumX.X); // MUST qualify the name or the file won't compile
static final String X = "X";
private final String v;
EnumX(final String v) {
this.v = v;
}
public String getV() {
return v;
}
}
]]></code>
</test-code>
<test-code>
<description>#5263 FP when forward reference is illegal (class)</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class EnumX {
static final String Y = EnumX.X; // MUST qualify the name or the file won't compile
static final String X = "X";
private final String v;
EnumX(final String v) {
this.v = v;
}
public String getV() {
return v;
}
}
]]></code>
</test-code>
<test-code>
<description>#5263 not forward references</description>
<expected-problems>4</expected-problems>
<code><![CDATA[
public class EnumX {
static final Object Y = new Object() {
String q = EnumX.X; // not FWR
};
private final String v = EnumX.X; // not FWR bc not static
EnumX() {
this.v = EnumX.X; // not FWR
}
void foo() {
System.out.println(EnumX.X); // not FWR
}
static final String X = "X";
}
]]></code>
</test-code>
</test-data>