Merge pull request #2422 from ahoppen/ahoppen/fixme-cleanup

ahoppen · web-flow · commit f2ef161a8d3b · 2024-01-10T15:28:05.000-08:00
Removes TODOs and FIXMEs that are no longer applicable
diff --git a/Sources/SwiftParser/Availability.swift b/Sources/SwiftParser/Availability.swift
@@ -145,8 +145,6 @@ extension Parser {
       case (.unavailable, let handle)?,
         (.noasync, let handle)?:
         let argument = self.eat(handle)
-        // FIXME: Can we model this in SwiftSyntax by making the
-        // 'labeled' argument part optional?
         entry = .token(argument)
       case (.star, _)?:
         entry = self.parseAvailabilitySpec()
@@ -177,8 +175,6 @@ extension Parser {
   /// Parse an availability argument.
   mutating func parseAvailabilitySpec() -> RawAvailabilityArgumentSyntax.Argument {
     if let star = self.consumeIfContextualPunctuator("*") {
-      // FIXME: Use makeAvailabilityVersionRestriction here - but swift-format
-      // doesn't expect it.
       return .token(star)
     }
 
diff --git a/Sources/SwiftParser/Declarations.swift b/Sources/SwiftParser/Declarations.swift
@@ -135,8 +135,6 @@ extension TokenConsumer {
       }
 
       // Otherwise, parse it as an expression.
-      // FIXME: C++ parser returns true if this is a top-level non-"script" files.
-      // But we don't have "is library" flag.
       return false
     case .some(_):
       // All other decl start keywords unconditionally start a decl.
@@ -1670,8 +1668,10 @@ extension Parser {
       let (unexpectedBeforeIdentifier, identifier) = self.expectIdentifier(allowSelfOrCapitalSelfAsIdentifier: true)
       var types = [RawDesignatedTypeSyntax]()
       while let comma = self.consume(if: .comma) {
-        // FIXME: The compiler accepts... anything here. This is a bug.
-        // let (unexpectedBeforeDesignatedType, designatedType) = self.expectIdentifier()
+        // Technically, we should only accept identifiers for the designated
+        // types but the C++ parser accepted anything, which we mimick.
+        // It's not worth fixing since designated types are no longer allowed
+        // anyway.
         let designatedType = self.consumeAnyToken()
         types.append(
           RawDesignatedTypeSyntax(
diff --git a/Sources/SwiftParser/Expressions.swift b/Sources/SwiftParser/Expressions.swift
@@ -791,7 +791,6 @@ extension Parser {
 
       // Check for a trailing closure, if allowed.
       if self.at(.leftBrace) && !leadingExpr.raw.kind.isLiteral && self.withLookahead({ $0.atValidTrailingClosure(flavor: flavor) }) {
-        // FIXME: if Result has a trailing closure, break out.
         // Add dummy blank argument list to the call expression syntax.
         let list = RawLabeledExprListSyntax(elements: [], arena: self.arena)
         let (first, rest) = self.parseTrailingClosures(flavor: flavor)
@@ -1472,7 +1471,6 @@ extension Parser {
     }
 
     if let (colon, rsquare) = self.consume(if: .colon, followedBy: .rightSquare) {
-      // FIXME: We probably want a separate node for the empty case.
       return RawExprSyntax(
         RawDictionaryExprSyntax(
           unexpectedBeforeLSquare,
diff --git a/Sources/SwiftParser/TopLevel.swift b/Sources/SwiftParser/TopLevel.swift
@@ -150,8 +150,6 @@ extension Parser {
       )
     }
 
-    // FIXME: It is unfortunate that the Swift book refers to these as
-    // "statements" and not "items".
     let item = self.parseItem(isAtTopLevel: isAtTopLevel, allowInitDecl: allowInitDecl)
     let semi = self.consume(if: .semicolon)
     var trailingSemis: [RawTokenSyntax] = []
diff --git a/Sources/SwiftParser/Types.swift b/Sources/SwiftParser/Types.swift
@@ -380,7 +380,6 @@ extension Parser {
   /// Parse a type placeholder.
   mutating func parsePlaceholderType() -> RawIdentifierTypeSyntax {
     let (unexpectedBeforeName, name) = self.expect(.wildcard)
-    // FIXME: Need a better syntax node than this
     return RawIdentifierTypeSyntax(
       unexpectedBeforeName,
       name: name,
diff --git a/Sources/SwiftParserDiagnostics/MissingTokenError.swift b/Sources/SwiftParserDiagnostics/MissingTokenError.swift
@@ -49,8 +49,6 @@ extension ParseDiagnosticsGenerator {
   private func handleInvalidIdentifier(invalidToken: TokenSyntax, missingToken: TokenSyntax, invalidTokenContainer: UnexpectedNodesSyntax) -> Bool {
     let fixIts: [FixIt]
     if invalidToken.tokenKind.isLexerClassifiedKeyword || invalidToken.tokenKind.isDollarIdentifier {
-      // TODO: Should the parser add the text with backticks to the missing
-      // node? Then this could just make missing/present.
       fixIts = [
         FixIt(
           message: .wrapInBackticks,
diff --git a/Sources/SwiftParserDiagnostics/MultiLineStringLiteralDiagnosticsGenerator.swift b/Sources/SwiftParserDiagnostics/MultiLineStringLiteralDiagnosticsGenerator.swift
@@ -133,7 +133,6 @@ final class MultiLineStringLiteralIndentationDiagnosticsGenerator: SyntaxVisitor
     case .stringSegment(let stringSegment):
       return stringSegment.last?.isNewline ?? false
     default:
-      // FIXME: newlines should never be part of trailing trivia
       return previousToken.trailingTrivia.contains(where: { $0.isNewline })
     }
   }
diff --git a/Sources/SwiftSyntax/Raw/RawSyntaxNodeProtocol.swift b/Sources/SwiftSyntax/Raw/RawSyntaxNodeProtocol.swift
@@ -270,9 +270,6 @@ public struct RawTokenSyntax: RawSyntaxNodeProtocol {
     trailingTriviaPieces: [RawTriviaPiece] = [],
     arena: __shared SyntaxArena
   ) {
-    // FIXME: Allow creating a `RawSyntax.parsedToken()` with a string literal
-    // for text. Currently it asserts that the string buffer is not contained
-    // within the `arena`.
     self.init(
       materialized: kind,
       text: text ?? kind.defaultText ?? "",
diff --git a/Sources/SwiftSyntax/SyntaxChildren.swift b/Sources/SwiftSyntax/SyntaxChildren.swift
@@ -153,9 +153,9 @@ struct RawSyntaxChildren: BidirectionalCollection {
 
   /// The rootId of the tree the child nodes belong to
   private let rootId: UInt
+
   /// The number of children in `parent`. Cached to avoid reaching into `parent` for every index
   /// advancement
-  // FIXME: Do we need this cached?
   private let numberOfChildren: Int
 
   let startIndex: SyntaxChildrenIndex
diff --git a/Sources/SwiftSyntaxBuilder/Syntax+StringInterpolation.swift b/Sources/SwiftSyntaxBuilder/Syntax+StringInterpolation.swift
@@ -268,7 +268,6 @@ extension String: ExpressibleByLiteralSyntax {
 
 extension ExpressibleByLiteralSyntax where Self: BinaryInteger {
   public func makeLiteralSyntax() -> IntegerLiteralExprSyntax {
-    // TODO: Radix selection? Thousands separators?
     let digits = String(self, radix: 10)
     return IntegerLiteralExprSyntax(literal: .integerLiteral(digits))
   }
@@ -305,7 +304,6 @@ extension ExpressibleByLiteralSyntax where Self: FloatingPoint, Self: LosslessSt
       )
 
     case .negativeNormal, .negativeSubnormal, .positiveZero, .positiveSubnormal, .positiveNormal:
-      // TODO: Thousands separators?
       let digits = String(self)
       return ExprSyntax(FloatLiteralExprSyntax(literal: .floatLiteral(digits)))
     }
diff --git a/Sources/SwiftSyntaxBuilder/SyntaxParsable+ExpressibleByStringInterpolation.swift b/Sources/SwiftSyntaxBuilder/SyntaxParsable+ExpressibleByStringInterpolation.swift
@@ -71,8 +71,6 @@ extension SyntaxParseable {
   public init(stringInterpolation: SyntaxStringInterpolation) {
     self = stringInterpolation.sourceText.withUnsafeBufferPointer { buffer in
       var parser = Parser(buffer)
-      // FIXME: When the parser supports incremental parsing, put the
-      // interpolatedSyntaxNodes in so we don't have to parse them again.
       let result = Self.parse(from: &parser)
       return result
     }
diff --git a/Tests/SwiftParserTest/translated/DollarIdentifierTests.swift b/Tests/SwiftParserTest/translated/DollarIdentifierTests.swift
@@ -188,13 +188,13 @@ final class DollarIdentifierTests: ParserTestCase {
     assertParse(
       """
       func escapedDollarAnd() {
-        // FIXME: Bad diagnostics.
         1️⃣`$0` = 1
         `$$` = 2
         `$abc` = 3
       }
       """,
       diagnostics: [
+        // FIXME: Bad diagnostic
         DiagnosticSpec(message: "unexpected code in function")
       ]
     )
diff --git a/Tests/SwiftParserTest/translated/EnumElementPatternSwift4Tests.swift b/Tests/SwiftParserTest/translated/EnumElementPatternSwift4Tests.swift
@@ -15,16 +15,6 @@
 import XCTest
 
 final class EnumElementPatternSwift4Tests: ParserTestCase {
-  func testEnumElementPatternSwift41() {
-    assertParse(
-      """
-      // https://github.com/apple/swift/issues/46040
-      // See test/Compatibility/enum_element_pattern.swift for Swift3 behavior.
-      // As for FIXME cases: see https://github.com/apple/swift/issues/46054
-      """
-    )
-  }
-
   func testEnumElementPatternSwift42() {
     assertParse(
       """
diff --git a/Tests/SwiftParserTest/translated/EnumTests.swift b/Tests/SwiftParserTest/translated/EnumTests.swift
@@ -15,15 +15,6 @@
 import XCTest
 
 final class EnumTests: ParserTestCase {
-  func testEnum1() {
-    assertParse(
-      """
-      // FIXME: this test only passes on platforms which have Float80.
-      // <rdar://problem/19508460> Floating point enum raw values are not portable
-      """
-    )
-  }
-
   func testEnum2() {
     assertParse(
       """
@@ -705,7 +696,6 @@ final class EnumTests: ParserTestCase {
     assertParse(
       """
       enum RawTypeWithRepeatValues5 : Double {
-        // FIXME: should reject.
         // 2^65-1
         case Vaughn = 36893488147419103231
         case Wilson = 36893488147419103231.0
@@ -718,7 +708,6 @@ final class EnumTests: ParserTestCase {
     assertParse(
       """
       enum RawTypeWithRepeatValues6 : Double {
-        // FIXME: should reject.
         // 2^127-1
         case Vaughn = 170141183460469231731687303715884105727
         case Wilson = 170141183460469231731687303715884105727.0
@@ -731,7 +720,6 @@ final class EnumTests: ParserTestCase {
     assertParse(
       """
       enum RawTypeWithRepeatValues7 : Double {
-        // FIXME: should reject.
         // 2^128-1
         case Vaughn = 340282366920938463463374607431768211455
         case Wilson = 340282366920938463463374607431768211455.0
diff --git a/Tests/SwiftParserTest/translated/GenericDisambiguationTests.swift b/Tests/SwiftParserTest/translated/GenericDisambiguationTests.swift
@@ -213,11 +213,14 @@ final class GenericDisambiguationTests: ParserTestCase {
   }
 
   func testGenericDisambiguation15() {
-    // TODO: parse empty <> list
     assertParse(
       """
       A<>.c()
-      """
+      """,
+      substructure: PostfixOperatorExprSyntax(
+        expression: ExprSyntax(DeclReferenceExprSyntax(baseName: .identifier("A"))),
+        operator: .postfixOperator("<>")
+      )
     )
   }
 
diff --git a/Tests/SwiftParserTest/translated/IfconfigExprTests.swift b/Tests/SwiftParserTest/translated/IfconfigExprTests.swift
@@ -500,7 +500,6 @@ final class IfconfigExprTests: ParserTestCase {
     )
   }
 
-  // FIXME: Diagnostics could be better.
   func testIfConfigExpr34() {
     assertParse(
       """
diff --git a/Tests/SwiftParserTest/translated/MatchingPatternsTests.swift b/Tests/SwiftParserTest/translated/MatchingPatternsTests.swift
@@ -509,8 +509,6 @@ final class MatchingPatternsTests: ParserTestCase {
   func testMatchingPatterns31() {
     assertParse(
       #"""
-      // FIXME: We don't currently allow subpatterns for "isa" patterns that
-      // require interesting conditional downcasts.
       class Base { }
       class Derived : Base { }
       """#
diff --git a/Tests/SwiftParserTest/translated/OperatorDeclTests.swift b/Tests/SwiftParserTest/translated/OperatorDeclTests.swift
@@ -351,8 +351,6 @@ final class OperatorDeclTests: ParserTestCase {
   }
 
   func testOperatorDecl13() {
-    // FIXME: Ideally, we shouldn't emit the «consistent whitespace» diagnostic
-    // where = cannot possibly mean an assignment.
     assertParse(
       """
       infix operator =1️⃣#=
@@ -528,7 +526,6 @@ final class OperatorDeclTests: ParserTestCase {
   }
 
   func testOperatorDecl21() {
-    // TODO: We should not allow specification of multiple precedence groups
     assertParse(
       """
       protocol Proto {}
diff --git a/Tests/SwiftParserTest/translated/RecoveryTests.swift b/Tests/SwiftParserTest/translated/RecoveryTests.swift
@@ -12,6 +12,7 @@
 
 // This test file has been translated from swift/test/Parse/recovery.swift
 
+import SwiftSyntax
 import XCTest
 
 final class RecoveryTests: ParserTestCase {
@@ -290,10 +291,6 @@ final class RecoveryTests: ParserTestCase {
       """,
       diagnostics: [
         DiagnosticSpec(message: "missing condition in 'if' statement")
-        // TODO: Old parser expected error on line 3: consecutive statements on a line must be separated by ';', Fix-It replacements: 14 - 14 = ';'
-        // TODO: Old parser expected error on line 3: closure expression is unused
-        // TODO: Old parser expected note on line 3: did you mean to use a 'do' statement?, Fix-It replacements: 15 - 15 = 'do '
-        // TODO: Old parser expected warning on line 3: boolean literal is unused
       ]
     )
   }
@@ -318,8 +315,6 @@ final class RecoveryTests: ParserTestCase {
       """,
       diagnostics: [
         DiagnosticSpec(message: "missing condition in 'if' statement")
-        // TODO: Old parser expected error on line 2: closure expression is unused
-        // TODO: Old parser expected note on line 2: did you mean to use a 'do' statement?, Fix-It replacements: 8 - 8 = 'do '
       ]
     )
   }
@@ -374,10 +369,6 @@ final class RecoveryTests: ParserTestCase {
       """,
       diagnostics: [
         DiagnosticSpec(message: "missing condition in 'while' statement")
-        // TODO: Old parser expected error on line 3: consecutive statements on a line must be separated by ';', Fix-It replacements: 17 - 17 = ';'
-        // TODO: Old parser expected error on line 3: closure expression is unused
-        // TODO: Old parser expected note on line 3: did you mean to use a 'do' statement?, Fix-It replacements: 18 - 18 = 'do '
-        // TODO: Old parser expected warning on line 3: boolean literal is unused
       ]
     )
   }
@@ -402,8 +393,6 @@ final class RecoveryTests: ParserTestCase {
       """,
       diagnostics: [
         DiagnosticSpec(message: "missing condition in 'while' statement")
-        // TODO: Old parser expected error on line 2: closure expression is unused
-        // TODO: Old parser expected note on line 2: did you mean to use a 'do' statement?, Fix-It replacements: 11 - 11 = 'do '
       ]
     )
   }
@@ -814,7 +803,6 @@ final class RecoveryTests: ParserTestCase {
       """,
       diagnostics: [
         DiagnosticSpec(locationMarker: "1️⃣", message: "expected expression in 'switch' statement")
-        // TODO: Old parser expected error on line 2: 'is' keyword required to pattern match against type name, Fix-It replacements: 10 - 10 = 'is '
       ]
     )
   }
@@ -2871,9 +2859,23 @@ final class RecoveryTests: ParserTestCase {
           &(Int:x)
       }
       """,
-      diagnostics: [
-        // TODO: Old parser expected error on line 4: '&' may only be used to pass an argument to inout parameter
-      ]
+      substructure: InOutExprSyntax(
+        ampersand: .prefixAmpersandToken(),
+        expression: ExprSyntax(
+          TupleExprSyntax(
+            leftParen: .leftParenToken(),
+            elements: LabeledExprListSyntax([
+              LabeledExprSyntax(
+                label: .identifier("Int"),
+                colon: .colonToken(),
+                expression: ExprSyntax(DeclReferenceExprSyntax(baseName: .identifier("x")))
+              )
+            ]),
+            rightParen: .rightParenToken()
+          )
+        )
+      )
+
     )
   }
 
diff --git a/Tests/SwiftParserTest/translated/StringLiteralEofTests.swift b/Tests/SwiftParserTest/translated/StringLiteralEofTests.swift
@@ -164,7 +164,6 @@ final class StringLiteralEofTests: ParserTestCase {
             \(<#expression#>)
         """
         """##
-      // FIXME: The closing delimiter should be put on the new line
     )
   }
 
diff --git a/Tests/SwiftParserTest/translated/SubscriptingTests.swift b/Tests/SwiftParserTest/translated/SubscriptingTests.swift
@@ -176,7 +176,6 @@ final class SubscriptingTests: ParserTestCase {
   func testSubscripting11() {
     assertParse(
       """
-      // FIXME: This test case does not belong in Parse/
       let y2 = Y2()
       _ = y2[0]
       """
diff --git a/Tests/SwiftParserTest/translated/SuperTests.swift b/Tests/SwiftParserTest/translated/SuperTests.swift
@@ -92,7 +92,6 @@ final class SuperTests: ParserTestCase {
           super.foo.bar
           super.bar
           super.bar()
-          // FIXME: should also say "'super.init' cannot be referenced outside of an initializer"
           super.init
           super.init()
           super.init(0)
diff --git a/Tests/SwiftParserTest/translated/TypeExprTests.swift b/Tests/SwiftParserTest/translated/TypeExprTests.swift
@@ -43,7 +43,6 @@ final class TypeExprTests: ParserTestCase {
       protocol Zim {
         associatedtype Zang
         init()
-        // TODO class var prop: Int { get }
         static func meth() {}
         func instMeth() {}
       }
@@ -224,7 +223,6 @@ final class TypeExprTests: ParserTestCase {
       func archetype<T: Zim>(_: T) {
         _ = T.self
         _ = T()
-        // TODO let prop = T.prop
         _ = T.meth
         let _ : () = T.meth()
         _ = T
@@ -239,7 +237,6 @@ final class TypeExprTests: ParserTestCase {
       func assocType<T: Zim>(_: T) where T.Zang: Zim {
         _ = T.Zang.self
         _ = T.Zang()
-        // TODO _ = T.Zang.prop
         _ = T.Zang.meth
         let _ : () = T.Zang.meth()
         _ = T.Zang
diff --git a/Tests/SwiftSyntaxBuilderTest/TriviaTests.swift b/Tests/SwiftSyntaxBuilderTest/TriviaTests.swift

Original file line number	Diff line number	Diff line change
`@@ -150,8 +150,6 @@ extension Parser {`
`150`	`150`	`)`
`151`	`151`	`}`
`152`	`152`
`153`		`- // FIXME: It is unfortunate that the Swift book refers to these as`
`154`		`- // "statements" and not "items".`
`155`	`153`	`let item = self.parseItem(isAtTopLevel: isAtTopLevel, allowInitDecl: allowInitDecl)`
`156`	`154`	`let semi = self.consume(if: .semicolon)`
`157`	`155`	`var trailingSemis: [RawTokenSyntax] = []`
Original file line number	Diff line number	Diff line change
`@@ -133,7 +133,6 @@ final class MultiLineStringLiteralIndentationDiagnosticsGenerator: SyntaxVisitor`
`133`	`133`	`case .stringSegment(let stringSegment):`
`134`	`134`	`return stringSegment.last?.isNewline ?? false`
`135`	`135`	`default:`
`136`		`- // FIXME: newlines should never be part of trailing trivia`
`137`	`136`	`return previousToken.trailingTrivia.contains(where: { $0.isNewline })`
`138`	`137`	`}`
`139`	`138`	`}`
Original file line number	Diff line number	Diff line change
`@@ -268,7 +268,6 @@ extension String: ExpressibleByLiteralSyntax {`
`268`	`268`
`269`	`269`	`extension ExpressibleByLiteralSyntax where Self: BinaryInteger {`
`270`	`270`	`public func makeLiteralSyntax() -> IntegerLiteralExprSyntax {`
`271`		`- // TODO: Radix selection? Thousands separators?`
`272`	`271`	`let digits = String(self, radix: 10)`
`273`	`272`	`return IntegerLiteralExprSyntax(literal: .integerLiteral(digits))`
`274`	`273`	`}`
`@@ -305,7 +304,6 @@ extension ExpressibleByLiteralSyntax where Self: FloatingPoint, Self: LosslessSt`
`305`	`304`	`)`
`306`	`305`
`307`	`306`	`case .negativeNormal, .negativeSubnormal, .positiveZero, .positiveSubnormal, .positiveNormal:`
`308`		`- // TODO: Thousands separators?`
`309`	`307`	`let digits = String(self)`
`310`	`308`	`return ExprSyntax(FloatLiteralExprSyntax(literal: .floatLiteral(digits)))`
`311`	`309`	`}`