From 60fa6706f352f5367e650b9b7ede483fe0ff83c5 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Thu, 4 May 2023 09:29:40 +0200 Subject: [PATCH 1/4] Change test class name to match syntax type --- ...ingLiteralTests.swift => StringLiteralExprSyntaxTests.swift} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename Tests/SwiftSyntaxBuilderTest/{StringLiteralTests.swift => StringLiteralExprSyntaxTests.swift} (99%) diff --git a/Tests/SwiftSyntaxBuilderTest/StringLiteralTests.swift b/Tests/SwiftSyntaxBuilderTest/StringLiteralExprSyntaxTests.swift similarity index 99% rename from Tests/SwiftSyntaxBuilderTest/StringLiteralTests.swift rename to Tests/SwiftSyntaxBuilderTest/StringLiteralExprSyntaxTests.swift index 1de0bf7a62b..4b761ce623b 100644 --- a/Tests/SwiftSyntaxBuilderTest/StringLiteralTests.swift +++ b/Tests/SwiftSyntaxBuilderTest/StringLiteralExprSyntaxTests.swift @@ -14,7 +14,7 @@ import XCTest import SwiftSyntax import SwiftSyntaxBuilder -final class StringLiteralTests: XCTestCase { +final class StringLiteralExprSyntaxTests: XCTestCase { func testStringLiteral() { let leadingTrivia = Trivia.unexpectedText("␣") let testCases: [UInt: (String, String)] = [ From 3bd861f157aa9c0f7fcd55c1a3e318748b47b922 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Mon, 1 May 2023 17:14:57 +0200 Subject: [PATCH 2/4] Fix wrong foratting of multiline string literal --- Sources/SwiftBasicFormat/BasicFormat.swift | 87 +++++++++++-------- .../DiagnosticExtensions.swift | 5 +- .../MissingNodesError.swift | 2 +- .../ParseDiagnosticsGenerator.swift | 4 +- Tests/SwiftParserTest/ExpressionTests.swift | 12 ++- .../translated/AvailabilityQueryTests.swift | 4 +- ...AvailabilityQueryUnavailabilityTests.swift | 4 +- .../translated/MultilineErrorsTests.swift | 4 +- ...ePoundDiagnosticArgRdar41154797Tests.swift | 3 +- .../translated/MultilineStringTests.swift | 76 ++++++++++++++++ .../translated/StringLiteralEofTests.swift | 12 ++- .../UnclosedStringInterpolationTests.swift | 4 +- 12 files changed, 162 insertions(+), 55 deletions(-) diff --git a/Sources/SwiftBasicFormat/BasicFormat.swift b/Sources/SwiftBasicFormat/BasicFormat.swift index de1e796f10d..1bc9c8d8f14 100644 --- a/Sources/SwiftBasicFormat/BasicFormat.swift +++ b/Sources/SwiftBasicFormat/BasicFormat.swift @@ -143,35 +143,49 @@ open class BasicFormat: SyntaxRewriter { return node.requiresIndent } - /// Whether a leading newline on `token` should be added. - open func requiresLeadingNewline(_ token: TokenSyntax) -> Bool { - // We don't want to add newlines inside string interpolation - if isInsideStringInterpolation(token) { + open func requiresNewline(between first: TokenSyntax?, and second: TokenSyntax?) -> Bool { + // We don't want to add newlines inside string interpolation. + // When first or second `TokenSyntax` is a multiline quote we want special handling + // even if it's inside a string interpolation, because it still requires newline + // after open quote and before close quote. + if let first, + isInsideStringInterpolation(first), + first.tokenKind != .multilineStringQuote, + second?.tokenKind != .multilineStringQuote + { return false - } - - if token.requiresLeadingNewline { - return true - } - - var ancestor: Syntax = Syntax(token) - while let parent = ancestor.parent { - ancestor = parent - if ancestor.position != token.position || ancestor.firstToken(viewMode: viewMode) != token { - break - } - if let ancestorsParent = ancestor.parent, childrenSeparatedByNewline(ancestorsParent) { + } else if let second { + if second.requiresLeadingNewline { return true } - switch ancestor.keyPathInParent { - case \IfConfigClauseSyntax.elements: - return true - default: - break + + var ancestor: Syntax = Syntax(second) + while let parent = ancestor.parent { + ancestor = parent + if ancestor.position != second.position || ancestor.firstToken(viewMode: viewMode) != second { + break + } + if let ancestorsParent = ancestor.parent, childrenSeparatedByNewline(ancestorsParent) { + return true + } + switch ancestor.keyPathInParent { + case \IfConfigClauseSyntax.elements: + return true + default: + break + } } } - return false + switch (first?.tokenKind, second?.tokenKind) { + case (.multilineStringQuote, .multilineStringQuote), // string interpolation segment inside a multi-line string literal + (.multilineStringQuote, .stringSegment), // empty multi-line string literal + (.stringSegment, .multilineStringQuote), // segment starting a multi-line string literal + (.rightParen, .multilineStringQuote): // ending a multi-line string literal that has a string interpolation segment at its end + return true + default: + return false + } } open func requiresWhitespace(between first: TokenSyntax?, and second: TokenSyntax?) -> Bool { @@ -298,10 +312,7 @@ open class BasicFormat: SyntaxRewriter { if previousToken.trailingTrivia.endsWithNewline { return true } - if case .stringSegment(let segment) = previousToken.tokenKind, segment.last?.isNewline ?? false { - return true - } - return false + return previousToken.isStringSegmentWithLastCharacterBeingNewline }() lazy var previousTokenIsStringLiteralEndingInNewline: Bool = { @@ -310,10 +321,7 @@ open class BasicFormat: SyntaxRewriter { // don't add a leading newline to the file. return true } - if case .stringSegment(let segment) = previousToken.tokenKind, segment.last?.isNewline ?? false { - return true - } - return false + return previousToken.isStringSegmentWithLastCharacterBeingNewline }() lazy var nextTokenWillStartWithWhitespace: Bool = { @@ -321,7 +329,7 @@ open class BasicFormat: SyntaxRewriter { return false } return nextToken.leadingTrivia.startsWithWhitespace - || (requiresLeadingNewline(nextToken) && isMutable(nextToken)) + || (requiresNewline(between: token, and: nextToken) && isMutable(nextToken)) }() lazy var nextTokenWillStartWithNewline: Bool = { @@ -329,7 +337,7 @@ open class BasicFormat: SyntaxRewriter { return false } return nextToken.leadingTrivia.startsWithNewline - || (requiresLeadingNewline(nextToken) && isMutable(nextToken)) + || (requiresNewline(between: token, and: nextToken) && isMutable(nextToken) && !token.trailingTrivia.endsWithNewline && !token.isStringSegmentWithLastCharacterBeingNewline) }() /// This token's trailing trivia + any spaces or tabs at the start of the @@ -342,7 +350,7 @@ open class BasicFormat: SyntaxRewriter { var leadingTrivia = token.leadingTrivia var trailingTrivia = token.trailingTrivia - if requiresLeadingNewline(token) { + if requiresNewline(between: previousToken, and: token) { // Add a leading newline if the token requires it unless // - it already starts with a newline or // - the previous token ends with a newline @@ -407,3 +415,14 @@ open class BasicFormat: SyntaxRewriter { return token.detach().with(\.leadingTrivia, leadingTrivia).with(\.trailingTrivia, trailingTrivia) } } + +fileprivate extension TokenSyntax { + var isStringSegmentWithLastCharacterBeingNewline: Bool { + switch self.tokenKind { + case .stringSegment(let segment): + return segment.last?.isNewline ?? false + default: + return false + } + } +} diff --git a/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift b/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift index ad20ff98ad8..0e196e514d8 100644 --- a/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift +++ b/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift @@ -152,11 +152,12 @@ extension FixIt.MultiNodeChange { } } - if let previousToken = node.previousToken(viewMode: .all), + if let previousToken = node.previousToken(viewMode: .fixedUp), previousToken.presence == .present, let firstToken = node.firstToken(viewMode: .all), previousToken.trailingTrivia.allSatisfy({ $0.isWhitespace }), - !BasicFormat().requiresWhitespace(between: previousToken, and: firstToken) + !BasicFormat().requiresWhitespace(between: previousToken, and: firstToken), + !BasicFormat().requiresNewline(between: previousToken, and: firstToken) { // If the previous token and this node don't need to be separated, remove // the separation. diff --git a/Sources/SwiftParserDiagnostics/MissingNodesError.swift b/Sources/SwiftParserDiagnostics/MissingNodesError.swift index 216c8a69e59..dd7ba898ab4 100644 --- a/Sources/SwiftParserDiagnostics/MissingNodesError.swift +++ b/Sources/SwiftParserDiagnostics/MissingNodesError.swift @@ -22,7 +22,7 @@ fileprivate func findCommonAncestor(_ nodes: [Syntax]) -> Syntax? { } class NoNewlinesFormat: BasicFormat { - override func requiresLeadingNewline(_ token: TokenSyntax) -> Bool { + override func requiresNewline(between first: TokenSyntax?, and second: TokenSyntax?) -> Bool { return false } } diff --git a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift index 3b46b9c53ba..8a5acb476d8 100644 --- a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift +++ b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift @@ -1252,12 +1252,10 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { } if case .stringSegment(let segment) = node.segments.last { if let invalidContent = segment.unexpectedBeforeContent?.onlyToken(where: { $0.trailingTrivia.contains(where: { $0.isBackslash }) }) { - let leadingTrivia = segment.content.leadingTrivia - let trailingTrivia = segment.content.trailingTrivia let fixIt = FixIt( message: .removeBackslash, changes: [ - .makePresent(segment.content, leadingTrivia: leadingTrivia, trailingTrivia: trailingTrivia), + .makePresent(segment.content), .makeMissing(invalidContent, transferTrivia: false), ] ) diff --git a/Tests/SwiftParserTest/ExpressionTests.swift b/Tests/SwiftParserTest/ExpressionTests.swift index f43a92fc2c7..fdc62a4e23f 100644 --- a/Tests/SwiftParserTest/ExpressionTests.swift +++ b/Tests/SwiftParserTest/ExpressionTests.swift @@ -603,7 +603,8 @@ final class ExpressionTests: XCTestCase { ) ], fixedSource: ##""" - """"""" + """" + """ """## ) @@ -619,7 +620,8 @@ final class ExpressionTests: XCTestCase { ) ], fixedSource: ##""" - """""""" + """"" + """ """## ) @@ -656,7 +658,8 @@ final class ExpressionTests: XCTestCase { DiagnosticSpec(message: ##"expected '"""#' to end string literal"##, fixIts: [##"insert '"""#'"##]) ], fixedSource: ##""" - #""""""# + #""" + """# """## ) @@ -668,7 +671,8 @@ final class ExpressionTests: XCTestCase { DiagnosticSpec(message: ##"expected '"""#' to end string literal"##, fixIts: [##"insert '"""#'"##]) ], fixedSource: ##""" - #"""a"""# + #"""a + """# """## ) diff --git a/Tests/SwiftParserTest/translated/AvailabilityQueryTests.swift b/Tests/SwiftParserTest/translated/AvailabilityQueryTests.swift index 6f4dbc40d99..5c0cd63b8cb 100644 --- a/Tests/SwiftParserTest/translated/AvailabilityQueryTests.swift +++ b/Tests/SwiftParserTest/translated/AvailabilityQueryTests.swift @@ -100,7 +100,7 @@ final class AvailabilityQueryTests: XCTestCase { DiagnosticSpec(message: "expected ',' joining parts of a multi-clause condition", fixIts: ["replace '&&' with ','"]) ], fixedSource: """ - if #available(OSX 10.51, *) , #available(OSX 10.52, *) { + if #available(OSX 10.51, *), #available(OSX 10.52, *) { } """ ) @@ -440,7 +440,7 @@ final class AvailabilityQueryTests: XCTestCase { DiagnosticSpec(message: "expected ',' joining platforms in availability condition", fixIts: ["replace '||' with ','"]) ], fixedSource: """ - if #available(OSX 10.51 , iOS 8.0) { + if #available(OSX 10.51, iOS 8.0) { } """ ) diff --git a/Tests/SwiftParserTest/translated/AvailabilityQueryUnavailabilityTests.swift b/Tests/SwiftParserTest/translated/AvailabilityQueryUnavailabilityTests.swift index e89c8613fc7..52ffaa36fb3 100644 --- a/Tests/SwiftParserTest/translated/AvailabilityQueryUnavailabilityTests.swift +++ b/Tests/SwiftParserTest/translated/AvailabilityQueryUnavailabilityTests.swift @@ -103,7 +103,7 @@ final class AvailabilityQueryUnavailabilityTests: XCTestCase { ) ], fixedSource: """ - if #unavailable(OSX 10.51) , #unavailable(OSX 10.52) { + if #unavailable(OSX 10.51), #unavailable(OSX 10.52) { } """ ) @@ -447,7 +447,7 @@ final class AvailabilityQueryUnavailabilityTests: XCTestCase { ) ], fixedSource: """ - if #unavailable(OSX 10.51 , iOS 8.0) { + if #unavailable(OSX 10.51, iOS 8.0) { } """ ) diff --git a/Tests/SwiftParserTest/translated/MultilineErrorsTests.swift b/Tests/SwiftParserTest/translated/MultilineErrorsTests.swift index 4f6343968bb..dfb4a4fd95b 100644 --- a/Tests/SwiftParserTest/translated/MultilineErrorsTests.swift +++ b/Tests/SwiftParserTest/translated/MultilineErrorsTests.swift @@ -705,7 +705,8 @@ final class MultilineErrorsTests: XCTestCase { ], fixedSource: ##""" _ = """ - foo\""" + foo\ + """ """## ) } @@ -741,6 +742,7 @@ final class MultilineErrorsTests: XCTestCase { fixedSource: #""" _ = """ \#u{20} + """ """# ) diff --git a/Tests/SwiftParserTest/translated/MultilinePoundDiagnosticArgRdar41154797Tests.swift b/Tests/SwiftParserTest/translated/MultilinePoundDiagnosticArgRdar41154797Tests.swift index 0183eced16f..d819fe38459 100644 --- a/Tests/SwiftParserTest/translated/MultilinePoundDiagnosticArgRdar41154797Tests.swift +++ b/Tests/SwiftParserTest/translated/MultilinePoundDiagnosticArgRdar41154797Tests.swift @@ -35,7 +35,8 @@ final class MultilinePoundDiagnosticArgRdar41154797Tests: XCTestCase { ), ], fixedSource: ##""" - #error("""""") + #error(""" + """) """## ) } diff --git a/Tests/SwiftParserTest/translated/MultilineStringTests.swift b/Tests/SwiftParserTest/translated/MultilineStringTests.swift index 0ca78fdf1dc..263ed297fa0 100644 --- a/Tests/SwiftParserTest/translated/MultilineStringTests.swift +++ b/Tests/SwiftParserTest/translated/MultilineStringTests.swift @@ -411,6 +411,82 @@ final class MultilineStringTests: XCTestCase { ) } + func testMultilineString47() { + assertParse( + #""" + _ = """1️⃣""" + """#, + diagnostics: [ + DiagnosticSpec( + message: "multi-line string literal closing delimiter must begin on a new line", + fixIts: ["insert newline"] + ) + ], + fixedSource: #""" + _ = """ + """ + """# + ) + } + + func testMultilineString48() { + assertParse( + #""" + _ = """A1️⃣""" + """#, + diagnostics: [ + DiagnosticSpec( + message: "multi-line string literal closing delimiter must begin on a new line", + fixIts: ["insert newline"] + ) + ], + fixedSource: #""" + _ = """A + """ + """# + ) + } + + func testMultilineString49() { + assertParse( + #""" + _ = ℹ️"""1️⃣ + """#, + diagnostics: [ + DiagnosticSpec( + message: #"expected '"""' to end string literal"#, + notes: [NoteSpec(message: #"to match this opening '"""'"#)], + fixIts: [#"insert '"""'"#] + ) + ], + fixedSource: #""" + _ = """ + """ + """# + ) + } + + func testMultilineString50() { + assertParse( + #""" + _ = ℹ️""" + A1️⃣ + """#, + diagnostics: [ + DiagnosticSpec( + message: #"expected '"""' to end string literal"#, + notes: [NoteSpec(message: #"to match this opening '"""'"#)], + fixIts: [#"insert '"""'"#] + ) + ], + fixedSource: #""" + _ = """ + A + """ + """# + ) + } + func testEscapeNewlineInRawString() { assertParse( ##""" diff --git a/Tests/SwiftParserTest/translated/StringLiteralEofTests.swift b/Tests/SwiftParserTest/translated/StringLiteralEofTests.swift index 25e8986da35..5d98c476d28 100644 --- a/Tests/SwiftParserTest/translated/StringLiteralEofTests.swift +++ b/Tests/SwiftParserTest/translated/StringLiteralEofTests.swift @@ -133,7 +133,8 @@ final class StringLiteralEofTests: XCTestCase { fixedSource: #""" // NOTE: DO NOT add a newline at EOF. _ = """ - foo""" + foo + """ """# ) } @@ -160,7 +161,8 @@ final class StringLiteralEofTests: XCTestCase { fixedSource: ##""" _ = """ foo - \(<#expression#>)""" + \(<#expression#>) + """ """## // FIXME: The closing delimiter should be put on the new line ) @@ -197,7 +199,8 @@ final class StringLiteralEofTests: XCTestCase { fixedSource: ##""" _ = """ foo - \("bar")""" + \("bar") + """ """## ) } @@ -236,7 +239,8 @@ final class StringLiteralEofTests: XCTestCase { fixedSource: ##""" _ = """ \("bar" - baz)""" + baz) + """ """## ) } diff --git a/Tests/SwiftParserTest/translated/UnclosedStringInterpolationTests.swift b/Tests/SwiftParserTest/translated/UnclosedStringInterpolationTests.swift index 9a8868c6002..a76ea0fb07f 100644 --- a/Tests/SwiftParserTest/translated/UnclosedStringInterpolationTests.swift +++ b/Tests/SwiftParserTest/translated/UnclosedStringInterpolationTests.swift @@ -187,7 +187,9 @@ final class UnclosedStringInterpolationTests: XCTestCase { fixedSource: ##""" _ = """ \( - """""")""" + """ + """) + """ """## ) } From a64d0efd34175b8af6477dfdfc0cb9dfd4b878d3 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Thu, 4 May 2023 10:23:11 +0200 Subject: [PATCH 3/4] Fix missing newline for opening quote for multilines strings --- Sources/SwiftBasicFormat/BasicFormat.swift | 7 ++-- .../StringLiteralExprSyntaxTests.swift | 40 +++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/Sources/SwiftBasicFormat/BasicFormat.swift b/Sources/SwiftBasicFormat/BasicFormat.swift index 1bc9c8d8f14..696c0641197 100644 --- a/Sources/SwiftBasicFormat/BasicFormat.swift +++ b/Sources/SwiftBasicFormat/BasicFormat.swift @@ -178,9 +178,10 @@ open class BasicFormat: SyntaxRewriter { } switch (first?.tokenKind, second?.tokenKind) { - case (.multilineStringQuote, .multilineStringQuote), // string interpolation segment inside a multi-line string literal - (.multilineStringQuote, .stringSegment), // empty multi-line string literal - (.stringSegment, .multilineStringQuote), // segment starting a multi-line string literal + case (.multilineStringQuote, .backslash), // string interpolation segment inside a multi-line string literal + (.multilineStringQuote, .multilineStringQuote), // empty multi-line string literal + (.multilineStringQuote, .stringSegment), // segment starting a multi-line string literal + (.stringSegment, .multilineStringQuote), // ending a multi-line string literal that has a string interpolation segment at its end (.rightParen, .multilineStringQuote): // ending a multi-line string literal that has a string interpolation segment at its end return true default: diff --git a/Tests/SwiftSyntaxBuilderTest/StringLiteralExprSyntaxTests.swift b/Tests/SwiftSyntaxBuilderTest/StringLiteralExprSyntaxTests.swift index 4b761ce623b..b760e2dd309 100644 --- a/Tests/SwiftSyntaxBuilderTest/StringLiteralExprSyntaxTests.swift +++ b/Tests/SwiftSyntaxBuilderTest/StringLiteralExprSyntaxTests.swift @@ -334,4 +334,44 @@ final class StringLiteralExprSyntaxTests: XCTestCase { """# ) } + + func testMultiStringOpeningQuote() { + assertBuildResult( + StringLiteralExprSyntax(openQuote: .multilineStringQuoteToken(), content: "a", closeQuote: .multilineStringQuoteToken()), + #""" + """ + a + """ + """# + ) + + assertBuildResult( + StringLiteralExprSyntax( + openQuote: .multilineStringQuoteToken(), + segments: StringLiteralSegmentsSyntax { + .expressionSegment( + ExpressionSegmentSyntax( + expressions: TupleExprElementListSyntax { + TupleExprElementSyntax( + expression: StringLiteralExprSyntax( + openQuote: .multilineStringQuoteToken(), + content: "a", + closeQuote: .multilineStringQuoteToken() + ) + ) + } + ) + ) + }, + closeQuote: .multilineStringQuoteToken() + ), + #""" + """ + \(""" + a + """) + """ + """# + ) + } } From 6f412218aeff81faa1232001300ba89e7b39a847 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Wed, 10 May 2023 09:22:55 +0200 Subject: [PATCH 4/4] Add documentation for local helper variables --- Sources/SwiftBasicFormat/BasicFormat.swift | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Sources/SwiftBasicFormat/BasicFormat.swift b/Sources/SwiftBasicFormat/BasicFormat.swift index 696c0641197..dbdffe9b028 100644 --- a/Sources/SwiftBasicFormat/BasicFormat.swift +++ b/Sources/SwiftBasicFormat/BasicFormat.swift @@ -296,6 +296,12 @@ open class BasicFormat: SyntaxRewriter { let previousToken = self.previousToken ?? token.previousToken(viewMode: viewMode) let nextToken = token.nextToken(viewMode: viewMode) + /// In addition to existing trivia of `previousToken`, also considers + /// `previousToken` as ending with whitespace if it and `token` should be + /// separated by whitespace. + /// It does not consider whether a newline should be added between + /// `previousToken` and the `token` because that newline should be added to + /// the next token's trailing trivia. lazy var previousTokenWillEndWithWhitespace: Bool = { guard let previousToken = previousToken else { return false @@ -304,6 +310,8 @@ open class BasicFormat: SyntaxRewriter { || (requiresWhitespace(between: previousToken, and: token) && isMutable(previousToken)) }() + /// This method does not consider any posssible mutations to `previousToken` + /// because newlines should be added to the next token's leading trivia. lazy var previousTokenWillEndWithNewline: Bool = { guard let previousToken = previousToken else { // Assume that the start of the tree is equivalent to a newline so we @@ -325,6 +333,10 @@ open class BasicFormat: SyntaxRewriter { return previousToken.isStringSegmentWithLastCharacterBeingNewline }() + /// Also considers `nextToken` as starting with a whitespace if a newline + /// should be added to it. It does not check whether `token` and `nextToken` + /// should be separated by whitespace because the whitespace should be added + /// to the `token`’s leading trivia. lazy var nextTokenWillStartWithWhitespace: Bool = { guard let nextToken = nextToken else { return false @@ -333,6 +345,8 @@ open class BasicFormat: SyntaxRewriter { || (requiresNewline(between: token, and: nextToken) && isMutable(nextToken)) }() + /// Also considers `nextToken` as starting with a leading newline if `token` + /// and `nextToken` should be separated by a newline. lazy var nextTokenWillStartWithNewline: Bool = { guard let nextToken = nextToken else { return false