From 596f1296001ede2c4047f767b08bc196cc72f9de Mon Sep 17 00:00:00 2001
From: Andreas Dangel <andreas.dangel@pmd-code.org>
Date: Wed, 24 Aug 2022 15:04:43 +0200
Subject: [PATCH] Fix False positive with public method call on new instance

---
 ...ConstructorCallsOverridableMethodRule.java | 18 ++++++------
 .../xml/ConstructorCallsOverridableMethod.xml | 28 +++++++++++++++++++
 2 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/ConstructorCallsOverridableMethodRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/ConstructorCallsOverridableMethodRule.java
index 9e60c58230..343498ba38 100644
--- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/ConstructorCallsOverridableMethodRule.java
+++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/ConstructorCallsOverridableMethodRule.java
@@ -14,6 +14,7 @@ import java.util.Set;
 import java.util.TreeMap;
 
 import net.sourceforge.pmd.lang.ast.Node;
+import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTArgumentList;
 import net.sourceforge.pmd.lang.java.ast.ASTArguments;
@@ -275,21 +276,18 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul
                                 // check prefix type match
                                 ASTPrimaryPrefix child2 = (ASTPrimaryPrefix) child;
                                 if (getNameFromPrefix(child2) == null) {
-                                    if (child2.getImage() == null) {
+                                    if (child2.usesThisModifier()) {
                                         thisIndex = x;
                                         break;
-                                    } else {
-                                        // happens when super is used
-                                        // [super.method(): image = 'method']
+                                    } else if (child2.usesSuperModifier()) {
                                         superFirst = true;
                                         thisIndex = x;
-                                        // the true super is at an unusable
-                                        // index because super.method() has only
-                                        // 2 nodes [method=0,()=1]
-                                        // as opposed to the 3 you might expect
-                                        // and which this.method() actually has.
-                                        // [this=0,method=1.()=2]
                                         break;
+                                    } else if (child2.getFirstChildOfType(ASTAllocationExpression.class) != null) {
+                                        // change of scope - the method call is neither on this or super,
+                                        // but on a different instance.
+                                        // ignore this method call
+                                        return null;
                                     }
                                 }
                             }
diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/ConstructorCallsOverridableMethod.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/ConstructorCallsOverridableMethod.xml
index bb4568c073..82903ec954 100644
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/ConstructorCallsOverridableMethod.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/ConstructorCallsOverridableMethod.xml
@@ -346,6 +346,34 @@ class Foo3 {
   }
   public void bar(String s) {}
 }
+]]></code>
+    </test-code>
+    <test-code>
+        <description>False positive with public method call on new instance</description>
+        <expected-problems>2</expected-problems>
+        <expected-linenumbers>4,5</expected-linenumbers>
+        <code><![CDATA[
+class Foo {
+  public Foo() {
+    privateInitOk(); // false positive
+    privateInitBad1(); // warning expected here
+    privateInitBad2(); // warning expected here
+  }
+
+  private void privateInitOk() {
+    new Foo().publicMethod();
+  }
+
+  private void privateInitBad1() {
+    publicMethod();
+  }
+
+  private void privateInitBad2() {
+    this.publicMethod();
+  }
+
+  public void publicMethod() { }
+}
 ]]></code>
     </test-code>
 </test-data>