-
Notifications
You must be signed in to change notification settings - Fork 441
Merge missing token diagnostics if they occur at the same source location #838
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
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
38 changes: 38 additions & 0 deletions
38
Sources/SwiftParser/Diagnostics/DiagnosticExtensions.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
//===--- DiagnosticExtensions.swift ---------------------------------------===// | ||
// | ||
// This source file is part of the Swift.org open source project | ||
// | ||
// Copyright (c) 2014 - 2022 Apple Inc. and the Swift project authors | ||
// Licensed under Apache License v2.0 with Runtime Library Exception | ||
// | ||
// See https://swift.org/LICENSE.txt for license information | ||
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
import SwiftDiagnostics | ||
import SwiftSyntax | ||
|
||
extension FixIt { | ||
init(message: StaticParserFixIt, changes: [Change]) { | ||
self.init(message: message as FixItMessage, changes: changes) | ||
} | ||
} | ||
|
||
extension FixIt.Change { | ||
/// Replaced a present node with a missing node | ||
static func makeMissing(node: TokenSyntax) -> FixIt.Change { | ||
assert(node.presence == .present) | ||
return .replace( | ||
oldNode: Syntax(node), | ||
newNode: Syntax(TokenSyntax(node.tokenKind, leadingTrivia: [], trailingTrivia: [], presence: .missing)) | ||
) | ||
} | ||
|
||
static func makePresent<T: SyntaxProtocol>(node: T) -> FixIt.Change { | ||
return .replace( | ||
oldNode: Syntax(node), | ||
newNode: PresentMaker().visit(Syntax(node)) | ||
) | ||
} | ||
} |
252 changes: 252 additions & 0 deletions
252
Sources/SwiftParser/Diagnostics/MissingNodesError.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,252 @@ | ||
//===--- MissingNodesError.swift ------------------------------------------===// | ||
// | ||
// This source file is part of the Swift.org open source project | ||
// | ||
// Copyright (c) 2014 - 2022 Apple Inc. and the Swift project authors | ||
// Licensed under Apache License v2.0 with Runtime Library Exception | ||
// | ||
// See https://swift.org/LICENSE.txt for license information | ||
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
import SwiftDiagnostics | ||
@_spi(RawSyntax) import SwiftSyntax | ||
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 all tokens in the parent are missing, return the parent type name. | ||
if let commonParent = commonParent, | ||
commonParent.isMissingAllTokens, | ||
let firstToken = commonParent.firstToken(viewMode: .all), | ||
let lastToken = commonParent.lastToken(viewMode: .all), | ||
missingNodes.contains(Syntax(firstToken)), | ||
missingNodes.contains(Syntax(lastToken)) { | ||
switch commonParent.as(SyntaxEnum.self) { | ||
case .codeBlock: | ||
return "code block" | ||
case .memberDeclBlock: | ||
return "member block" | ||
default: | ||
if let nodeTypeName = commonParent.nodeTypeNameForDiagnostics { | ||
return nodeTypeName | ||
} | ||
} | ||
} | ||
|
||
enum DescriptionPart { | ||
case tokensWithDefaultText([TokenSyntax]) | ||
case tokenWithoutDefaultText(RawTokenKind) | ||
case node(Syntax) | ||
|
||
var description: String { | ||
switch self { | ||
case .tokensWithDefaultText(let tokens): | ||
let tokenContents = tokens.map({ Format().format(syntax: $0).description }).joined() | ||
return "'\(tokenContents.trimmingSpaces())'" | ||
case .tokenWithoutDefaultText(let tokenKind): | ||
return tokenKind.nameForDiagnostics | ||
case .node(let node): | ||
if let parent = node.parent, | ||
let childName = parent.childNameForDiagnostics(node.index) { | ||
return "\(childName)" | ||
} else { | ||
return "\(node.nodeTypeNameForDiagnostics ?? "syntax")" | ||
} | ||
} | ||
} | ||
} | ||
|
||
var parts: [DescriptionPart] = [] | ||
for missingNode in missingNodes { | ||
if let missingToken = missingNode.as(TokenSyntax.self) { | ||
let newPart: DescriptionPart | ||
let (rawKind, text) = missingToken.tokenKind.decomposeToRaw() | ||
if let text = text, !text.isEmpty { | ||
let presentToken = TokenSyntax(missingToken.tokenKind, presence: .present) | ||
newPart = .tokensWithDefaultText([presentToken]) | ||
} else { | ||
if let defaultText = rawKind.defaultText { | ||
let newKind = TokenKind.fromRaw(kind: rawKind, text: String(syntaxText: defaultText)) | ||
let presentToken = TokenSyntax(newKind, presence: .present) | ||
newPart = .tokensWithDefaultText([presentToken]) | ||
} else { | ||
newPart = .tokenWithoutDefaultText(rawKind) | ||
} | ||
} | ||
|
||
switch (parts.last, newPart) { | ||
case (.tokensWithDefaultText(let previousTokens), .tokensWithDefaultText(let newTokens)): | ||
parts[parts.count - 1] = .tokensWithDefaultText(previousTokens + newTokens) | ||
default: | ||
parts.append(newPart) | ||
} | ||
} else { | ||
parts.append(.node(missingNode)) | ||
} | ||
} | ||
let partDescriptions = parts.map({ $0.description }) | ||
switch partDescriptions.count { | ||
case 0: | ||
return "syntax" | ||
case 1: | ||
return "\(partDescriptions.first!)" | ||
default: | ||
return "\(partDescriptions[0..<partDescriptions.count - 1].joined(separator: ", ")) and \(partDescriptions.last!)" | ||
} | ||
} | ||
|
||
// MARK: - Error | ||
|
||
public struct MissingNodesError: ParserError { | ||
public let missingNodes: [Syntax] | ||
public let commonParent: Syntax? | ||
|
||
init(missingNodes: [Syntax]) { | ||
assert(!missingNodes.isEmpty) | ||
self.missingNodes = missingNodes | ||
self.commonParent = missingNodes.first?.parent | ||
assert(missingNodes.allSatisfy({ $0.parent == self.commonParent })) | ||
} | ||
|
||
/// If applicable, returns a string that describes after which node the nodes are expected. | ||
private var afterClause: String? { | ||
guard let firstMissingNode = missingNodes.first else { | ||
return nil | ||
} | ||
if let missingDecl = firstMissingNode.as(MissingDeclSyntax.self) { | ||
if let lastModifier = missingDecl.modifiers?.last { | ||
return "after '\(lastModifier.name.text)' modifier" | ||
} else if missingDecl.attributes != nil { | ||
return "after attribute" | ||
} | ||
} | ||
|
||
// The after clause only provides value if the first missing node is not a token. | ||
// TODO: Revisit whether we want to have this clause at all. | ||
if !firstMissingNode.is(TokenSyntax.self) { | ||
if let previousToken = firstMissingNode.previousToken(viewMode: .fixedUp), previousToken.presence == .present { | ||
return "after '\(previousToken.text)'" | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
/// If applicable, returns a string that describes the node in which the missing nodes are expected. | ||
private var parentContextClause: String? { | ||
// anchorParent is the first parent that has a type name for diagnostics. | ||
guard let anchorParent = commonParent?.ancestorOrSelf(where: { $0.nodeTypeNameForDiagnostics != nil }) else { | ||
return nil | ||
} | ||
let anchorTypeName = anchorParent.nodeTypeNameForDiagnostics! | ||
if anchorParent.is(SourceFileSyntax.self) { | ||
return nil | ||
} | ||
|
||
var isFirstTokenStartMarker: Bool | ||
switch missingNodes.first?.as(TokenSyntax.self)?.tokenKind { | ||
case .leftBrace, .leftAngle, .leftParen, .leftSquareBracket: | ||
isFirstTokenStartMarker = true | ||
default: | ||
isFirstTokenStartMarker = false | ||
} | ||
|
||
var isLastTokenEndMarker: Bool | ||
switch missingNodes.last?.as(TokenSyntax.self)?.tokenKind { | ||
case .rightBrace, .rightAngle, .rightParen, .rightSquareBracket, .stringQuote, .multilineStringQuote, .rawStringDelimiter(_): | ||
isLastTokenEndMarker = true | ||
default: | ||
isLastTokenEndMarker = false | ||
} | ||
|
||
switch (isFirstTokenStartMarker, isLastTokenEndMarker) { | ||
case (true, false) where Syntax(anchorParent.firstToken(viewMode: .all)) == missingNodes.first: | ||
return "to start \(anchorTypeName)" | ||
case (false, true) where Syntax(anchorParent.lastToken(viewMode: .all)) == missingNodes.last: | ||
return "to end \(anchorTypeName)" | ||
default: | ||
return "in \(anchorTypeName)" | ||
} | ||
} | ||
|
||
public var message: String { | ||
var message = "Expected \(missingNodesDescription(missingNodes: missingNodes, commonParent: commonParent))" | ||
if let afterClause = afterClause { | ||
message += " \(afterClause)" | ||
} | ||
if let parentContextClause = parentContextClause { | ||
message += " \(parentContextClause)" | ||
} | ||
return message | ||
} | ||
} | ||
|
||
// MARK: - Fix-It | ||
|
||
public struct InsertTokenFixIt: ParserFixIt { | ||
public let missingNodes: [Syntax] | ||
public let commonParent: Syntax? | ||
|
||
init(missingNodes: [Syntax]) { | ||
assert(!missingNodes.isEmpty) | ||
self.missingNodes = missingNodes | ||
self.commonParent = missingNodes.first?.parent | ||
assert(missingNodes.allSatisfy({ $0.parent == self.commonParent })) | ||
} | ||
|
||
public var message: String { "Insert \(missingNodesDescription(missingNodes: missingNodes, commonParent: commonParent))" } | ||
} | ||
|
||
// MARK: - Generate Error | ||
|
||
extension ParseDiagnosticsGenerator { | ||
func handleMissingSyntax<T: SyntaxProtocol>(_ node: T) -> SyntaxVisitorContinueKind { | ||
if shouldSkip(node) { | ||
return .skipChildren | ||
} | ||
|
||
// Walk all upcoming sibling to see if they are also missing to handle them in this diagnostic. | ||
// If this is the case, handle all of them in this diagnostic. | ||
var missingNodes = [Syntax(node)] | ||
if let parent = node.parent { | ||
let siblings = parent.children(viewMode: .all) | ||
let siblingsAfter = siblings[siblings.index(after: node.index)...] | ||
for sibling in siblingsAfter { | ||
if sibling.as(TokenSyntax.self)?.presence == .missing { | ||
// Handle missing sibling tokens | ||
missingNodes += [sibling] | ||
} else if sibling.raw.kind.isMissing { | ||
// Handle missing sibling nodes (e.g. MissingDeclSyntax) | ||
missingNodes += [sibling] | ||
} else if sibling.isCollection && sibling.children(viewMode: .sourceAccurate).count == 0 { | ||
// Skip over any syntax collections without any elements while looking ahead for further missing nodes. | ||
} else { | ||
// Otherwise we have found a present node, so stop looking ahead. | ||
break | ||
} | ||
} | ||
} else { | ||
missingNodes = [] | ||
} | ||
|
||
let fixIt = FixIt( | ||
message: InsertTokenFixIt(missingNodes: missingNodes), | ||
changes: missingNodes.map(FixIt.Change.makePresent) | ||
) | ||
|
||
addDiagnostic( | ||
node, | ||
position: node.endPosition, | ||
MissingNodesError(missingNodes: missingNodes), | ||
fixIts: [fixIt], | ||
handledNodes: missingNodes.map(\.id) | ||
) | ||
return .visitChildren | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to add all the new files to CMakeLists.txt?