Fix-its for misplaced typed throws could be improved

[DougGregor]

Issue Kind

Bad Diagnostic Produced

Source Code

func foo() -> throws(MyError) Int {}

Description

Applying the Fix-It for the parser diagnostics here produces malformed code, because the thrown error type gets lost. The "fixed" code is:

func foo() throws -> (MyError) Int {}

It should be

func foo() throws(MyError) -> Int {}

[ahoppen]

Tracked in Apple’s issue tracker as rdar://119341656

[vj-codes]

@ahoppen hey can I work on this?

[DougGregor]

@vj-codes yes, please!

[matthewcheok]

I looked into this briefly and it looks like when the following syntax is parsed and the first throws clause has no type - wonder if that's the reason why the type is later omitted and is thought to be part of the return clause? Also wondering what the difference is between materializedToken and parsedToken?

▿ func foo() -> throws(MyError) Int {}
  ▿ raw : .layout(sourceFile byteLength=36 descendantCount=33)
    - 0 : nil
    - 1 : nil
    - 2 : nil
    ▿ 3 : .layout(codeBlockItemList byteLength=36 descendantCount=31)
      ▿ 0 : .layout(codeBlockItem byteLength=36 descendantCount=30)
        - 0 : nil
        ▿ 1 : .layout(functionDecl byteLength=36 descendantCount=29)
          - 0 : nil
          - 1 : .layout(attributeList byteLength=0 descendantCount=0)
          - 2 : nil
          - 3 : .layout(declModifierList byteLength=0 descendantCount=0)
          - 4 : nil
          - 5 : .parsedToken(keyword wholeText="func " textRange=0..<4)
          - 6 : nil
          - 7 : .parsedToken(identifier wholeText="foo" textRange=0..<3)
          - 8 : nil
          - 9 : nil
          - 10 : nil
          ▿ 11 : .layout(functionSignature byteLength=22 descendantCount=18)
            - 0 : nil
            ▿ 1 : .layout(functionParameterClause byteLength=3 descendantCount=3)
              - 0 : nil
              - 1 : .parsedToken(leftParen wholeText="(" textRange=0..<1)
              - 2 : nil
              - 3 : .layout(functionParameterList byteLength=0 descendantCount=0)
              - 4 : nil
              - 5 : .parsedToken(rightParen wholeText=") " textRange=0..<1)
              - 6 : nil
            - 2 : nil
            ▿ 3 : .layout(functionEffectSpecifiers byteLength=0 descendantCount=2)
              - 0 : nil
              - 1 : nil
              - 2 : nil
              ▿ 3 : .layout(throwsClause byteLength=0 descendantCount=1)
                - 0 : nil
                - 1 : .materializedToken(keyword text="throws" numLeadingTrivia=0 byteLength=6)
                - 2 : nil
                - 3 : nil
                - 4 : nil
                - 5 : nil
                - 6 : nil
                - 7 : nil
                - 8 : nil
              - 4 : nil
            - 4 : nil
            ▿ 5 : .layout(returnClause byteLength=19 descendantCount=10)
              - 0 : nil
              - 1 : .parsedToken(arrow wholeText="-> " textRange=0..<2)
              ▿ 2 : .layout(unexpectedNodes byteLength=6 descendantCount=1)
                - 0 : .parsedToken(keyword wholeText="throws" textRange=0..<6)
              ▿ 3 : .layout(tupleType byteLength=10 descendantCount=6)
                - 0 : nil
                - 1 : .parsedToken(leftParen wholeText="(" textRange=0..<1)
                - 2 : nil
                ▿ 3 : .layout(tupleTypeElementList byteLength=7 descendantCount=3)
                  ▿ 0 : .layout(tupleTypeElement byteLength=7 descendantCount=2)
                    - 0 : nil
                    - 1 : nil
                    - 2 : nil
                    - 3 : nil
                    - 4 : nil
                    - 5 : nil
                    - 6 : nil
                    - 7 : nil
                    - 8 : nil
                    ▿ 9 : .layout(identifierType byteLength=7 descendantCount=1)
                      - 0 : nil
                      - 1 : .parsedToken(identifier wholeText="MyError" textRange=0..<7)
                      - 2 : nil
                      - 3 : nil
                      - 4 : nil
                    - 10 : nil
                    - 11 : nil
                    - 12 : nil
                    - 13 : nil
                    - 14 : nil
                - 4 : nil
                - 5 : .parsedToken(rightParen wholeText=") " textRange=0..<1)
                - 6 : nil
              - 4 : nil
            - 6 : nil
          - 12 : nil
          - 13 : nil
          - 14 : nil
          ▿ 15 : .layout(codeBlock byteLength=6 descendantCount=5)
            ▿ 0 : .layout(unexpectedNodes byteLength=4 descendantCount=1)
              - 0 : .parsedToken(identifier wholeText="Int " textRange=0..<3)
            - 1 : .parsedToken(leftBrace wholeText="{" textRange=0..<1)
            - 2 : nil
            - 3 : .layout(codeBlockItemList byteLength=0 descendantCount=0)
            - 4 : nil
            - 5 : .parsedToken(rightBrace wholeText="}" textRange=0..<1)
            - 6 : nil
          - 16 : nil
        - 2 : nil
        - 3 : nil
        - 4 : nil
    - 4 : nil
    - 5 : .parsedToken(endOfFile wholeText="" textRange=0..<0)
    - 6 : nil

[ahoppen]

Yes, if I remember correctly parsing the type in a misspelled throws position is the core piece that needs to be done to fix this issue.

Parsed tokens are a version of materialized tokens that are optimized for parsing speed. The main difference is that a ParsedToken does not contain the trivia as individual pieces but as raw text, which will get lexed into the trivia pieces only when accessed. This removes a bit of work during parse time, especially considering that most users of swift-syntax will not access the trivia of most tokens. Here’s where ParsedToken and MaterializedToken are defined, if you are interested.

[AppAppWorks]

Pardon for my lack of understanding of SwiftSyntax, I've experimented with the following code in Parser.parseMisplacedEffectSpecifiers

while self.hasProgressed(&loopProgress) {
if let (spec, handle, matchedSubset) = self.at(
  anyIn: S.MisspelledAsyncTokenKinds.self,
  or: S.CorrectAsyncTokenKinds.self
) {
  ...
  previousIsThrow = false
} else if let (spec, handle, matchedSubset) = self.at(
  anyIn: S.MisspelledThrowsTokenKinds.self,
  or: S.CorrectThrowsTokenKinds.self
) {
  ...
  previousIsThrow = true
} else {
  if previousIsThrow {
    previousIsThrow = false
    
    guard self.at(prefix: "(") else {
      break
    }
    let rawLeftParen = consumeAnyToken()
    leftParen = missingToken(.leftParen)
    
    rawType = parseType()
    
    guard self.at(prefix: ")") else {
      leftParen = nil
      rawType = nil
      break
    }
    let rawRightParen = consumeAnyToken()
    rightParen = missingToken(.rightParen)
    
    unexpected.append(rawLeftParen)
    
    unexpected += RawSyntax(rawType!).allTokens()
    
    unexpected.append(rawRightParen)
  }
  break
}
if synthesizedAsync != nil || synthesizedThrows != nil {
  let synthesizedThrowsClause = synthesizedThrows.map {
    RawThrowsClauseSyntax(throwsSpecifier: $0, leftParen: leftParen, type: rawType, rightParen: rightParen, arena: self.arena)
  }
...

and in ParseDiagnosticsGenerator.handleMisplacedEffectSpecifiersAfterArrow

private func handleMisplacedEffectSpecifiersAfterArrow(
    effectSpecifiers: (some EffectSpecifiersSyntax)?,
    misplacedSpecifiers: UnexpectedNodesSyntax?
  ) {
  var correctTokens = [effectSpecifiers?.asyncSpecifier, effectSpecifiers?.throwsClause?.throwsSpecifier, effectSpecifiers?.throwsClause?.leftParen]
  if let throwingType = effectSpecifiers?.throwsClause?.type {
    correctTokens += Array(throwingType.tokens(viewMode: .all))
    correctTokens.append(effectSpecifiers?.throwsClause?.rightParen)
  }
  exchangeTokens(
    unexpected: misplacedSpecifiers,
    unexpectedTokenCondition: { _ in true },
...

Given the source text func test() -> 1️⃣throws (any Error) Int, I can generate

DiagnosticSpec(message: "'throws (any Error)' must precede '->'", fixIts: ["move 'throws (any Error)' in front of '->'"])

and the almost-correct fixedSource func test() throws(any Error) -> Int, but apparently I've messed up the syntax text offset in the parsing process. The fix-it message's location is at func test() -> throws (a1️⃣ny Error) Int for some reason.

Also, the mutated test cases in DefinitionTests, e.g. func test() any Error-> throws (any Error) Int, func test() any ErrorError-> throws (any Error) Int are causing serious errors.