Merge pull request #3756 from oowekyala:issue3754-singularfield-fp

[java] Fix #3754 singularfield fp #3756

* pr-3756:
  Fix #3754 - SingularField false positive with read in while condition

pmd-core/src/main/java/net/sourceforge/pmd/util/DataMap.java

@@ -6,6 +6,7 @@ package net.sourceforge.pmd.util;
 
 import java.util.IdentityHashMap;
 import java.util.Map;
+import java.util.function.Function;
 import java.util.function.Supplier;
 
 import org.checkerframework.checker.nullness.qual.Nullable;
@@ -67,6 +68,21 @@ public final class DataMap<K> {
         return (T) getMap().computeIfAbsent(key, k -> supplier.get());
     }
 
+    /**
+     * Create or replace a mapping with a value computed from the current
+     * value (or null if missing).
+     *
+     * @param key      Key
+     * @param function Supplier for a value
+     * @param <T>      Type of the data
+     *
+     * @return Value returned by the parameter function
+     */
+    @SuppressWarnings("unchecked")
+    public <T> T compute(DataKey<? extends K, T> key, Function<? super @Nullable T, ? extends T> function) {
+        return (T) getMap().compute(key, (k, v) -> function.apply((T) v));
+    }
+
     private Map<DataKey<? extends K, ?>, Object> getMap() {
         // the map is lazily created, it's only needed if set() is called
         // at least once, but get() might be called many more times, as

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

@@ -168,9 +168,9 @@ public final class DataflowPass {
      */
     public static final class ReachingDefinitionSet {
 
-        private final Set<AssignmentEntry> reaching;
-        private final boolean isNotFullyKnown;
-        private final boolean containsInitialFieldValue;
+        private Set<AssignmentEntry> reaching;
+        private boolean isNotFullyKnown;
+        private boolean containsInitialFieldValue;
 
         ReachingDefinitionSet(/*Mutable*/Set<AssignmentEntry> reaching) {
             this.reaching = reaching;
@@ -199,6 +199,12 @@ public final class DataflowPass {
         public boolean containsInitialFieldValue() {
             return containsInitialFieldValue;
         }
+
+        void absorb(ReachingDefinitionSet reaching) {
+            this.containsInitialFieldValue |= reaching.containsInitialFieldValue;
+            this.isNotFullyKnown |= reaching.isNotFullyKnown;
+            this.reaching.addAll(reaching.reaching);
+        }
     }
 
     /**
@@ -811,7 +817,6 @@ public final class DataflowPass {
 
         @Override
         public SpanInfo visit(ASTUnaryExpression node, SpanInfo data) {
-            super.visit(node, data);
             data = acceptOpt(node.getOperand(), data);
 
             if (node.getOperator().isPure()) {
@@ -1183,7 +1188,15 @@ public final class DataflowPass {
                 global.usedAssignments.addAll(info.reachingDefs);
                 if (reachingDefSink != null) {
                     ReachingDefinitionSet reaching = new ReachingDefinitionSet(new HashSet<>(info.reachingDefs));
-                    reachingDefSink.getUserMap().set(REACHING_DEFS, reaching);
+                    // need to merge into previous to account for cyclic control flow
+                    reachingDefSink.getUserMap().compute(REACHING_DEFS, current -> {
+                        if (current != null) {
+                            current.absorb(reaching);
+                            return current;
+                        } else {
+                            return reaching;
+                        }
+                    });
                 }
             }
         }

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

@@ -694,7 +694,7 @@ public final class JavaRuleUtil {
                 // not inherited
                 && ((JFieldSymbol) sym).getEnclosingClass().equals(e.getEnclosingType().getSymbol())
                 // correct syntactic form
-                && e instanceof ASTVariableAccess || isSyntacticThisFieldAccess(e);
+                && (e instanceof ASTVariableAccess || isSyntacticThisFieldAccess(e));
         }
         return false;
     }

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SingularField.xml

@@ -802,4 +802,58 @@ public class Foo {
 }
             ]]></code>
     </test-code>
+    <test-code>
+        <description>#3754: failure case with field used in loop</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+            public class SingularFieldFP {
+                public Object case1() {
+                    return new Object() {
+                        private int field; // <---------- false positive
+                        public boolean foo() {
+                            while (field++ < 10);
+                            return false;
+                        }
+                    };
+                }
+                public Object case2() {
+                    return new Object() {
+                        private int field; // <---------- false positive
+                        public boolean foo() {
+                            while (field < 10) {
+                                field++;
+                            }
+                            return false;
+                        }
+                    };
+                }
+                public Object case3() {
+                    return new Object() {
+                        private int field;
+                        public boolean foo() {
+                            if (field < 10) {
+                                field++;
+                            }
+                            while (field < 10) {
+                                field++;
+                            }
+                            return false;
+                        }
+                    };
+                }
+                public Object case4() {
+                    return new Object() {
+                        private int field;
+                        public boolean foo() {
+                            if (field > 10) {
+                                return true;
+                            }
+                            field++;
+                            return false;
+                        }
+                    };
+                }
+            }
+            ]]></code>
+    </test-code>
 </test-data>