Skip to content

Commit

Permalink
Fix resolution of children of override lazy val modules (#3270)
Browse files Browse the repository at this point in the history
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: #3270
  • Loading branch information
lihaoyi authored Jul 18, 2024
1 parent 862a4dc commit 6ff0e6c
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 1 deletion.
1 change: 0 additions & 1 deletion main/define/src/mill/define/Reflect.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 7 additions & 0 deletions main/resolve/test/src/mill/main/ResolveTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))
)
}
}
}
19 changes: 19 additions & 0 deletions main/test/src/mill/util/TestGraphs.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
}
}
}
}
}

}

0 comments on commit 6ff0e6c

Please sign in to comment.