From e4f55173230892a270b2453eb1e1da506875c160 Mon Sep 17 00:00:00 2001
From: Doug Gregor <dgregor@apple.com>
Date: Sun, 4 Aug 2024 07:35:40 -0700
Subject: [PATCH 01/11] Implement warning to recognize conditions that should
 be simulator checks

Implement a warning to recognize `#if` conditions like this:

```swift
((os(iOS) || os(tvOS)) && (arch(i386) || arch(x86_64)))
```

and suggest `targetEnvironment(simultator)` instead.
---
 Package.swift                                 |   2 +-
 Sources/SwiftIfConfig/CMakeLists.txt          |   1 +
 Sources/SwiftIfConfig/IfConfigError.swift     |  26 ++++-
 .../SwiftIfConfig/IfConfigEvaluation.swift    | 103 +++++++++++++++++-
 Tests/SwiftIfConfigTest/EvaluateTests.swift   |  24 ++++
 5 files changed, 149 insertions(+), 7 deletions(-)

diff --git a/Package.swift b/Package.swift
index 2d626ac9ab5..a9d2f7ae572 100644
--- a/Package.swift
+++ b/Package.swift
@@ -143,7 +143,7 @@ let package = Package(
 
     .target(
       name: "SwiftIfConfig",
-      dependencies: ["SwiftSyntax", "SwiftDiagnostics", "SwiftOperators"],
+      dependencies: ["SwiftSyntax", "SwiftSyntaxBuilder", "SwiftDiagnostics", "SwiftOperators"],
       exclude: ["CMakeLists.txt"]
     ),
 
diff --git a/Sources/SwiftIfConfig/CMakeLists.txt b/Sources/SwiftIfConfig/CMakeLists.txt
index 350c134b9f9..7425b0df75d 100644
--- a/Sources/SwiftIfConfig/CMakeLists.txt
+++ b/Sources/SwiftIfConfig/CMakeLists.txt
@@ -24,5 +24,6 @@ add_swift_syntax_library(SwiftIfConfig
 
 target_link_swift_syntax_libraries(SwiftIfConfig PUBLIC
   SwiftSyntax
+  SwiftSyntaxBuilder
   SwiftDiagnostics
   SwiftOperators)
diff --git a/Sources/SwiftIfConfig/IfConfigError.swift b/Sources/SwiftIfConfig/IfConfigError.swift
index a0e33506c68..67958e46761 100644
--- a/Sources/SwiftIfConfig/IfConfigError.swift
+++ b/Sources/SwiftIfConfig/IfConfigError.swift
@@ -12,6 +12,7 @@
 
 import SwiftDiagnostics
 import SwiftSyntax
+import SwiftSyntaxBuilder
 
 /// Describes the kinds of errors that can occur when processing #if conditions.
 enum IfConfigError: Error, CustomStringConvertible {
@@ -29,6 +30,7 @@ enum IfConfigError: Error, CustomStringConvertible {
   case canImportTwoParameters(syntax: ExprSyntax)
   case ignoredTrailingComponents(version: VersionTuple, syntax: ExprSyntax)
   case integerLiteralCondition(syntax: ExprSyntax, replacement: Bool)
+  case likelySimulatorPlatform(syntax: ExprSyntax)
 
   var description: String {
     switch self {
@@ -75,6 +77,10 @@ enum IfConfigError: Error, CustomStringConvertible {
 
     case .integerLiteralCondition(syntax: let syntax, replacement: let replacement):
       return "'\(syntax.trimmedDescription)' is not a valid conditional compilation expression, use '\(replacement)'"
+
+    case .likelySimulatorPlatform:
+      return
+        "platform condition appears to be testing for simulator environment; use 'targetEnvironment(simulator)' instead"
     }
   }
 
@@ -93,7 +99,8 @@ enum IfConfigError: Error, CustomStringConvertible {
       .canImportLabel(syntax: let syntax),
       .canImportTwoParameters(syntax: let syntax),
       .ignoredTrailingComponents(version: _, syntax: let syntax),
-      .integerLiteralCondition(syntax: let syntax, replacement: _):
+      .integerLiteralCondition(syntax: let syntax, replacement: _),
+      .likelySimulatorPlatform(syntax: let syntax):
       return Syntax(syntax)
 
     case .unsupportedVersionOperator(name: _, operator: let op):
@@ -111,7 +118,7 @@ extension IfConfigError: DiagnosticMessage {
 
   var severity: SwiftDiagnostics.DiagnosticSeverity {
     switch self {
-    case .ignoredTrailingComponents: return .warning
+    case .ignoredTrailingComponents, .likelySimulatorPlatform: return .warning
     default: return .error
     }
   }
@@ -142,6 +149,21 @@ extension IfConfigError: DiagnosticMessage {
       )
     }
 
+    // For the likely targetEnvironment(simulator) condition we have a Fix-It.
+    if case .likelySimulatorPlatform(let syntax) = self {
+      return Diagnostic(
+        node: syntax,
+        message: self,
+        fixIt: .replace(
+          message: SimpleFixItMessage(
+            message: "replace with 'targetEnvironment(simulator)'"
+          ),
+          oldNode: syntax,
+          newNode: "targetEnvironment(simulator)" as ExprSyntax
+        )
+      )
+    }
+
     return Diagnostic(node: syntax, message: self)
   }
 }
diff --git a/Sources/SwiftIfConfig/IfConfigEvaluation.swift b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
index 2ed3ccb4c37..cec3d48171b 100644
--- a/Sources/SwiftIfConfig/IfConfigEvaluation.swift
+++ b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
@@ -123,6 +123,11 @@ func evaluateIfConfig(
     let op = binOp.operator.as(BinaryOperatorExprSyntax.self),
     (op.operator.text == "&&" || op.operator.text == "||")
   {
+    // Check whether this was likely to be a check for targetEnvironment(simulator).
+    if let targetEnvironmentDiag = diagnoseLikelySimulatorEnvironmentTest(binOp) {
+      extraDiagnostics.append(targetEnvironmentDiag)
+    }
+
     // Evaluate the left-hand side.
     let (lhsActive, lhssyntaxErrorsAllowed, lhsDiagnostics) = evaluateIfConfig(
       condition: binOp.leftOperand,
@@ -134,9 +139,17 @@ func evaluateIfConfig(
     if lhssyntaxErrorsAllowed {
       switch (lhsActive, op.operator.text) {
       case (true, "||"):
-        return (active: true, syntaxErrorsAllowed: lhssyntaxErrorsAllowed, diagnostics: lhsDiagnostics)
+        return (
+          active: true,
+          syntaxErrorsAllowed: lhssyntaxErrorsAllowed,
+          diagnostics: extraDiagnostics + lhsDiagnostics
+        )
       case (false, "&&"):
-        return (active: false, syntaxErrorsAllowed: lhssyntaxErrorsAllowed, diagnostics: lhsDiagnostics)
+        return (
+          active: false,
+          syntaxErrorsAllowed: lhssyntaxErrorsAllowed,
+          diagnostics: extraDiagnostics + lhsDiagnostics
+        )
       default:
         break
       }
@@ -153,14 +166,14 @@ func evaluateIfConfig(
       return (
         active: lhsActive || rhsActive,
         syntaxErrorsAllowed: lhssyntaxErrorsAllowed && rhssyntaxErrorsAllowed,
-        diagnostics: lhsDiagnostics + rhsDiagnostics
+        diagnostics: extraDiagnostics + lhsDiagnostics + rhsDiagnostics
       )
 
     case "&&":
       return (
         active: lhsActive && rhsActive,
         syntaxErrorsAllowed: lhssyntaxErrorsAllowed || rhssyntaxErrorsAllowed,
-        diagnostics: lhsDiagnostics + rhsDiagnostics
+        diagnostics: extraDiagnostics + lhsDiagnostics + rhsDiagnostics
       )
 
     default:
@@ -433,6 +446,88 @@ func evaluateIfConfig(
   return recordError(.unknownExpression(condition))
 }
 
+/// Determine whether the given condition only involves disjunctions that
+/// check the given config function against one of the provided values.
+///
+/// For example, this will match a condition like `os(iOS) ||  os(tvOS)`
+/// when passed `IfConfigFunctions.os` and `["iOS", "tvOS"]`.
+private func isConditionDisjunction(
+  _ condition: some ExprSyntaxProtocol,
+  function: IfConfigFunctions,
+  anyOf values: [String]
+) -> Bool {
+  // Recurse into disjunctions. Both sides need to match.
+  if let binOp = condition.as(InfixOperatorExprSyntax.self),
+    let op = binOp.operator.as(BinaryOperatorExprSyntax.self),
+    op.operator.text == "||"
+  {
+    return isConditionDisjunction(binOp.leftOperand, function: function, anyOf: values)
+      && isConditionDisjunction(binOp.rightOperand, function: function, anyOf: values)
+  }
+
+  // Look through parentheses.
+  if let tuple = condition.as(TupleExprSyntax.self), tuple.isParentheses,
+    let element = tuple.elements.first
+  {
+    return isConditionDisjunction(element.expression, function: function, anyOf: values)
+  }
+
+  // If we have a call to this function, check whether the argument is one of
+  // the acceptable values.
+  if let call = condition.as(FunctionCallExprSyntax.self),
+    let fnName = call.calledExpression.simpleIdentifierExpr,
+    let callFn = IfConfigFunctions(rawValue: fnName),
+    callFn == function,
+    let argExpr = call.arguments.singleUnlabeledExpression,
+    let arg = argExpr.simpleIdentifierExpr
+  {
+    return values.contains(arg)
+  }
+
+  return false
+}
+
+/// If this binary operator looks like it could be replaced by a
+/// targetEnvironment(simulator) check, produce a diagnostic that does so.
+///
+/// For example, this checks for conditions like:
+///
+/// ```
+/// #if (os(iOS) ||  os(tvOS)) && (arch(i386) || arch(x86_64))
+/// ```
+///
+/// which should be replaced with
+///
+/// ```
+/// #if targetEnvironment(simulator)
+/// ```
+private func diagnoseLikelySimulatorEnvironmentTest(
+  _ binOp: InfixOperatorExprSyntax
+) -> Diagnostic? {
+  guard let op = binOp.operator.as(BinaryOperatorExprSyntax.self),
+    op.operator.text == "&&"
+  else {
+    return nil
+  }
+
+  func isSimulatorPlatformOSTest(_ condition: ExprSyntax) -> Bool {
+    return isConditionDisjunction(condition, function: .os, anyOf: ["iOS", "tvOS", "watchOS"])
+  }
+
+  func isSimulatorPlatformArchTest(_ condition: ExprSyntax) -> Bool {
+    return isConditionDisjunction(condition, function: .arch, anyOf: ["i386", "x86_64"])
+  }
+
+  guard
+    (isSimulatorPlatformOSTest(binOp.leftOperand) && isSimulatorPlatformArchTest(binOp.rightOperand))
+      || (isSimulatorPlatformOSTest(binOp.rightOperand) && isSimulatorPlatformArchTest(binOp.leftOperand))
+  else {
+    return nil
+  }
+
+  return IfConfigError.likelySimulatorPlatform(syntax: ExprSyntax(binOp)).asDiagnostic
+}
+
 extension IfConfigClauseSyntax {
   /// Fold the operators within an #if condition, turning sequence expressions
   /// involving the various allowed operators (&&, ||, !) into well-structured
diff --git a/Tests/SwiftIfConfigTest/EvaluateTests.swift b/Tests/SwiftIfConfigTest/EvaluateTests.swift
index 5ea79172176..25a6fe2c1ee 100644
--- a/Tests/SwiftIfConfigTest/EvaluateTests.swift
+++ b/Tests/SwiftIfConfigTest/EvaluateTests.swift
@@ -278,6 +278,30 @@ public class EvaluateTests: XCTestCase {
       ]
     )
   }
+
+  func testLikelySimulatorEnvironment() throws {
+    assertIfConfig(
+      "((os(iOS) || os(tvOS)) && (arch(i386) || arch(x86_64))) && DEBUG",
+      .inactive,
+      diagnostics: [
+        DiagnosticSpec(
+          message:
+            "platform condition appears to be testing for simulator environment; use 'targetEnvironment(simulator)' instead",
+          line: 1,
+          column: 2,
+          severity: .warning,
+          fixIts: [
+            FixItSpec(message: "replace with 'targetEnvironment(simulator)'")
+          ]
+        )
+      ]
+    )
+
+    assertIfConfig(
+      "((os(iOS) || os(tvOS)) && (arch(arm64) || arch(x86_64))) && DEBUG",
+      .inactive
+    )
+  }
 }
 
 /// Assert the results of evaluating the condition within an `#if` against the

From d5e37175f09e1851d4f30aaa2be1b09c0e86c9a3 Mon Sep 17 00:00:00 2001
From: Doug Gregor <dgregor@apple.com>
Date: Sun, 4 Aug 2024 07:55:02 -0700
Subject: [PATCH 02/11] Slight changes in diagnostic wording to more closely
 match the compiler

---
 Sources/SwiftIfConfig/IfConfigError.swift     |  2 +-
 .../SwiftIfConfig/IfConfigEvaluation.swift    |  8 +++---
 Tests/SwiftIfConfigTest/EvaluateTests.swift   | 26 +++++++++++++++++++
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/Sources/SwiftIfConfig/IfConfigError.swift b/Sources/SwiftIfConfig/IfConfigError.swift
index 67958e46761..e2205d43e8a 100644
--- a/Sources/SwiftIfConfig/IfConfigError.swift
+++ b/Sources/SwiftIfConfig/IfConfigError.swift
@@ -67,7 +67,7 @@ enum IfConfigError: Error, CustomStringConvertible {
       return "canImport requires a module name"
 
     case .canImportLabel(syntax: _):
-      return "2nd parameter of canImport should be labeled as _version or _underlyingVersion"
+      return "second parameter of canImport should be labeled as _version or _underlyingVersion"
 
     case .canImportTwoParameters(syntax: _):
       return "canImport can take only two parameters"
diff --git a/Sources/SwiftIfConfig/IfConfigEvaluation.swift b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
index cec3d48171b..387ecd0f78b 100644
--- a/Sources/SwiftIfConfig/IfConfigEvaluation.swift
+++ b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
@@ -374,6 +374,10 @@ func evaluateIfConfig(
         return recordError(.canImportMissingModule(syntax: ExprSyntax(call)))
       }
 
+      if call.arguments.count > 2 {
+        return recordError(.canImportTwoParameters(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: ".")
@@ -423,10 +427,6 @@ func evaluateIfConfig(
           assert(secondArg.label?.text == "_underlyingVersion")
           version = .underlyingVersion(versionTuple)
         }
-
-        if call.arguments.count > 2 {
-          return recordError(.canImportTwoParameters(syntax: ExprSyntax(call)))
-        }
       } else {
         version = .unversioned
       }
diff --git a/Tests/SwiftIfConfigTest/EvaluateTests.swift b/Tests/SwiftIfConfigTest/EvaluateTests.swift
index 25a6fe2c1ee..284df37e9f6 100644
--- a/Tests/SwiftIfConfigTest/EvaluateTests.swift
+++ b/Tests/SwiftIfConfigTest/EvaluateTests.swift
@@ -277,6 +277,32 @@ public class EvaluateTests: XCTestCase {
         )
       ]
     )
+
+    assertIfConfig(
+      "canImport(A, 2.2)",
+      .unparsed,
+      diagnostics: [
+        DiagnosticSpec(
+          message: #"second parameter of canImport should be labeled as _version or _underlyingVersion"#,
+          line: 1,
+          column: 14,
+          severity: .error
+        )
+      ]
+    )
+
+    assertIfConfig(
+      "canImport(A, 2.2, 1.1)",
+      .unparsed,
+      diagnostics: [
+        DiagnosticSpec(
+          message: #"canImport can take only two parameters"#,
+          line: 1,
+          column: 1,
+          severity: .error
+        )
+      ]
+    )
   }
 
   func testLikelySimulatorEnvironment() throws {

From 2f24394ba134bbe34186742dac105148fa11e8ac Mon Sep 17 00:00:00 2001
From: Doug Gregor <dgregor@apple.com>
Date: Sun, 4 Aug 2024 14:58:26 -0700
Subject: [PATCH 03/11] Downgrade the error about a
 `_compiler_version("1.N.2")` to a warning

The error about only a `*` being meaningful in the second position is
a warning in the compiler, so match that behavior.
---
 Sources/SwiftIfConfig/IfConfigError.swift     |  3 +-
 .../SwiftIfConfig/IfConfigEvaluation.swift    |  6 ++-
 .../SwiftIfConfig/VersionTuple+Parsing.swift  | 39 ++++++++++++++++---
 Tests/SwiftIfConfigTest/EvaluateTests.swift   | 13 +++++++
 4 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/Sources/SwiftIfConfig/IfConfigError.swift b/Sources/SwiftIfConfig/IfConfigError.swift
index e2205d43e8a..adcf7b94e6e 100644
--- a/Sources/SwiftIfConfig/IfConfigError.swift
+++ b/Sources/SwiftIfConfig/IfConfigError.swift
@@ -118,7 +118,8 @@ extension IfConfigError: DiagnosticMessage {
 
   var severity: SwiftDiagnostics.DiagnosticSeverity {
     switch self {
-    case .ignoredTrailingComponents, .likelySimulatorPlatform: return .warning
+    case .compilerVersionSecondComponentNotWildcard, .ignoredTrailingComponents, .likelySimulatorPlatform:
+      return .warning
     default: return .error
     }
   }
diff --git a/Sources/SwiftIfConfig/IfConfigEvaluation.swift b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
index 387ecd0f78b..0071f83a2ac 100644
--- a/Sources/SwiftIfConfig/IfConfigEvaluation.swift
+++ b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
@@ -354,7 +354,11 @@ func evaluateIfConfig(
       let versionString = stringSegment.content.text
       let expectedVersion: VersionTuple
       do {
-        expectedVersion = try VersionTuple(parsingCompilerBuildVersion: versionString, argExpr)
+        expectedVersion = try VersionTuple.parseCompilerBuildVersion(
+          versionString,
+          argExpr,
+          extraDiagnostics: &extraDiagnostics
+        )
       } catch {
         return recordError(error, at: stringSegment.content)
       }
diff --git a/Sources/SwiftIfConfig/VersionTuple+Parsing.swift b/Sources/SwiftIfConfig/VersionTuple+Parsing.swift
index c5d301c2991..e0b16b6b57c 100644
--- a/Sources/SwiftIfConfig/VersionTuple+Parsing.swift
+++ b/Sources/SwiftIfConfig/VersionTuple+Parsing.swift
@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+import SwiftDiagnostics
 import SwiftSyntax
 
 extension VersionTuple {
@@ -20,11 +21,35 @@ extension VersionTuple {
   ///   we are parsing.
   ///   - versionSyntax: The syntax node that contains the version string, used
   ///   only for diagnostic purposes.
-  init(
-    parsingCompilerBuildVersion versionString: String,
+  static func parseCompilerBuildVersion(
+    _ versionString: String,
     _ versionSyntax: ExprSyntax
-  ) throws {
-    components = []
+  ) -> (version: VersionTuple?, diagnostics: [Diagnostic]) {
+    var extraDiagnostics: [Diagnostic] = []
+    let version: VersionTuple?
+    do {
+      version = try parseCompilerBuildVersion(versionString, versionSyntax, extraDiagnostics: &extraDiagnostics)
+    } catch {
+      version = nil
+      extraDiagnostics.append(contentsOf: error.asDiagnostics(at: versionSyntax))
+    }
+
+    return (version, extraDiagnostics)
+  }
+
+  /// 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.
+  static func parseCompilerBuildVersion(
+    _ versionString: String,
+    _ versionSyntax: ExprSyntax,
+    extraDiagnostics: inout [Diagnostic]
+  ) throws -> VersionTuple {
+    var components: [Int] = []
 
     // Version value are separated by periods.
     let componentStrings = versionString.split(separator: ".", omittingEmptySubsequences: false)
@@ -49,7 +74,9 @@ extension VersionTuple {
       // The second component is always "*", and is never used for comparison.
       if index == 1 {
         if componentString != "*" {
-          throw IfConfigError.compilerVersionSecondComponentNotWildcard(syntax: versionSyntax)
+          extraDiagnostics.append(
+            IfConfigError.compilerVersionSecondComponentNotWildcard(syntax: versionSyntax).asDiagnostic
+          )
         }
         try recordComponent(0)
         continue
@@ -102,5 +129,7 @@ extension VersionTuple {
       }
       components[0] = components[0] / 1000
     }
+
+    return VersionTuple(components: components)
   }
 }
diff --git a/Tests/SwiftIfConfigTest/EvaluateTests.swift b/Tests/SwiftIfConfigTest/EvaluateTests.swift
index 284df37e9f6..1210071e1eb 100644
--- a/Tests/SwiftIfConfigTest/EvaluateTests.swift
+++ b/Tests/SwiftIfConfigTest/EvaluateTests.swift
@@ -230,6 +230,19 @@ public class EvaluateTests: XCTestCase {
         )
       ]
     )
+
+    assertIfConfig(
+      #"_compiler_version("5.7.100")"#,
+      .active,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "the second version component is not used for comparison in legacy compiler versions",
+          line: 1,
+          column: 19,
+          severity: .warning
+        )
+      ]
+    )
   }
 
   func testCanImport() throws {

From 1425e5a16671486638aeb27a811de7c2717f10ca Mon Sep 17 00:00:00 2001
From: Doug Gregor <dgregor@apple.com>
Date: Sun, 4 Aug 2024 16:30:15 -0700
Subject: [PATCH 04/11] Downgrade error about bad "_endian" platform condition
 to a warning

The compiler issues a warning, so do the same here.
---
 Sources/SwiftIfConfig/IfConfigError.swift     | 10 ++++++--
 .../SwiftIfConfig/IfConfigEvaluation.swift    | 25 ++++++++++++++-----
 Tests/SwiftIfConfigTest/EvaluateTests.swift   | 13 ++++++++++
 3 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/Sources/SwiftIfConfig/IfConfigError.swift b/Sources/SwiftIfConfig/IfConfigError.swift
index adcf7b94e6e..10bc7250164 100644
--- a/Sources/SwiftIfConfig/IfConfigError.swift
+++ b/Sources/SwiftIfConfig/IfConfigError.swift
@@ -31,6 +31,7 @@ enum IfConfigError: Error, CustomStringConvertible {
   case ignoredTrailingComponents(version: VersionTuple, syntax: ExprSyntax)
   case integerLiteralCondition(syntax: ExprSyntax, replacement: Bool)
   case likelySimulatorPlatform(syntax: ExprSyntax)
+  case endiannessDoesNotMatch(syntax: ExprSyntax, argument: String)
 
   var description: String {
     switch self {
@@ -81,6 +82,9 @@ enum IfConfigError: Error, CustomStringConvertible {
     case .likelySimulatorPlatform:
       return
         "platform condition appears to be testing for simulator environment; use 'targetEnvironment(simulator)' instead"
+
+    case .endiannessDoesNotMatch:
+      return "unknown endianness for build configuration '_endian' (must be 'big' or 'little')"
     }
   }
 
@@ -100,7 +104,8 @@ enum IfConfigError: Error, CustomStringConvertible {
       .canImportTwoParameters(syntax: let syntax),
       .ignoredTrailingComponents(version: _, syntax: let syntax),
       .integerLiteralCondition(syntax: let syntax, replacement: _),
-      .likelySimulatorPlatform(syntax: let syntax):
+      .likelySimulatorPlatform(syntax: let syntax),
+      .endiannessDoesNotMatch(syntax: let syntax, argument: _):
       return Syntax(syntax)
 
     case .unsupportedVersionOperator(name: _, operator: let op):
@@ -118,7 +123,8 @@ extension IfConfigError: DiagnosticMessage {
 
   var severity: SwiftDiagnostics.DiagnosticSeverity {
     switch self {
-    case .compilerVersionSecondComponentNotWildcard, .ignoredTrailingComponents, .likelySimulatorPlatform:
+    case .compilerVersionSecondComponentNotWildcard, .ignoredTrailingComponents,
+      .likelySimulatorPlatform, .endiannessDoesNotMatch:
       return .warning
     default: return .error
     }
diff --git a/Sources/SwiftIfConfig/IfConfigEvaluation.swift b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
index 0071f83a2ac..3702aacdd3e 100644
--- a/Sources/SwiftIfConfig/IfConfigEvaluation.swift
+++ b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
@@ -283,23 +283,36 @@ func evaluateIfConfig(
       )
 
     case ._endian:
-      // Ensure that we have a single argument that is a simple identifier,
-      // either "little" or "big".
+      // Ensure that we have a single argument that is a simple identifier.
       guard let argExpr = call.arguments.singleUnlabeledExpression,
-        let arg = argExpr.simpleIdentifierExpr,
-        let expectedEndianness = Endianness(rawValue: arg)
+        let arg = argExpr.simpleIdentifierExpr
       else {
         return recordError(
           .requiresUnlabeledArgument(
             name: fnName,
-            role: "endiannes ('big' or 'little')",
+            role: "endianness ('big' or 'little')",
             syntax: ExprSyntax(call)
           )
         )
       }
 
+      // The argument needs to be either "little" or "big". Otherwise, we assume
+      // it fails.
+      let isActive: Bool
+      if let expectedEndianness = Endianness(rawValue: arg) {
+        isActive = configuration.endianness == expectedEndianness
+      } else {
+        // Complain about unknown endianness
+        extraDiagnostics.append(
+          contentsOf: IfConfigError.endiannessDoesNotMatch(syntax: argExpr, argument: arg)
+            .asDiagnostics(at: argExpr)
+        )
+
+        isActive = false
+      }
+
       return (
-        active: configuration.endianness == expectedEndianness,
+        active: isActive,
         syntaxErrorsAllowed: fn.syntaxErrorsAllowed,
         diagnostics: extraDiagnostics
       )
diff --git a/Tests/SwiftIfConfigTest/EvaluateTests.swift b/Tests/SwiftIfConfigTest/EvaluateTests.swift
index 1210071e1eb..87b9550dffe 100644
--- a/Tests/SwiftIfConfigTest/EvaluateTests.swift
+++ b/Tests/SwiftIfConfigTest/EvaluateTests.swift
@@ -170,6 +170,19 @@ public class EvaluateTests: XCTestCase {
     assertIfConfig("_pointerBitWidth(_32)", .inactive)
     assertIfConfig("_hasAtomicBitWidth(_64)", .active)
     assertIfConfig("_hasAtomicBitWidth(_128)", .inactive)
+
+    assertIfConfig(
+      "_endian(mid)",
+      .inactive,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "unknown endianness for build configuration '_endian' (must be 'big' or 'little')",
+          line: 1,
+          column: 9,
+          severity: .warning
+        )
+      ]
+    )
   }
 
   func testVersions() throws {

From f3c3f141a66b356d7ea57e94f6f780495ee5a492 Mon Sep 17 00:00:00 2001
From: Doug Gregor <dgregor@apple.com>
Date: Sun, 4 Aug 2024 19:30:06 -0700
Subject: [PATCH 05/11] Introduce warning to rename targetEnvironment(macabi)
 to macCatalyst

This matches the behavior of the compiler.
---
 Sources/SwiftIfConfig/IfConfigError.swift     | 24 +++++++++++++++++--
 .../SwiftIfConfig/IfConfigEvaluation.swift    | 16 ++++++++++---
 Tests/SwiftIfConfigTest/EvaluateTests.swift   | 16 +++++++++++++
 3 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/Sources/SwiftIfConfig/IfConfigError.swift b/Sources/SwiftIfConfig/IfConfigError.swift
index 10bc7250164..7f70763bb87 100644
--- a/Sources/SwiftIfConfig/IfConfigError.swift
+++ b/Sources/SwiftIfConfig/IfConfigError.swift
@@ -32,6 +32,7 @@ enum IfConfigError: Error, CustomStringConvertible {
   case integerLiteralCondition(syntax: ExprSyntax, replacement: Bool)
   case likelySimulatorPlatform(syntax: ExprSyntax)
   case endiannessDoesNotMatch(syntax: ExprSyntax, argument: String)
+  case macabiIsMacCatalyst(syntax: ExprSyntax)
 
   var description: String {
     switch self {
@@ -83,6 +84,9 @@ enum IfConfigError: Error, CustomStringConvertible {
       return
         "platform condition appears to be testing for simulator environment; use 'targetEnvironment(simulator)' instead"
 
+    case .macabiIsMacCatalyst:
+      return "'macabi' has been renamed to 'macCatalyst'"
+
     case .endiannessDoesNotMatch:
       return "unknown endianness for build configuration '_endian' (must be 'big' or 'little')"
     }
@@ -105,7 +109,8 @@ enum IfConfigError: Error, CustomStringConvertible {
       .ignoredTrailingComponents(version: _, syntax: let syntax),
       .integerLiteralCondition(syntax: let syntax, replacement: _),
       .likelySimulatorPlatform(syntax: let syntax),
-      .endiannessDoesNotMatch(syntax: let syntax, argument: _):
+      .endiannessDoesNotMatch(syntax: let syntax, argument: _),
+      .macabiIsMacCatalyst(syntax: let syntax):
       return Syntax(syntax)
 
     case .unsupportedVersionOperator(name: _, operator: let op):
@@ -124,7 +129,7 @@ extension IfConfigError: DiagnosticMessage {
   var severity: SwiftDiagnostics.DiagnosticSeverity {
     switch self {
     case .compilerVersionSecondComponentNotWildcard, .ignoredTrailingComponents,
-      .likelySimulatorPlatform, .endiannessDoesNotMatch:
+      .likelySimulatorPlatform, .endiannessDoesNotMatch, .macabiIsMacCatalyst:
       return .warning
     default: return .error
     }
@@ -171,6 +176,21 @@ extension IfConfigError: DiagnosticMessage {
       )
     }
 
+    // For the targetEnvironment(macabi) -> macCatalyst rename we have a Fix-It.
+    if case .macabiIsMacCatalyst(syntax: let syntax) = self {
+      return Diagnostic(
+        node: syntax,
+        message: self,
+        fixIt: .replace(
+          message: SimpleFixItMessage(
+            message: "replace with 'macCatalyst'"
+          ),
+          oldNode: syntax,
+          newNode: "macCatalyst" as ExprSyntax
+        )
+      )
+    }
+
     return Diagnostic(node: syntax, message: self)
   }
 }
diff --git a/Sources/SwiftIfConfig/IfConfigEvaluation.swift b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
index 3702aacdd3e..e984d243360 100644
--- a/Sources/SwiftIfConfig/IfConfigEvaluation.swift
+++ b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
@@ -203,13 +203,23 @@ func evaluateIfConfig(
     ) -> (active: Bool, syntaxErrorsAllowed: Bool, diagnostics: [Diagnostic]) {
       // Ensure that we have a single argument that is a simple identifier.
       guard let argExpr = call.arguments.singleUnlabeledExpression,
-        let arg = argExpr.simpleIdentifierExpr
+        var arg = argExpr.simpleIdentifierExpr
       else {
         return recordError(
           .requiresUnlabeledArgument(name: fnName, role: role, syntax: ExprSyntax(call))
         )
       }
 
+      // The historical "macabi" environment has been renamed to "macCatalyst".
+      if role == "environment" && arg == "macabi" {
+        extraDiagnostics.append(
+          IfConfigError.macabiIsMacCatalyst(syntax: argExpr)
+            .asDiagnostic
+        )
+
+        arg = "macCatalyst"
+      }
+
       return checkConfiguration(at: argExpr) {
         (active: try body(arg), syntaxErrorsAllowed: fn.syntaxErrorsAllowed)
       }
@@ -304,8 +314,8 @@ func evaluateIfConfig(
       } else {
         // Complain about unknown endianness
         extraDiagnostics.append(
-          contentsOf: IfConfigError.endiannessDoesNotMatch(syntax: argExpr, argument: arg)
-            .asDiagnostics(at: argExpr)
+          IfConfigError.endiannessDoesNotMatch(syntax: argExpr, argument: arg)
+            .asDiagnostic
         )
 
         isActive = false
diff --git a/Tests/SwiftIfConfigTest/EvaluateTests.swift b/Tests/SwiftIfConfigTest/EvaluateTests.swift
index 87b9550dffe..be0af28922b 100644
--- a/Tests/SwiftIfConfigTest/EvaluateTests.swift
+++ b/Tests/SwiftIfConfigTest/EvaluateTests.swift
@@ -183,6 +183,22 @@ public class EvaluateTests: XCTestCase {
         )
       ]
     )
+
+    assertIfConfig(
+      "targetEnvironment(macabi)",
+      .inactive,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "'macabi' has been renamed to 'macCatalyst'",
+          line: 1,
+          column: 19,
+          severity: .warning,
+          fixIts: [
+            FixItSpec(message: "replace with 'macCatalyst'")
+          ]
+        )
+      ]
+    )
   }
 
   func testVersions() throws {

From 150e569886807ebff10e984eb37ae05c0accd5ab Mon Sep 17 00:00:00 2001
From: Doug Gregor <dgregor@apple.com>
Date: Sun, 4 Aug 2024 22:37:45 -0700
Subject: [PATCH 06/11] Properly translate import path expressions to an import
 path

My old hack of taking the text of the whole expression and splitting it on
a period was showing its age by failing to reject invalid code. Perform
the proper translation, throwing errors if the import path doesn't
match the proper A.B.C form.
---
 Sources/SwiftIfConfig/IfConfigError.swift     |  7 ++-
 .../SwiftIfConfig/IfConfigEvaluation.swift    | 44 +++++++++++++++++--
 Tests/SwiftIfConfigTest/EvaluateTests.swift   | 13 ++++++
 3 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/Sources/SwiftIfConfig/IfConfigError.swift b/Sources/SwiftIfConfig/IfConfigError.swift
index 7f70763bb87..0905d033675 100644
--- a/Sources/SwiftIfConfig/IfConfigError.swift
+++ b/Sources/SwiftIfConfig/IfConfigError.swift
@@ -33,6 +33,7 @@ enum IfConfigError: Error, CustomStringConvertible {
   case likelySimulatorPlatform(syntax: ExprSyntax)
   case endiannessDoesNotMatch(syntax: ExprSyntax, argument: String)
   case macabiIsMacCatalyst(syntax: ExprSyntax)
+  case expectedModuleName(syntax: ExprSyntax)
 
   var description: String {
     switch self {
@@ -89,6 +90,9 @@ enum IfConfigError: Error, CustomStringConvertible {
 
     case .endiannessDoesNotMatch:
       return "unknown endianness for build configuration '_endian' (must be 'big' or 'little')"
+
+    case .expectedModuleName:
+      return "expected module name"
     }
   }
 
@@ -110,7 +114,8 @@ enum IfConfigError: Error, CustomStringConvertible {
       .integerLiteralCondition(syntax: let syntax, replacement: _),
       .likelySimulatorPlatform(syntax: let syntax),
       .endiannessDoesNotMatch(syntax: let syntax, argument: _),
-      .macabiIsMacCatalyst(syntax: let syntax):
+      .macabiIsMacCatalyst(syntax: let syntax),
+      .expectedModuleName(syntax: let syntax):
       return Syntax(syntax)
 
     case .unsupportedVersionOperator(name: _, operator: let op):
diff --git a/Sources/SwiftIfConfig/IfConfigEvaluation.swift b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
index e984d243360..1632dd49933 100644
--- a/Sources/SwiftIfConfig/IfConfigEvaluation.swift
+++ b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
@@ -405,9 +405,13 @@ func evaluateIfConfig(
         return recordError(.canImportTwoParameters(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: ".")
+      // Extract the import path.
+      let importPath: [String]
+      do {
+        importPath = try extractImportPath(firstArg.expression)
+      } catch {
+        return recordError(error, at: firstArg.expression)
+      }
 
       // If there is a second argument, it shall have the label _version or
       // _underlyingVersion.
@@ -473,6 +477,40 @@ func evaluateIfConfig(
   return recordError(.unknownExpression(condition))
 }
 
+/// Given an expression with the expected form A.B.C, extract the import path
+/// ["A", "B", "C"] from it. Throws an error if the expression doesn't match
+/// this form.
+private func extractImportPath(_ expression: some ExprSyntaxProtocol) throws -> [String] {
+  // Member access.
+  if let memberAccess = expression.as(MemberAccessExprSyntax.self),
+    let base = memberAccess.base,
+    let memberName = memberAccess.declName.simpleIdentifier
+  {
+    return try extractImportPath(base) + [memberName]
+  }
+
+  // Declaration reference.
+  if let declRef = expression.as(DeclReferenceExprSyntax.self),
+    let name = declRef.simpleIdentifier
+  {
+    return [name]
+  }
+
+  throw IfConfigError.expectedModuleName(syntax: ExprSyntax(expression))
+}
+
+extension DeclReferenceExprSyntax {
+  /// If this declaration reference is a simple identifier, return that
+  /// string.
+  fileprivate var simpleIdentifier: String? {
+    guard argumentNames == nil else {
+      return nil
+    }
+
+    return baseName.text
+  }
+}
+
 /// Determine whether the given condition only involves disjunctions that
 /// check the given config function against one of the provided values.
 ///
diff --git a/Tests/SwiftIfConfigTest/EvaluateTests.swift b/Tests/SwiftIfConfigTest/EvaluateTests.swift
index be0af28922b..d16a51a5c85 100644
--- a/Tests/SwiftIfConfigTest/EvaluateTests.swift
+++ b/Tests/SwiftIfConfigTest/EvaluateTests.swift
@@ -345,6 +345,19 @@ public class EvaluateTests: XCTestCase {
         )
       ]
     )
+
+    assertIfConfig(
+      "canImport(A(b: 1, c: 2).B.C)",
+      .unparsed,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "expected module name",
+          line: 1,
+          column: 11,
+          severity: .error
+        )
+      ]
+    )
   }
 
   func testLikelySimulatorEnvironment() throws {

From 757d2b3848e3b1ec6d4fc77e5fccc13686343ffe Mon Sep 17 00:00:00 2001
From: Doug Gregor <dgregor@apple.com>
Date: Sun, 4 Aug 2024 22:42:07 -0700
Subject: [PATCH 07/11] Ensure that we diagnose compound names where they
 shouldn't be

---
 .../SwiftIfConfig/IfConfigEvaluation.swift    | 19 +++---------------
 .../SwiftIfConfig/SyntaxLiteralUtils.swift    | 20 ++++++++++++++-----
 Tests/SwiftIfConfigTest/EvaluateTests.swift   | 11 ++++++++++
 3 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/Sources/SwiftIfConfig/IfConfigEvaluation.swift b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
index 1632dd49933..270e82dc120 100644
--- a/Sources/SwiftIfConfig/IfConfigEvaluation.swift
+++ b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
@@ -96,10 +96,9 @@ func evaluateIfConfig(
   }
 
   // Declaration references are for custom compilation flags.
-  if let identExpr = condition.as(DeclReferenceExprSyntax.self) {
-    // FIXME: Need a real notion of an identifier.
-    let ident = identExpr.baseName.text
-
+  if let identExpr = condition.as(DeclReferenceExprSyntax.self),
+    let ident = identExpr.simpleIdentifier
+  {
     // Evaluate the custom condition. If the build configuration cannot answer this query, fail.
     return checkConfiguration(at: identExpr) {
       (active: try configuration.isCustomConditionSet(name: ident), syntaxErrorsAllowed: false)
@@ -499,18 +498,6 @@ private func extractImportPath(_ expression: some ExprSyntaxProtocol) throws ->
   throw IfConfigError.expectedModuleName(syntax: ExprSyntax(expression))
 }
 
-extension DeclReferenceExprSyntax {
-  /// If this declaration reference is a simple identifier, return that
-  /// string.
-  fileprivate var simpleIdentifier: String? {
-    guard argumentNames == nil else {
-      return nil
-    }
-
-    return baseName.text
-  }
-}
-
 /// Determine whether the given condition only involves disjunctions that
 /// check the given config function against one of the provided values.
 ///
diff --git a/Sources/SwiftIfConfig/SyntaxLiteralUtils.swift b/Sources/SwiftIfConfig/SyntaxLiteralUtils.swift
index ebe6ec9af69..3278bbb7e2e 100644
--- a/Sources/SwiftIfConfig/SyntaxLiteralUtils.swift
+++ b/Sources/SwiftIfConfig/SyntaxLiteralUtils.swift
@@ -36,13 +36,23 @@ extension LabeledExprListSyntax {
 extension ExprSyntax {
   /// Whether this is a simple identifier expression and, if so, what the identifier string is.
   var simpleIdentifierExpr: String? {
-    guard let identExpr = self.as(DeclReferenceExprSyntax.self),
-      identExpr.argumentNames == nil
-    else {
+    guard let identExpr = self.as(DeclReferenceExprSyntax.self) else {
       return nil
     }
 
-    // FIXME: Handle escaping here.
-    return identExpr.baseName.text
+    return identExpr.simpleIdentifier
+  }
+}
+
+extension DeclReferenceExprSyntax {
+  /// If this declaration reference is a simple identifier, return that
+  /// string.
+  var simpleIdentifier: String? {
+    guard argumentNames == nil else {
+      return nil
+    }
+
+    /// FIXME: Make this an Identifier so we handle escaping properly.
+    return baseName.text
   }
 }
diff --git a/Tests/SwiftIfConfigTest/EvaluateTests.swift b/Tests/SwiftIfConfigTest/EvaluateTests.swift
index d16a51a5c85..a0a9f037e31 100644
--- a/Tests/SwiftIfConfigTest/EvaluateTests.swift
+++ b/Tests/SwiftIfConfigTest/EvaluateTests.swift
@@ -120,6 +120,17 @@ public class EvaluateTests: XCTestCase {
         )
       ]
     )
+    assertIfConfig(
+      "BAR(_:)",
+      .unparsed,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "invalid conditional compilation expression",
+          line: 1,
+          column: 1
+        )
+      ]
+    )
   }
 
   func testBadExpressions() throws {

From fab75788c1303e8b842f2ff4ecaf5b7d3326211b Mon Sep 17 00:00:00 2001
From: Doug Gregor <dgregor@apple.com>
Date: Sun, 4 Aug 2024 22:51:05 -0700
Subject: [PATCH 08/11] Customize "unknown infix operator" diagnostic to make
 it clearer

Only && and || are allowed as infix operators within an #if condition, so
note that in the diagnostic (just like the compiler does).
---
 Sources/SwiftIfConfig/IfConfigError.swift      |  7 ++++++-
 Sources/SwiftIfConfig/IfConfigEvaluation.swift | 12 ++++++++++++
 Tests/SwiftIfConfigTest/EvaluateTests.swift    | 18 ++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/Sources/SwiftIfConfig/IfConfigError.swift b/Sources/SwiftIfConfig/IfConfigError.swift
index 0905d033675..163a5ae0982 100644
--- a/Sources/SwiftIfConfig/IfConfigError.swift
+++ b/Sources/SwiftIfConfig/IfConfigError.swift
@@ -34,6 +34,7 @@ enum IfConfigError: Error, CustomStringConvertible {
   case endiannessDoesNotMatch(syntax: ExprSyntax, argument: String)
   case macabiIsMacCatalyst(syntax: ExprSyntax)
   case expectedModuleName(syntax: ExprSyntax)
+  case badInfixOperator(syntax: ExprSyntax)
 
   var description: String {
     switch self {
@@ -93,6 +94,9 @@ enum IfConfigError: Error, CustomStringConvertible {
 
     case .expectedModuleName:
       return "expected module name"
+
+    case .badInfixOperator:
+      return "expected '&&' or '||' expression"
     }
   }
 
@@ -115,7 +119,8 @@ enum IfConfigError: Error, CustomStringConvertible {
       .likelySimulatorPlatform(syntax: let syntax),
       .endiannessDoesNotMatch(syntax: let syntax, argument: _),
       .macabiIsMacCatalyst(syntax: let syntax),
-      .expectedModuleName(syntax: let syntax):
+      .expectedModuleName(syntax: let syntax),
+      .badInfixOperator(syntax: let syntax):
       return Syntax(syntax)
 
     case .unsupportedVersionOperator(name: _, operator: let op):
diff --git a/Sources/SwiftIfConfig/IfConfigEvaluation.swift b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
index 270e82dc120..8bb7c232f61 100644
--- a/Sources/SwiftIfConfig/IfConfigEvaluation.swift
+++ b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
@@ -589,6 +589,18 @@ extension IfConfigClauseSyntax {
   ) -> (folded: ExprSyntax, diagnostics: [Diagnostic]) {
     var foldingDiagnostics: [Diagnostic] = []
     let foldedCondition = OperatorTable.logicalOperators.foldAll(condition) { error in
+      // Replace the "unknown infix operator" diagnostic with a custom one
+      // that mentions that only '&&' and '||' are allowed.
+      if case .missingOperator(_, referencedFrom: let syntax) = error,
+        let binOp = syntax.parent?.as(BinaryOperatorExprSyntax.self)
+      {
+
+        foldingDiagnostics.append(
+          IfConfigError.badInfixOperator(syntax: ExprSyntax(binOp)).asDiagnostic
+        )
+        return
+      }
+
       foldingDiagnostics.append(contentsOf: error.asDiagnostics(at: condition))
     }.cast(ExprSyntax.self)
     return (folded: foldedCondition, diagnostics: foldingDiagnostics)
diff --git a/Tests/SwiftIfConfigTest/EvaluateTests.swift b/Tests/SwiftIfConfigTest/EvaluateTests.swift
index a0a9f037e31..30eb2558bba 100644
--- a/Tests/SwiftIfConfigTest/EvaluateTests.swift
+++ b/Tests/SwiftIfConfigTest/EvaluateTests.swift
@@ -148,6 +148,24 @@ public class EvaluateTests: XCTestCase {
         )
       ]
     )
+
+    assertIfConfig(
+      "A == B",
+      .unparsed,
+      configuration: buildConfig,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "expected '&&' or '||' expression",
+          line: 1,
+          column: 3
+        ),
+        DiagnosticSpec(
+          message: "invalid conditional compilation expression",
+          line: 1,
+          column: 1
+        ),
+      ]
+    )
   }
 
   func testFeatures() throws {

From 3f5c9ea12f3a7563315b7e845737270b22d48cb2 Mon Sep 17 00:00:00 2001
From: Doug Gregor <dgregor@apple.com>
Date: Sun, 4 Aug 2024 23:03:49 -0700
Subject: [PATCH 09/11] Only diagnostic likely targetEnvironment(simulator)
 conditions at the top level

This matches the compiler's behavior.
---
 .../SwiftIfConfig/IfConfigEvaluation.swift    | 19 +++++++++++++------
 Tests/SwiftIfConfigTest/EvaluateTests.swift   |  9 +++++++--
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/Sources/SwiftIfConfig/IfConfigEvaluation.swift b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
index 8bb7c232f61..1a0a2531ec5 100644
--- a/Sources/SwiftIfConfig/IfConfigEvaluation.swift
+++ b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
@@ -30,7 +30,8 @@ import SwiftSyntax
 ///   diagnose syntax errors in blocks where the check fails.
 func evaluateIfConfig(
   condition: ExprSyntax,
-  configuration: some BuildConfiguration
+  configuration: some BuildConfiguration,
+  outermostCondition: Bool = true
 ) -> (active: Bool, syntaxErrorsAllowed: Bool, diagnostics: [Diagnostic]) {
   var extraDiagnostics: [Diagnostic] = []
 
@@ -111,7 +112,8 @@ func evaluateIfConfig(
   {
     let (innerActive, innerSyntaxErrorsAllowed, innerDiagnostics) = evaluateIfConfig(
       condition: prefixOp.expression,
-      configuration: configuration
+      configuration: configuration,
+      outermostCondition: outermostCondition
     )
 
     return (active: !innerActive, syntaxErrorsAllowed: innerSyntaxErrorsAllowed, diagnostics: innerDiagnostics)
@@ -123,14 +125,17 @@ func evaluateIfConfig(
     (op.operator.text == "&&" || op.operator.text == "||")
   {
     // Check whether this was likely to be a check for targetEnvironment(simulator).
-    if let targetEnvironmentDiag = diagnoseLikelySimulatorEnvironmentTest(binOp) {
+    if outermostCondition,
+      let targetEnvironmentDiag = diagnoseLikelySimulatorEnvironmentTest(binOp)
+    {
       extraDiagnostics.append(targetEnvironmentDiag)
     }
 
     // Evaluate the left-hand side.
     let (lhsActive, lhssyntaxErrorsAllowed, lhsDiagnostics) = evaluateIfConfig(
       condition: binOp.leftOperand,
-      configuration: configuration
+      configuration: configuration,
+      outermostCondition: false
     )
 
     // Short-circuit evaluation if we know the answer and the left-hand side
@@ -157,7 +162,8 @@ func evaluateIfConfig(
     // Evaluate the right-hand side.
     let (rhsActive, rhssyntaxErrorsAllowed, rhsDiagnostics) = evaluateIfConfig(
       condition: binOp.rightOperand,
-      configuration: configuration
+      configuration: configuration,
+      outermostCondition: false
     )
 
     switch op.operator.text {
@@ -186,7 +192,8 @@ func evaluateIfConfig(
   {
     return evaluateIfConfig(
       condition: element.expression,
-      configuration: configuration
+      configuration: configuration,
+      outermostCondition: outermostCondition
     )
   }
 
diff --git a/Tests/SwiftIfConfigTest/EvaluateTests.swift b/Tests/SwiftIfConfigTest/EvaluateTests.swift
index 30eb2558bba..6db77be8af4 100644
--- a/Tests/SwiftIfConfigTest/EvaluateTests.swift
+++ b/Tests/SwiftIfConfigTest/EvaluateTests.swift
@@ -391,7 +391,7 @@ public class EvaluateTests: XCTestCase {
 
   func testLikelySimulatorEnvironment() throws {
     assertIfConfig(
-      "((os(iOS) || os(tvOS)) && (arch(i386) || arch(x86_64))) && DEBUG",
+      "((os(iOS) || os(tvOS)) && (arch(i386) || arch(x86_64)))",
       .inactive,
       diagnostics: [
         DiagnosticSpec(
@@ -408,7 +408,12 @@ public class EvaluateTests: XCTestCase {
     )
 
     assertIfConfig(
-      "((os(iOS) || os(tvOS)) && (arch(arm64) || arch(x86_64))) && DEBUG",
+      "((os(iOS) || os(tvOS)) && (arch(arm64) || arch(x86_64)))",
+      .inactive
+    )
+
+    assertIfConfig(
+      "((os(iOS) || os(tvOS)) && (arch(i386) || arch(x86_64))) && DEBUG",
       .inactive
     )
   }

From 1a3f156329f09947413a23409f4675dc221a18bc Mon Sep 17 00:00:00 2001
From: Doug Gregor <dgregor@apple.com>
Date: Sun, 4 Aug 2024 23:46:02 -0700
Subject: [PATCH 10/11] Avoid evaluating canImport when the result of the #if
 condition is known

Within the compiler, canImport involves side effects, so we need to follow
along with how the compiler works precisely to avoid changing behavior.
---
 .../SwiftIfConfig/IfConfigEvaluation.swift    | 117 ++++++++++++++----
 Tests/SwiftIfConfigTest/EvaluateTests.swift   |   5 +
 .../TestingBuildConfiguration.swift           |   9 +-
 3 files changed, 103 insertions(+), 28 deletions(-)

diff --git a/Sources/SwiftIfConfig/IfConfigEvaluation.swift b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
index 1a0a2531ec5..75ac2ac58f3 100644
--- a/Sources/SwiftIfConfig/IfConfigEvaluation.swift
+++ b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
@@ -132,52 +132,63 @@ func evaluateIfConfig(
     }
 
     // Evaluate the left-hand side.
-    let (lhsActive, lhssyntaxErrorsAllowed, lhsDiagnostics) = evaluateIfConfig(
+    let (lhsActive, lhsSyntaxErrorsAllowed, lhsDiagnostics) = evaluateIfConfig(
       condition: binOp.leftOperand,
       configuration: configuration,
       outermostCondition: false
     )
 
-    // Short-circuit evaluation if we know the answer and the left-hand side
-    // was syntaxErrorsAllowed.
-    if lhssyntaxErrorsAllowed {
-      switch (lhsActive, op.operator.text) {
-      case (true, "||"):
-        return (
-          active: true,
-          syntaxErrorsAllowed: lhssyntaxErrorsAllowed,
-          diagnostics: extraDiagnostics + lhsDiagnostics
-        )
-      case (false, "&&"):
-        return (
-          active: false,
-          syntaxErrorsAllowed: lhssyntaxErrorsAllowed,
-          diagnostics: extraDiagnostics + lhsDiagnostics
-        )
-      default:
-        break
-      }
+    // Short-circuit evaluation if we know the answer. We still recurse into
+    // the right-hand side, but with a dummy configuration that won't have
+    // side effects, so we only get validation-related errors.
+    let shortCircuitResult: Bool?
+    switch (lhsActive, op.operator.text) {
+    case (true, "||"): shortCircuitResult = true
+    case (false, "&&"): shortCircuitResult = false
+    default: shortCircuitResult = nil
+    }
+
+    // If we are supposed to short-circuit and the left-hand side of this
+    // operator with inactive &&, stop now: we shouldn't evaluate the right-
+    // hand side at all.
+    if let isActive = shortCircuitResult, lhsSyntaxErrorsAllowed {
+      return (
+        active: isActive,
+        syntaxErrorsAllowed: lhsSyntaxErrorsAllowed,
+        diagnostics: extraDiagnostics + lhsDiagnostics
+      )
     }
 
     // Evaluate the right-hand side.
-    let (rhsActive, rhssyntaxErrorsAllowed, rhsDiagnostics) = evaluateIfConfig(
-      condition: binOp.rightOperand,
-      configuration: configuration,
-      outermostCondition: false
-    )
+    let rhsActive: Bool
+    let rhsSyntaxErrorsAllowed: Bool
+    let rhsDiagnostics: [Diagnostic]
+    if shortCircuitResult != nil {
+      (rhsActive, rhsSyntaxErrorsAllowed, rhsDiagnostics) = evaluateIfConfig(
+        condition: binOp.rightOperand,
+        configuration: CanImportSuppressingBuildConfiguration(other: configuration),
+        outermostCondition: false
+      )
+    } else {
+      (rhsActive, rhsSyntaxErrorsAllowed, rhsDiagnostics) = evaluateIfConfig(
+        condition: binOp.rightOperand,
+        configuration: configuration,
+        outermostCondition: false
+      )
+    }
 
     switch op.operator.text {
     case "||":
       return (
         active: lhsActive || rhsActive,
-        syntaxErrorsAllowed: lhssyntaxErrorsAllowed && rhssyntaxErrorsAllowed,
+        syntaxErrorsAllowed: lhsSyntaxErrorsAllowed && rhsSyntaxErrorsAllowed,
         diagnostics: extraDiagnostics + lhsDiagnostics + rhsDiagnostics
       )
 
     case "&&":
       return (
         active: lhsActive && rhsActive,
-        syntaxErrorsAllowed: lhssyntaxErrorsAllowed || rhssyntaxErrorsAllowed,
+        syntaxErrorsAllowed: lhsSyntaxErrorsAllowed || rhsSyntaxErrorsAllowed,
         diagnostics: extraDiagnostics + lhsDiagnostics + rhsDiagnostics
       )
 
@@ -674,3 +685,55 @@ extension ExprSyntaxProtocol {
     return false
   }
 }
+
+/// Build configuration adaptor that suppresses calls to canImport, which
+/// can have side effects. This is somewhat of a hack for the compiler.
+private struct CanImportSuppressingBuildConfiguration<Other: BuildConfiguration>: BuildConfiguration {
+  var other: Other
+
+  func isCustomConditionSet(name: String) throws -> Bool {
+    return try other.isCustomConditionSet(name: name)
+  }
+
+  func hasFeature(name: String) throws -> Bool {
+    return try other.hasFeature(name: name)
+  }
+
+  func hasAttribute(name: String) throws -> Bool {
+    return try other.hasAttribute(name: name)
+  }
+
+  func canImport(importPath: [String], version: CanImportVersion) throws -> Bool {
+    return false
+  }
+
+  func isActiveTargetOS(name: String) throws -> Bool {
+    return try other.isActiveTargetOS(name: name)
+  }
+
+  func isActiveTargetArchitecture(name: String) throws -> Bool {
+    return try other.isActiveTargetArchitecture(name: name)
+  }
+
+  func isActiveTargetEnvironment(name: String) throws -> Bool {
+    return try other.isActiveTargetEnvironment(name: name)
+  }
+
+  func isActiveTargetRuntime(name: String) throws -> Bool {
+    return try other.isActiveTargetRuntime(name: name)
+  }
+
+  func isActiveTargetPointerAuthentication(name: String) throws -> Bool {
+    return try other.isActiveTargetPointerAuthentication(name: name)
+  }
+
+  var targetPointerBitWidth: Int { return other.targetPointerBitWidth }
+
+  var targetAtomicBitWidths: [Int] { return other.targetAtomicBitWidths }
+
+  var endianness: Endianness { return other.endianness }
+
+  var languageVersion: VersionTuple { return other.languageVersion }
+
+  var compilerVersion: VersionTuple { return other.compilerVersion }
+}
diff --git a/Tests/SwiftIfConfigTest/EvaluateTests.swift b/Tests/SwiftIfConfigTest/EvaluateTests.swift
index 6db77be8af4..01b7bc63652 100644
--- a/Tests/SwiftIfConfigTest/EvaluateTests.swift
+++ b/Tests/SwiftIfConfigTest/EvaluateTests.swift
@@ -387,6 +387,11 @@ public class EvaluateTests: XCTestCase {
         )
       ]
     )
+
+    assertIfConfig(
+      "canImport(SwiftSyntax) || canImport(ExplodingModule)",
+      .active
+    )
   }
 
   func testLikelySimulatorEnvironment() throws {
diff --git a/Tests/SwiftIfConfigTest/TestingBuildConfiguration.swift b/Tests/SwiftIfConfigTest/TestingBuildConfiguration.swift
index 5f807fe0b1e..6d6beb7caee 100644
--- a/Tests/SwiftIfConfigTest/TestingBuildConfiguration.swift
+++ b/Tests/SwiftIfConfigTest/TestingBuildConfiguration.swift
@@ -15,11 +15,14 @@ import SwiftSyntax
 
 enum BuildConfigurationError: Error, CustomStringConvertible {
   case badAttribute(String)
+  case badModule(String)
 
   var description: String {
     switch self {
     case .badAttribute(let attribute):
       return "unacceptable attribute '\(attribute)'"
+    case .badModule(let module):
+      return "unacceptable module '\(module)'"
     }
   }
 }
@@ -53,11 +56,15 @@ struct TestingBuildConfiguration: BuildConfiguration {
   func canImport(
     importPath: [String],
     version: CanImportVersion
-  ) -> Bool {
+  ) throws -> Bool {
     guard let moduleName = importPath.first else {
       return false
     }
 
+    if moduleName == "ExplodingModule" {
+      throw BuildConfigurationError.badModule(moduleName)
+    }
+
     guard moduleName == "SwiftSyntax" else { return false }
 
     switch version {

From 24ead55397ec6f91ff2b852d365deb7c7ee24f0c Mon Sep 17 00:00:00 2001
From: Doug Gregor <dgregor@apple.com>
Date: Sun, 4 Aug 2024 23:51:46 -0700
Subject: [PATCH 11/11] Slight diagnostics cleanup for incorrect infix and
 prefix operators

---
 Sources/SwiftIfConfig/IfConfigError.swift      |  7 ++++++-
 Sources/SwiftIfConfig/IfConfigEvaluation.swift | 18 +++++++++++++-----
 Tests/SwiftIfConfigTest/EvaluateTests.swift    | 13 ++++++++++---
 3 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/Sources/SwiftIfConfig/IfConfigError.swift b/Sources/SwiftIfConfig/IfConfigError.swift
index 163a5ae0982..d42f84da3b5 100644
--- a/Sources/SwiftIfConfig/IfConfigError.swift
+++ b/Sources/SwiftIfConfig/IfConfigError.swift
@@ -35,6 +35,7 @@ enum IfConfigError: Error, CustomStringConvertible {
   case macabiIsMacCatalyst(syntax: ExprSyntax)
   case expectedModuleName(syntax: ExprSyntax)
   case badInfixOperator(syntax: ExprSyntax)
+  case badPrefixOperator(syntax: ExprSyntax)
 
   var description: String {
     switch self {
@@ -97,6 +98,9 @@ enum IfConfigError: Error, CustomStringConvertible {
 
     case .badInfixOperator:
       return "expected '&&' or '||' expression"
+
+    case .badPrefixOperator:
+      return "expected unary '!' expression"
     }
   }
 
@@ -120,7 +124,8 @@ enum IfConfigError: Error, CustomStringConvertible {
       .endiannessDoesNotMatch(syntax: let syntax, argument: _),
       .macabiIsMacCatalyst(syntax: let syntax),
       .expectedModuleName(syntax: let syntax),
-      .badInfixOperator(syntax: let syntax):
+      .badInfixOperator(syntax: let syntax),
+      .badPrefixOperator(syntax: let syntax):
       return Syntax(syntax)
 
     case .unsupportedVersionOperator(name: _, operator: let op):
diff --git a/Sources/SwiftIfConfig/IfConfigEvaluation.swift b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
index 75ac2ac58f3..5a7d5d2ad9c 100644
--- a/Sources/SwiftIfConfig/IfConfigEvaluation.swift
+++ b/Sources/SwiftIfConfig/IfConfigEvaluation.swift
@@ -107,9 +107,12 @@ func evaluateIfConfig(
   }
 
   // Logical '!'.
-  if let prefixOp = condition.as(PrefixOperatorExprSyntax.self),
-    prefixOp.operator.text == "!"
-  {
+  if let prefixOp = condition.as(PrefixOperatorExprSyntax.self) {
+    // If this isn't '!', complain.
+    guard prefixOp.operator.text == "!" else {
+      return recordError(.badPrefixOperator(syntax: condition))
+    }
+
     let (innerActive, innerSyntaxErrorsAllowed, innerDiagnostics) = evaluateIfConfig(
       condition: prefixOp.expression,
       configuration: configuration,
@@ -121,9 +124,14 @@ func evaluateIfConfig(
 
   // Logical '&&' and '||'.
   if let binOp = condition.as(InfixOperatorExprSyntax.self),
-    let op = binOp.operator.as(BinaryOperatorExprSyntax.self),
-    (op.operator.text == "&&" || op.operator.text == "||")
+    let op = binOp.operator.as(BinaryOperatorExprSyntax.self)
   {
+    // If this is neither && nor ||, it was already diagnosed as part of
+    // operator folding. Just return this as inactive.
+    guard op.operator.text == "&&" || op.operator.text == "||" else {
+      return (active: false, syntaxErrorsAllowed: true, diagnostics: extraDiagnostics)
+    }
+
     // Check whether this was likely to be a check for targetEnvironment(simulator).
     if outermostCondition,
       let targetEnvironmentDiag = diagnoseLikelySimulatorEnvironmentTest(binOp)
diff --git a/Tests/SwiftIfConfigTest/EvaluateTests.swift b/Tests/SwiftIfConfigTest/EvaluateTests.swift
index 01b7bc63652..5b3444654ac 100644
--- a/Tests/SwiftIfConfigTest/EvaluateTests.swift
+++ b/Tests/SwiftIfConfigTest/EvaluateTests.swift
@@ -158,12 +158,19 @@ public class EvaluateTests: XCTestCase {
           message: "expected '&&' or '||' expression",
           line: 1,
           column: 3
-        ),
+        )
+      ]
+    )
+
+    assertIfConfig(
+      "^DEBUG",
+      .unparsed,
+      diagnostics: [
         DiagnosticSpec(
-          message: "invalid conditional compilation expression",
+          message: "expected unary '!' expression",
           line: 1,
           column: 1
-        ),
+        )
       ]
     )
   }