diff --git a/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift b/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift index 27fb25ccb5b..1d1f9983df5 100644 --- a/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift +++ b/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift @@ -48,6 +48,7 @@ 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 { @@ -55,21 +56,13 @@ extension FixIt.MultiNodeChange { } /// 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 @@ -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 diff --git a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift index 2c8f339176d..067ae580b55 100644 --- a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift +++ b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift @@ -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, @@ -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) } @@ -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) } @@ -764,10 +764,8 @@ 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, @@ -775,9 +773,8 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { 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), ] ) ], @@ -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 } @@ -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 } @@ -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), ] ) ], @@ -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) diff --git a/Sources/_SwiftSyntaxTestSupport/FixItApplier.swift b/Sources/_SwiftSyntaxTestSupport/FixItApplier.swift new file mode 100644 index 00000000000..967bdc8440a --- /dev/null +++ b/Sources/_SwiftSyntaxTestSupport/FixItApplier.swift @@ -0,0 +1,123 @@ +//===----------------------------------------------------------------------===// +// +// 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 { + return startUtf8Offset.. 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.edits + 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.. 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 { + // Take the largest value as we in some cases can end up with a negative offset. + remainingEdit.startUtf8Offset = remainingEdit.startUtf8Offset - edit.replacementRange.count + edit.replacementLength + + // If we need to replace a string at the end, we need to ensure we don't get an out-of-bounds error. + 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 + ) + } + } +} + +fileprivate extension Array where Element == FixIt.Change { + var edits: [FixItApplier.Edit] { + return self.map { $0.edit } + } +} diff --git a/Tests/SwiftParserTest/Assertions.swift b/Tests/SwiftParserTest/Assertions.swift index ffe3ccc4f7f..29216e25c02 100644 --- a/Tests/SwiftParserTest/Assertions.swift +++ b/Tests/SwiftParserTest/Assertions.swift @@ -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(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( _ location: SourceLocation, @@ -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 = diff --git a/Tests/SwiftParserTest/DeclarationTests.swift b/Tests/SwiftParserTest/DeclarationTests.swift index 8ca6fb7ff40..de2bd77be3e 100644 --- a/Tests/SwiftParserTest/DeclarationTests.swift +++ b/Tests/SwiftParserTest/DeclarationTests.swift @@ -922,6 +922,7 @@ final class DeclarationTests: ParserTestCase { fixedSource: "func test() async throws(MyError) {}", experimentalFeatures: [.typedThrows] ) + assertParse( "func test() throws 1️⃣async2️⃣(MyError) {}", diagnostics: [ @@ -931,6 +932,7 @@ final class DeclarationTests: ParserTestCase { fixedSource: "func test() async throws (MyError) {}", experimentalFeatures: [.typedThrows] ) + assertParse( "func test() 1️⃣try2️⃣(MyError) async {}", diagnostics: [ @@ -940,6 +942,7 @@ final class DeclarationTests: ParserTestCase { fixedSource: "func test() throws (MyError) async {}", experimentalFeatures: [.typedThrows] ) + assertParse( "func test() 1️⃣try 2️⃣async3️⃣(MyError) {}", diagnostics: [ @@ -950,6 +953,7 @@ final class DeclarationTests: ParserTestCase { fixedSource: "func test() async throws (MyError) {}", experimentalFeatures: [.typedThrows] ) + assertParse( "func test() throws(MyError) 1️⃣await {}", diagnostics: [ @@ -958,6 +962,7 @@ final class DeclarationTests: ParserTestCase { fixedSource: "func test() async throws(MyError) {}", experimentalFeatures: [.typedThrows] ) + assertParse( "func test() throws 1️⃣await2️⃣(MyError) {}", diagnostics: [ @@ -967,6 +972,7 @@ final class DeclarationTests: ParserTestCase { fixedSource: "func test() async throws (MyError) {}", experimentalFeatures: [.typedThrows] ) + assertParse( "func test() 1️⃣try2️⃣(MyError) await {}", diagnostics: [ @@ -976,6 +982,7 @@ final class DeclarationTests: ParserTestCase { fixedSource: "func test() throws (MyError) await {}", experimentalFeatures: [.typedThrows] ) + assertParse( "func test() 1️⃣try await2️⃣(MyError) {}", diagnostics: [ @@ -985,6 +992,7 @@ final class DeclarationTests: ParserTestCase { fixedSource: "func test() awaitthrows (MyError) {}", // FIXME: spacing experimentalFeatures: [.typedThrows] ) + assertParse( "func test() async1️⃣(MyError) {}", diagnostics: [ @@ -992,6 +1000,7 @@ final class DeclarationTests: ParserTestCase { ], experimentalFeatures: [.typedThrows] ) + assertParse( "func test() 1️⃣await2️⃣(MyError) {}", diagnostics: [ @@ -1001,6 +1010,7 @@ final class DeclarationTests: ParserTestCase { fixedSource: "func test() async (MyError) {}", experimentalFeatures: [.typedThrows] ) + assertParse( "func test() 1️⃣try2️⃣(MyError) {}", diagnostics: [ @@ -1010,10 +1020,12 @@ final class DeclarationTests: ParserTestCase { fixedSource: "func test() throws (MyError) {}", experimentalFeatures: [.typedThrows] ) + assertParse( "func test() throws(MyError) {}", experimentalFeatures: [.typedThrows] ) + assertParse( "func test() throws(MyError)1️⃣async {}", // no space between closing parenthesis and `async` diagnostics: [