Skip to content

Commit 8b12ce2

Browse files
authored
Let Task-macros fail if their return value has a Task-type (#5494)
Returning a `Task[_]` from a `Task[_]` (for example, `Task[Task[Int]]`) should be always an error because the inner task is never detected as a dependency in the task graph and thus never executed. Fixes #5263
1 parent 62f6be1 commit 8b12ce2

File tree

2 files changed

+104
-2
lines changed

2 files changed

+104
-2
lines changed

core/api/src/mill/api/internal/Applicative.scala

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package mill.api.internal
22

3+
import mill.api.Task
34
import mill.api.daemon.internal.internal
45
import scala.annotation.compileTimeOnly
56

@@ -24,15 +25,69 @@ object Applicative {
2425

2526
type Id[+T] = T
2627

28+
/**
29+
* @param allowNestedTasks whether `Task[Task[A]]` or `Task[Something[Task[A]]]` and similar structures
30+
* are allowed.
31+
*/
2732
private[mill] def impl[M[_]: Type, W[_]: Type, Z[_]: Type, T: Type, Ctx: Type](using
2833
Quotes
2934
)(
3035
traverseCtx: (Expr[Seq[W[Any]]], Expr[(Seq[Any], Ctx) => Z[T]]) => Expr[M[T]],
3136
t: Expr[Z[T]],
32-
allowTaskReferences: Boolean = true
37+
allowTaskReferences: Boolean = true,
38+
allowNestedTasks: Boolean = false
3339
): Expr[M[T]] = {
3440
import quotes.reflect.*
3541

42+
def checkForNestedTasks() = {
43+
val taskType = TypeRepr.of[Task[_]]
44+
45+
def isTask(tpr: TypeRepr): Boolean =
46+
// We use `typeSymbol` for a robust check.
47+
tpr.typeSymbol == taskType.typeSymbol
48+
49+
def containsTask(tpr: TypeRepr): Boolean = {
50+
// Dealias to handle type aliases like `type MyTask[A] = Task[A]`
51+
val currentType = tpr.dealias
52+
53+
if (isTask(currentType)) true
54+
else
55+
// Otherwise, check all of its type arguments recursively.
56+
// e.g., for Map[Int, String], this would be [Int, String]
57+
currentType.typeArgs.exists(containsTask)
58+
}
59+
60+
val resultTypeRepr = TypeRepr.of[M[T]].dealias
61+
62+
if (isTask(resultTypeRepr)) {
63+
64+
/** The `A` to the `Task[A]`. */
65+
val innerTypeRepr = resultTypeRepr.typeArgs match {
66+
case innerTypeRepr :: Nil => innerTypeRepr
67+
case other => throw new IllegalStateException(
68+
s"Task[_] should have exactly one type parameter, but got ${other.map(_.show)}. " +
69+
"This is a bug in mill."
70+
)
71+
}
72+
73+
if (containsTask(innerTypeRepr)) {
74+
report.errorAndAbort(
75+
// report.warning(
76+
s"""|A `Task[A]` cannot be a parameter of another `Task[A]` because the inner task would not
77+
|be executed.
78+
|
79+
|See https://github.com/com-lihaoyi/mill/issues/5263 for more information.
80+
|
81+
|Type of the result: ${resultTypeRepr.show}
82+
|""".stripMargin,
83+
Position.ofMacroExpansion
84+
)
85+
}
86+
}
87+
}
88+
89+
if (!allowNestedTasks) checkForNestedTasks()
90+
3691
val targetApplySym = TypeRepr.of[Applyable[Nothing, ?]].typeSymbol.methodMember("apply").head
3792

3893
// Derived from @olafurpg's
@@ -99,6 +154,7 @@ object Applicative {
99154

100155
val newBody = treeMap.transformTree(t.asTerm)(Symbol.spliceOwner).asExprOf[Z[T]]
101156
val exprsList = Expr.ofList(exprs.toList.map(_.asExprOf[W[Any]]))
157+
102158
(newBody, exprsList)
103159
}
104160

@@ -119,5 +175,4 @@ object Applicative {
119175
else
120176
traverseCtx(exprsList, callback)
121177
}
122-
123178
}

core/api/test/src/mill/api/MacroErrorTests.scala

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package mill.api
22

33
import utest._
44
import mill.testkit.TestRootModule
5+
56
object MacroErrorTests extends TestSuite {
67

78
val tests = Tests {
@@ -215,5 +216,51 @@ object MacroErrorTests extends TestSuite {
215216
"Could not summon ToSegments[sun.misc.Unsafe]"
216217
))
217218
}
219+
220+
test("taskWithinATask") {
221+
val nestedTaskError = "A `Task[A]` cannot be a parameter of another `Task[A]`"
222+
223+
test("simple") {
224+
val error = utest.compileError(
225+
"""
226+
object foo extends TestRootModule {
227+
def taskWithinTask = Task.Anon { Task.Anon { 42 } }
228+
229+
lazy val millDiscover = Discover[this.type]
230+
}
231+
"""
232+
)
233+
234+
assert(error.msg.contains(nestedTaskError))
235+
}
236+
237+
test("nested") {
238+
val error = utest.compileError(
239+
"""
240+
object foo extends TestRootModule {
241+
def taskWithinTask = Task.Anon { Seq(Task.Anon { 42 }) }
242+
243+
lazy val millDiscover = Discover[this.type]
244+
}
245+
"""
246+
)
247+
248+
assert(error.msg.contains(nestedTaskError))
249+
}
250+
251+
test("inAnotherType") {
252+
val error = utest.compileError(
253+
"""
254+
object foo extends TestRootModule {
255+
def taskWithinTask = Seq(Task.Anon { Task.Anon { 42 } })
256+
257+
lazy val millDiscover = Discover[this.type]
258+
}
259+
"""
260+
)
261+
262+
assert(error.msg.contains(nestedTaskError))
263+
}
264+
}
218265
}
219266
}

0 commit comments

Comments
 (0)