Skip to content

Add diagnostic to move 'async' and 'throws' in 'ArrowExpr' in front of '->' #881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ private func createTokenFormatFunction() -> FunctionDecl {
) {
VariableDecl("var node = node")
SwitchStmt(expression: MemberAccessExpr(base: "node", name: "tokenKind")) {
for token in SYNTAX_TOKENS {
for token in SYNTAX_TOKENS where token.name != "ContextualKeyword" {
SwitchCase(label: SwitchCaseLabel(caseItems: CaseItem(pattern: ExpressionPattern(expression: MemberAccessExpr(name: token.swiftKind))))) {
if token.requiresLeadingSpace {
IfStmt(
Expand Down Expand Up @@ -196,6 +196,20 @@ private func createTokenFormatFunction() -> FunctionDecl {
SwitchCase(label: SwitchCaseLabel(caseItems: CaseItem(pattern: ExpressionPattern(expression: MemberAccessExpr(name: "eof"))))) {
BreakStmt("break")
}
SwitchCase(label: SwitchCaseLabel(caseItems: CaseItem(pattern: ExpressionPattern(expression: MemberAccessExpr(name: "contextualKeyword"))))) {
SwitchStmt(
"""
switch node.text {
case "async":
if node.trailingTrivia.isEmpty {
node.trailingTrivia += .space
}
default:
break
}
"""
)
}
}
SequenceExpr("node.leadingTrivia = node.leadingTrivia.indented(indentation: indentation)")
SequenceExpr("node.trailingTrivia = node.trailingTrivia.indented(indentation: indentation)")
Expand Down
11 changes: 9 additions & 2 deletions Sources/SwiftBasicFormat/generated/BasicFormat.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3147,8 +3147,6 @@ open class BasicFormat: SyntaxRewriter {
break
case .dollarIdentifier:
break
case .contextualKeyword:
break
case .rawStringDelimiter:
break
case .stringSegment:
Expand All @@ -3159,6 +3157,15 @@ open class BasicFormat: SyntaxRewriter {
break
case .eof:
break
case .contextualKeyword:
switch node.text {
case "async":
if node.trailingTrivia.isEmpty {
node.trailingTrivia += .space
}
default:
break
}
}
node.leadingTrivia = node.leadingTrivia.indented(indentation: indentation)
node.trailingTrivia = node.trailingTrivia.indented(indentation: indentation)
Expand Down
28 changes: 23 additions & 5 deletions Sources/SwiftDiagnostics/FixIt.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,39 @@ public protocol FixItMessage {

/// A Fix-It that can be applied to resolve a diagnostic.
public struct FixIt {
public struct Changes: ExpressibleByArrayLiteral {
public var changes: [Change]

public init(changes: [Change]) {
self.changes = changes
}

public init(arrayLiteral elements: FixIt.Change...) {
self.init(changes: elements)
}

public init(combining: [Changes]) {
self.init(changes: combining.flatMap(\.changes))
}
}

public enum Change {
/// Replace `oldNode` by `newNode`.
case replace(oldNode: Syntax, newNode: Syntax)
/// Remove the trailing trivia of the given token
case removeTrailingTrivia(TokenSyntax)
/// Replace the leading trivia on the given token
case replaceLeadingTrivia(token: TokenSyntax, newTrivia: Trivia)
/// Replace the trailing trivia on the given token
case replaceTrailingTrivia(token: TokenSyntax, newTrivia: Trivia)
}

/// A description of what this Fix-It performs.
public let message: FixItMessage

/// The changes that need to be performed when the Fix-It is applied.
public let changes: [Change]
public let changes: Changes

public init(message: FixItMessage, changes: [Change]) {
assert(!changes.isEmpty, "A Fix-It must have at least one change associated with it")
public init(message: FixItMessage, changes: Changes) {
assert(!changes.changes.isEmpty, "A Fix-It must have at least one change associated with it")
self.message = message
self.changes = changes
}
Expand Down
64 changes: 47 additions & 17 deletions Sources/SwiftParser/Diagnostics/DiagnosticExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,42 +14,72 @@ import SwiftDiagnostics
import SwiftSyntax

extension FixIt {
init(message: StaticParserFixIt, changes: [Change]) {
init(message: StaticParserFixIt, changes: Changes) {
self.init(message: message as FixItMessage, changes: changes)
}

public init(message: FixItMessage, changes: [Changes]) {
self.init(message: message, changes: FixIt.Changes(combining: changes))
}

init(message: StaticParserFixIt, changes: [Changes]) {
self.init(message: message as FixItMessage, changes: FixIt.Changes(combining: changes))
}
}

extension FixIt.Change {
extension FixIt.Changes {
/// Replaced a present node with a missing node
static func makeMissing(node: TokenSyntax) -> FixIt.Change {
static func makeMissing(node: TokenSyntax) -> Self {
assert(node.presence == .present)
return .replace(
oldNode: Syntax(node),
newNode: Syntax(TokenSyntax(node.tokenKind, leadingTrivia: [], trailingTrivia: [], presence: .missing))
)
var changes = [
FixIt.Change.replace(
oldNode: Syntax(node),
newNode: Syntax(TokenSyntax(node.tokenKind, leadingTrivia: [], trailingTrivia: [], presence: .missing))
)
]
if !node.leadingTrivia.isEmpty, let nextToken = node.nextToken(viewMode: .sourceAccurate) {
changes.append(.replaceLeadingTrivia(token: nextToken, newTrivia: node.leadingTrivia))
}
return FixIt.Changes(changes: changes)
}

static func makePresent<T: SyntaxProtocol>(node: T) -> FixIt.Change {
return .replace(
static func makePresent<T: SyntaxProtocol>(node: T) -> Self {
return [.replace(
oldNode: Syntax(node),
newNode: PresentMaker().visit(Syntax(node))
)
)]
}

/// Make a token present. If `leadingTrivia` or `trailingTrivia` is specified,
/// override the default leading/trailing trivia inferred from `BasicFormat`.
static func makePresent(
node: TokenSyntax,
leadingTrivia: Trivia? = nil,
trailingTrivia: Trivia? = nil
) -> Self {
var presentNode = PresentMaker().visit(Syntax(node)).as(TokenSyntax.self)!
if let leadingTrivia = leadingTrivia {
presentNode.leadingTrivia = leadingTrivia
}
if let trailingTrivia = trailingTrivia {
presentNode.trailingTrivia = trailingTrivia
}
return [.replace(
oldNode: Syntax(node),
newNode: Syntax(presentNode)
)]
}
}

extension Array where Element == FixIt.Change {
/// Makes the `token` present, moving it in front of the previous token's trivia.
static func makePresentBeforeTrivia(token: TokenSyntax) -> [FixIt.Change] {
static func makePresentBeforeTrivia(token: TokenSyntax) -> Self {
if let previousToken = token.previousToken(viewMode: .sourceAccurate) {
let presentToken = PresentMaker().visit(token).withTrailingTrivia(previousToken.trailingTrivia)
return [
.removeTrailingTrivia(previousToken),
.replaceTrailingTrivia(token: previousToken, newTrivia: []),
.replace(oldNode: Syntax(token), newNode: presentToken),
]
} else {
return [
.makePresent(node: token)
]
return .makePresent(node: token)
}
}
}
10 changes: 6 additions & 4 deletions Sources/SwiftParser/Diagnostics/MissingNodesError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ import SwiftBasicFormat
// MARK: - Shared code

/// Returns a string that describes `missingNodes`.
/// `missingNodes` are expected to all be children of `commonParent`.
private func missingNodesDescription(missingNodes: [Syntax], commonParent: Syntax?) -> String {
assert(missingNodes.allSatisfy({ $0.parent == commonParent }))
/// If `commonParent` is not `nil`, `missingNodes` are expected to all be children of `commonParent`.
func missingNodesDescription(missingNodes: [Syntax], commonParent: Syntax?) -> String {
if commonParent != nil {
assert(missingNodes.allSatisfy({ $0.parent == commonParent }))
}

// If all tokens in the parent are missing, return the parent type name.
if let commonParent = commonParent,
Expand Down Expand Up @@ -256,7 +258,7 @@ extension ParseDiagnosticsGenerator {

let fixIt = FixIt(
message: InsertTokenFixIt(missingNodes: missingNodes),
changes: missingNodes.map(FixIt.Change.makePresent)
changes: missingNodes.map(FixIt.Changes.makePresent)
)

var notes: [Note] = []
Expand Down
79 changes: 66 additions & 13 deletions Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,51 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
return handledNodes.contains(node.id)
}

/// Utility function to emit a diagnostic that removes a misplaced token and instead inserts an equivalent token at the corrected location.
///
/// If `incorrectContainer` contains only tokens that satisfy `unexpectedTokenCondition`, emit a diagnostic with message `message` that marks this token as misplaced.
/// If `correctTokens` contains missing tokens, also emit a Fix-It with message `fixIt` that marks the unexpected token as missing and instead inserts `correctTokens`.
public func exchangeTokens(
unexpected: UnexpectedNodesSyntax?,
unexpectedTokenCondition: (TokenSyntax) -> Bool,
correctTokens: [TokenSyntax?],
message: (_ misplacedTokens: [TokenSyntax]) -> DiagnosticMessage,
fixIt: (_ misplacedTokens: [TokenSyntax]) -> FixItMessage
) {
guard let incorrectContainer = unexpected,
let misplacedTokens = incorrectContainer.onlyTokens(satisfying: unexpectedTokenCondition) else {
// If there are no unexpected nodes or the unexpected contain multiple tokens, don't emit a diagnostic.
return
}

// Ignore `correctTokens` that are already present.
let correctTokens = correctTokens.compactMap({ $0 }).filter({ $0.presence == .missing })
var changes = misplacedTokens.map(FixIt.Changes.makeMissing)
for correctToken in correctTokens {
if misplacedTokens.count == 1, let misplacedToken = misplacedTokens.first,
misplacedToken.nextToken(viewMode: .all) == correctToken || misplacedToken.previousToken(viewMode: .all) == correctToken {
changes.append(FixIt.Changes.makePresent(
node: correctToken,
leadingTrivia: misplacedToken.leadingTrivia,
trailingTrivia: misplacedToken.trailingTrivia
))
} else {
changes.append(FixIt.Changes.makePresent(
node: correctToken,
leadingTrivia: nil,
trailingTrivia: nil
))
}
}
var fixIts: [FixIt] = []
if changes.count > 1 {
// Only emit a Fix-It if we are moving a token, i.e. also making a token present.
fixIts.append(FixIt(message: fixIt(misplacedTokens), changes: changes))
}

addDiagnostic(incorrectContainer, message(misplacedTokens), fixIts: fixIts, handledNodes: [incorrectContainer.id] + correctTokens.map(\.id))
}

// MARK: - Generic diagnostic generation

public override func visitAny(_ node: Syntax) -> SyntaxVisitorContinueKind {
Expand Down Expand Up @@ -180,6 +225,20 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
return .visitChildren
}

public override func visit(_ node: ArrowExprSyntax) -> SyntaxVisitorContinueKind {
if shouldSkip(node) {
return .skipChildren
}
exchangeTokens(
unexpected: node.unexpectedAfterArrowToken,
unexpectedTokenCondition: { $0.tokenKind == .contextualKeyword("async") || $0.tokenKind == .throwsKeyword },
correctTokens: [node.asyncKeyword, node.throwsToken],
message: { EffectsSpecifierAfterArrow(effectsSpecifiersAfterArrow: $0) },
fixIt: { MoveTokensInFrontOfFixIt(movedTokens: $0, inFrontOf: .arrow) }
)
return .visitChildren
}

public override func visit(_ node: ForInStmtSyntax) -> SyntaxVisitorContinueKind {
if shouldSkip(node) {
return .skipChildren
Expand Down Expand Up @@ -215,19 +274,13 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
if shouldSkip(node) {
return .skipChildren
}
if let output = node.output,
let missingThrowsKeyword = node.throwsOrRethrowsKeyword,
missingThrowsKeyword.presence == .missing,
let unexpectedBeforeReturnType = output.unexpectedBetweenArrowAndReturnType,
let throwsInReturnPosition = unexpectedBeforeReturnType.tokens(withKind: .throwsKeyword).first {
addDiagnostic(throwsInReturnPosition, .throwsInReturnPosition, fixIts: [
FixIt(message: .moveThrowBeforeArrow, changes: [
.makeMissing(node: throwsInReturnPosition),
.makePresent(node: missingThrowsKeyword),
])
], handledNodes: [unexpectedBeforeReturnType.id, missingThrowsKeyword.id, throwsInReturnPosition.id])
return .visitChildren
}
exchangeTokens(
unexpected: node.output?.unexpectedBetweenArrowAndReturnType,
unexpectedTokenCondition: { $0.tokenKind == .throwsKeyword },
correctTokens: [node.throwsOrRethrowsKeyword],
message: { _ in StaticParserError.throwsInReturnPosition },
fixIt: { MoveTokensInFrontOfFixIt(movedTokens: $0, inFrontOf: .arrow) }
)
return .visitChildren
}

Expand Down
22 changes: 21 additions & 1 deletion Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,14 @@ public enum StaticParserError: String, DiagnosticMessage {

// MARK: - Diagnostics (please sort alphabetically)

public struct EffectsSpecifierAfterArrow: ParserError {
public let effectsSpecifiersAfterArrow: [TokenSyntax]

public var message: String {
"\(missingNodesDescription(missingNodes: effectsSpecifiersAfterArrow.map(Syntax.init), commonParent: nil)) may only occur before '->'"
}
}

public struct ExtaneousCodeAtTopLevel: ParserError {
public let extraneousCode: UnexpectedNodesSyntax

Expand Down Expand Up @@ -141,11 +149,23 @@ public struct UnexpectedNodesError: ParserError {
public enum StaticParserFixIt: String, FixItMessage {
case insertSemicolon = "insert ';'"
case insertAttributeArguments = "insert attribute argument"
case moveThrowBeforeArrow = "move 'throws' before '->'"

public var message: String { self.rawValue }

public var fixItID: MessageID {
MessageID(domain: diagnosticDomain, id: "\(type(of: self)).\(self)")
}
}

public struct MoveTokensInFrontOfFixIt: ParserFixIt {
/// The token that should be moved
public let movedTokens: [TokenSyntax]

/// The token after which 'try' should be moved
public let inFrontOf: RawTokenKind

public var message: String {
"move \(missingNodesDescription(missingNodes: movedTokens.map(Syntax.init), commonParent: nil)) in front of '\(inFrontOf.nameForDiagnostics)'"
}
}

24 changes: 24 additions & 0 deletions Sources/SwiftParser/Diagnostics/SyntaxExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,30 @@ extension UnexpectedNodesSyntax {
func tokens(withKind kind: TokenKind) -> [TokenSyntax] {
return self.tokens(satisfying: { $0.tokenKind == kind })
}

/// If this only contains a single item that is a token, return that token, otherwise return `nil`.
var onlyToken: TokenSyntax? {
return onlyToken(where: { _ in true })
}

/// If this only contains a single item, which is a token satisfying `condition`, return that token, otherwise return `nil`.
func onlyToken(where condition: (TokenSyntax) -> Bool) -> TokenSyntax? {
if self.count == 1, let token = self.first?.as(TokenSyntax.self), condition(token) {
return token
} else {
return nil
}
}

/// If this only contains tokens satisfying `condition`, return an array containing those tokens, otherwise return `nil`.
func onlyTokens(satisfying condition: (TokenSyntax) -> Bool) -> [TokenSyntax]? {
let tokens = tokens(satisfying: condition)
if tokens.count == self.count {
return tokens
} else {
return nil
}
}
}

extension Syntax {
Expand Down
Loading