Merge pull request #1809 from TTOzzi/remove-unexpected-node-between-argument-and-right-paren-in-attribute-syntax

Improve diagnostics for invalid version tuples

Author: ahoppen

Sources/SwiftParser/Availability.swift

@@ -253,6 +253,23 @@ extension Parser {
     return self.consumeAnyToken()
   }
 
+  /// Consume the unexpected version token(e.g. integerLiteral, floatingLiteral, identifier) until the period no longer appears.
+  private mutating func parseUnexpectedVersionTokens() -> RawUnexpectedNodesSyntax? {
+    var unexpectedTokens: [RawTokenSyntax] = []
+    var keepGoing: RawTokenSyntax? = nil
+    var loopProgress = LoopProgressCondition()
+    repeat {
+      if let keepGoing {
+        unexpectedTokens.append(keepGoing)
+      }
+      if let unexpectedVersion = self.consume(if: .integerLiteral, .floatingLiteral, .identifier) {
+        unexpectedTokens.append(unexpectedVersion)
+      }
+      keepGoing = self.consume(if: .period)
+    } while keepGoing != nil && loopProgress.evaluate(currentToken)
+    return RawUnexpectedNodesSyntax(unexpectedTokens, arena: self.arena)
+  }
+
   /// Parse a dot-separated list of version numbers.
   ///
   /// Grammar
@@ -286,24 +303,29 @@ extension Parser {
         } else {
           trailingComponents.append(versionComponent)
         }
-      }
-
-      var unexpectedTrailingComponents: RawUnexpectedNodesSyntax?
 
-      if !trailingComponents.isEmpty {
-        unexpectedTrailingComponents = RawUnexpectedNodesSyntax(elements: trailingComponents.compactMap { $0.as(RawSyntax.self) }, arena: self.arena)
+        if version.isMissing {
+          break
+        }
       }
 
+      let unexpectedTrailingComponents = RawUnexpectedNodesSyntax(trailingComponents, arena: self.arena)
+      let unexpectedAfterComponents = self.parseUnexpectedVersionTokens()
       return RawVersionTupleSyntax(
         major: major,
         components: RawVersionComponentListSyntax(elements: components, arena: self.arena),
-        unexpectedTrailingComponents,
+        RawUnexpectedNodesSyntax(combining: unexpectedTrailingComponents, unexpectedAfterComponents, arena: self.arena),
         arena: self.arena
       )
-
     } else {
       let major = self.expectDecimalIntegerWithoutRecovery()
-      return RawVersionTupleSyntax(major: major, components: nil, arena: self.arena)
+      let unexpectedAfterComponents = self.parseUnexpectedVersionTokens()
+      return RawVersionTupleSyntax(
+        major: major,
+        components: nil,
+        unexpectedAfterComponents,
+        arena: self.arena
+      )
     }
   }
 }

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

@@ -1938,14 +1938,22 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
       return .skipChildren
     }
 
-    if let trailingComponents = node.unexpectedAfterComponents,
-      let components = node.components
-    {
-      addDiagnostic(
-        trailingComponents,
-        TrailingVersionAreIgnored(major: node.major, components: components),
-        handledNodes: [trailingComponents.id]
-      )
+    if let unexpectedAfterComponents = node.unexpectedAfterComponents {
+      if let components = node.components,
+        unexpectedAfterComponents.allSatisfy({ $0.is(VersionComponentSyntax.self) })
+      {
+        addDiagnostic(
+          unexpectedAfterComponents,
+          TrailingVersionAreIgnored(major: node.major, components: components),
+          handledNodes: [unexpectedAfterComponents.id]
+        )
+      } else {
+        addDiagnostic(
+          unexpectedAfterComponents,
+          CannotParseVersionTuple(versionTuple: unexpectedAfterComponents),
+          handledNodes: [node.major.id, node.components?.id, unexpectedAfterComponents.id].compactMap { $0 }
+        )
+      }
     }
 
     return .visitChildren

Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

@@ -292,7 +292,7 @@ public struct CannotParseVersionTuple: ParserError {
   public let versionTuple: UnexpectedNodesSyntax
 
   public var message: String {
-    return "cannot parse version \(versionTuple)"
+    return "cannot parse version component \(versionTuple.shortSingleLineContentDescription)"
   }
 }
 

Tests/SwiftParserTest/AvailabilityTests.swift

@@ -142,19 +142,21 @@ final class AvailabilityTests: XCTestCase {
       )
     )
 
+    assertParse(
+      """
+      @available(OSX 10.0.1, *)
+      func test() {}
+      """
+    )
+
     assertParse(
       """
       @available(OSX 1️⃣10e10)
       func test() {}
       """,
       diagnostics: [
-        DiagnosticSpec(message: "expected version tuple in version restriction", fixIts: ["insert version tuple"]),
-        DiagnosticSpec(message: "unexpected code '10e10' in attribute"),
-      ],
-      fixedSource: """
-        @available(OSX <#integer literal#>10e10)
-        func test() {}
-        """
+        DiagnosticSpec(message: "cannot parse version component code '10e10'")
+      ]
     )
 
     assertParse(
@@ -163,13 +165,8 @@ final class AvailabilityTests: XCTestCase {
       func test() {}
       """,
       diagnostics: [
-        DiagnosticSpec(message: "expected integer literal in version tuple", fixIts: ["insert integer literal"]),
-        DiagnosticSpec(message: "unexpected code '0e10' in attribute"),
-      ],
-      fixedSource: """
-        @available(OSX 10.<#integer literal#>0e10)
-        func test() {}
-        """
+        DiagnosticSpec(message: "cannot parse version component code '0e10'")
+      ]
     )
 
     assertParse(
@@ -178,13 +175,8 @@ final class AvailabilityTests: XCTestCase {
       func test() {}
       """,
       diagnostics: [
-        DiagnosticSpec(message: "expected version tuple in version restriction", fixIts: ["insert version tuple"]),
-        DiagnosticSpec(message: "unexpected code '0xff' in attribute"),
-      ],
-      fixedSource: """
-        @available(OSX <#integer literal#>0xff)
-        func test() {}
-        """
+        DiagnosticSpec(message: "cannot parse version component code '0xff'")
+      ]
     )
 
     assertParse(
@@ -193,13 +185,28 @@ final class AvailabilityTests: XCTestCase {
       func test() {}
       """,
       diagnostics: [
-        DiagnosticSpec(message: "expected integer literal in version tuple", fixIts: ["insert integer literal"]),
-        DiagnosticSpec(message: "unexpected code '0xff' in attribute"),
-      ],
-      fixedSource: """
-        @available(OSX 1.0.<#integer literal#>0xff)
-        func test() {}
-        """
+        DiagnosticSpec(message: "cannot parse version component code '0xff'")
+      ]
+    )
+
+    assertParse(
+      """
+      @available(OSX 1.0.1️⃣0xff, *)
+      func test() {}
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "cannot parse version component code '0xff'")
+      ]
+    )
+
+    assertParse(
+      """
+      @available(OSX 1.0.1️⃣asdf)
+      func test() {}
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "cannot parse version component code 'asdf'")
+      ]
     )
   }
 }

Tests/SwiftParserTest/translated/IfconfigExprTests.swift

@@ -388,7 +388,7 @@ final class IfconfigExprTests: XCTestCase {
       #endif
       """#,
       diagnostics: [
-        DiagnosticSpec(message: "cannot parse version \"\"")
+        DiagnosticSpec(message: #"cannot parse version component code '""'"#)
       ]
     )
   }
@@ -401,7 +401,7 @@ final class IfconfigExprTests: XCTestCase {
       #endif
       """,
       diagnostics: [
-        DiagnosticSpec(message: "cannot parse version >=2.2")
+        DiagnosticSpec(message: "cannot parse version component code '>=2.2'")
       ]
     )
   }
@@ -414,7 +414,7 @@ final class IfconfigExprTests: XCTestCase {
       #endif
       """,
       diagnostics: [
-        DiagnosticSpec(message: "cannot parse version 20A301")
+        DiagnosticSpec(message: "cannot parse version component code '20A301'")
       ]
     )
   }
@@ -427,7 +427,7 @@ final class IfconfigExprTests: XCTestCase {
       #endif
       """#,
       diagnostics: [
-        DiagnosticSpec(message: "cannot parse version \"20A301\"")
+        DiagnosticSpec(message: #"cannot parse version component code '"20A301"'"#)
       ]
     )
   }