Skip to content

[5.9] Slightly clean up FixIt.Change and FixIt.Changes #1496

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
Apr 6, 2023
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 @@ -84,7 +84,7 @@ extension PluginMessage.Diagnostic {
self.fixIts = syntaxDiag.fixIts.compactMap {
PluginMessage.Diagnostic.FixIt(
message: $0.message.message,
changes: $0.changes.changes.compactMap {
changes: $0.changes.compactMap {
let range: SourceManager.SourceRange?
let text: String
switch $0 {
Expand Down
22 changes: 3 additions & 19 deletions Sources/SwiftDiagnostics/FixIt.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,6 @@ 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)
Expand All @@ -54,10 +38,10 @@ public struct FixIt {
public let message: FixItMessage

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

public init(message: FixItMessage, changes: Changes) {
precondition(!changes.changes.isEmpty, "A Fix-It must have at least one change associated with it")
public init(message: FixItMessage, changes: [Change]) {
precondition(!changes.isEmpty, "A Fix-It must have at least one change associated with it")
self.message = message
self.changes = changes
}
Expand Down
79 changes: 53 additions & 26 deletions Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,51 @@ import SwiftBasicFormat
import SwiftSyntax

extension FixIt {
public init(message: FixItMessage, changes: [Changes]) {
self.init(message: message, changes: FixIt.Changes(combining: changes))
/// A more complex set of changes that affects multiple syntax nodes and thus
/// produces multiple `FixIt.Change`s. This allows us to e.g. mark a node as
/// missing but keep the trivia by transferring it to the previous or next
/// token.
struct MultiNodeChange {
var primitiveChanges: [Change]

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

init(_ primitiveChanges: Change...) {
self.init(primitiveChanges: primitiveChanges)
}

init(combining: [MultiNodeChange]) {
self.init(primitiveChanges: combining.flatMap(\.primitiveChanges))
}
}

init(message: FixItMessage, changes: [MultiNodeChange]) {
self.init(message: message, changes: MultiNodeChange(combining: changes))
}

init(message: FixItMessage, changes: MultiNodeChange) {
self.init(message: message, changes: changes.primitiveChanges)
}

// These overloads shouldn't be needed, but are currently required for the
// Swift 5.5 compiler to handle non-trivial FixIt initializations using
// leading-dot syntax.
// TODO: These can be dropped once we require a minimum of Swift 5.6 to
// compile the library.
init(message: StaticParserFixIt, changes: Changes) {
self.init(message: message as FixItMessage, changes: changes)
init(message: StaticParserFixIt, changes: MultiNodeChange) {
self.init(message: message as FixItMessage, changes: changes.primitiveChanges)
}
init(message: StaticParserFixIt, changes: [Changes]) {
self.init(message: message as FixItMessage, changes: FixIt.Changes(combining: changes))
init(message: StaticParserFixIt, changes: [MultiNodeChange]) {
self.init(message: message as FixItMessage, changes: MultiNodeChange(combining: changes).primitiveChanges)
}
public init(message: StaticParserFixIt, changes: [Change]) {
self.init(message: message as FixItMessage, changes: changes)
}
}

extension FixIt.Changes {
extension FixIt.MultiNodeChange {
/// Replaced a present token with a missing node.
/// If `transferTrivia` is `true`, the leading and trailing trivia of the
/// removed node will be transferred to the trailing trivia of the previous token.
Expand All @@ -49,26 +76,26 @@ extension FixIt.Changes {
var changes = tokens.map {
FixIt.Change.replace(
oldNode: Syntax($0),
newNode: Syntax(TokenSyntax($0.tokenKind, leadingTrivia: [], trailingTrivia: [], presence: .missing))
newNode: Syntax($0.with(\.presence, .missing))
)
}
if transferTrivia {
changes += FixIt.Changes.transferTriviaAtSides(from: tokens).changes
changes += FixIt.MultiNodeChange.transferTriviaAtSides(from: tokens).primitiveChanges
}
return FixIt.Changes(changes: changes)
return FixIt.MultiNodeChange(primitiveChanges: changes)
}

/// If `transferTrivia` is `true`, the leading and trailing trivia of the
/// removed node will be transferred to the trailing trivia of the previous token.
static func makeMissing<SyntaxType: SyntaxProtocol>(_ node: SyntaxType?, transferTrivia: Bool = true) -> Self {
guard let node = node else {
return FixIt.Changes(changes: [])
return FixIt.MultiNodeChange(primitiveChanges: [])
}
var changes = [FixIt.Change.replace(oldNode: Syntax(node), newNode: MissingMaker().visit(Syntax(node)))]
if transferTrivia {
changes += FixIt.Changes.transferTriviaAtSides(from: [node]).changes
changes += FixIt.MultiNodeChange.transferTriviaAtSides(from: [node]).primitiveChanges
}
return FixIt.Changes(changes: changes)
return FixIt.MultiNodeChange(primitiveChanges: changes)
}

/// Make a node present. If `leadingTrivia` or `trailingTrivia` is specified,
Expand All @@ -89,13 +116,13 @@ extension FixIt.Changes {
let nextToken = node.nextToken(viewMode: .sourceAccurate),
leadingTrivia == nil
{
return [
return FixIt.MultiNodeChange(
.replace(
oldNode: Syntax(node),
newNode: Syntax(presentNode).with(\.leadingTrivia, nextToken.leadingTrivia)
),
.replaceLeadingTrivia(token: nextToken, newTrivia: []),
]
.replaceLeadingTrivia(token: nextToken, newTrivia: [])
)
} else if node.leadingTrivia.isEmpty,
let previousToken = node.previousToken(viewMode: .fixedUp),
previousToken.presence == .present,
Expand All @@ -105,19 +132,19 @@ extension FixIt.Changes {
{
/// If neither this nor the previous token are punctionation make sure they
/// are separated by a space.
return [
return FixIt.MultiNodeChange(
.replace(
oldNode: Syntax(node),
newNode: Syntax(presentNode).with(\.leadingTrivia, .space)
)
]
)
} else {
return [
return FixIt.MultiNodeChange(
.replace(
oldNode: Syntax(node),
newNode: Syntax(presentNode)
)
]
)
}
}

Expand All @@ -128,10 +155,10 @@ extension FixIt.Changes {
if !previousToken.trailingTrivia.isEmpty {
presentToken = presentToken.with(\.trailingTrivia, previousToken.trailingTrivia)
}
return [
return FixIt.MultiNodeChange(
.replaceTrailingTrivia(token: previousToken, newTrivia: []),
.replace(oldNode: Syntax(token), newNode: Syntax(presentToken)),
]
.replace(oldNode: Syntax(token), newNode: Syntax(presentToken))
)
} else {
return .makePresent(token)
}
Expand All @@ -149,11 +176,11 @@ extension FixIt.Changes {
// Punctuation is generally not followed by spaces in Swift.
// If this action would only add spaces to the punctuation, drop it.
// This generally yields better results.
return []
return FixIt.MultiNodeChange()
}
return [.replaceTrailingTrivia(token: previousToken, newTrivia: mergedTrivia)]
return FixIt.MultiNodeChange(.replaceTrailingTrivia(token: previousToken, newTrivia: mergedTrivia))
} else {
return []
return FixIt.MultiNodeChange()
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ public extension SwiftSyntax.TokenDiagnostic {
.with(\.leadingTrivia, Trivia(pieces: token.leadingTrivia.map(replaceNonBreakingSpace)))
.with(\.trailingTrivia, Trivia(pieces: token.trailingTrivia.map(replaceNonBreakingSpace)))
return [
FixIt(message: .replaceNonBreakingSpaceBySpace, changes: [[.replace(oldNode: Syntax(token), newNode: Syntax(fixedToken))]])
FixIt(message: .replaceNonBreakingSpaceBySpace, changes: [.replace(oldNode: Syntax(token), newNode: Syntax(fixedToken))])
]
case .unicodeCurlyQuote:
let (rawKind, text) = token.tokenKind.decomposeToRaw()
Expand All @@ -206,7 +206,7 @@ public extension SwiftSyntax.TokenDiagnostic {

let fixedToken = token.withKind(TokenKind.fromRaw(kind: rawKind, text: replacedText))
return [
FixIt(message: .replaceCurlyQuoteByNormalQuote, changes: [[.replace(oldNode: Syntax(token), newNode: Syntax(fixedToken))]])
FixIt(message: .replaceCurlyQuoteByNormalQuote, changes: [.replace(oldNode: Syntax(token), newNode: Syntax(fixedToken))])
]
case .equalMustHaveConsistentWhitespaceOnBothSides:
let hasLeadingSpace = token.previousToken(viewMode: .all)?.trailingTrivia.contains(where: { $0.isSpaceOrTab }) ?? false
Expand All @@ -226,7 +226,7 @@ public extension SwiftSyntax.TokenDiagnostic {
}

return [
FixIt(message: .insertWhitespace, changes: FixIt.Changes(changes: changes))
FixIt(message: .insertWhitespace, changes: changes)
]
default:
return []
Expand Down
4 changes: 2 additions & 2 deletions Sources/SwiftParserDiagnostics/MissingNodesError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ extension ParseDiagnosticsGenerator {
func handleMissingSyntax<T: SyntaxProtocol>(
_ node: T,
overridePosition: AbsolutePosition? = nil,
additionalChanges: [FixIt.Changes] = [],
additionalChanges: [FixIt.MultiNodeChange] = [],
additionalHandledNodes: [SyntaxIdentifier] = []
) -> SyntaxVisitorContinueKind {
if shouldSkip(node) {
Expand Down Expand Up @@ -356,7 +356,7 @@ extension ParseDiagnosticsGenerator {
}
}

let changes = missingNodes.enumerated().map { (index, missingNode) -> FixIt.Changes in
let changes = missingNodes.enumerated().map { (index, missingNode) -> FixIt.MultiNodeChange in
if index == 0,
let token = missingNode.as(TokenSyntax.self),
let previousTokenKind = missingNode.previousToken(viewMode: .sourceAccurate)?.tokenKind
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftParserDiagnostics/MissingTokenError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ extension ParseDiagnosticsGenerator {

private func handleInvalidPeriod(invalidToken: TokenSyntax, missingToken: TokenSyntax, invalidTokenContainer: UnexpectedNodesSyntax) -> Bool {
// Trailing trivia is the cause of this diagnostic, don't transfer it.
let changes: [FixIt.Changes] = [
let changes: [FixIt.MultiNodeChange] = [
.makeMissing(invalidToken, transferTrivia: false),
.makePresent(missingToken),
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ final class MultiLineStringLiteralIndentatinDiagnosticsGenerator: SyntaxVisitor
message: InvalidIndentationInMultiLineStringLiteralError(kind: currentDiagnostic.kind, lines: currentDiagnostic.lines),
highlights: [],
notes: [Note(node: Syntax(closeQuote), message: .shouldMatchIndentationOfClosingQuote)],
fixIts: [FixIt(message: .changeIndentationToMatchClosingDelimiter, changes: FixIt.Changes(changes: currentDiagnostic.changes))]
fixIts: [FixIt(message: .changeIndentationToMatchClosingDelimiter, changes: currentDiagnostic.changes)]
)

finishedDiagnostics.append((diagnostic, currentDiagnostic.handledNodes))
Expand Down
18 changes: 9 additions & 9 deletions Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -143,23 +143,23 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {

// Ignore `correctTokens` that are already present.
let correctAndMissingTokens = correctTokens.filter({ $0.presence == .missing })
var changes: [FixIt.Changes] = []
var changes: [FixIt.MultiNodeChange] = []
if let misplacedToken = misplacedTokens.only, let correctToken = correctTokens.only,
misplacedToken.nextToken(viewMode: .all) == correctToken || misplacedToken.previousToken(viewMode: .all) == correctToken,
correctToken.presence == .missing
{
// We are exchanging two adjacent tokens, transfer the trivia from the incorrect token to the corrected token.
changes += misplacedTokens.map { FixIt.Changes.makeMissing($0, transferTrivia: false) }
changes += misplacedTokens.map { FixIt.MultiNodeChange.makeMissing($0, transferTrivia: false) }
changes.append(
FixIt.Changes.makePresent(
FixIt.MultiNodeChange.makePresent(
correctToken,
leadingTrivia: misplacedToken.leadingTrivia,
trailingTrivia: misplacedToken.trailingTrivia
)
)
} else {
changes += misplacedTokens.map { FixIt.Changes.makeMissing($0) }
changes += correctAndMissingTokens.map { FixIt.Changes.makePresent($0) }
changes += misplacedTokens.map { FixIt.MultiNodeChange.makeMissing($0) }
changes += correctAndMissingTokens.map { FixIt.MultiNodeChange.makePresent($0) }
}
var fixIts: [FixIt] = []
if changes.count > 1 {
Expand Down Expand Up @@ -309,7 +309,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
node,
.unexpectedSemicolon,
fixIts: [
FixIt(message: RemoveNodesFixIt(semicolons), changes: semicolons.map { FixIt.Changes.makeMissing($0) })
FixIt(message: RemoveNodesFixIt(semicolons), changes: semicolons.map { FixIt.MultiNodeChange.makeMissing($0) })
]
)
} else if node.first?.as(TokenSyntax.self)?.tokenKind.isIdentifier == true,
Expand All @@ -327,7 +327,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
FixIt(
message: .joinIdentifiers,
changes: [
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joined), presence: .present)))],
FixIt.MultiNodeChange(.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joined), presence: .present)))),
.makeMissing(tokens),
]
)
Expand All @@ -338,7 +338,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
FixIt(
message: .joinIdentifiersWithCamelCase,
changes: [
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))],
FixIt.MultiNodeChange(.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))),
.makeMissing(tokens),
]
)
Expand Down Expand Up @@ -821,7 +821,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
message: RemoveNodesFixIt(unexpectedName),
changes: [
.makeMissing(unexpectedName),
FixIt.Changes(changes: [.replaceTrailingTrivia(token: previous, newTrivia: [])]),
FixIt.MultiNodeChange(.replaceTrailingTrivia(token: previous, newTrivia: [])),
]
)
],
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftSyntax/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ add_swift_host_library(SwiftSyntax
SyntaxArena.swift
SyntaxChildren.swift
SyntaxData.swift
SyntaxOtherNodes.swift
SyntaxText.swift
SyntaxTreeViewMode.swift
TokenDiagnostic.swift
TokenSyntax.swift
Utils.swift

Raw/RawSyntax.swift
Expand Down
15 changes: 15 additions & 0 deletions Sources/SwiftSyntax/Raw/RawSyntaxTokenView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,21 @@ public struct RawSyntaxTokenView {
}
}

/// Returns a `RawSyntax` node with the presence changed to `newValue`.
@_spi(RawSyntax)
public func withPresence(_ newValue: SourcePresence, arena: SyntaxArena) -> RawSyntax {
switch raw.rawData.payload {
case .parsedToken(var payload):
payload.presence = newValue
return RawSyntax(arena: arena, payload: .parsedToken(payload))
case .materializedToken(var payload):
payload.presence = newValue
return RawSyntax(arena: arena, payload: .materializedToken(payload))
default:
preconditionFailure("'withKind()' is called on non-token raw syntax")
}
}

/// The length of the token without leading or trailing trivia, assuming this
/// is a token node.
@_spi(RawSyntax)
Expand Down
8 changes: 8 additions & 0 deletions Sources/SwiftSyntax/SyntaxData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,14 @@ struct SyntaxData {
return self
}
}

func withPresence(_ presence: SourcePresence, arena: SyntaxArena) -> SyntaxData {
if let raw = raw.tokenView?.withPresence(presence, arena: arena) {
return replacingSelf(raw, arena: arena)
} else {
return self
}
}
}

#if DEBUG
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,12 @@ public struct TokenSyntax: SyntaxProtocol, SyntaxHashable {
}

public var presence: SourcePresence {
return tokenView.presence
get {
return tokenView.presence
}
set {
self = TokenSyntax(data.withPresence(newValue, arena: SyntaxArena()))
}
}

/// The text of the token as written in the source code, without any trivia.
Expand Down
Loading