diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 14dc38def2..aecf590b88 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -37,6 +37,8 @@ This is a {{ site.pmd.release_type }} release. * [#5083](https://github.com/pmd/pmd/issues/5083): \[java] UnusedPrivateMethod false positive when method reference has no target type * [#5097](https://github.com/pmd/pmd/issues/5097): \[java] UnusedPrivateMethod FP with raw type missing from the classpath * [#5318](https://github.com/pmd/pmd/issues/5318): \[java] PreserveStackTraceRule: false-positive on Pattern Matching with instanceof +* java-codestyle + * [#5263](https://github.com/pmd/pmd/issues/5263): \[java] UnnecessaryFullyQualifiedName: false-positive in an enum that uses its own static variables * java-performance * [#5287](https://github.com/pmd/pmd/issues/5287): \[java] TooFewBranchesForSwitch false-positive with switch using list of case constants * [#5314](https://github.com/pmd/pmd/issues/5314): \[java] InsufficientStringBufferDeclarationRule: Lack of handling for char type parameters diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/internal/JavaAstUtils.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/internal/JavaAstUtils.java index 3c32710efe..9de709b90f 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/internal/JavaAstUtils.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/internal/JavaAstUtils.java @@ -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 ); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryFullyQualifiedNameRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryFullyQualifiedNameRule.java index 4c2b79b4bc..20e072efed 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryFullyQualifiedNameRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryFullyQualifiedNameRule.java @@ -13,11 +13,18 @@ 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.ASTEnumConstant; import net.sourceforge.pmd.lang.java.ast.ASTFieldAccess; +import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTInitializer; 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 +101,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 +259,50 @@ public class UnnecessaryFullyQualifiedNameRule extends AbstractJavaRulechainRule throw AssertionUtil.shouldNotReachHere("unknown constant ScopeInfo: " + scopeInfo); } } + + private static boolean isPartOfStaticInitialization(ASTBodyDeclaration decl) { + return decl instanceof ASTFieldDeclaration && ((ASTFieldDeclaration) decl).isStatic() + || decl instanceof ASTInitializer && ((ASTInitializer) decl).isStatic() + || decl instanceof ASTEnumConstant; + } + + /** + * 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 || !fieldDecl.isStatic()) { + return false; + } + ASTBodyDeclaration enclosing = fieldAccess.ancestors(ASTBodyDeclaration.class) + .first(); + if (isPartOfStaticInitialization(enclosing) + && 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; + } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryFullyQualifiedName.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryFullyQualifiedName.xml index 056c77453e..25a69a3bc1 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryFullyQualifiedName.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryFullyQualifiedName.xml @@ -1125,4 +1125,73 @@ public class Foo { } ]]> + + #5263 FP when forward reference is illegal + 0 + + + + #5263 FP when forward reference is illegal (class) + 0 + + + + #5263 not forward references + 5 + 3,6,9,13,17 + +