Address a number of review comments (thanks, Alex!)

Author: DougGregor

Sources/SwiftIfConfig/IfConfigEvaluation.swift

@@ -72,9 +72,14 @@ enum IfConfigError: Error, CustomStringConvertible {
 extension VersionTuple {
   /// Parse a compiler build version of the form "5007.*.1.2.3*", which is
   /// used by an older if configuration form `_compiler_version("...")`.
+  /// - Parameters:
+  ///   - versionString: The version string for the compiler build version that
+  ///   we are parsing.
+  ///   - versionSyntax: The syntax node that contains the version string, used
+  ///   only for diagnostic purposes.
   fileprivate init(
     parsingCompilerBuildVersion versionString: String,
-    _ syntax: ExprSyntax
+    _ versionSyntax: ExprSyntax
   ) throws {
     components = []
 
@@ -86,7 +91,7 @@ extension VersionTuple {
       let limit = components.isEmpty ? 9223371 : 999
       if value < 0 || value > limit {
         // FIXME: Can we provide a more precise location here?
-        throw IfConfigError.compilerVersionOutOfRange(value: value, upperLimit: limit, syntax: syntax)
+        throw IfConfigError.compilerVersionOutOfRange(value: value, upperLimit: limit, syntax: versionSyntax)
       }
 
       components.append(value)
@@ -96,29 +101,29 @@ extension VersionTuple {
     for (index, componentString) in componentStrings.enumerated() {
       // Check ahead of time for empty version components
       if componentString.isEmpty {
-        throw IfConfigError.emptyVersionComponent(syntax: syntax)
+        throw IfConfigError.emptyVersionComponent(syntax: versionSyntax)
       }
 
       // The second component is always "*", and is never used for comparison.
       if index == 1 {
         if componentString != "*" {
-          throw IfConfigError.compilerVersionSecondComponentNotWildcard(syntax: syntax)
+          throw IfConfigError.compilerVersionSecondComponentNotWildcard(syntax: versionSyntax)
         }
         try recordComponent(0)
         continue
       }
 
       // Every other component must be an integer value.
       guard let component = Int(componentString) else {
-        throw IfConfigError.invalidVersionOperand(name: "_compiler_version", syntax: syntax)
+        throw IfConfigError.invalidVersionOperand(name: "_compiler_version", syntax: versionSyntax)
       }
 
       try recordComponent(component)
     }
 
     // Only allowed to specify up to 5 version components.
     if components.count > 5 {
-      throw IfConfigError.compilerVersionTooManyComponents(syntax: syntax)
+      throw IfConfigError.compilerVersionTooManyComponents(syntax: versionSyntax)
     }
 
     // In the beginning, '_compiler_version(string-literal)' was designed for a
@@ -159,6 +164,13 @@ extension VersionTuple {
 }
 
 /// Evaluate the condition of an `#if`.
+/// - Parameters:
+///   - condition: The condition to evaluate, which we assume has already been
+///     folded according to the logical operators table.
+///   - configuration: The configuration against which the condition will be
+///     evaluated.
+/// - Throws: Throws if an errors occur during evaluation.
+/// - Returns: Whether the condition holds with the given build configuration.
 private func evaluateIfConfig(
   condition: ExprSyntax,
   configuration: some BuildConfiguration
@@ -223,7 +235,6 @@ private func evaluateIfConfig(
     let fnName = call.calledExpression.simpleIdentifierExpr,
     let fn = IfConfigFunctions(rawValue: fnName)
   {
-
     /// Perform a check for an operation that takes a single identifier argument.
     func doSingleIdentifierArgumentCheck(_ body: (String, ExprSyntax) -> Bool?) throws -> Bool? {
       // Ensure that we have a single argument that is a simple identifier.
@@ -297,6 +308,8 @@ private func evaluateIfConfig(
         let arg = argExpr.simpleIdentifierExpr,
         let expectedEndianness = Endianness(rawValue: arg)
       else {
+        // FIXME: Custom error message when the endianness doesn't match any
+        // case.
         result = nil
         break
       }
@@ -365,8 +378,9 @@ private func evaluateIfConfig(
         throw IfConfigError.canImportMissingModule(syntax: ExprSyntax(call))
       }
 
+      // FIXME: This is a gross hack. Actually look at the sequence of
+      // `MemberAccessExprSyntax` nodes and pull out the identifiers.
       let importPath = firstArg.expression.trimmedDescription.split(separator: ".")
-      // FIXME: Check to make sure we have all identifiers here.
 
       // If there is a second argument, it shall have the label _version or
       // _underlyingVersion.
@@ -428,7 +442,7 @@ extension IfConfigState {
   /// insufficient information to make a determination.
   public init(condition: some ExprSyntaxProtocol, configuration: some BuildConfiguration) throws {
     // Apply operator folding for !/&&/||.
-    let foldedCondition = try OperatorTable.logicalOperators.foldAll(condition).as(ExprSyntax.self)!
+    let foldedCondition = try OperatorTable.logicalOperators.foldAll(condition).cast(ExprSyntax.self)
 
     let result = try evaluateIfConfig(condition: foldedCondition, configuration: configuration)
     self = result ? .active : .inactive

Sources/SwiftIfConfig/SwiftIfConfig.docc/SwiftIfConfig.md

@@ -1,7 +1,5 @@
 # `SwiftIfConfig`
 
-
-
 A library to evaluate `#if` conditionals within a Swift syntax tree.
 
 ## Overview
@@ -29,6 +27,6 @@ The syntax tree and its parser do not reason about the build configuration. Rath
 The `SwiftIfConfig` library provides utilities to determine which syntax nodes are part of a particular build configuration. Each utility requires that one provide a specific build configuration (i.e., an instance of a type that conforms to the <doc:BuildConfiguration> protocol), and provides a different view on essentially the same information:
 
 * <doc:ActiveSyntaxVisitor> and <doc:ActiveSyntaxAnyVisitor> are visitor types that only visit the syntax nodes that are included ("active") for a given build configuration, implicitly skipping any nodes within inactive `#if` clauses.
-* `SyntaxProtocol.removingInactive(in:)` produces a syntax node that removes all inactive regions (and their corresponding `IfConfigDeclSyntax` nodes) from the given syntax tree, returning a new tree that is free of `#if` conditions.
-* `IfConfigDeclSyntax.activeClause(in:)` determines which of the clauses of an `#if` is active for the given build configuration, returning the active clause.
-* `SyntaxProtocol.isActive(in:)` determines whether the given syntax node is active for the given build configuration.
+* ``SyntaxProtocol/removingInactive(in:)`` produces a syntax node that removes all inactive regions (and their corresponding `IfConfigDeclSyntax` nodes) from the given syntax tree, returning a new tree that is free of `#if` conditions.
+* ``IfConfigDeclSyntax.activeClause(in:)`` determines which of the clauses of an `#if` is active for the given build configuration, returning the active clause.
+* ``SyntaxProtocol.isActive(in:)`` determines whether the given syntax node is active for the given build configuration.

Sources/SwiftIfConfig/SyntaxLiteralUtils.swift

@@ -20,8 +20,7 @@ extension BooleanLiteralExprSyntax {
 extension TupleExprSyntax {
   /// Whether this tuple is a parenthesized expression, e.g., (x).
   var isParentheses: Bool {
-    guard elements.count == 1, let element = elements.first else { return false }
-    return element.label == nil
+    return elements.singleUnlabeledExpression != nil
   }
 }
 

Sources/SwiftIfConfig/VersionTuple.swift

@@ -14,14 +14,20 @@
 public struct VersionTuple {
   /// The components of the version tuple, start with the major version.
   public var components: [Int]
-}
 
-extension VersionTuple {
-  /// Create a version tuple from its components.
-  public init(_ components: Int...) {
+  /// Create a version tuple from a non-empty array of components.
+  public init(components: [Int]) {
+    precondition(!components.isEmpty)
     self.components = components
   }
 
+  /// Create a version tuple from its components.
+  public init(_ firstComponent: Int, _ remainingComponents: Int...) {
+    self.components = []
+    self.components.append(firstComponent)
+    self.components.append(contentsOf: remainingComponents)
+  }
+
   /// Parse a string into a version tuple, returning `nil` if any errors were
   /// present.
   public init?(parsing string: String) {
@@ -34,10 +40,10 @@ extension VersionTuple {
 
       components.append(component)
     }
+
+    if components.isEmpty { return nil }
   }
-}
 
-extension VersionTuple {
   /// Normalize the version tuple by removing trailing zeroes.
   var normalized: VersionTuple {
     var newComponents = components

Tests/SwiftIfConfigTest/VisitorTests.swift

@@ -24,7 +24,9 @@ class AllActiveVisitor: ActiveSyntaxAnyVisitor<TestingBuildConfiguration> {
     super.init(viewMode: .sourceAccurate, configuration: configuration)
   }
   open override func visitAny(_ node: Syntax) -> SyntaxVisitorContinueKind {
-    XCTAssertTrue(try! node.isActive(in: configuration))
+    var active: Bool = false
+    XCTAssertNoThrow(try active = node.isActive(in: configuration))
+    XCTAssertTrue(active)
     return .visitChildren
   }
 }