Skip to content

Commit

Permalink
Merge pull request swiftlang#2226 from kimdv/kimdv/rewrite-fit-it-app…
Browse files Browse the repository at this point in the history
…lier

Rewrite FixItApplier to be string based
  • Loading branch information
kimdv committed Oct 17, 2023
2 parents 106183a + 33c4b08 commit 02a1330
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 81 deletions.
40 changes: 26 additions & 14 deletions Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,28 +48,21 @@ extension FixIt {

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.
static func makeMissing(_ token: TokenSyntax, transferTrivia: Bool = true) -> Self {
return makeMissing([token], transferTrivia: transferTrivia)
}

/// Replace present tokens with missing tokens.
/// If `transferTrivia` is `true`, the leading and trailing trivia of the
/// removed node will be transferred to the trailing trivia of the previous token.
///
/// If `transferTrivia` is `true`, the leading trivia of the first token and
/// the trailing trivia of the last token will be transferred to their adjecent
/// tokens.
static func makeMissing(_ tokens: [TokenSyntax], transferTrivia: Bool = true) -> Self {
precondition(!tokens.isEmpty)
precondition(tokens.allSatisfy({ $0.isPresent }))
var changes = tokens.map {
FixIt.Change.replace(
oldNode: Syntax($0),
newNode: Syntax($0.with(\.presence, .missing))
)
}
if transferTrivia {
changes += FixIt.MultiNodeChange.transferTriviaAtSides(from: tokens).primitiveChanges
}
return FixIt.MultiNodeChange(primitiveChanges: changes)
precondition(tokens.allSatisfy(\.isPresent))
return .makeMissing(tokens.map(Syntax.init), transferTrivia: transferTrivia)
}

/// If `transferTrivia` is `true`, the leading and trailing trivia of the
Expand Down Expand Up @@ -104,6 +97,25 @@ extension FixIt.MultiNodeChange {
return FixIt.MultiNodeChange()
}
}

/// Replace present nodes with their missing equivalents.
///
/// If `transferTrivia` is `true`, the leading trivia of the first node and
/// the trailing trivia of the last node will be transferred to their adjecent
/// tokens.
static func makeMissing(_ nodes: [Syntax], transferTrivia: Bool = true) -> Self {
precondition(!nodes.isEmpty)
var changes = nodes.map {
FixIt.Change.replace(
oldNode: $0,
newNode: MissingMaker().rewrite($0, detach: true)
)
}
if transferTrivia {
changes += FixIt.MultiNodeChange.transferTriviaAtSides(from: nodes).primitiveChanges
}
return FixIt.MultiNodeChange(primitiveChanges: changes)
}
}

// MARK: - Make present
Expand Down
26 changes: 12 additions & 14 deletions Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
correctToken.isMissing
{
// We are exchanging two adjacent tokens, transfer the trivia from the incorrect token to the corrected token.
changes += misplacedTokens.map { FixIt.MultiNodeChange.makeMissing($0, transferTrivia: false) }
changes.append(
FixIt.MultiNodeChange.makePresent(
correctToken,
Expand All @@ -191,6 +190,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
trailingTrivia: misplacedToken.trailingTrivia.isEmpty ? nil : misplacedToken.trailingTrivia
)
)
changes.append(FixIt.MultiNodeChange.makeMissing(misplacedTokens, transferTrivia: false))
} else {
changes += misplacedTokens.map { FixIt.MultiNodeChange.makeMissing($0) }
changes += correctAndMissingTokens.map { FixIt.MultiNodeChange.makePresent($0) }
Expand Down Expand Up @@ -236,7 +236,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
exchangeTokens(
unexpected: misplacedSpecifiers,
unexpectedTokenCondition: { EffectSpecifier(token: $0) != nil },
correctTokens: [effectSpecifiers?.throwsSpecifier, effectSpecifiers?.asyncSpecifier],
correctTokens: [effectSpecifiers?.asyncSpecifier, effectSpecifiers?.throwsSpecifier],
message: { EffectsSpecifierAfterArrow(effectsSpecifiersAfterArrow: $0) },
moveFixIt: { MoveTokensInFrontOfFixIt(movedTokens: $0, inFrontOf: .arrow) },
removeRedundantFixIt: { RemoveRedundantFixIt(removeTokens: $0) }
Expand Down Expand Up @@ -764,20 +764,17 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
if let unexpected = node.unexpectedBetweenRequirementAndTrailingComma,
let token = unexpected.presentTokens(satisfying: { $0.tokenKind == .binaryOperator("&&") }).first,
let trailingComma = node.trailingComma,
trailingComma.isMissing,
let previous = node.unexpectedBetweenRequirementAndTrailingComma?.previousToken(viewMode: .sourceAccurate)
trailingComma.isMissing
{

addDiagnostic(
unexpected,
.expectedCommaInWhereClause,
fixIts: [
FixIt(
message: ReplaceTokensFixIt(replaceTokens: [token], replacements: [.commaToken()]),
changes: [
.makeMissing(token),
.makePresent(trailingComma),
FixIt.MultiNodeChange(.replaceTrailingTrivia(token: previous, newTrivia: [])),
.makeMissing(token, transferTrivia: false),
.makePresent(trailingComma, leadingTrivia: token.leadingTrivia, trailingTrivia: token.trailingTrivia),
]
)
],
Expand Down Expand Up @@ -818,7 +815,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
fixIts: [
FixIt(
message: RemoveNodesFixIt(nodes),
changes: nodes.map { .makeMissing($0) }
changes: .makeMissing(nodes)
)
],
handledNodes: nodes.map { $0.id }
Expand Down Expand Up @@ -1542,7 +1539,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
fixIts: [
FixIt(
message: RemoveNodesFixIt(rawDelimiters),
changes: rawDelimiters.map { .makeMissing($0) }
changes: .makeMissing(rawDelimiters)
)
],
handledNodes: rawDelimiters.map { $0.id }
Expand Down Expand Up @@ -1862,8 +1859,8 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
replacements: [node.colon]
),
changes: [
FixIt.MultiNodeChange.makeMissing(equalToken),
FixIt.MultiNodeChange.makePresent(node.colon),
.makeMissing(equalToken, transferTrivia: false),
.makePresent(node.colon, leadingTrivia: equalToken.leadingTrivia, trailingTrivia: equalToken.trailingTrivia),
]
)
],
Expand Down Expand Up @@ -1971,8 +1968,9 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
FixIt(
message: fixItMessage,
changes: [
FixIt.MultiNodeChange.makePresent(detail.detail)
] + unexpectedTokens.map { FixIt.MultiNodeChange.makeMissing($0) }
.makePresent(detail.detail),
.makeMissing(unexpectedTokens),
]
)
],
handledNodes: [detail.id] + unexpectedTokens.map(\.id)
Expand Down
114 changes: 114 additions & 0 deletions Sources/_SwiftSyntaxTestSupport/FixItApplier.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2023 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

public enum FixItApplier {
struct Edit: Equatable {
var startUtf8Offset: Int
var endUtf8Offset: Int
let replacement: String

var replacementLength: Int {
return replacement.utf8.count
}

var replacementRange: Range<Int> {
return startUtf8Offset..<endUtf8Offset
}
}

/// Applies selected or all Fix-Its from the provided diagnostics to a given syntax tree.
///
/// - Parameters:
/// - diagnostics: An array of `Diagnostic` objects, each containing one or more Fix-Its.
/// - filterByMessages: An optional array of message strings to filter which Fix-Its to apply.
/// If `nil`, the first Fix-It from each diagnostic is applied.
/// - tree: The syntax tree to which the Fix-Its will be applied.
///
/// - Returns: A ``String`` representation of the modified syntax tree after applying the Fix-Its.
public static func applyFixes(
from diagnostics: [Diagnostic],
filterByMessages messages: [String]?,
to tree: any SyntaxProtocol
) -> String {
let messages = messages ?? diagnostics.compactMap { $0.fixIts.first?.message.message }

let changes =
diagnostics
.flatMap(\.fixIts)
.filter { messages.contains($0.message.message) }
.flatMap(\.changes)

var edits: [Edit] = changes.map(\.edit)
var source = tree.description

while let edit = edits.first {
edits = Array(edits.dropFirst())

let startIndex = source.utf8.index(source.utf8.startIndex, offsetBy: edit.startUtf8Offset)
let endIndex = source.utf8.index(source.utf8.startIndex, offsetBy: edit.endUtf8Offset)

source.replaceSubrange(startIndex..<endIndex, with: edit.replacement)

edits = edits.compactMap { remainingEdit -> FixItApplier.Edit? in
var remainingEdit = remainingEdit

if remainingEdit.replacementRange.overlaps(edit.replacementRange) {
// The edit overlaps with the previous edit. We can't apply both
// without conflicts. Apply the one that's listed first and drop the
// later edit.
return nil
}

// If the remaining edit starts after or at the end of the edit that we just applied,
// shift it by the current edit's difference in length.
if edit.endUtf8Offset <= remainingEdit.startUtf8Offset {
remainingEdit.startUtf8Offset = remainingEdit.startUtf8Offset - edit.replacementRange.count + edit.replacementLength
remainingEdit.endUtf8Offset = remainingEdit.endUtf8Offset - edit.replacementRange.count + edit.replacementLength
}

return remainingEdit
}
}

return source
}
}

fileprivate extension FixIt.Change {
var edit: FixItApplier.Edit {
switch self {
case .replace(let oldNode, let newNode):
return FixItApplier.Edit(
startUtf8Offset: oldNode.position.utf8Offset,
endUtf8Offset: oldNode.endPosition.utf8Offset,
replacement: newNode.description
)

case .replaceLeadingTrivia(let token, let newTrivia):
return FixItApplier.Edit(
startUtf8Offset: token.position.utf8Offset,
endUtf8Offset: token.positionAfterSkippingLeadingTrivia.utf8Offset,
replacement: newTrivia.description
)

case .replaceTrailingTrivia(let token, let newTrivia):
return FixItApplier.Edit(
startUtf8Offset: token.endPositionBeforeTrailingTrivia.utf8Offset,
endUtf8Offset: token.endPosition.utf8Offset,
replacement: newTrivia.description
)
}
}
}
54 changes: 1 addition & 53 deletions Tests/SwiftParserTest/Assertions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -276,58 +276,6 @@ struct DiagnosticSpec {
}
}

class FixItApplier: SyntaxRewriter {
var changes: [FixIt.Change]

init(diagnostics: [Diagnostic], withMessages messages: [String]?) {
let messages = messages ?? diagnostics.compactMap { $0.fixIts.first?.message.message }

self.changes =
diagnostics
.flatMap { $0.fixIts }
.filter {
return messages.contains($0.message.message)
}
.flatMap { $0.changes }

super.init(viewMode: .all)
}

public override func visitAny(_ node: Syntax) -> Syntax? {
for change in changes {
switch change {
case .replace(oldNode: let oldNode, newNode: let newNode) where oldNode.id == node.id:
return newNode
default:
break
}
}
return nil
}

override func visit(_ node: TokenSyntax) -> TokenSyntax {
var modifiedNode = node
for change in changes {
switch change {
case .replaceLeadingTrivia(token: let changedNode, newTrivia: let newTrivia) where changedNode.id == node.id:
modifiedNode = node.with(\.leadingTrivia, newTrivia)
case .replaceTrailingTrivia(token: let changedNode, newTrivia: let newTrivia) where changedNode.id == node.id:
modifiedNode = node.with(\.trailingTrivia, newTrivia)
default:
break
}
}
return modifiedNode
}

/// If `messages` is `nil`, applies all Fix-Its in `diagnostics` to `tree` and returns the fixed syntax tree.
/// If `messages` is not `nil`, applies only Fix-Its whose message is in `messages`.
public static func applyFixes<T: SyntaxProtocol>(in diagnostics: [Diagnostic], withMessages messages: [String]?, to tree: T) -> Syntax {
let applier = FixItApplier(diagnostics: diagnostics, withMessages: messages)
return applier.rewrite(tree)
}
}

/// Assert that `location` is the same as that of `locationMarker` in `tree`.
func assertLocation<T: SyntaxProtocol>(
_ location: SourceLocation,
Expand Down Expand Up @@ -679,7 +627,7 @@ extension ParserTestCase {
if expectedDiagnostics.contains(where: { !$0.fixIts.isEmpty }) && expectedFixedSource == nil {
XCTFail("Expected a fixed source if the test case produces diagnostics with Fix-Its", file: file, line: line)
} else if let expectedFixedSource = expectedFixedSource {
let fixedTree = FixItApplier.applyFixes(in: diags, withMessages: applyFixIts, to: tree)
let fixedTree = FixItApplier.applyFixes(from: diags, filterByMessages: applyFixIts, to: tree)
var fixedTreeDescription = fixedTree.description
if options.contains(.normalizeNewlinesInFixedSource) {
fixedTreeDescription =
Expand Down
Loading

0 comments on commit 02a1330

Please sign in to comment.