A few cleanups based on code review

Author: DougGregor

Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift

@@ -10,15 +10,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-//
-// This file defines the SyntaxRewriter, a class that performs a standard walk
-// and tree-rebuilding pattern.
-//
-// Subclassers of this class can override the walking behavior for any syntax
-// node and transform nodes however they like.
-//
-//===----------------------------------------------------------------------===//
-
 import SwiftDiagnostics
 import SwiftSyntax
 
@@ -88,12 +79,13 @@ extension SyntaxProtocol {
 /// than trivia).
 class ActiveSyntaxRewriter<Configuration: BuildConfiguration>: SyntaxRewriter {
   let configuration: Configuration
+  var diagnostics: [Diagnostic] = []
 
   init(configuration: Configuration) {
     self.configuration = configuration
   }
 
-  private func dropInactive<List: Collection & SyntaxCollection>(
+  private func dropInactive<List: SyntaxCollection>(
     _ node: List,
     elementAsIfConfig: (List.Element) -> IfConfigDeclSyntax?
   ) -> List {
@@ -105,7 +97,10 @@ class ActiveSyntaxRewriter<Configuration: BuildConfiguration>: SyntaxRewriter {
       // Find #ifs within the list.
       if let ifConfigDecl = elementAsIfConfig(element) {
         // Retrieve the active `#if` clause
-        let activeClause = ifConfigDecl.activeClause(in: configuration)
+        let (activeClause, localDiagnostics) = ifConfigDecl.activeClause(in: configuration)
+
+        // Add these diagnostics.
+        diagnostics.append(contentsOf: localDiagnostics)
 
         // If this is the first element that changed, note that we have
         // changes and add all prior elements to the list of new elements.
@@ -255,7 +250,8 @@ class ActiveSyntaxRewriter<Configuration: BuildConfiguration>: SyntaxRewriter {
       return dropInactive(outerBase: base, postfixIfConfig: postfixIfConfig)
     }
 
-    preconditionFailure("Unhandled postfix expression in #if elimination")
+    assertionFailure("Unhandled postfix expression in #if elimination")
+    return postfix
   }
 
   /// Drop inactive regions from a postfix `#if` configuration, applying the
@@ -265,7 +261,10 @@ class ActiveSyntaxRewriter<Configuration: BuildConfiguration>: SyntaxRewriter {
     postfixIfConfig: PostfixIfConfigExprSyntax
   ) -> ExprSyntax {
     // Retrieve the active `if` clause.
-    let activeClause = postfixIfConfig.config.activeClause(in: configuration)
+    let (activeClause, localDiagnostics) = postfixIfConfig.config.activeClause(in: configuration)
+
+    // Record these diagnostics.
+    diagnostics.append(contentsOf: localDiagnostics)
 
     guard case .postfixExpression(let postfixExpr) = activeClause?.elements
     else {

Sources/SwiftIfConfig/ActiveSyntaxVisitor.swift

@@ -9,6 +9,7 @@
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
+
 import SwiftDiagnostics
 import SwiftSyntax
 
@@ -55,9 +56,8 @@ open class ActiveSyntaxVisitor<Configuration: BuildConfiguration>: SyntaxVisitor
   }
 
   open override func visit(_ node: IfConfigDeclSyntax) -> SyntaxVisitorContinueKind {
-    let activeClause = node.activeClause(in: configuration) { diag in
-      self.diagnostics.append(diag)
-    }
+    let (activeClause, localDiagnostics) = node.activeClause(in: configuration)
+    diagnostics.append(contentsOf: localDiagnostics)
 
     numIfClausesVisited += 1
 
@@ -116,9 +116,9 @@ open class ActiveSyntaxAnyVisitor<Configuration: BuildConfiguration>: SyntaxAnyV
 
   open override func visit(_ node: IfConfigDeclSyntax) -> SyntaxVisitorContinueKind {
     // If there is an active clause, visit it's children.
-    let activeClause = node.activeClause(in: configuration) { diag in
-      self.diagnostics.append(diag)
-    }
+    let (activeClause, localDiagnostics) = node.activeClause(in: configuration)
+    diagnostics.append(contentsOf: localDiagnostics)
+
     if let activeClause, let elements = activeClause.elements {
       walk(Syntax(elements))
     }

Sources/SwiftIfConfig/ConfiguredRegionState.swift

@@ -9,6 +9,7 @@
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
+
 import SwiftDiagnostics
 import SwiftOperators
 import SwiftSyntax

Sources/SwiftIfConfig/ConfiguredRegions.swift

@@ -9,6 +9,7 @@
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
+
 import SwiftDiagnostics
 import SwiftSyntax
 
@@ -62,7 +63,7 @@ fileprivate class ConfiguredRegionVisitor<Configuration: BuildConfiguration>: Sy
   override func visit(_ node: IfConfigDeclSyntax) -> SyntaxVisitorContinueKind {
     // If we're in an active region, find the active clause. Otherwise,
     // there isn't one.
-    let activeClause = inActiveRegion ? node.activeClause(in: configuration) : nil
+    let activeClause = inActiveRegion ? node.activeClause(in: configuration).clause : nil
     for clause in node.clauses {
       // If this is the active clause, record it and then recurse into the
       // elements.

Sources/SwiftIfConfig/IfConfigDecl+IfConfig.swift

@@ -9,6 +9,7 @@
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
+
 import SwiftDiagnostics
 import SwiftSyntax
 
@@ -29,31 +30,32 @@ extension IfConfigDeclSyntax {
   /// command line, the second clause (containing `func g()`) would be returned. If neither was
   /// passed, this function will return `nil` to indicate that none of the regions are active.
   ///
-  /// If an error occurrs while processing any of the `#if` clauses,
+  /// If an error occurs while processing any of the `#if` clauses,
   /// that clause will be considered inactive and this operation will
   /// continue to evaluate later clauses.
   public func activeClause(
-    in configuration: some BuildConfiguration,
-    diagnosticHandler: ((Diagnostic) -> Void)? = nil
-  ) -> IfConfigClauseSyntax? {
+    in configuration: some BuildConfiguration
+  ) -> (clause: IfConfigClauseSyntax?, diagnostics: [Diagnostic]) {
+    var diagnostics: [Diagnostic] = []
     for clause in clauses {
       // If there is no condition, we have reached an unconditional clause. Return it.
       guard let condition = clause.condition else {
-        return clause
+        return (clause, diagnostics: diagnostics)
       }
 
       // If this condition evaluates true, return this clause.
       let isActive =
         (try? evaluateIfConfig(
           condition: condition,
-          configuration: configuration,
-          diagnosticHandler: diagnosticHandler
-        ))?.active ?? false
+          configuration: configuration
+        ) { 
+          diagnostics.append($0)
+        })?.active ?? false
       if isActive {
-        return clause
+        return (clause, diagnostics: diagnostics)
       }
     }
 
-    return nil
+    return (nil, diagnostics: diagnostics)
   }
 }

Sources/SwiftIfConfig/IfConfigError.swift

@@ -9,6 +9,7 @@
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
+
 import SwiftDiagnostics
 import SwiftSyntax
 

Sources/SwiftIfConfig/IfConfigEvaluation.swift

@@ -9,6 +9,7 @@
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
+
 import SwiftDiagnostics
 import SwiftSyntax
 

Sources/SwiftIfConfig/SwiftIfConfig.docc/SwiftIfConfig.md

@@ -1,4 +1,4 @@
-# `SwiftIfConfig`
+# SwiftIfConfig
 
 A library to evaluate `#if` conditionals within a Swift syntax tree.
 

Sources/SwiftIfConfig/SyntaxLiteralUtils.swift

@@ -9,6 +9,7 @@
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
+
 import SwiftSyntax
 
 extension BooleanLiteralExprSyntax {

Sources/SwiftIfConfig/SyntaxProtocol+IfConfig.swift

@@ -9,6 +9,7 @@
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
+
 import SwiftDiagnostics
 import SwiftSyntax
 
@@ -43,10 +44,11 @@ extension SyntaxProtocol {
       if let ifConfigClause = currentNode.as(IfConfigClauseSyntax.self),
         let ifConfigDecl = ifConfigClause.parent?.parent?.as(IfConfigDeclSyntax.self)
       {
-        let activeClause = ifConfigDecl.activeClause(
-          in: configuration,
-          diagnosticHandler: diagnosticHandler
-        )
+        let (activeClause, localDiagnostics) = ifConfigDecl.activeClause(in: configuration)
+
+        for diag in localDiagnostics {
+          diagnosticHandler?(diag)
+        }
 
         if activeClause != ifConfigClause {
           // This was not the active clause, so we know that we're in an