Async deinit diagnostics in swift-syntax

I’m working on adding support for parsing async deinit in swift-syntax, but I’m a bit stuck with producing diagnostics.

For which cases does it make sense to produce customised diagnostics?

These two probably are must haves:

// error: expected async specifier; did you mean 'async'?
// fixit: replace 'reasync' with 'async' 
deinit await

// error: deinit cannot throw
// fixit: remove 'throws'
deinit throws

Is one error sufficient here? Is second error useful in this case, or it does more harm than good?

// error: deinit cannot throw
// fixit: remove 'throws'
// error: 'async' must precede 'throws'
// fixit: move 'async' in front of 'throws'
deinit throws async

Should there be a diagnostics for async before deinit? To my surprise currently this parses without any diagnostics:

async func foo()
async deint

How should the parse tree like?

I’ve found existing code for parsing and producing diagnostics for effect specifiers for functions, accessors and types, and my first instinct was to generalise and reuse it for deinit case, but actually so far it's not working great.

I've created DeinitEffectSpecifiers node, which contains only AsyncSpecifier, without ThrowsSpecifier, and then in extension conforming RawDeinitEffectSpecifiersSyntax to RawEffectSpecifiersTrait added convenience initialiser that combines unexpectedBetweenAsyncSpecifierAndThrowsSpecifier, throwsSpecifier and unexpectedAfterThrowsSpecifier into unexpectedAfterAsyncSpecifier. MisspelledThrowsTokenKinds contains all 4 keywords, and CorrectThrowsTokenKinds is uninhabited. With minimal changes I was able to reuse parseEffectSpecifiers(), but diagnostics does not work well in this case.

In particular when trying to parse deinit throws async, I end up with asyncSpecifier being a synthesised token, while both real tokens end up in unexpectedAfterAsyncSpecifier.

I was hoping to reuse logic for producing AsyncMustPrecedeThrows, but exchangeTokens() expect a single unexpected token, but I have two in this case.

I might be able to fix it by handling throws and other keywords as MisspelledAsyncSpecifiers, but that feels too hacky.

So, I'm considering the following options:

  • place AsyncSpecifier directly into DeinitializerDecl.
  • create DeinitEffectSpecifiers without ThrowsSpecifier .
  • create DeinitEffectSpecifiers with ThrowsSpecifier.