[SwiftParser] Lookahead: Teach skipTypeAttributeList about modifiers with arguments

Author: xedin

Sources/SwiftParser/Types.swift

@@ -691,11 +691,27 @@ extension Parser.Lookahead {
     var specifierProgress = LoopProgressCondition()
     // TODO: Can we model isolated/_const so that they're specified in both canParse* and parse*?
     while canHaveParameterSpecifier,
-      self.at(anyIn: SimpleTypeSpecifierSyntax.SpecifierOptions.self) != nil || self.at(.keyword(.isolated))
-        || self.at(.keyword(._const)),
+      self.at(anyIn: SimpleTypeSpecifierSyntax.SpecifierOptions.self) != nil
+        || self.at(.keyword(.nonisolated), .keyword(.dependsOn)),
       self.hasProgressed(&specifierProgress)
     {
-      self.consumeAnyToken()
+      switch self.currentToken {
+      case .keyword(.nonisolated), .keyword(.dependsOn):
+        self.consumeAnyToken()
+
+        // The argument is missing but it still could be a valid modifier,
+        // i.e. `nonisolated` in an inheritance clause.
+        guard self.at(.leftParen) else {
+          continue
+        }
+
+        if self.withLookahead({ $0.atAttributeOrSpecifierArgument() }) {
+          skipSingle()
+        }
+
+      default:
+        self.consumeAnyToken()
+      }
     }
 
     var attributeProgress = LoopProgressCondition()
@@ -1059,10 +1075,24 @@ extension Parser {
   private mutating func parseNonisolatedTypeSpecifier() -> RawTypeSpecifierListSyntax.Element {
     let (unexpectedBeforeNonisolatedKeyword, nonisolatedKeyword) = self.expect(.keyword(.nonisolated))
 
-    // Avoid being to greedy about `(` since this modifier should be associated with
-    // function types, it's possible that the argument is omitted and what follows
-    // is a function type i.e. `nonisolated () async -> Void`.
-    if self.at(.leftParen) && !withLookahead({ $0.atAttributeOrSpecifierArgument() }) {
+    // If the next token is not '(' this could mean two things:
+    //  - What follows is a type and we should allow it because
+    //    using `nonsisolated` without an argument is allowed in
+    //    an inheritance clause.
+    //  - The '(nonsending)' was omitted.
+    if !self.at(.leftParen) {
+      // `nonisolated P<...>` is allowed in an inheritance clause.
+      if withLookahead({ $0.canParseTypeIdentifier() }) {
+        let nonisolatedSpecifier = RawNonisolatedTypeSpecifierSyntax(
+          unexpectedBeforeNonisolatedKeyword,
+          nonisolatedKeyword: nonisolatedKeyword,
+          argument: nil,
+          arena: self.arena
+        )
+        return .nonisolatedTypeSpecifier(nonisolatedSpecifier)
+      }
+
+      // Otherwise require '(nonsending)'
       let argument = RawNonisolatedSpecifierArgumentSyntax(
         leftParen: missingToken(.leftParen),
         nonsendingKeyword: missingToken(.keyword(.nonsending)),
@@ -1076,24 +1106,37 @@ extension Parser {
         argument: argument,
         arena: self.arena
       )
+
       return .nonisolatedTypeSpecifier(nonisolatedSpecifier)
     }
 
-    // nonisolated without an argument is valid in some positions i.e. inheritance clause.
-    guard let leftParen = self.consume(if: .leftParen) else {
+    // Avoid being to greedy about `(` since this modifier should be associated with
+    // function types, it's possible that the argument is omitted and what follows
+    // is a function type i.e. `nonisolated () async -> Void`.
+    if self.at(.leftParen) && !withLookahead({ $0.atAttributeOrSpecifierArgument() }) {
+      let argument = RawNonisolatedSpecifierArgumentSyntax(
+        leftParen: missingToken(.leftParen),
+        nonsendingKeyword: missingToken(.keyword(.nonsending)),
+        rightParen: missingToken(.rightParen),
+        arena: self.arena
+      )
+
       let nonisolatedSpecifier = RawNonisolatedTypeSpecifierSyntax(
         unexpectedBeforeNonisolatedKeyword,
         nonisolatedKeyword: nonisolatedKeyword,
-        argument: nil,
+        argument: argument,
         arena: self.arena
       )
+
       return .nonisolatedTypeSpecifier(nonisolatedSpecifier)
     }
 
+    let (unexpectedBeforeLeftParen, leftParen) = self.expect(.leftParen)
     let (unexpectedBeforeModifier, modifier) = self.expect(.keyword(.nonsending))
     let (unexpectedBeforeRightParen, rightParen) = self.expect(.rightParen)
 
     let argument = RawNonisolatedSpecifierArgumentSyntax(
+      unexpectedBeforeLeftParen,
       leftParen: leftParen,
       unexpectedBeforeModifier,
       nonsendingKeyword: modifier,

Tests/SwiftParserTest/TypeTests.swift

@@ -485,6 +485,20 @@ final class TypeTests: ParserTestCase {
       experimentalFeatures: [.nonescapableTypes]
     )
 
+    assertParse(
+      "func foo() -> dependsOn1️⃣ @Sendable (Int, isolated (any Actor)?) async throws -> Void",
+      diagnostics: [
+        DiagnosticSpec(
+          locationMarker: "1️⃣",
+          message: "expected '(', parameter reference, and ')' in lifetime specifier",
+          fixIts: ["insert '(', parameter reference, and ')'"]
+        )
+      ],
+      fixedSource:
+        "func foo() -> dependsOn(<#identifier#>) @Sendable (Int, isolated (any Actor)?) async throws -> Void",
+      experimentalFeatures: [.nonescapableTypes]
+    )
+
     assertParse(
       "func foo() -> dependsOn(1️⃣*) X",
       diagnostics: [
@@ -545,6 +559,23 @@ final class TypeTests: ParserTestCase {
     assertParse("let _ = Array<nonisolated(nonsending) () async -> Void>()")
     assertParse("func foo(test: nonisolated(nonsending) () async -> Void)")
     assertParse("func foo(test: nonisolated(nonsending) @escaping () async -> Void) {}")
+    assertParse("test(S<nonisolated(nonsending) () async -> Void>(), type(of: concurrentTest))")
+    assertParse("S<nonisolated(nonsending) @Sendable (Int) async -> Void>()")
+    assertParse("let _ = S<nonisolated(nonsending) consuming @Sendable (Int) async -> Void>()")
+    assertParse("struct S : nonisolated P {}")
+    assertParse("let _ = [nonisolated()]")
+
+    assertParse(
+      "Foo<Int, nonisolated1️⃣ @Sendable (Int, inout (any Actor)?) async throws -> Void>()",
+      diagnostics: [
+        DiagnosticSpec(
+          locationMarker: "1️⃣",
+          message: "expected '(nonsending)' in 'nonisolated' specifier",
+          fixIts: ["insert '(nonsending)'"]
+        )
+      ],
+      fixedSource: "Foo<Int, nonisolated(nonsending) @Sendable (Int, inout (any Actor)?) async throws -> Void>()"
+    )
 
     assertParse(
       "func foo(test: nonisolated1️⃣ () async -> Void)",