Remove DataflowPass#ensureProcessed

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/BinaryOp.java

@@ -11,7 +11,6 @@ import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 import net.sourceforge.pmd.util.CollectionUtil;
-import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
  * Represents the operator of an {@linkplain ASTInfixExpression infix expression}.

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedAssignmentRule.java

@@ -21,10 +21,10 @@ import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
 import net.sourceforge.pmd.lang.java.ast.JavaNode;
 import net.sourceforge.pmd.lang.java.ast.UnaryOp;
 import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
-import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
 import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass;
 import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass.AssignmentEntry;
 import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass.DataflowResult;
+import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
 import net.sourceforge.pmd.properties.PropertyDescriptor;
 import net.sourceforge.pmd.properties.PropertyFactory;
 

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/AvoidThrowingNullPointerExceptionRule.java

@@ -10,6 +10,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTVariableAccess;
 import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
 import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass;
 import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass.AssignmentEntry;
+import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass.DataflowResult;
 import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass.ReachingDefinitionSet;
 import net.sourceforge.pmd.lang.java.symbols.JLocalVariableSymbol;
 import net.sourceforge.pmd.lang.java.symbols.JVariableSymbol;
@@ -43,8 +44,8 @@ public class AvoidThrowingNullPointerExceptionRule extends AbstractJavaRulechain
     }
 
     private boolean hasNpeValue(ASTVariableAccess thrown, JLocalVariableSymbol sym) {
-        DataflowPass.ensureProcessed(thrown.getRoot());
-        ReachingDefinitionSet reaching = DataflowPass.getReachingDefinitions(thrown);
+        DataflowResult dataflow = DataflowPass.getDataflowResult(thrown.getRoot());
+        ReachingDefinitionSet reaching = dataflow.getReachingDefinitions(thrown);
         if (reaching == null || reaching.isNotFullyKnown()) {
             // we lean towards false negatives... maybe we should be able
             // to report this with a lower priority

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SingularFieldRule.java

@@ -1,4 +1,4 @@
-/**
+/*
  * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
  */
 
@@ -19,14 +19,15 @@ import org.checkerframework.checker.nullness.qual.Nullable;
 import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr;
 import net.sourceforge.pmd.lang.java.ast.ASTBodyDeclaration;
-import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
 import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTLambdaExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
+import net.sourceforge.pmd.lang.java.ast.AccessNode;
 import net.sourceforge.pmd.lang.java.ast.AccessNode.Visibility;
 import net.sourceforge.pmd.lang.java.ast.JavaNode;
 import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
 import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass;
+import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass.DataflowResult;
 import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass.ReachingDefinitionSet;
 import net.sourceforge.pmd.lang.java.rule.internal.JavaPropertyUtil;
 import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil;
@@ -65,43 +66,43 @@ public class SingularFieldRule extends AbstractJavaRulechainRule {
         );
 
     public SingularFieldRule() {
-        super(ASTCompilationUnit.class, ASTFieldDeclaration.class);
+        super(ASTAnyTypeDeclaration.class);
         definePropertyDescriptor(IGNORED_FIELD_ANNOTATIONS);
     }
 
     @Override
-    public Object visit(ASTCompilationUnit node, Object data) {
-        DataflowPass.ensureProcessed(node);
-        return data;
-    }
-
-    @Override
-    public Object visit(ASTFieldDeclaration node, Object data) {
-        ASTAnyTypeDeclaration enclosingType = node.getEnclosingType();
-        if (node.getVisibility() != Visibility.V_PRIVATE
-            || node.hasModifiers(STATIC)
-            || JavaRuleUtil.hasAnyAnnotation(enclosingType, INVALIDATING_CLASS_ANNOT)
-            || JavaRuleUtil.hasAnyAnnotation(node, getProperty(IGNORED_FIELD_ANNOTATIONS))) {
-            return data;
+    public Object visitJavaNode(JavaNode node, Object data) {
+        ASTAnyTypeDeclaration enclosingType = (ASTAnyTypeDeclaration) node;
+        if (JavaRuleUtil.hasAnyAnnotation(enclosingType, INVALIDATING_CLASS_ANNOT)) {
+            return null;
         }
 
-        for (ASTVariableDeclaratorId varId : node.getVarIds()) {
-            if (mayBeSingular(varId) && isSingularField(enclosingType, varId)) {
-                addViolation(data, varId, varId.getName());
+        DataflowResult dataflow = null;
+        for (ASTFieldDeclaration fieldDecl : enclosingType.getDeclarations(ASTFieldDeclaration.class)) {
+            if (!mayBeSingular(fieldDecl)
+                || JavaRuleUtil.hasAnyAnnotation(fieldDecl, getProperty(IGNORED_FIELD_ANNOTATIONS))) {
+                continue;
+            }
+            for (ASTVariableDeclaratorId varId : fieldDecl.getVarIds()) {
+                if (dataflow == null) { //compute lazily
+                    dataflow = DataflowPass.getDataflowResult(node.getRoot());
+                }
+                if (isSingularField(enclosingType, varId, dataflow)) {
+                    addViolation(data, varId, varId.getName());
+                }
             }
         }
-
-        return data;
+        return null;
     }
 
-    public static boolean mayBeSingular(ASTVariableDeclaratorId varId) {
+    public static boolean mayBeSingular(AccessNode varId) {
         return varId.getEffectiveVisibility().isAtMost(Visibility.V_PRIVATE)
             && !varId.getModifiers().hasAny(STATIC, FINAL);
     }
 
-    private boolean isSingularField(ASTAnyTypeDeclaration fieldOwner, ASTVariableDeclaratorId varId) {
+    private boolean isSingularField(ASTAnyTypeDeclaration fieldOwner, ASTVariableDeclaratorId varId, DataflowResult dataflow) {
         if (JavaRuleUtil.isNeverUsed(varId)) {
-            return false;// don't report unused field
+            return false; // don't report unused field
         }
 
         //Check usages for validity & group them by scope
@@ -120,7 +121,7 @@ public class SingularFieldRule extends AbstractJavaRulechainRule {
 
         // the field is singular if it is used as a local var in every method.
         for (ASTBodyDeclaration method : usagesByScope.keySet()) {
-            if (method != null && !usagesDontObserveValueBeforeMethodCall(usagesByScope.get(method))) {
+            if (method != null && !usagesDontObserveValueBeforeMethodCall(usagesByScope.get(method), dataflow)) {
                 return false;
             }
         }
@@ -140,9 +141,9 @@ public class SingularFieldRule extends AbstractJavaRulechainRule {
                     .any(it -> it instanceof ASTLambdaExpression);
     }
 
-    private boolean usagesDontObserveValueBeforeMethodCall(List<ASTNamedReferenceExpr> usages) {
+    private boolean usagesDontObserveValueBeforeMethodCall(List<ASTNamedReferenceExpr> usages, DataflowResult dataflow) {
         for (ASTNamedReferenceExpr usage : usages) {
-            ReachingDefinitionSet reaching = DataflowPass.getReachingDefinitions(usage);
+            ReachingDefinitionSet reaching = dataflow.getReachingDefinitions(usage);
             if (reaching != null && reaching.containsInitialFieldValue()) {
                 return false;
             }

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/MissingBreakInSwitchRule.java

@@ -8,7 +8,6 @@ import java.util.regex.Pattern;
 
 import net.sourceforge.pmd.lang.ast.GenericToken;
 import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken;
-import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
 import net.sourceforge.pmd.lang.java.ast.ASTSwitchBranch;
 import net.sourceforge.pmd.lang.java.ast.ASTSwitchFallthroughBranch;
 import net.sourceforge.pmd.lang.java.ast.ASTSwitchStatement;
@@ -16,6 +15,7 @@ import net.sourceforge.pmd.lang.java.ast.JavaNode;
 import net.sourceforge.pmd.lang.java.ast.JavaTokenKinds;
 import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule;
 import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass;
+import net.sourceforge.pmd.lang.java.rule.internal.DataflowPass.DataflowResult;
 import net.sourceforge.pmd.util.OptionalBool;
 
 public class MissingBreakInSwitchRule extends AbstractJavaRulechainRule {
@@ -27,20 +27,17 @@ public class MissingBreakInSwitchRule extends AbstractJavaRulechainRule {
                                                                    Pattern.DOTALL | Pattern.CASE_INSENSITIVE);
 
     public MissingBreakInSwitchRule() {
-        super(ASTCompilationUnit.class, ASTSwitchStatement.class);
-    }
-
-    public Object visit(ASTCompilationUnit node, Object data) {
-        DataflowPass.ensureProcessed(node);
-        return null;
+        super(ASTSwitchStatement.class);
     }
 
     @Override
     public Object visit(ASTSwitchStatement node, Object data) {
+        DataflowResult dataflow = DataflowPass.getDataflowResult(node.getRoot());
+
         for (ASTSwitchBranch branch : node.getBranches()) {
             if (branch instanceof ASTSwitchFallthroughBranch && branch != node.getLastChild()) {
                 ASTSwitchFallthroughBranch fallthrough = (ASTSwitchFallthroughBranch) branch;
-                OptionalBool bool = DataflowPass.switchBranchFallsThrough(branch);
+                OptionalBool bool = dataflow.switchBranchFallsThrough(branch);
                 if (bool != OptionalBool.NO
                     && fallthrough.getStatements().nonEmpty()
                     && !nextBranchHasComment(branch)) {

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/DataflowPass.java

@@ -108,19 +108,19 @@ public final class DataflowPass {
     private static final SimpleDataKey<ReachingDefinitionSet> REACHING_DEFS = DataMap.simpleDataKey("java.dataflow.reaching.backwards");
     private static final SimpleDataKey<AssignmentEntry> VAR_DEFINITION = DataMap.simpleDataKey("java.dataflow.field.def");
     private static final SimpleDataKey<OptionalBool> SWITCH_BRANCH_FALLS_THROUGH = DataMap.simpleDataKey("java.dataflow.switch.fallthrough");
+    
+    private DataflowPass() {
+        // utility class
+    }
 
+    /**
+     * Returns the info computed by the dataflow pass for the given file.
+     * The computation is done at most once.
+     */
     public static DataflowResult getDataflowResult(ASTCompilationUnit acu) {
         return acu.getUserMap().computeIfAbsent(DATAFLOW_RESULT_K, () -> process(acu));
     }
 
-    public static void ensureProcessed(ASTCompilationUnit acu) {
-        getDataflowResult(acu);
-    }
-
-    public static @Nullable ReachingDefinitionSet getReachingDefinitions(ASTNamedReferenceExpr expr) {
-        return expr.getUserMap().get(REACHING_DEFS);
-    }
-
     /**
      * If the var id is that of a field, returns the assignment entry that
      * corresponds to its definition (either blank or its initializer). From
@@ -134,13 +134,6 @@ public final class DataflowPass {
         return varId.getUserMap().get(VAR_DEFINITION);
     }
 
-    public static @NonNull OptionalBool switchBranchFallsThrough(ASTSwitchBranch b) {
-        if (b instanceof ASTSwitchFallthroughBranch) {
-            return Objects.requireNonNull(b.getUserMap().get(SWITCH_BRANCH_FALLS_THROUGH));
-        }
-        return OptionalBool.NO;
-    }
-
     private static DataflowResult process(ASTCompilationUnit node) {
         DataflowResult dataflowResult = new DataflowResult();
         for (ASTAnyTypeDeclaration typeDecl : node.getTypeDeclarations()) {
@@ -155,7 +148,8 @@ public final class DataflowPass {
             }
 
             CollectionUtil.mergeMaps(
-                dataflowResult.killRecord, subResult.killRecord,
+                dataflowResult.killRecord,
+                subResult.killRecord,
                 (s1, s2) -> {
                     s1.addAll(s2);
                     return s1;
@@ -167,7 +161,7 @@ public final class DataflowPass {
 
     /**
      * A set of reaching definitions, ie the assignments that are visible
-     * at some point. One can use {@link #getReachingDefinitions(ASTNamedReferenceExpr)}
+     * at some point. One can use {@link DataflowResult#getReachingDefinitions(ASTNamedReferenceExpr)}
      * to get the data flow that reaches a variable usage (and go backwards with
      * the {@linkplain DataflowResult#getKillers(AssignmentEntry) kill record}).
      */
@@ -209,6 +203,7 @@ public final class DataflowPass {
     /**
      * Global result of the dataflow analysis.
      */
+    // this is a façade class
     public static final class DataflowResult {
 
         final Set<AssignmentEntry> unusedAssignments;
@@ -233,6 +228,25 @@ public final class DataflowPass {
         public @NonNull Set<AssignmentEntry> getKillers(AssignmentEntry assignment) {
             return killRecord.getOrDefault(assignment, Collections.emptySet());
         }
+
+        // These methods are only valid to be called if the dataflow pass has run.
+        // This is why they are instance methods here: by asking for the DataflowResult
+        // instance to get access to them, you ensure that the pass has been executed properly.
+
+        /**
+         * Returns whether the switch branch falls-through to the next one (or the end of the switch).
+         */
+        public @NonNull OptionalBool switchBranchFallsThrough(ASTSwitchBranch b) {
+            if (b instanceof ASTSwitchFallthroughBranch) {
+                return Objects.requireNonNull(b.getUserMap().get(SWITCH_BRANCH_FALLS_THROUGH));
+            }
+            return OptionalBool.NO;
+        }
+
+
+        public @Nullable ReachingDefinitionSet getReachingDefinitions(ASTNamedReferenceExpr expr) {
+            return expr.getUserMap().get(REACHING_DEFS);
+        }
     }
 
     private static class ReachingDefsVisitor extends JavaVisitorBase<SpanInfo, SpanInfo> {