Merge pull request #937 from ahoppen/ahoppen/review-comments-2

Address review comments from merged PRs

Author: ahoppen

Sources/SwiftParser/Declarations.swift

@@ -1592,6 +1592,14 @@ extension Parser {
     _ handle: RecoveryConsumptionHandle
   ) -> RawSubscriptDeclSyntax {
     let (unexpectedBeforeSubscriptKeyword, subscriptKeyword) = self.eat(handle)
+
+    let unexpectedName: RawTokenSyntax?
+    if self.at(.identifier) && self.peek().starts(with: "<") || self.peek().tokenKind == .leftParen {
+      unexpectedName = self.consumeAnyToken()
+    } else {
+      unexpectedName = nil
+    }
+
     let genericParameterClause: RawGenericParameterClauseSyntax?
     if self.currentToken.starts(with: "<") {
       genericParameterClause = self.parseGenericParameters()
@@ -1624,6 +1632,7 @@ extension Parser {
       modifiers: attrs.modifiers,
       unexpectedBeforeSubscriptKeyword,
       subscriptKeyword: subscriptKeyword,
+      RawUnexpectedNodesSyntax([unexpectedName], arena: self.arena),
       genericParameterClause: genericParameterClause,
       indices: indices,
       result: result,

Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift

@@ -305,6 +305,12 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     if shouldSkip(node) {
       return .skipChildren
     }
+    if let unexpected = node.unexpectedBetweenSubscriptKeywordAndGenericParameterClause,
+        let nameTokens = unexpected.onlyTokens(satisfying: { !$0.tokenKind.isKeyword }) {
+      addDiagnostic(unexpected, .subscriptsCannotHaveNames, fixIts: [
+        FixIt(message: RemoveTokensFixIt(tokensToRemove: nameTokens), changes: .makeMissing(tokens: nameTokens))
+      ], handledNodes: [unexpected.id])
+    }
     if let unexpected = node.indices.unexpectedBeforeLeftParen,
         let nameTokens = unexpected.onlyTokens(satisfying: { !$0.tokenKind.isKeyword }) {
       addDiagnostic(unexpected, .subscriptsCannotHaveNames, fixIts: [

Sources/SwiftParser/Expressions.swift

@@ -1162,7 +1162,7 @@ extension Parser {
   public mutating func parseIdentifierExpression() -> RawExprSyntax {
     let (name, args) = self.parseDeclNameRef(.compoundNames)
     guard self.lookahead().canParseAsGenericArgumentList() else {
-      if name.tokenText.hasPrefix("<#") && name.tokenText.hasSuffix("#>") && args == nil {
+      if name.tokenText.isEditorPlaceholder && args == nil {
         return RawExprSyntax(
           RawEditorPlaceholderExprSyntax(
             identifier: name,

Sources/SwiftParser/Lexer.swift

@@ -52,9 +52,7 @@ public struct Lexer {
     }
 
     var isEditorPlaceholder: Bool {
-      return self.tokenKind == .identifier &&
-      self.tokenText.starts(with: SyntaxText("<#")) &&
-      self.tokenText.hasSuffix(SyntaxText("#>"))
+      return self.tokenKind == .identifier && self.tokenText.isEditorPlaceholder
     }
 
     @_spi(RawSyntax)

Sources/SwiftParser/SyntaxUtils.swift

@@ -24,13 +24,26 @@ extension RawUnexpectedNodesSyntax {
     })
   }
 
-  /// If `tokens` is not empty, construct a `RawUnexpectedNodesSyntax`
+  /// If `nodes` is not empty, construct a `RawUnexpectedNodesSyntax`
   /// containing those tokens, otherwise return `nil`.
-  init?(_ tokens: [RawTokenSyntax], arena: SyntaxArena) {
-    if tokens.isEmpty {
+  init?<SyntaxType: RawSyntaxNodeProtocol>(_ nodes: [SyntaxType], arena: SyntaxArena) {
+    if nodes.isEmpty {
       return nil
     } else {
-      self.init(elements: tokens.map(RawSyntax.init), arena: arena)
+      self.init(elements: nodes.map(RawSyntax.init), arena: arena)
     }
   }
+
+  /// If `nodes` contains non-`nil` values, construct a `RawUnexpectedNodesSyntax`
+  /// containing those tokens, otherwise return `nil`.
+  init?<SyntaxType: RawSyntaxNodeProtocol>(_ nodes: [SyntaxType?], arena: SyntaxArena) {
+    self.init(nodes.compactMap({ $0 }), arena: arena)
+  }
+}
+
+extension SyntaxText {
+  var isEditorPlaceholder: Bool {
+    return self.starts(with: SyntaxText("<#")) &&
+    self.hasSuffix(SyntaxText("#>"))
+  }
 }

Tests/SwiftParserTest/translated/SubscriptingTests.swift

@@ -422,13 +422,13 @@ final class SubscriptingTests: XCTestCase {
       """,
       diagnostics: [
         DiagnosticSpec(locationMarker: "1️⃣", message: "subscripts cannot have a name", fixIts: ["remove 'x'"]),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "subscripts cannot have a name", fixIts: ["remove 'x<T>'"]),
+        DiagnosticSpec(locationMarker: "2️⃣", message: "subscripts cannot have a name", fixIts: ["remove 'x'"]),
       ], fixedSource: """
       struct A10 {
         subscript (i: Int) -> Int {
           return 0
         }
-        subscript (i: T) -> Int {
+        subscript <T>(i: T) -> Int {
           return 0
         }
       }
@@ -440,7 +440,7 @@ final class SubscriptingTests: XCTestCase {
     AssertParse(
       """
       struct A11 {
-        subscript 1️⃣x y : 2️⃣Int 3️⃣-> Int 4️⃣{ 
+        subscript 1️⃣x y : 2️⃣Int 3️⃣-> Int 4️⃣{
           return 0
         }
       }