Skip to content

Commit 0711d21

Browse files
kimdvkimdv
authored
Merge pull request #1623 from kimdv/kimdv/fix-newline-for-multiline-string-literal
2 parents 93e905c + 6f41221 commit 0711d21

13 files changed

+218
-56
lines changed

‎Sources/SwiftBasicFormat/BasicFormat.swift‎

Lines changed: 68 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -143,35 +143,50 @@ open class BasicFormat: SyntaxRewriter {
143143
return node.requiresIndent
144144
}
145145

146-
/// Whether a leading newline on `token` should be added.
147-
open func requiresLeadingNewline(_ token: TokenSyntax) -> Bool {
148-
// We don't want to add newlines inside string interpolation
149-
if isInsideStringInterpolation(token) {
146+
open func requiresNewline(between first: TokenSyntax?, and second: TokenSyntax?) -> Bool {
147+
// We don't want to add newlines inside string interpolation.
148+
// When first or second `TokenSyntax` is a multiline quote we want special handling
149+
// even if it's inside a string interpolation, because it still requires newline
150+
// after open quote and before close quote.
151+
if let first,
152+
isInsideStringInterpolation(first),
153+
first.tokenKind != .multilineStringQuote,
154+
second?.tokenKind != .multilineStringQuote
155+
{
150156
return false
151-
}
152-
153-
if token.requiresLeadingNewline {
154-
return true
155-
}
156-
157-
var ancestor: Syntax = Syntax(token)
158-
while let parent = ancestor.parent {
159-
ancestor = parent
160-
if ancestor.position != token.position || ancestor.firstToken(viewMode: viewMode) != token {
161-
break
162-
}
163-
if let ancestorsParent = ancestor.parent, childrenSeparatedByNewline(ancestorsParent) {
157+
} else if let second {
158+
if second.requiresLeadingNewline {
164159
return true
165160
}
166-
switch ancestor.keyPathInParent {
167-
case \IfConfigClauseSyntax.elements:
168-
return true
169-
default:
170-
break
161+
162+
var ancestor: Syntax = Syntax(second)
163+
while let parent = ancestor.parent {
164+
ancestor = parent
165+
if ancestor.position != second.position || ancestor.firstToken(viewMode: viewMode) != second {
166+
break
167+
}
168+
if let ancestorsParent = ancestor.parent, childrenSeparatedByNewline(ancestorsParent) {
169+
return true
170+
}
171+
switch ancestor.keyPathInParent {
172+
case \IfConfigClauseSyntax.elements:
173+
return true
174+
default:
175+
break
176+
}
171177
}
172178
}
173179

174-
return false
180+
switch (first?.tokenKind, second?.tokenKind) {
181+
case (.multilineStringQuote, .backslash), // string interpolation segment inside a multi-line string literal
182+
(.multilineStringQuote, .multilineStringQuote), // empty multi-line string literal
183+
(.multilineStringQuote, .stringSegment), // segment starting a multi-line string literal
184+
(.stringSegment, .multilineStringQuote), // ending a multi-line string literal that has a string interpolation segment at its end
185+
(.rightParen, .multilineStringQuote): // ending a multi-line string literal that has a string interpolation segment at its end
186+
return true
187+
default:
188+
return false
189+
}
175190
}
176191

177192
open func requiresWhitespace(between first: TokenSyntax?, and second: TokenSyntax?) -> Bool {
@@ -281,6 +296,12 @@ open class BasicFormat: SyntaxRewriter {
281296
let previousToken = self.previousToken ?? token.previousToken(viewMode: viewMode)
282297
let nextToken = token.nextToken(viewMode: viewMode)
283298

299+
/// In addition to existing trivia of `previousToken`, also considers
300+
/// `previousToken` as ending with whitespace if it and `token` should be
301+
/// separated by whitespace.
302+
/// It does not consider whether a newline should be added between
303+
/// `previousToken` and the `token` because that newline should be added to
304+
/// the next token's trailing trivia.
284305
lazy var previousTokenWillEndWithWhitespace: Bool = {
285306
guard let previousToken = previousToken else {
286307
return false
@@ -289,6 +310,8 @@ open class BasicFormat: SyntaxRewriter {
289310
|| (requiresWhitespace(between: previousToken, and: token) && isMutable(previousToken))
290311
}()
291312

313+
/// This method does not consider any posssible mutations to `previousToken`
314+
/// because newlines should be added to the next token's leading trivia.
292315
lazy var previousTokenWillEndWithNewline: Bool = {
293316
guard let previousToken = previousToken else {
294317
// Assume that the start of the tree is equivalent to a newline so we
@@ -298,10 +321,7 @@ open class BasicFormat: SyntaxRewriter {
298321
if previousToken.trailingTrivia.endsWithNewline {
299322
return true
300323
}
301-
if case .stringSegment(let segment) = previousToken.tokenKind, segment.last?.isNewline ?? false {
302-
return true
303-
}
304-
return false
324+
return previousToken.isStringSegmentWithLastCharacterBeingNewline
305325
}()
306326

307327
lazy var previousTokenIsStringLiteralEndingInNewline: Bool = {
@@ -310,26 +330,29 @@ open class BasicFormat: SyntaxRewriter {
310330
// don't add a leading newline to the file.
311331
return true
312332
}
313-
if case .stringSegment(let segment) = previousToken.tokenKind, segment.last?.isNewline ?? false {
314-
return true
315-
}
316-
return false
333+
return previousToken.isStringSegmentWithLastCharacterBeingNewline
317334
}()
318335

336+
/// Also considers `nextToken` as starting with a whitespace if a newline
337+
/// should be added to it. It does not check whether `token` and `nextToken`
338+
/// should be separated by whitespace because the whitespace should be added
339+
/// to the `token`’s leading trivia.
319340
lazy var nextTokenWillStartWithWhitespace: Bool = {
320341
guard let nextToken = nextToken else {
321342
return false
322343
}
323344
return nextToken.leadingTrivia.startsWithWhitespace
324-
|| (requiresLeadingNewline(nextToken) && isMutable(nextToken))
345+
|| (requiresNewline(between: token, and: nextToken) && isMutable(nextToken))
325346
}()
326347

348+
/// Also considers `nextToken` as starting with a leading newline if `token`
349+
/// and `nextToken` should be separated by a newline.
327350
lazy var nextTokenWillStartWithNewline: Bool = {
328351
guard let nextToken = nextToken else {
329352
return false
330353
}
331354
return nextToken.leadingTrivia.startsWithNewline
332-
|| (requiresLeadingNewline(nextToken) && isMutable(nextToken))
355+
|| (requiresNewline(between: token, and: nextToken) && isMutable(nextToken) && !token.trailingTrivia.endsWithNewline && !token.isStringSegmentWithLastCharacterBeingNewline)
333356
}()
334357

335358
/// This token's trailing trivia + any spaces or tabs at the start of the
@@ -342,7 +365,7 @@ open class BasicFormat: SyntaxRewriter {
342365
var leadingTrivia = token.leadingTrivia
343366
var trailingTrivia = token.trailingTrivia
344367

345-
if requiresLeadingNewline(token) {
368+
if requiresNewline(between: previousToken, and: token) {
346369
// Add a leading newline if the token requires it unless
347370
// - it already starts with a newline or
348371
// - the previous token ends with a newline
@@ -407,3 +430,14 @@ open class BasicFormat: SyntaxRewriter {
407430
return token.detach().with(\.leadingTrivia, leadingTrivia).with(\.trailingTrivia, trailingTrivia)
408431
}
409432
}
433+
434+
fileprivate extension TokenSyntax {
435+
var isStringSegmentWithLastCharacterBeingNewline: Bool {
436+
switch self.tokenKind {
437+
case .stringSegment(let segment):
438+
return segment.last?.isNewline ?? false
439+
default:
440+
return false
441+
}
442+
}
443+
}

‎Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,11 +152,12 @@ extension FixIt.MultiNodeChange {
152152
}
153153
}
154154

155-
if let previousToken = node.previousToken(viewMode: .all),
155+
if let previousToken = node.previousToken(viewMode: .fixedUp),
156156
previousToken.presence == .present,
157157
let firstToken = node.firstToken(viewMode: .all),
158158
previousToken.trailingTrivia.allSatisfy({ $0.isWhitespace }),
159-
!BasicFormat().requiresWhitespace(between: previousToken, and: firstToken)
159+
!BasicFormat().requiresWhitespace(between: previousToken, and: firstToken),
160+
!BasicFormat().requiresNewline(between: previousToken, and: firstToken)
160161
{
161162
// If the previous token and this node don't need to be separated, remove
162163
// the separation.

‎Sources/SwiftParserDiagnostics/MissingNodesError.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ fileprivate func findCommonAncestor(_ nodes: [Syntax]) -> Syntax? {
2222
}
2323

2424
class NoNewlinesFormat: BasicFormat {
25-
override func requiresLeadingNewline(_ token: TokenSyntax) -> Bool {
25+
override func requiresNewline(between first: TokenSyntax?, and second: TokenSyntax?) -> Bool {
2626
return false
2727
}
2828
}

‎Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1252,12 +1252,10 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
12521252
}
12531253
if case .stringSegment(let segment) = node.segments.last {
12541254
if let invalidContent = segment.unexpectedBeforeContent?.onlyToken(where: { $0.trailingTrivia.contains(where: { $0.isBackslash }) }) {
1255-
let leadingTrivia = segment.content.leadingTrivia
1256-
let trailingTrivia = segment.content.trailingTrivia
12571255
let fixIt = FixIt(
12581256
message: .removeBackslash,
12591257
changes: [
1260-
.makePresent(segment.content, leadingTrivia: leadingTrivia, trailingTrivia: trailingTrivia),
1258+
.makePresent(segment.content),
12611259
.makeMissing(invalidContent, transferTrivia: false),
12621260
]
12631261
)

‎Tests/SwiftParserTest/ExpressionTests.swift‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,8 @@ final class ExpressionTests: XCTestCase {
603603
)
604604
],
605605
fixedSource: ##"""
606-
"""""""
606+
""""
607+
"""
607608
"""##
608609
)
609610

@@ -619,7 +620,8 @@ final class ExpressionTests: XCTestCase {
619620
)
620621
],
621622
fixedSource: ##"""
622-
""""""""
623+
"""""
624+
"""
623625
"""##
624626
)
625627

@@ -656,7 +658,8 @@ final class ExpressionTests: XCTestCase {
656658
DiagnosticSpec(message: ##"expected '"""#' to end string literal"##, fixIts: [##"insert '"""#'"##])
657659
],
658660
fixedSource: ##"""
659-
#""""""#
661+
#"""
662+
"""#
660663
"""##
661664
)
662665

@@ -668,7 +671,8 @@ final class ExpressionTests: XCTestCase {
668671
DiagnosticSpec(message: ##"expected '"""#' to end string literal"##, fixIts: [##"insert '"""#'"##])
669672
],
670673
fixedSource: ##"""
671-
#"""a"""#
674+
#"""a
675+
"""#
672676
"""##
673677
)
674678

‎Tests/SwiftParserTest/translated/AvailabilityQueryTests.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ final class AvailabilityQueryTests: XCTestCase {
100100
DiagnosticSpec(message: "expected ',' joining parts of a multi-clause condition", fixIts: ["replace '&&' with ','"])
101101
],
102102
fixedSource: """
103-
if #available(OSX 10.51, *) , #available(OSX 10.52, *) {
103+
if #available(OSX 10.51, *), #available(OSX 10.52, *) {
104104
}
105105
"""
106106
)
@@ -440,7 +440,7 @@ final class AvailabilityQueryTests: XCTestCase {
440440
DiagnosticSpec(message: "expected ',' joining platforms in availability condition", fixIts: ["replace '||' with ','"])
441441
],
442442
fixedSource: """
443-
if #available(OSX 10.51 , iOS 8.0) {
443+
if #available(OSX 10.51, iOS 8.0) {
444444
}
445445
"""
446446
)

‎Tests/SwiftParserTest/translated/AvailabilityQueryUnavailabilityTests.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ final class AvailabilityQueryUnavailabilityTests: XCTestCase {
103103
)
104104
],
105105
fixedSource: """
106-
if #unavailable(OSX 10.51) , #unavailable(OSX 10.52) {
106+
if #unavailable(OSX 10.51), #unavailable(OSX 10.52) {
107107
}
108108
"""
109109
)
@@ -447,7 +447,7 @@ final class AvailabilityQueryUnavailabilityTests: XCTestCase {
447447
)
448448
],
449449
fixedSource: """
450-
if #unavailable(OSX 10.51 , iOS 8.0) {
450+
if #unavailable(OSX 10.51, iOS 8.0) {
451451
}
452452
"""
453453
)

‎Tests/SwiftParserTest/translated/MultilineErrorsTests.swift‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,8 @@ final class MultilineErrorsTests: XCTestCase {
705705
],
706706
fixedSource: ##"""
707707
_ = """
708-
foo\"""
708+
foo\
709+
"""
709710
"""##
710711
)
711712
}
@@ -741,6 +742,7 @@ final class MultilineErrorsTests: XCTestCase {
741742
fixedSource: #"""
742743
_ = """
743744
\#u{20}
745+
744746
"""
745747
"""#
746748
)

‎Tests/SwiftParserTest/translated/MultilinePoundDiagnosticArgRdar41154797Tests.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ final class MultilinePoundDiagnosticArgRdar41154797Tests: XCTestCase {
3535
),
3636
],
3737
fixedSource: ##"""
38-
#error("""""")
38+
#error("""
39+
""")
3940
"""##
4041
)
4142
}

‎Tests/SwiftParserTest/translated/MultilineStringTests.swift‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,82 @@ final class MultilineStringTests: XCTestCase {
411411
)
412412
}
413413

414+
func testMultilineString47() {
415+
assertParse(
416+
#"""
417+
_ = """1️⃣"""
418+
"""#,
419+
diagnostics: [
420+
DiagnosticSpec(
421+
message: "multi-line string literal closing delimiter must begin on a new line",
422+
fixIts: ["insert newline"]
423+
)
424+
],
425+
fixedSource: #"""
426+
_ = """
427+
"""
428+
"""#
429+
)
430+
}
431+
432+
func testMultilineString48() {
433+
assertParse(
434+
#"""
435+
_ = """A1️⃣"""
436+
"""#,
437+
diagnostics: [
438+
DiagnosticSpec(
439+
message: "multi-line string literal closing delimiter must begin on a new line",
440+
fixIts: ["insert newline"]
441+
)
442+
],
443+
fixedSource: #"""
444+
_ = """A
445+
"""
446+
"""#
447+
)
448+
}
449+
450+
func testMultilineString49() {
451+
assertParse(
452+
#"""
453+
_ = ℹ️"""1️⃣
454+
"""#,
455+
diagnostics: [
456+
DiagnosticSpec(
457+
message: #"expected '"""' to end string literal"#,
458+
notes: [NoteSpec(message: #"to match this opening '"""'"#)],
459+
fixIts: [#"insert '"""'"#]
460+
)
461+
],
462+
fixedSource: #"""
463+
_ = """
464+
"""
465+
"""#
466+
)
467+
}
468+
469+
func testMultilineString50() {
470+
assertParse(
471+
#"""
472+
_ = ℹ️"""
473+
A1️⃣
474+
"""#,
475+
diagnostics: [
476+
DiagnosticSpec(
477+
message: #"expected '"""' to end string literal"#,
478+
notes: [NoteSpec(message: #"to match this opening '"""'"#)],
479+
fixIts: [#"insert '"""'"#]
480+
)
481+
],
482+
fixedSource: #"""
483+
_ = """
484+
A
485+
"""
486+
"""#
487+
)
488+
}
489+
414490
func testEscapeNewlineInRawString() {
415491
assertParse(
416492
##"""

0 commit comments

Comments
 (0)