Skip to content

Commit 99e684e

Browse files
Support command overloading in a binary compatible fashion (#5376)
Fixes #5281 The previous code already collected all command overloads in the `Discover` data structure, but it returned one arbitrarily when queried. This PR makes it return the one with the longer argument list, which supports `@unroll`-style signature evolution of adding parameters with default values to the right side of a parameter list. Added a unit test that fails on main, passes on this PR --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent 480f62c commit 99e684e

File tree

2 files changed

+26
-1
lines changed

2 files changed

+26
-1
lines changed

core/define/src/mill/define/Discover.scala

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@ class Discover(val classInfo: Map[Class[?], Discover.ClassInfo]) {
2323
if ep.mainName.getOrElse(ep.defaultName) == name
2424
} yield ep
2525

26-
res.headOption
26+
// When there are multiple entrypoints matching the class and name,
27+
// return the one with the longest argument lists, since users may
28+
// add additional arguments to a command (with default values) to
29+
// preserve binary compatibility while evolving the method signature
30+
res.maxByOption(_.argSigs0.length)
2731
}
2832
}
2933

core/exec/test/src/mill/exec/ExecutionTests.scala

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@ object ExecutionTests extends TestSuite {
4545
lazy val millDiscover = Discover[this.type]
4646
}
4747

48+
object overloads extends TestRootModule {
49+
def overloaded(x: Int) = Task.Command { x + 1 }
50+
def overloaded(x: Int, y: Int = 0) = Task.Command { x + y + 1 }
51+
lazy val millDiscover = Discover[this.type]
52+
}
53+
4854
class Checker[T <: mill.testkit.TestRootModule](module: T)
4955
extends exec.Checker(module)
5056

@@ -448,5 +454,20 @@ object ExecutionTests extends TestSuite {
448454
assert(res.executionResults.transitiveFailing.keySet == Set(anonTaskFailure.task))
449455
}
450456
}
457+
test("overloaded") {
458+
UnitTester(overloads, null).scoped { tester =>
459+
val res = tester.apply(Seq(overloads.overloaded(1)))
460+
assert(res == Right(UnitTester.Result(Vector(2), 1)))
461+
462+
val res2 = tester.apply(Seq(overloads.overloaded(1, 2)))
463+
assert(res2 == Right(UnitTester.Result(Vector(4), 1)))
464+
465+
val res3 = tester.apply("overloaded", "-x", "1")
466+
assert(res3 == Right(UnitTester.Result(Vector(2), 1)))
467+
468+
val res4 = tester.apply("overloaded", "-x", "1", "-y", "2")
469+
assert(res4 == Right(UnitTester.Result(Vector(4), 1)))
470+
}
471+
}
451472
}
452473
}

0 commit comments

Comments
 (0)