Skip to content

Add to match this opening <thing> notes to the diagnostics #862

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 1 commit into from
Sep 30, 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 @@ -705,7 +705,7 @@ public let STMT_NODES: [Node] = [
]),

Node(name: "PoundAssertStmt",
nameForDiagnostics: "'#assert' statement",
nameForDiagnostics: "'#assert' directive",
kind: "Stmt",
children: [
Child(name: "PoundAssert",
Expand Down
3 changes: 2 additions & 1 deletion Sources/SwiftDiagnostics/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@
# See http://swift.org/CONTRIBUTORS.txt for Swift project authors

add_library(SwiftDiagnostics STATIC
Diagnostic.swift
FixIt.swift
Message.swift
Diagnostic.swift
Note.swift
)

target_link_libraries(SwiftDiagnostics PUBLIC
Expand Down
13 changes: 12 additions & 1 deletion Sources/SwiftDiagnostics/Diagnostic.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,26 @@ public struct Diagnostic: CustomDebugStringConvertible {
/// Nodes that should be highlighted in the source code.
public let highlights: [Syntax]

/// Notes that point to additional locations which are relevant for this diagnostic.
public let notes: [Note]

/// Fix-Its that can be applied to resolve this diagnostic.
/// Each Fix-It offers a different way to resolve the diagnostic. Usually, there's only one.
public let fixIts: [FixIt]

public init(node: Syntax, position: AbsolutePosition? = nil, message: DiagnosticMessage, highlights: [Syntax] = [], fixIts: [FixIt] = []) {
public init(
node: Syntax,
position: AbsolutePosition? = nil,
message: DiagnosticMessage,
highlights: [Syntax] = [],
notes: [Note] = [],
fixIts: [FixIt] = []
) {
self.node = node
self.position = position ?? node.positionAfterSkippingLeadingTrivia
self.diagMessage = message
self.highlights = highlights
self.notes = notes
self.fixIts = fixIts
}

Expand Down
4 changes: 2 additions & 2 deletions Sources/SwiftDiagnostics/FixIt.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import SwiftSyntax
/// shown to the client.
/// The messages should describe the change that the Fix-It will perform
public protocol FixItMessage {
/// The diagnostic message that should be displayed in the client.
/// The Fix-It message that should be displayed in the client.
var message: String { get }

/// See ``MessageID``.
Expand All @@ -38,7 +38,7 @@ public struct FixIt {
public let changes: [Change]

public init(message: FixItMessage, changes: [Change]) {
assert(!changes.isEmpty, "A Fix-It must have at least one diagnostic associated with it")
assert(!changes.isEmpty, "A Fix-It must have at least one change associated with it")
self.message = message
self.changes = changes
}
Expand Down
68 changes: 68 additions & 0 deletions Sources/SwiftDiagnostics/Note.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
//===--- Note.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 SwiftSyntax

/// Types conforming to this protocol represent note messages that can be
/// shown to the client.
/// The messages should describe what the note is pointing at.
public protocol NoteMessage {
/// The message that should be displayed in the client.
var message: String { get }

/// See ``MessageID``.
var fixItID: MessageID { get }
}

/// A note that points to another node that's relevant for a Diagnostic.
public struct Note: CustomDebugStringConvertible {
/// The node whose location the node is pointing.
public let node: Syntax

/// The position at which the location should be anchored.
/// By default, this is the start location of `node`.
public let position: AbsolutePosition

/// A description of what this note is pointing at.
public let noteMessage: NoteMessage
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not sure having an ID for note/diagnostic is really worth it. But we can keep it for now 🤷

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think it’s very little overhead at this point and once a use case comes up we will be glad that we have so we don’t need to do string matching for the error message.


public init(
node: Syntax,
position: AbsolutePosition? = nil,
message: NoteMessage
) {
self.node = node
self.position = position ?? node.positionAfterSkippingLeadingTrivia
self.noteMessage = message
}

/// The message that should be displayed to the user.
public var message: String {
return noteMessage.message
}

/// The location at which the note should be displayed.
public func location(converter: SourceLocationConverter) -> SourceLocation {
return converter.location(for: position)
}

public var debugDescription: String {
if let root = node.root.as(SourceFileSyntax.self) {
let locationConverter = SourceLocationConverter(file: "", tree: root)
let location = location(converter: locationConverter)
return "\(location): \(message)"
} else {
return "<unknown>: \(message)"
}
}
}

67 changes: 51 additions & 16 deletions Sources/SwiftParser/Diagnostics/MissingNodesError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,38 @@ private func missingNodesDescription(missingNodes: [Syntax], commonParent: Synta
}
}

fileprivate extension TokenKind {
var isStartMarker: Bool {
switch self {
case .leftBrace, .leftAngle, .leftParen, .leftSquareBracket:
return true
default:
return false
}
}

var isEndMarker: Bool {
return matchingStartMarkerKind != nil
}

var matchingStartMarkerKind: TokenKind? {
switch self {
case .rightBrace:
return .leftBrace
case .rightAngle:
return .leftAngle
case .rightParen:
return .leftParen
case .rightSquareBracket:
return .leftSquareBracket
case .stringQuote, .multilineStringQuote, .rawStringDelimiter:
return self
default:
return nil
}
}
}

// MARK: - Error

public struct MissingNodesError: ParserError {
Expand Down Expand Up @@ -142,22 +174,8 @@ public struct MissingNodesError: ParserError {
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
}

let isFirstTokenStartMarker = missingNodes.first?.as(TokenSyntax.self)?.tokenKind.isStartMarker ?? false
let isLastTokenEndMarker = missingNodes.last?.as(TokenSyntax.self)?.tokenKind.isEndMarker ?? false
switch (isFirstTokenStartMarker, isLastTokenEndMarker) {
case (true, false) where Syntax(anchorParent.firstToken(viewMode: .all)) == missingNodes.first:
return "to start \(anchorTypeName)"
Expand All @@ -180,6 +198,14 @@ public struct MissingNodesError: ParserError {
}
}

// MARK: - Note

public struct MatchingOpeningTokenNote: ParserNote {
let openingToken: TokenSyntax

public var message: String { "to match this opening '\(openingToken.text)'" }
}

// MARK: - Fix-It

public struct InsertTokenFixIt: ParserFixIt {
Expand Down Expand Up @@ -233,10 +259,19 @@ extension ParseDiagnosticsGenerator {
changes: missingNodes.map(FixIt.Change.makePresent)
)

var notes: [Note] = []
if missingNodes.count == 1,
let token = missingNodes.last?.as(TokenSyntax.self),
let matchingStartMarkerKind = token.tokenKind.matchingStartMarkerKind,
let startToken = token.parent?.children(viewMode: .sourceAccurate).lazy.compactMap({ $0.as(TokenSyntax.self) }).first(where: { $0.tokenKind == matchingStartMarkerKind }) {
notes.append(Note(node: Syntax(startToken), message: MatchingOpeningTokenNote(openingToken: startToken)))
}

addDiagnostic(
node,
position: node.endPosition,
MissingNodesError(missingNodes: missingNodes),
notes: notes,
fixIts: [fixIt],
handledNodes: missingNodes.map(\.id)
)
Expand Down
32 changes: 28 additions & 4 deletions Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,39 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
// MARK: - Private helper functions

/// Produce a diagnostic.
func addDiagnostic<T: SyntaxProtocol>(_ node: T, position: AbsolutePosition? = nil, _ message: DiagnosticMessage, highlights: [Syntax] = [], fixIts: [FixIt] = [], handledNodes: [SyntaxIdentifier] = []) {
func addDiagnostic<T: SyntaxProtocol>(
_ node: T,
position: AbsolutePosition? = nil,
_ message: DiagnosticMessage,
highlights: [Syntax] = [],
notes: [Note] = [],
fixIts: [FixIt] = [],
handledNodes: [SyntaxIdentifier] = []
) {
diagnostics.removeAll(where: { handledNodes.contains($0.node.id) })
diagnostics.append(Diagnostic(node: Syntax(node), position: position, message: message, highlights: highlights, fixIts: fixIts))
diagnostics.append(Diagnostic(node: Syntax(node), position: position, message: message, highlights: highlights, notes: notes, fixIts: fixIts))
self.handledNodes.append(contentsOf: handledNodes)
}

/// Produce a diagnostic.
func addDiagnostic<T: SyntaxProtocol>(_ node: T,position: AbsolutePosition? = nil, _ message: StaticParserError, highlights: [Syntax] = [], fixIts: [FixIt] = [], handledNodes: [SyntaxIdentifier] = []) {
addDiagnostic(node, position: position, message as DiagnosticMessage, highlights: highlights, fixIts: fixIts, handledNodes: handledNodes)
func addDiagnostic<T: SyntaxProtocol>(
_ node: T,
position: AbsolutePosition? = nil,
_ message: StaticParserError,
highlights: [Syntax] = [],
notes: [Note] = [],
fixIts: [FixIt] = [],
handledNodes: [SyntaxIdentifier] = []
) {
addDiagnostic(
node,
position: position,
message as DiagnosticMessage,
highlights: highlights,
notes: notes,
fixIts: fixIts,
handledNodes: handledNodes
)
}

/// Whether the node should be skipped for diagnostic emission.
Expand Down
14 changes: 14 additions & 0 deletions Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,20 @@ public extension ParserError {
}
}

public protocol ParserNote: NoteMessage {
var fixItID: MessageID { get }
}

public extension ParserNote {
static var fixItID: MessageID {
return MessageID(domain: diagnosticDomain, id: "\(self)")
}

var fixItID: MessageID {
return Self.fixItID
}
}

public protocol ParserFixIt: FixItMessage {
var fixItID: MessageID { get }
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftSyntax/gyb_generated/SyntaxEnum.swift
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ public enum SyntaxEnum {
case .catchClause:
return "'catch' clause"
case .poundAssertStmt:
return "'#assert' statement"
return "'#assert' directive"
case .genericWhereClause:
return "'where' clause"
case .genericRequirementList:
Expand Down
Loading