Address a bunch of minor code review comments

Author: DougGregor

Package.swift

@@ -143,7 +143,7 @@ let package = Package(
 
     .target(
       name: "SwiftIfConfig",
-      dependencies: ["SwiftSyntax", "SwiftOperators"],
+      dependencies: ["SwiftSyntax", "SwiftDiagnostics", "SwiftOperators"],
       exclude: ["CMakeLists.txt"]
     ),
 

Sources/SwiftIfConfig/ActiveSyntaxRewriter.swift

@@ -25,15 +25,17 @@ extension SyntaxProtocol {
   /// clauses, e.g., `#if FOO > 10`, then the condition will be
   /// considered to have failed and the clauses's elements will be
   /// removed.
-  public func removingInactive(in configuration: some BuildConfiguration) -> (Syntax, [Diagnostic]) {
+  public func removingInactive(
+    in configuration: some BuildConfiguration
+  ) -> (result: Syntax, diagnostics: [Diagnostic]) {
     // First pass: Find all of the active clauses for the #ifs we need to
     // visit, along with any diagnostics produced along the way. This process
     // does not change the tree in any way.
     let visitor = ActiveSyntaxVisitor(viewMode: .sourceAccurate, configuration: configuration)
     visitor.walk(self)
 
     // If there were no active clauses to visit, we're done!
-    if visitor.numIfClausesVisited == 0 {
+    if !visitor.visitedAnyIfClauses {
       return (Syntax(self), visitor.diagnostics)
     }
 

Sources/SwiftIfConfig/ActiveSyntaxVisitor.swift

@@ -37,33 +37,35 @@ import SwiftSyntax
 /// it would not visit either `f` or `g`.
 ///
 /// All notes visited by this visitor will have the "active" state, i.e.,
-/// `node.isActive(in: configuration)` will evaluate to `.active` or will
-/// throw. When errors occur, they will be recorded in the set of
+/// `node.isActive(in: configuration)` will have evaluated to `.active`
+/// When errors occur, they will be recorded in the set of
 /// diagnostics.
 open class ActiveSyntaxVisitor<Configuration: BuildConfiguration>: SyntaxVisitor {
   /// The build configuration, which will be queried for each relevant `#if`.
   public let configuration: Configuration
 
-  /// The set of diagnostics accumulated during this walk of active syntax.
-  public var diagnostics: [Diagnostic] = []
+  /// The diagnostics accumulated during this walk of active syntax.
+  public private(set) var diagnostics: [Diagnostic] = []
 
-  /// The number of "#if" clauses that were visited.
-  var numIfClausesVisited: Int = 0
+  /// Whether we visited any "#if" clauses.
+  var visitedAnyIfClauses: Bool = false
 
   public init(viewMode: SyntaxTreeViewMode, configuration: Configuration) {
     self.configuration = configuration
     super.init(viewMode: viewMode)
   }
 
   open override func visit(_ node: IfConfigDeclSyntax) -> SyntaxVisitorContinueKind {
+    // Note: there is a clone of this code in ActiveSyntaxAnyVisitor. If you
+    // change one, please also change the other.
     let (activeClause, localDiagnostics) = node.activeClause(in: configuration)
     diagnostics.append(contentsOf: localDiagnostics)
 
-    numIfClausesVisited += 1
+    visitedAnyIfClauses = true
 
     // If there is an active clause, visit it's children.
     if let activeClause, let elements = activeClause.elements {
-      walk(Syntax(elements))
+      walk(elements)
     }
 
     // Skip everything else in the #if.
@@ -95,32 +97,30 @@ open class ActiveSyntaxVisitor<Configuration: BuildConfiguration>: SyntaxVisitor
 /// it would not visit either `f` or `g`.
 ///
 /// All notes visited by this visitor will have the "active" state, i.e.,
-/// `node.isActive(in: configuration)` will evaluate to `.active` or will
-/// throw.
-///
-/// All notes visited by this visitor will have the "active" state, i.e.,
-/// `node.isActive(in: configuration)` will evaluate to `.active` or will
-/// throw. When errors occur, they will be recorded in the set of
-/// diagnostivs.
+/// `node.isActive(in: configuration)` will have evaluated to `.active`.
+/// When errors occur, they will be recorded in the array of diagnostics.
 open class ActiveSyntaxAnyVisitor<Configuration: BuildConfiguration>: SyntaxAnyVisitor {
   /// The build configuration, which will be queried for each relevant `#if`.
   public let configuration: Configuration
 
-  /// The set of diagnostics accumulated during this walk of active syntax.
-  public var diagnostics: [Diagnostic] = []
+  /// The diagnostics accumulated during this walk of active syntax.
+  public private(set) var diagnostics: [Diagnostic] = []
 
   public init(viewMode: SyntaxTreeViewMode, configuration: Configuration) {
     self.configuration = configuration
     super.init(viewMode: viewMode)
   }
 
   open override func visit(_ node: IfConfigDeclSyntax) -> SyntaxVisitorContinueKind {
+    // Note: there is a clone of this code in ActiveSyntaxVisitor. If you
+    // change one, please also change the other.
+
     // If there is an active clause, visit it's children.
     let (activeClause, localDiagnostics) = node.activeClause(in: configuration)
     diagnostics.append(contentsOf: localDiagnostics)
 
     if let activeClause, let elements = activeClause.elements {
-      walk(Syntax(elements))
+      walk(elements)
     }
 
     // Skip everything else in the #if.

Sources/SwiftIfConfig/BuildConfiguration.swift

@@ -253,7 +253,7 @@ public protocol BuildConfiguration {
   ///
   /// The language version can be queried with the `swift` directive that checks
   /// how the supported language version compares, as described by
-  /// [SE-0212](https://github.com/apple/swift-evolution/blob/main/proposals/0212-compiler-version-directive.md). For example:
+  /// [SE-0212](https://github.com/swiftlang/swift-evolution/blob/main/proposals/0212-compiler-version-directive.md). For example:
   ///
   /// ```swift
   /// #if swift(>=5.5)

Sources/SwiftIfConfig/CMakeLists.txt

@@ -25,5 +25,4 @@ add_swift_syntax_library(SwiftIfConfig
 target_link_swift_syntax_libraries(SwiftIfConfig PUBLIC
   SwiftSyntax
   SwiftDiagnostics
-  SwiftOperators
-  SwiftParser)
+  SwiftOperators)