From 6ff0e6c28d12b89fb7ca5df3775a686bc6c68980 Mon Sep 17 00:00:00 2001
From: Li Haoyi <haoyi.sg@gmail.com>
Date: Thu, 18 Jul 2024 16:41:45 +0800
Subject: [PATCH] Fix resolution of children of `override lazy val` modules
 (#3270)

This was preventing resolution of modules in the form `override lazy val
foo: FooModule = new FooModule{...}`, which are necessary when you want
to override one module with another. You couldn't resolve them via
`resolve` or run them from the command line, but you could depend on
them from other modules and have then get picked up by planning logic

These turn up in Mill's own build, and I notice I couldn't resolve e.g.
`main.codesig.test.cases[callgraph-basic-1-static-method].compile`
before this PR. After this PR, I can.

Filtering out `abstract` methods in `Reflect` seems unnecessary, since
compilation checks that every method is implemented already.

Added a unit test to cover this edge case.

Pull request: https://github.com/com-lihaoyi/mill/pull/3270
---
 main/define/src/mill/define/Reflect.scala     |  1 -
 .../test/src/mill/main/ResolveTests.scala     |  7 +++++++
 main/test/src/mill/util/TestGraphs.scala      | 19 +++++++++++++++++++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/main/define/src/mill/define/Reflect.scala b/main/define/src/mill/define/Reflect.scala
index 5b6b7e438a2..beb743274e3 100644
--- a/main/define/src/mill/define/Reflect.scala
+++ b/main/define/src/mill/define/Reflect.scala
@@ -29,7 +29,6 @@ private[mill] object Reflect {
         isLegalIdentifier(n) &&
         (!noParams || m.getParameterCount == 0) &&
         (m.getModifiers & Modifier.STATIC) == 0 &&
-        (m.getModifiers & Modifier.ABSTRACT) == 0 &&
         inner.isAssignableFrom(m.getReturnType)
     } yield m
 
diff --git a/main/resolve/test/src/mill/main/ResolveTests.scala b/main/resolve/test/src/mill/main/ResolveTests.scala
index 6c8218db9fd..45d65bde8fd 100644
--- a/main/resolve/test/src/mill/main/ResolveTests.scala
+++ b/main/resolve/test/src/mill/main/ResolveTests.scala
@@ -1099,5 +1099,12 @@ object ResolveTests extends TestSuite {
         )
       }
     }
+    test("abstractModule") {
+      val check = new Checker(TestGraphs.AbstractModule)
+      test - check(
+        "__",
+        Right(Set(_.concrete.tests.inner.foo, _.concrete.tests.inner.innerer.bar))
+      )
+    }
   }
 }
diff --git a/main/test/src/mill/util/TestGraphs.scala b/main/test/src/mill/util/TestGraphs.scala
index 3482f6e0636..dd92bd3d9b3 100644
--- a/main/test/src/mill/util/TestGraphs.scala
+++ b/main/test/src/mill/util/TestGraphs.scala
@@ -608,4 +608,23 @@ object TestGraphs {
     }
   }
 
+  object AbstractModule extends TestUtil.BaseModule {
+    trait Abstract extends Module {
+      lazy val tests: Tests = new Tests {}
+      trait Tests extends Module {}
+    }
+
+    object concrete extends Abstract {
+      override lazy val tests: ConcreteTests = new ConcreteTests {}
+      trait ConcreteTests extends Tests {
+        object inner extends Module {
+          def foo = T { "foo" }
+          object innerer extends Module {
+            def bar = T { "bar" }
+          }
+        }
+      }
+    }
+  }
+
 }