Change Graph's 'mapValues' APIs to pass key path in addition to value to transform closure (#372)

stmontgomery · web-flow · commit 6676949df4ac · 2024-04-23T20:29:09.000-05:00
This is a small change which begins passing key path _in addition to_ value to Graph's `mapValue` and `compactMapValue` APIs. ### Motivation: This was extracted out from work in #366, and is initially intended for use in that PR but is a generally-useful enhancement. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
diff --git a/Sources/Testing/Running/Configuration.TestFilter.swift b/Sources/Testing/Running/Configuration.TestFilter.swift
@@ -219,7 +219,7 @@ extension Configuration.TestFilter.Kind {
     case .unfiltered:
       return testGraph
     case let .precomputed(selection, membership):
-      return testGraph.mapValues { test in
+      return testGraph.mapValues { _, test in
         guard let test else {
           return nil
         }
@@ -250,7 +250,9 @@ extension Configuration.TestFilter.Kind {
       return zip(
         lhs.apply(to: testGraph),
         rhs.apply(to: testGraph)
-      ).mapValues(op.functionValue)
+      ).mapValues { _, value in
+        op.functionValue(value.0, value.1)
+      }
     }
   }
 }
@@ -271,7 +273,7 @@ extension Configuration.TestFilter {
     // combined test filters. It is only consulted on the outermost call to
     // apply(to:), not in _apply(to:).
     if !includeHiddenTests {
-      result = result.mapValues { test in
+      result = result.mapValues { _, test in
         (test?.isHidden == true) ? nil : test
       }
     }
diff --git a/Sources/Testing/Running/Runner.Plan.swift b/Sources/Testing/Running/Runner.Plan.swift
@@ -237,13 +237,13 @@ extension Runner.Plan {
 
     // Now that we have allowed all the traits to update their corresponding
     // actions, recursively apply those actions to child tests in the graph.
-    actionGraph = actionGraph.mapValues { action in
+    actionGraph = actionGraph.mapValues { _, action in
       (action, recursivelyApply: action.isRecursive)
     }
 
     // Zip the tests and actions together and return them.
-    return zip(testGraph, actionGraph).mapValues { test, action in
-      test.map { Step(test: $0, action: action) }
+    return zip(testGraph, actionGraph).mapValues { _, pair in
+      pair.0.map { Step(test: $0, action: pair.1) }
     }
   }
 
diff --git a/Sources/Testing/Support/Graph.swift b/Sources/Testing/Support/Graph.swift
@@ -406,7 +406,7 @@ extension Graph {
   ///
   /// - Parameters:
   ///   - keyPath: The key path to use for the root node when passing it to
-  ///    `body`.
+  ///     `body`.
   ///   - body: A closure that is invoked once per element in the graph. The
   ///     key path and leaf value of each node are passed to the closure.
   ///
@@ -485,18 +485,18 @@ extension Graph {
   ///
   /// - Parameters:
   ///   - transform: A closure that is invoked once per element in the graph.
-  ///     The leaf value of each node is passed to this closure and its result
-  ///     is used as the corresponding value in the new graph. If the result is
-  ///     `nil`, the node and all of its child nodes are omitted from the new
-  ///     graph.
+  ///     The key path and leaf value of each node are passed to this closure
+  ///     and its result is used as the corresponding value in the new graph. If
+  ///     the result is `nil`, the node and all of its child nodes are omitted
+  ///     from the new graph.
   ///
   /// - Returns: A graph containing the transformed nodes of this graph at the
   ///   same key paths, with `nil` values omitted.
   ///
   /// - Throws: Whatever is thrown by `transform`.
   ///
   /// This function iterates depth-first.
-  func compactMapValues<U>(_ transform: (V) throws -> U?) rethrows -> Graph<K, U>? {
+  func compactMapValues<U>(_ transform: (Element) throws -> U?) rethrows -> Graph<K, U>? {
     try compactMapValues {
       try transform($0).map { ($0, false) }
     }
@@ -507,18 +507,18 @@ extension Graph {
   ///
   /// - Parameters:
   ///   - transform: A closure that is invoked once per element in the graph.
-  ///     The leaf value of each node is passed to this closure and its result
-  ///     is used as the corresponding value in the new graph. If the result is
-  ///     `nil`, the node and all of its child nodes are omitted from the new
-  ///     graph.
+  ///     The key path and leaf value of each node are passed to this closure
+  ///     and its result is used as the corresponding value in the new graph. If
+  ///     the result is `nil`, the node and all of its child nodes are omitted
+  ///     from the new graph.
   ///
   /// - Returns: A graph containing the transformed nodes of this graph at the
   ///   same key paths, with `nil` values omitted.
   ///
   /// - Throws: Whatever is thrown by `transform`.
   ///
   /// This function iterates depth-first.
-  func compactMapValues<U>(_ transform: (V) async throws -> U?) async rethrows -> Graph<K, U>? {
+  func compactMapValues<U>(_ transform: (Element) async throws -> U?) async rethrows -> Graph<K, U>? {
     try await compactMapValues {
       try await transform($0).map { ($0, false) }
     }
@@ -530,31 +530,54 @@ extension Graph {
   ///
   /// - Parameters:
   ///   - transform: A closure that is invoked once per element in the graph.
-  ///     The leaf value of each node is passed to this closure. The result of
-  ///     the closure is a tuple containing the new value and specifying whether
-  ///     or not the new value should also be applied to each descendant node.
-  ///     If `true`, `transform` is not invoked for those descendant nodes. If
-  ///     the result is `nil`, the node and all of its child nodes are omitted
-  ///     from the new graph.
+  ///     The key path and leaf value of each node are passed to this closure.
+  ///     The result of the closure is a tuple containing the new value and
+  ///     specifying whether or not the new value should also be applied to each
+  ///     descendant node. If `true`, `transform` is not invoked for those
+  ///     descendant nodes. If the result is `nil`, the node and all of its
+  ///     child nodes are omitted from the new graph.
   ///
   /// - Returns: A graph containing the transformed nodes of this graph at the
   ///   same key paths, with `nil` values omitted.
   ///
   /// - Throws: Whatever is thrown by `transform`.
   ///
   /// This function iterates depth-first.
-  func compactMapValues<U>(_ transform: (V) throws -> (U, recursivelyApply: Bool)?) rethrows -> Graph<K, U>? {
-    guard let (newValue, recursivelyApply) = try transform(value) else {
+  func compactMapValues<U>(_ transform: (Element) throws -> (U, recursivelyApply: Bool)?) rethrows -> Graph<K, U>? {
+    try _compactMapValues(keyPath: []) {
+      try transform(($0, $1))
+    }
+  }
+
+  /// The recursive implementation of `compactMapValues(_:)`.
+  ///
+  /// - Parameters:
+  ///   - keyPath: The key path to use for the root node when passing it to
+  ///     `transform`.
+  ///   - transform: A closure that is invoked once per element in the graph.
+  ///     The key path and leaf value of each node are passed to this closure.
+  ///     The result of the closure is a tuple containing the new value and
+  ///     specifying whether or not the new value should also be applied to each
+  ///     descendant node. If `true`, `transform` is not invoked for those
+  ///     descendant nodes. If the result is `nil`, the node and all of its
+  ///     child nodes are omitted from the new graph.
+  ///
+  /// - Throws: Whatever is thrown by `transform`.
+  private func _compactMapValues<U>(keyPath: [K], _ transform: (Element) throws -> (U, recursivelyApply: Bool)?) rethrows -> Graph<K, U>? {
+    guard let (newValue, recursivelyApply) = try transform((keyPath, value)) else {
       return nil
     }
 
     var newChildren = [K: Graph<K,U>]()
     newChildren.reserveCapacity(children.count)
     for (key, child) in children {
+      var childKeyPath = keyPath
+      childKeyPath.append(key)
+
       if recursivelyApply {
-        newChildren[key] = child.compactMapValues { _ in (newValue, true) }
+        newChildren[key] = child._compactMapValues(keyPath: childKeyPath) { _ in (newValue, true) }
       } else {
-        newChildren[key] = try child.compactMapValues(transform)
+        newChildren[key] = try child._compactMapValues(keyPath: childKeyPath, transform)
       }
     }
 
@@ -567,31 +590,54 @@ extension Graph {
   ///
   /// - Parameters:
   ///   - transform: A closure that is invoked once per element in the graph.
-  ///     The leaf value of each node is passed to this closure. The result of
-  ///     the closure is a tuple containing the new value and specifying whether
-  ///     or not the new value should also be applied to each descendant node.
-  ///     If `true`, `transform` is not invoked for those descendant nodes. If
-  ///     the result is `nil`, the node and all of its child nodes are omitted
-  ///     from the new graph.
+  ///     The key path and leaf value of each node are passed to this closure.
+  ///     The result of the closure is a tuple containing the new value and
+  ///     specifying whether or not the new value should also be applied to each
+  ///     descendant node. If `true`, `transform` is not invoked for those
+  ///     descendant nodes. If the result is `nil`, the node and all of its
+  ///     child nodes are omitted from the new graph.
   ///
   /// - Returns: A graph containing the transformed nodes of this graph at the
   ///   same key paths, with `nil` values omitted.
   ///
   /// - Throws: Whatever is thrown by `transform`.
   ///
   /// This function iterates depth-first.
-  func compactMapValues<U>(_ transform: (V) async throws -> (U, recursivelyApply: Bool)?) async rethrows -> Graph<K, U>? {
-    guard let (newValue, recursivelyApply) = try await transform(value) else {
+  func compactMapValues<U>(_ transform: (Element) async throws -> (U, recursivelyApply: Bool)?) async rethrows -> Graph<K, U>? {
+    try await _compactMapValues(keyPath: []) {
+      try await transform(($0, $1))
+    }
+  }
+
+  /// The recursive implementation of `compactMapValues(_:)`.
+  ///
+  /// - Parameters:
+  ///   - keyPath: The key path to use for the root node when passing it to
+  ///     `transform`.
+  ///   - transform: A closure that is invoked once per element in the graph.
+  ///     The key path and leaf value of each node are passed to this closure.
+  ///     The result of the closure is a tuple containing the new value and
+  ///     specifying whether or not the new value should also be applied to each
+  ///     descendant node. If `true`, `transform` is not invoked for those
+  ///     descendant nodes. If the result is `nil`, the node and all of its
+  ///     child nodes are omitted from the new graph.
+  ///
+  /// - Throws: Whatever is thrown by `transform`.
+  private func _compactMapValues<U>(keyPath: [K], _ transform: (Element) async throws -> (U, recursivelyApply: Bool)?) async rethrows -> Graph<K, U>? {
+    guard let (newValue, recursivelyApply) = try await transform((keyPath, value)) else {
       return nil
     }
 
     var newChildren = [K: Graph<K,U>]()
     newChildren.reserveCapacity(children.count)
     for (key, child) in children {
+      var childKeyPath = keyPath
+      childKeyPath.append(key)
+
       if recursivelyApply {
-        newChildren[key] = child.compactMapValues { _ in (newValue, true) }
+        newChildren[key] = child._compactMapValues(keyPath: childKeyPath) { _ in (newValue, true) }
       } else {
-        newChildren[key] = try await child.compactMapValues(transform)
+        newChildren[key] = try await child._compactMapValues(keyPath: childKeyPath, transform)
       }
     }
 
@@ -603,16 +649,16 @@ extension Graph {
   ///
   /// - Parameters:
   ///   - transform: A closure that is invoked once per element in the graph.
-  ///     The leaf value of each node is passed to this closure and its result
-  ///     is used as the corresponding value in the new graph.
+  ///     The key path and leaf value of each node are passed to this closure
+  ///     and its result is used as the corresponding value in the new graph.
   ///
   /// - Returns: A graph containing the transformed nodes of this graph at the
   ///   same key paths.
   ///
   /// - Throws: Whatever is thrown by `transform`.
   ///
   /// This function iterates depth-first.
-  func mapValues<U>(_ transform: (V) throws -> U) rethrows -> Graph<K, U> {
+  func mapValues<U>(_ transform: (Element) throws -> U) rethrows -> Graph<K, U> {
     try compactMapValues(transform)!
   }
 
@@ -621,16 +667,16 @@ extension Graph {
   ///
   /// - Parameters:
   ///   - transform: A closure that is invoked once per element in the graph.
-  ///     The leaf value of each node is passed to this closure and its result
-  ///     is used as the corresponding value in the new graph.
+  ///     The key path and leaf value of each node are passed to this closure
+  ///     and its result is used as the corresponding value in the new graph.
   ///
   /// - Returns: A graph containing the transformed nodes of this graph at the
   ///   same key paths.
   ///
   /// - Throws: Whatever is thrown by `transform`.
   ///
   /// This function iterates depth-first.
-  func mapValues<U>(_ transform: (V) async throws -> U) async rethrows -> Graph<K, U> {
+  func mapValues<U>(_ transform: (Element) async throws -> U) async rethrows -> Graph<K, U> {
     try await compactMapValues(transform)!
   }
 
@@ -640,18 +686,19 @@ extension Graph {
   ///
   /// - Parameters:
   ///   - transform: A closure that is invoked once per element in the graph.
-  ///     The leaf value of each node is passed to this closure. The result of
-  ///     the closure is a tuple containing the new value and specifying whether
-  ///     or not the new value should also be applied to each descendant node.
-  ///     If `true`, `transform` is not invoked for those descendant nodes.
+  ///     The key path and leaf value of each node are passed to this closure.
+  ///     The result of the closure is a tuple containing the new value and
+  ///     specifying whether or not the new value should also be applied to each
+  ///     descendant node. If `true`, `transform` is not invoked for those
+  ///     descendant nodes.
   ///
   /// - Returns: A graph containing the transformed nodes of this graph at the
   ///   same key paths.
   ///
   /// - Throws: Whatever is thrown by `transform`.
   ///
   /// This function iterates depth-first.
-  func mapValues<U>(_ transform: (V) throws -> (U, recursivelyApply: Bool)) rethrows -> Graph<K, U> {
+  func mapValues<U>(_ transform: (Element) throws -> (U, recursivelyApply: Bool)) rethrows -> Graph<K, U> {
     try compactMapValues(transform)!
   }
 
@@ -661,18 +708,19 @@ extension Graph {
   ///
   /// - Parameters:
   ///   - transform: A closure that is invoked once per element in the graph.
-  ///     The leaf value of each node is passed to this closure. The result of
-  ///     the closure is a tuple containing the new value and specifying whether
-  ///     or not the new value should also be applied to each descendant node.
-  ///     If `true`, `transform` is not invoked for those descendant nodes.
+  ///     The key path and leaf value of each node are passed to this closure.
+  ///     The result of the closure is a tuple containing the new value and
+  ///     specifying whether or not the new value should also be applied to each
+  ///     descendant node. If `true`, `transform` is not invoked for those
+  ///     descendant nodes.
   ///
   /// - Returns: A graph containing the transformed nodes of this graph at the
   ///   same key paths.
   ///
   /// - Throws: Whatever is thrown by `transform`.
   ///
   /// This function iterates depth-first.
-  func mapValues<U>(_ transform: (V) async throws -> (U, recursivelyApply: Bool)) async rethrows -> Graph<K, U> {
+  func mapValues<U>(_ transform: (Element) async throws -> (U, recursivelyApply: Bool)) async rethrows -> Graph<K, U> {
     try await compactMapValues(transform)!
   }
 
diff --git a/Tests/TestingTests/Support/GraphTests.swift b/Tests/TestingTests/Support/GraphTests.swift

Original file line number	Diff line number	Diff line change
`@@ -219,7 +219,7 @@ extension Configuration.TestFilter.Kind {`
`219`	`219`	`case .unfiltered:`
`220`	`220`	`return testGraph`
`221`	`221`	`case let .precomputed(selection, membership):`
`222`		`- return testGraph.mapValues { test in`
	`222`	`+ return testGraph.mapValues { _, test in`
`223`	`223`	`guard let test else {`
`224`	`224`	`return nil`
`225`	`225`	`}`
`@@ -250,7 +250,9 @@ extension Configuration.TestFilter.Kind {`
`250`	`250`	`return zip(`
`251`	`251`	`lhs.apply(to: testGraph),`
`252`	`252`	`rhs.apply(to: testGraph)`
`253`		`- ).mapValues(op.functionValue)`
	`253`	`+ ).mapValues { _, value in`
	`254`	`+ op.functionValue(value.0, value.1)`
	`255`	`+ }`
`254`	`256`	`}`
`255`	`257`	`}`
`256`	`258`	`}`
`@@ -271,7 +273,7 @@ extension Configuration.TestFilter {`
`271`	`273`	`// combined test filters. It is only consulted on the outermost call to`
`272`	`274`	`// apply(to:), not in _apply(to:).`
`273`	`275`	`if !includeHiddenTests {`
`274`		`- result = result.mapValues { test in`
	`276`	`+ result = result.mapValues { _, test in`
`275`	`277`	`(test?.isHidden == true) ? nil : test`
`276`	`278`	`}`
`277`	`279`	`}`
Original file line number	Diff line number	Diff line change
`@@ -237,13 +237,13 @@ extension Runner.Plan {`
`237`	`237`
`238`	`238`	`// Now that we have allowed all the traits to update their corresponding`
`239`	`239`	`// actions, recursively apply those actions to child tests in the graph.`
`240`		`- actionGraph = actionGraph.mapValues { action in`
	`240`	`+ actionGraph = actionGraph.mapValues { _, action in`
`241`	`241`	`(action, recursivelyApply: action.isRecursive)`
`242`	`242`	`}`
`243`	`243`
`244`	`244`	`// Zip the tests and actions together and return them.`
`245`		`- return zip(testGraph, actionGraph).mapValues { test, action in`
`246`		`- test.map { Step(test: $0, action: action) }`
	`245`	`+ return zip(testGraph, actionGraph).mapValues { _, pair in`
	`246`	`+ pair.0.map { Step(test: $0, action: pair.1) }`
`247`	`247`	`}`
`248`	`248`	`}`
`249`	`249`