From 69b219947f80ca7a3a9850d1d6789902864d662b Mon Sep 17 00:00:00 2001
From: Li Haoyi <haoyi.sg@gmail.com>
Date: Sun, 19 Nov 2023 19:13:08 +0800
Subject: [PATCH] Fix discovery of targets whose names get mangled due to their
 pseudo-private nature (#2883)

Fixes https://github.com/com-lihaoyi/mill/issues/2844

Adds an integration test to cover this edge case, a few other
permutations of it that I could come up with, and other
`private`-related behavior. I'm not sure if I managed to catch all
different ways these methods can be mangled, but if I missed any we can
discover/add them later. This new test fails if I remove any part of the
code change in `GroupEvaluator.scala`

I'm actually not sure if this should be an integration test or a unit
test.

* Technically this code path should be exercised in unit tests as well I
think, and I think this failure mode should be triggerable.
* But it might be affected by exactly how the `def`s are wrapped in
nested `object`s/`trait`s/`class`es, which is something that unit tests
do not do realistically.
* Also, a unit test cannot exercise the failure code path when you try
and specify a `private` target from the command, since it will simply
not compile (being `private` and all), and AFAICT this is somewhere
we're missing coverage (unrelated to this specific failure mode)

So I went with integration test.

Pull request: https://github.com/com-lihaoyi/mill/pull/2883
---
 .../feature/private-methods/repo/build.sc     | 43 +++++++++++++++++
 .../test/src/PrivateMethodsTests.scala        | 46 +++++++++++++++++++
 main/eval/src/mill/eval/GroupEvaluator.scala  | 10 +++-
 3 files changed, 97 insertions(+), 2 deletions(-)
 create mode 100644 integration/feature/private-methods/repo/build.sc
 create mode 100644 integration/feature/private-methods/test/src/PrivateMethodsTests.scala

diff --git a/integration/feature/private-methods/repo/build.sc b/integration/feature/private-methods/repo/build.sc
new file mode 100644
index 00000000000..62bf2f18791
--- /dev/null
+++ b/integration/feature/private-methods/repo/build.sc
@@ -0,0 +1,43 @@
+import mill._
+
+def pub = T {
+  priv()
+}
+
+private def priv = T {
+  "priv"
+}
+
+object foo extends Module {
+  def bar = T {
+    baz()
+  }
+}
+
+private def baz = T {
+  "bazOuter"
+}
+
+object qux extends Module {
+  object foo extends Module {
+    def bar = T {
+      baz()
+    }
+  }
+  private def baz = T {
+    "bazInner"
+  }
+}
+
+object cls extends cls
+class cls extends Module {
+  object foo extends Module {
+    def bar = T {
+      baz()
+    }
+  }
+
+  private def baz = T {
+    "bazCls"
+  }
+}
diff --git a/integration/feature/private-methods/test/src/PrivateMethodsTests.scala b/integration/feature/private-methods/test/src/PrivateMethodsTests.scala
new file mode 100644
index 00000000000..15a2a90e59a
--- /dev/null
+++ b/integration/feature/private-methods/test/src/PrivateMethodsTests.scala
@@ -0,0 +1,46 @@
+package mill.integration
+
+import utest._
+
+object PrivateMethodsTests extends IntegrationTestSuite {
+  val tests = Tests {
+    val wsRoot = initWorkspace()
+    "simple" - {
+      // Simple public target depending on private target works
+      val pub = evalStdout("show", "pub")
+      assert(pub.out == "\"priv\"")
+      assert(pub.isSuccess)
+
+      // Make sure calling private methods indirectly in a variety of scenarios works
+      // even when they're "not really private" due to
+      val fooBar = evalStdout("show", "foo.bar")
+      assert(fooBar.out == "\"bazOuter\"")
+      assert(fooBar.isSuccess)
+
+      val quxFooBar = evalStdout("show", "qux.foo.bar")
+      assert(quxFooBar.out == "\"bazInner\"")
+      assert(quxFooBar.isSuccess)
+
+      val clsFooBar = evalStdout("show", "cls.foo.bar")
+      assert(clsFooBar.out == "\"bazCls\"")
+      assert(clsFooBar.isSuccess)
+
+      // Make sure calling private methods directly fails
+      val priv = evalStdout("show", "priv")
+      assert(priv.err.contains("Cannot resolve priv"))
+      assert(priv.isSuccess == false)
+
+      val baz = evalStdout("show", "baz")
+      assert(baz.err.contains("Cannot resolve baz"))
+      assert(baz.isSuccess == false)
+
+      val quxBaz = evalStdout("show", "qux.baz")
+      assert(quxBaz.err.contains("Cannot resolve qux.baz"))
+      assert(quxBaz.isSuccess == false)
+
+      val clsBaz = evalStdout("show", "cls.baz")
+      assert(clsBaz.err.contains("Cannot resolve cls.baz"))
+      assert(clsBaz.isSuccess == false)
+    }
+  }
+}
diff --git a/main/eval/src/mill/eval/GroupEvaluator.scala b/main/eval/src/mill/eval/GroupEvaluator.scala
index f2f5f10f9ad..8754b67a539 100644
--- a/main/eval/src/mill/eval/GroupEvaluator.scala
+++ b/main/eval/src/mill/eval/GroupEvaluator.scala
@@ -8,7 +8,7 @@ import mill.eval.Evaluator.TaskResult
 import mill.util._
 
 import scala.collection.mutable
-import scala.reflect.NameTransformer.decode
+import scala.reflect.NameTransformer.{encode, decode}
 import scala.util.DynamicVariable
 import scala.util.control.NonFatal
 
@@ -98,7 +98,13 @@ private[mill] trait GroupEvaluator {
             val methods = for {
               c <- transitiveParents
               m <- c.getDeclaredMethods
-              if decode(m.getName) == namedTask.ctx.segment.pathSegments.head
+              encodedTaskName = encode(namedTask.ctx.segment.pathSegments.head)
+              if m.getName == encodedTaskName ||
+                // Handle scenarios where private method names get mangled when they are
+                // not really JVM-private due to being accessed by Scala nested objects
+                // or classes https://github.com/scala/bug/issues/9306
+                m.getName == (c.getName.replace('.', '$') + "$$" + encodedTaskName) ||
+                m.getName == (c.getName.replace('.', '$') + "$" + encodedTaskName)
             } yield m
 
             val methodClass = methods.head.getDeclaringClass.getName