Teach the ActiveSyntaxRewriter to use either ConfiguredRegions or BuildConfiguration

Instead of always forcing the construction of ConfiguredRegions ahead of
time, introduce the notion of an ActiveClauseEvaluator that stores
either configured regions or a build configuration, and can query
for the active clause within a given #if declaration from either
representation. Use this in ActiveSyntaxRewriter to make the operation
itself single-pass, always.

Always test both paths to make sure they stay in sync.

Author: DougGregor

Sources/SwiftIfConfig/ActiveClauseEvaluator.swift

@@ -0,0 +1,50 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import SwiftDiagnostics
+import SwiftSyntax
+
+/// Captures sufficient information to determine the active clause for an `#if`
+/// either by querying existing configured regions or by evaluating the
+/// clause's conditions against a build configuration.
+enum ActiveClauseEvaluator {
+  case configuredRegions(ConfiguredRegions)
+  case configuration(any BuildConfiguration)
+
+  /// Previously-known diagnostics.
+  var priorDiagnostics: [Diagnostic] {
+    switch self {
+    case .configuredRegions(let configuredRegions):
+      return configuredRegions.diagnostics
+    case .configuration:
+      return []
+    }
+  }
+
+  /// Determine which clause of an `#if` declaration is active, if any.
+  ///
+  /// If this evaluation produced any diagnostics, they will be appended to
+  /// the diagnostics parameter.
+  func activeClause(
+    for node: IfConfigDeclSyntax,
+    diagnostics: inout [Diagnostic]
+  ) -> IfConfigClauseSyntax? {
+    switch self {
+    case .configuredRegions(let configuredRegions):
+      return configuredRegions.activeClause(for: node)
+    case .configuration(let configuration):
+      let (activeClause, localDiagnostics) = node.activeClause(in: configuration)
+      diagnostics.append(contentsOf: localDiagnostics)
+      return activeClause
+    }
+  }
+}

Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift

@@ -52,13 +52,15 @@ extension SyntaxProtocol {
     in configuration: some BuildConfiguration,
     retainFeatureCheckIfConfigs: Bool
   ) -> (result: Syntax, diagnostics: [Diagnostic]) {
-    let configuredRegions = configuredRegions(in: configuration)
+    // Rewrite the syntax tree by removing the inactive clauses
+    // from each #if (along with the #ifs themselves).
+    let rewriter = ActiveSyntaxRewriter(
+      configuration: configuration,
+      retainFeatureCheckIfConfigs: retainFeatureCheckIfConfigs
+    )
     return (
-      result: configuredRegions.removingInactive(
-        from: self,
-        retainFeatureCheckIfConfigs: retainFeatureCheckIfConfigs
-      ),
-      diagnostics: configuredRegions.diagnostics
+      rewriter.rewrite(Syntax(self)),
+      rewriter.diagnostics
     )
   }
 }
@@ -144,15 +146,21 @@ extension ConfiguredRegions {
 /// For any other target platforms, the resulting tree will be empty (other
 /// than trivia).
 class ActiveSyntaxRewriter: SyntaxRewriter {
-  let configuredRegions: ConfiguredRegions
+  let activeClauses: ActiveClauseEvaluator
   var diagnostics: [Diagnostic]
 
   /// Whether to retain `#if` blocks containing compiler and feature checks.
   var retainFeatureCheckIfConfigs: Bool
 
   init(configuredRegions: ConfiguredRegions, retainFeatureCheckIfConfigs: Bool) {
-    self.configuredRegions = configuredRegions
-    self.diagnostics = configuredRegions.diagnostics
+    self.activeClauses = .configuredRegions(configuredRegions)
+    self.diagnostics = activeClauses.priorDiagnostics
+    self.retainFeatureCheckIfConfigs = retainFeatureCheckIfConfigs
+  }
+
+  init(configuration: some BuildConfiguration, retainFeatureCheckIfConfigs: Bool) {
+    self.activeClauses = .configuration(configuration)
+    self.diagnostics = activeClauses.priorDiagnostics
     self.retainFeatureCheckIfConfigs = retainFeatureCheckIfConfigs
   }
 
@@ -183,7 +191,7 @@ class ActiveSyntaxRewriter: SyntaxRewriter {
         (!retainFeatureCheckIfConfigs || !ifConfigDecl.containsFeatureCheck)
       {
         // Retrieve the active `#if` clause
-        let activeClause = configuredRegions.activeClause(for: ifConfigDecl)
+        let activeClause = activeClauses.activeClause(for: ifConfigDecl, diagnostics: &diagnostics)
 
         noteElementChanged(at: elementIndex)
 
@@ -346,7 +354,7 @@ class ActiveSyntaxRewriter: SyntaxRewriter {
     }
 
     // Retrieve the active `if` clause.
-    let activeClause = configuredRegions.activeClause(for: postfixIfConfig.config)
+    let activeClause = activeClauses.activeClause(for: postfixIfConfig.config, diagnostics: &diagnostics)
 
     guard case .postfixExpression(let postfixExpr) = activeClause?.elements
     else {

Sources/SwiftIfConfig/CMakeLists.txt

@@ -7,6 +7,7 @@
 # See http://swift.org/CONTRIBUTORS.txt for Swift project authors
 
 add_swift_syntax_library(SwiftIfConfig
+  ActiveClauseEvaluator.swift
   ActiveSyntaxVisitor.swift
   ActiveSyntaxRewriter.swift
   BuildConfiguration.swift

Tests/SwiftIfConfigTest/VisitorTests.swift

@@ -337,8 +337,8 @@ public class VisitorTests: XCTestCase {
   }
 }
 
-/// Assert that applying the given build configuration to the source code
-/// returns the expected source and diagnostics.
+/// Assert that removing any inactive code according to the given build
+/// configuration returns the expected source and diagnostics.
 fileprivate func assertRemoveInactive(
   _ source: String,
   configuration: some BuildConfiguration,
@@ -351,39 +351,59 @@ fileprivate func assertRemoveInactive(
   var parser = Parser(source)
   let tree = SourceFileSyntax.parse(from: &parser)
 
-  let (treeWithoutInactive, actualDiagnostics) = tree.removingInactive(
-    in: configuration,
-    retainFeatureCheckIfConfigs: retainFeatureCheckIfConfigs
-  )
-
-  // Check the resulting tree.
-  assertStringsEqualWithDiff(
-    treeWithoutInactive.description,
-    expectedSource,
-    file: file,
-    line: line
-  )
+  for useConfiguredRegions in [false, true] {
+    let fromDescription = useConfiguredRegions ? "configured regions" : "build configuration"
+    let treeWithoutInactive: Syntax
+    let actualDiagnostics: [Diagnostic]
+
+    if useConfiguredRegions {
+      let configuredRegions = tree.configuredRegions(in: configuration)
+      actualDiagnostics = configuredRegions.diagnostics
+      treeWithoutInactive = configuredRegions.removingInactive(
+        from: tree,
+        retainFeatureCheckIfConfigs: retainFeatureCheckIfConfigs
+      )
+    } else {
+      (treeWithoutInactive, actualDiagnostics) = tree.removingInactive(
+        in: configuration,
+        retainFeatureCheckIfConfigs: retainFeatureCheckIfConfigs
+      )
+    }
 
-  // Check the diagnostics.
-  if actualDiagnostics.count != expectedDiagnostics.count {
-    XCTFail(
-      """
-      Expected \(expectedDiagnostics.count) diagnostics, but got \(actualDiagnostics.count):
-      \(actualDiagnostics.map(\.debugDescription).joined(separator: "\n"))
-      """,
+    // Check the resulting tree.
+    assertStringsEqualWithDiff(
+      treeWithoutInactive.description,
+      expectedSource,
+      "Active code (\(fromDescription))",
       file: file,
       line: line
     )
-  } else {
-    for (actualDiag, expectedDiag) in zip(actualDiagnostics, expectedDiagnostics) {
-      assertDiagnostic(
-        actualDiag,
-        in: .tree(tree),
-        expected: expectedDiag,
-        failureHandler: {
-          XCTFail($0.message, file: $0.location.staticFilePath, line: $0.location.unsignedLine)
-        }
+
+    // Check the diagnostics.
+    if actualDiagnostics.count != expectedDiagnostics.count {
+      XCTFail(
+        """
+        Expected \(expectedDiagnostics.count) diagnostics, but got \(actualDiagnostics.count) via \(fromDescription):
+        \(actualDiagnostics.map(\.debugDescription).joined(separator: "\n"))
+        """,
+        file: file,
+        line: line
       )
+    } else {
+      for (actualDiag, expectedDiag) in zip(actualDiagnostics, expectedDiagnostics) {
+        assertDiagnostic(
+          actualDiag,
+          in: .tree(tree),
+          expected: expectedDiag,
+          failureHandler: {
+            XCTFail(
+              $0.message + " via \(fromDescription)",
+              file: $0.location.staticFilePath,
+              line: $0.location.unsignedLine
+            )
+          }
+        )
+      }
     }
   }
 }