Improve diagnostics for #available nodes in expression positions

Author: ahoppen

Sources/SwiftParser/Expressions.swift

@@ -1209,12 +1209,20 @@ extension Parser {
           arena: self.arena
         )
       )
-
     case (.pound, _)?:
       return RawExprSyntax(
         self.parseMacroExpansionExpr(pattern: pattern, flavor: flavor)
       )
-
+    case (.poundAvailableKeyword, _)?, (.poundUnavailableKeyword, _)?:
+      let poundAvailable = self.parsePoundAvailableConditionElement()
+      return RawExprSyntax(
+        RawIdentifierExprSyntax(
+          RawUnexpectedNodesSyntax([poundAvailable], arena: self.arena),
+          identifier: missingToken(.identifier),
+          declNameArguments: nil,
+          arena: self.arena
+        )
+      )
     case (.leftBrace, _)?:  // expr-closure
       return RawExprSyntax(self.parseClosureExpression())
     case (.period, let handle)?:  // .foo

Sources/SwiftParser/RawTokenKindSubset.swift

@@ -693,6 +693,8 @@ enum PrimaryExpressionStart: RawTokenKindSubset {
   case nilKeyword
   case period
   case pound
+  case poundAvailableKeyword  // For recovery
+  case poundUnavailableKeyword  // For recovery
   case regexLiteral
   case selfKeyword
   case stringLiteral
@@ -716,6 +718,8 @@ enum PrimaryExpressionStart: RawTokenKindSubset {
     case RawTokenKindMatch(.nil): self = .nilKeyword
     case RawTokenKindMatch(.period): self = .period
     case RawTokenKindMatch(.pound): self = .pound
+    case RawTokenKindMatch(.poundAvailableKeyword): self = .poundAvailableKeyword
+    case RawTokenKindMatch(.poundUnavailableKeyword): self = .poundUnavailableKeyword
     case RawTokenKindMatch(.regexLiteral): self = .regexLiteral
     case RawTokenKindMatch(.self): self = .selfKeyword
     case RawTokenKindMatch(.stringLiteral): self = .stringLiteral
@@ -742,6 +746,8 @@ enum PrimaryExpressionStart: RawTokenKindSubset {
     case .nilKeyword: return .keyword(.nil)
     case .period: return .period
     case .pound: return .pound
+    case .poundAvailableKeyword: return .poundAvailableKeyword
+    case .poundUnavailableKeyword: return .poundUnavailableKeyword
     case .regexLiteral: return .regexLiteral
     case .selfKeyword: return .keyword(.self)
     case .stringLiteral: return .stringLiteral

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

@@ -14,6 +14,22 @@ import SwiftDiagnostics
 @_spi(LexerDiagnostics) import SwiftParser
 @_spi(RawSyntax) import SwiftSyntax
 
+fileprivate extension TokenSyntax {
+  /// Assuming this token is a `poundAvailableKeyword` or `poundUnavailableKeyword`
+  /// returns the opposite keyword.
+  var negatedAvailabilityKeyword: TokenSyntax {
+    switch self.tokenKind {
+    case .poundAvailableKeyword:
+      return self.withKind(.poundUnavailableKeyword)
+    case .poundUnavailableKeyword:
+      return self.withKind(.poundAvailableKeyword)
+    default:
+      assertionFailure("The availability token of an AvailabilityConditionSyntax should always be #available or #unavailable")
+      return self
+    }
+  }
+}
+
 public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
   private var diagnostics: [Diagnostic] = []
 
@@ -464,11 +480,39 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     if shouldSkip(node) {
       return .skipChildren
     }
-    if node.identifier.presence == .missing,
-      let unexpected = node.unexpectedBeforeIdentifier,
-      unexpected.first?.as(TokenSyntax.self)?.tokenKind == .pound
-    {
-      addDiagnostic(unexpected, UnknownDirectiveError(unexpected: unexpected), handledNodes: [unexpected.id, node.identifier.id])
+    if node.identifier.presence == .missing, let unexpected = node.unexpectedBeforeIdentifier {
+      if unexpected.first?.as(TokenSyntax.self)?.tokenKind == .pound {
+        addDiagnostic(unexpected, UnknownDirectiveError(unexpected: unexpected), handledNodes: [unexpected.id, node.identifier.id])
+      } else if let availability = unexpected.first?.as(AvailabilityConditionSyntax.self) {
+        if let prefixOperatorExpr = node.parent?.as(PrefixOperatorExprSyntax.self),
+          let operatorToken = prefixOperatorExpr.operatorToken,
+          operatorToken.text == "!",
+          let conditionElement = prefixOperatorExpr.parent?.as(ConditionElementSyntax.self)
+        {
+          // Diagnose !#available(...) and !#unavailable(...)
+
+          let negatedAvailabilityKeyword = availability.availabilityKeyword.negatedAvailabilityKeyword
+          let negatedCoditionElement = ConditionElementSyntax(
+            condition: .availability(availability.withAvailabilityKeyword(negatedAvailabilityKeyword)),
+            trailingComma: conditionElement.trailingComma
+          )
+          addDiagnostic(
+            unexpected,
+            NegatedAvailabilityCondition(avaialabilityCondition: availability, negatedAvailabilityKeyword: negatedAvailabilityKeyword),
+            fixIts: [
+              FixIt(
+                message: ReplaceTokensFixIt(replaceTokens: [operatorToken, availability.availabilityKeyword], replacement: negatedAvailabilityKeyword),
+                changes: [
+                  .replace(oldNode: Syntax(conditionElement), newNode: Syntax(negatedCoditionElement))
+                ]
+              )
+            ],
+            handledNodes: [unexpected.id, node.identifier.id]
+          )
+        } else {
+          addDiagnostic(unexpected, AvailabilityConditionInExpression(avaialabilityCondition: availability), handledNodes: [unexpected.id, node.identifier.id])
+        }
+      }
     }
     return .visitChildren
   }

Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

@@ -171,6 +171,14 @@ extension DiagnosticMessage where Self == StaticParserError {
 
 // MARK: - Diagnostics (please sort alphabetically)
 
+public struct AvailabilityConditionInExpression: ParserError {
+  public let avaialabilityCondition: AvailabilityConditionSyntax
+
+  public var message: String {
+    return "\(nodesDescription([avaialabilityCondition], format: false)) cannot be used in an expression, only as a condition of 'if' or 'guard'"
+  }
+}
+
 public struct EffectsSpecifierAfterArrow: ParserError {
   public let effectsSpecifiersAfterArrow: [TokenSyntax]
 
@@ -225,6 +233,15 @@ public struct MissingAttributeArgument: ParserError {
   }
 }
 
+public struct NegatedAvailabilityCondition: ParserError {
+  public let avaialabilityCondition: AvailabilityConditionSyntax
+  public let negatedAvailabilityKeyword: TokenSyntax
+
+  public var message: String {
+    return "\(nodesDescription([avaialabilityCondition], format: false)) cannot be used in an expression; did you mean \(nodesDescription([negatedAvailabilityKeyword], format: false))?"
+  }
+}
+
 public struct SpaceSeparatedIdentifiersError: ParserError {
   public let firstToken: TokenSyntax
   public let additionalTokens: [TokenSyntax]

Tests/SwiftParserTest/translated/AvailabilityQueryTests.swift

@@ -31,9 +31,7 @@ final class AvailabilityQueryTests: XCTestCase {
       if (1️⃣#available(OSX 10.51, *)) {}
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 2: #available may only be used as condition of an 'if', 'guard'
-        DiagnosticSpec(message: "expected value in tuple"),
-        DiagnosticSpec(message: "unexpected code '#available(OSX 10.51, *)' in tuple"),
+        DiagnosticSpec(message: "availability condition cannot be used in an expression, only as a condition of 'if' or 'guard'")
       ]
     )
   }
@@ -44,9 +42,7 @@ final class AvailabilityQueryTests: XCTestCase {
       let x = 1️⃣#available(OSX 10.51, *)
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: #available may only be used as condition of
-        DiagnosticSpec(message: "expected expression in variable"),
-        DiagnosticSpec(message: "extraneous code '#available(OSX 10.51, *)' at top level"),
+        DiagnosticSpec(message: "availability condition cannot be used in an expression, only as a condition of 'if' or 'guard'")
       ]
     )
   }
@@ -57,28 +53,35 @@ final class AvailabilityQueryTests: XCTestCase {
       (1️⃣#available(OSX 10.51, *) ? 1 : 0)
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: #available may only be used as condition of an
-        DiagnosticSpec(message: "expected value in tuple"),
-        DiagnosticSpec(message: "unexpected code '#available(OSX 10.51, *) ? 1 : 0' in tuple"),
+        DiagnosticSpec(message: "availability condition cannot be used in an expression, only as a condition of 'if' or 'guard'")
       ]
     )
   }
 
-  func testAvailabilityQuery5() {
+  func testAvailabilityQuery5a() {
     AssertParse(
       """
       if !1️⃣#available(OSX 10.52, *) { 
       }
-      if let _ = Optional(5), !2️⃣#available(OSX 10.52, *) { 
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "availability condition cannot be used in an expression; did you mean '#unavailable'?", fixIts: ["replace '!#available' by '#unavailable'"])
+      ],
+      fixedSource: """
+        if #unavailable(OSX 10.52, *) {
+        }
+        """
+    )
+  }
+
+  func testAvailabilityQuery5b() {
+    AssertParse(
+      """
+      if let _ = Optional(5), !1️⃣#available(OSX 10.52, *) {
       }
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: #available cannot be used as an expression, did you mean to use '#unavailable'?, Fix-It replacements: 4 - 15 = '#unavailable'
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected expression in prefix operator expression"),
-        DiagnosticSpec(locationMarker: "1️⃣", message: "unexpected code '#available(OSX 10.52, *)' in 'if' statement"),
-        // TODO: Old parser expected error on line 3: #available cannot be used as an expression, did you mean to use '#unavailable'?, Fix-It replacements: 25 - 36 = '#unavailable'
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected expression in prefix operator expression"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "unexpected code '#available(OSX 10.52, *)' in 'if' statement"),
+        DiagnosticSpec(message: "availability condition cannot be used in an expression; did you mean '#unavailable'?", fixIts: ["replace '!#available' by '#unavailable'"])
       ]
     )
   }

Tests/SwiftParserTest/translated/AvailabilityQueryUnavailabilityTests.swift

@@ -25,34 +25,48 @@ final class AvailabilityQueryUnavailabilityTests: XCTestCase {
     )
   }
 
-  func testAvailabilityQueryUnavailability2() {
+  func testAvailabilityQueryUnavailability2a() {
     AssertParse(
       """
       // Disallow explicit wildcards.
       if #unavailable(OSX 10.51, *) {} 
       // Disallow use as an expression.
-      if (1️⃣#unavailable(OSX 10.51)) {}  
-      let x = 3️⃣#unavailable(OSX 10.51)
-      (#unavailable(OSX 10.51) ? 1 : 0) 
-      if !#unavailable(OSX 10.52) { 
+      if (1️⃣#unavailable(OSX 10.51)) {}
+      let x = 2️⃣#unavailable(OSX 10.51)
+      (3️⃣#unavailable(OSX 10.51) ? 1 : 0)
+      """,
+      diagnostics: [
+        DiagnosticSpec(locationMarker: "1️⃣", message: "availability condition cannot be used in an expression, only as a condition of 'if' or 'guard'"),
+        DiagnosticSpec(locationMarker: "2️⃣", message: "availability condition cannot be used in an expression, only as a condition of 'if' or 'guard'"),
+        DiagnosticSpec(locationMarker: "3️⃣", message: "availability condition cannot be used in an expression, only as a condition of 'if' or 'guard'"),
+      ]
+    )
+  }
+
+  func testAvailabilityQueryUnavailability2b() {
+    AssertParse(
+      """
+      if !1️⃣#unavailable(OSX 10.52) {
       }
-      if let _ = Optional(5), 5️⃣!6️⃣#unavailable(OSX 10.52) {
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "availability condition cannot be used in an expression; did you mean '#available'?", fixIts: ["replace '!#unavailable' by '#available'"])
+      ],
+      fixedSource: """
+        if #available(OSX 10.52) {
+        }
+        """
+    )
+  }
+
+  func testAvailabilityQueryUnavailability2c() {
+    AssertParse(
+      """
+      if let _ = Optional(5), !1️⃣#unavailable(OSX 10.52) {
       }
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 2: platform wildcard '*' is always implicit in #unavailable, Fix-It replacements: 28 - 29 = ''
-        // TODO: Old parser expected error on line 4: #unavailable may only be used as condition of an 'if', 'guard'
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected value in tuple"),
-        DiagnosticSpec(locationMarker: "1️⃣", message: "unexpected code '#unavailable(OSX 10.51)' in tuple"),
-        // TODO: Old parser expected error on line 5: #unavailable may only be used as condition of
-        DiagnosticSpec(locationMarker: "3️⃣", message: "expected expression in variable"),
-        DiagnosticSpec(locationMarker: "3️⃣", message: "unexpected code before variable"),
-        // TODO: Old parser expected error on line 6: #unavailable may only be used as condition of an
-        // TODO: Old parser expected error on line 7: #unavailable may only be used as condition of an
-        // TODO: Old parser expected error on line 9: #unavailable may only be used as condition
-        DiagnosticSpec(locationMarker: "5️⃣", message: "expected pattern in variable"),
-        DiagnosticSpec(locationMarker: "6️⃣", message: "expected expression in prefix operator expression"),
-        DiagnosticSpec(locationMarker: "6️⃣", message: "extraneous code at top level"),
+        DiagnosticSpec(message: "availability condition cannot be used in an expression; did you mean '#available'?", fixIts: ["replace '!#unavailable' by '#available'"])
       ]
     )
   }
@@ -459,23 +473,29 @@ final class AvailabilityQueryUnavailabilityTests: XCTestCase {
     )
   }
 
-  func testAvailabilityQueryUnavailability34() {
+  func testAvailabilityQueryUnavailability34a() {
     AssertParse(
       """
       // Diagnose wrong spellings of unavailability
-      if #available(*) 1️⃣== false { 
-      }
-      if !2️⃣#available(*) { 
+      if #available(*) 1️⃣== false {
       }
       """,
       diagnostics: [
         // TODO: Old parser expected error on line 2: #available cannot be used as an expression, did you mean to use '#unavailable'?, Fix-It replacements: 4 - 14 = '#unavailable', 18 - 27 = ''
-        DiagnosticSpec(locationMarker: "1️⃣", message: "unexpected code '== false' in 'if' statement"),
-        // TODO: Old parser expected error on line 4: #available cannot be used as an expression, did you mean to use '#unavailable'?, Fix-It replacements: 4 - 15 = '#unavailable'
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected expression in prefix operator expression"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "unexpected code '#available(*)' in 'if' statement"),
+        DiagnosticSpec(message: "unexpected code '== false' in 'if' statement")
       ]
     )
   }
 
+  func testAvailabilityQueryUnavailability34b() {
+    AssertParse(
+      """
+      if !1️⃣#available(*) {
+      }
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "availability condition cannot be used in an expression; did you mean '#unavailable'?", fixIts: ["replace '!#available' by '#unavailable'"])
+      ]
+    )
+  }
 }