Skip to content

Commit 3ba698a

Browse files
Add Result.traverse (#5493)
From the issue: > Currently we use `Result.sequence` everywhere, but many of the call sites could be replaced by `Result.traverse` for better conciseness and clarity. Added `Result.traverse`, optimized `Result.sequence` as well, and covered them with tests. I did not actually find any usages where `traverse` would be useful though. Fixes #5241. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent bf8f93a commit 3ba698a

File tree

4 files changed

+91
-14
lines changed

4 files changed

+91
-14
lines changed

core/api/daemon/src/mill/api/daemon/Result.scala

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package mill.api.daemon
22

3-
import scala.collection.{BuildFrom, mutable}
3+
import scala.collection.Factory
4+
import scala.util.boundary
45

56
/**
67
* Represents a computation that either succeeds with a value [[T]] or
@@ -47,16 +48,44 @@ object Result {
4748
case Right(value) => Result.Success(value)
4849
}
4950

50-
// implementation similar to scala.concurrent.Future#sequence
51-
def sequence[B, M[X] <: IterableOnce[X]](in: M[Result[B]])(
52-
implicit cbf: BuildFrom[M[Result[B]], B, M[B]]
51+
extension [A](rr: Result[Result[A]]) {
52+
def flatten: Result[A] = rr.flatMap(identity)
53+
}
54+
55+
/**
56+
* Converts a `Collection[Result[T]]` into a `Result[Collection[T]]`
57+
*/
58+
def sequence[B, M[X] <: IterableOnce[X]](in: M[Result[B]])(using
59+
factory: Factory[B, M[B]]
5360
): Result[M[B]] = {
54-
in.iterator
55-
.foldLeft[Result[mutable.Builder[B, M[B]]]](Success(cbf.newBuilder(in))) {
56-
case (acc, el) =>
57-
for (a <- acc; e <- el) yield a += e
61+
boundary {
62+
val builder = factory.newBuilder
63+
builder.sizeHint(in)
64+
in.iterator.foreach {
65+
case Success(b) => builder += b
66+
case Failure(error) => boundary.break(Failure(error))
5867
}
59-
.map(_.result())
68+
69+
builder.result()
70+
}
71+
}
72+
73+
/**
74+
* Converts a `Collection[T]` into a `Result[Collection[V]]` using the given `f: T => Result[V]`
75+
*/
76+
def traverse[A, B, Collection[x] <: IterableOnce[x]](collection: Collection[Result[A]])(
77+
f: A => Result[B]
78+
)(using factory: Factory[B, Collection[B]]): Result[Collection[B]] = {
79+
boundary {
80+
val builder = factory.newBuilder
81+
builder.sizeHint(collection)
82+
collection.iterator.map(_.flatMap(f)).foreach {
83+
case Success(b) => builder += b
84+
case Failure(error) => boundary.break(Failure(error))
85+
}
86+
87+
builder.result()
88+
}
6089
}
6190

6291
final class Exception(val error: String) extends java.lang.Exception(error)
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package mill.api.daemon
2+
3+
import utest.*
4+
5+
object ResultTests extends TestSuite {
6+
val tests = Tests {
7+
test("sequence") {
8+
test("empty") {
9+
val actual = Result.sequence(Seq.empty)
10+
assert(actual == Result.Success(Seq.empty))
11+
}
12+
13+
test("success") {
14+
val actual = Result.sequence(Seq(Result.Success(1), Result.Success(2)))
15+
assert(actual == Result.Success(Seq(1, 2)))
16+
}
17+
18+
test("failure") {
19+
val actual =
20+
Result.sequence(Seq(Result.Success(1), Result.Failure("fail"), Result.Success(2)))
21+
assert(actual == Result.Failure("fail"))
22+
}
23+
}
24+
25+
test("traverse") {
26+
test("empty") {
27+
test("success") {
28+
val actual = Result.traverse(Seq.empty)(Result.Success(_))
29+
assert(actual == Result.Success(Seq.empty))
30+
}
31+
32+
test("failure") {
33+
val actual = Result.traverse(Seq.empty)(_ => Result.Failure("fail"))
34+
assert(actual == Result.Success(Seq.empty))
35+
}
36+
}
37+
38+
test("success") {
39+
val actual = Result.traverse(Seq(1, 2))(Result.Success(_))
40+
assert(actual == Result.Success(Seq(1, 2)))
41+
}
42+
43+
test("failure") {
44+
val actual = Result.traverse(Seq(1, 2, 3)) { x =>
45+
if (x % 2 == 0) Result.Failure(s"fail $x") else Result.Success(x)
46+
}
47+
assert(actual == Result.Failure("fail 2"))
48+
}
49+
}
50+
}
51+
}

core/resolve/src/mill/resolve/ParseArgs.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ private[mill] object ParseArgs {
6161
.sequence(selectors.map(ExpandBraces.expandBraces))
6262
.map(_.flatten)
6363
selectors <- Result.sequence(expandedSelectors.map(extractSegments))
64-
} yield (selectors.toList, args)
64+
} yield (selectors.iterator.toList, args)
6565
}
6666

6767
def extractSelsAndArgs(

core/resolve/src/mill/resolve/Resolve.scala

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -316,10 +316,7 @@ private[mill] trait Resolve[T] {
316316
}
317317
}
318318

319-
Result
320-
.sequence(selected)
321-
.flatMap(Result.sequence(_))
322-
.map(_.flatten)
319+
Result.sequence(selected.map(_.flatten)).map(_.flatten)
323320
}
324321

325322
Result.sequence(resolved)

0 commit comments

Comments
 (0)