From 4d2212c69e2bd6fb56183bec0c7004d1c00b590e Mon Sep 17 00:00:00 2001 From: Ben Barham Date: Thu, 30 Mar 2023 21:14:22 -0700 Subject: [PATCH] Make leading/trailing trivia required fields Rather than having the possibility of `nil` trivia and requiring all clients to check every use, just return empty trivia when there are no nodes present. --- .../DiagnosticExtensions.swift | 2 +- .../ParseDiagnosticsGenerator.swift | 4 +-- Sources/SwiftSyntax/Raw/RawSyntax.swift | 8 +++--- Sources/SwiftSyntax/Syntax.swift | 14 +++++----- Sources/SwiftSyntax/generated/Trivia.swift | 5 ---- .../SyntaxProtocol+Initializer.swift | 4 +-- .../StringInterpolationTests.swift | 4 +-- .../MacroSystemTests.swift | 27 ++++--------------- .../AbsolutePositionTests.swift | 10 +++---- .../SyntaxVisitorTests.swift | 2 +- 10 files changed, 29 insertions(+), 51 deletions(-) diff --git a/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift b/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift index aadbf700765..9d3d0b58f3e 100644 --- a/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift +++ b/Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift @@ -96,7 +96,7 @@ extension FixIt.Changes { ), .replaceLeadingTrivia(token: nextToken, newTrivia: []), ] - } else if node.leadingTrivia?.isEmpty ?? true, + } else if node.leadingTrivia.isEmpty, let previousToken = node.previousToken(viewMode: .fixedUp), previousToken.presence == .present, previousToken.trailingTrivia.isEmpty, diff --git a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift index 599164b8543..b315dab41c5 100644 --- a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift +++ b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift @@ -821,7 +821,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { message: RemoveNodesFixIt(unexpectedName), changes: [ .makeMissing(unexpectedName), - FixIt.Changes(changes: [.replaceTrailingTrivia(token: previous, newTrivia: .zero)]), + FixIt.Changes(changes: [.replaceTrailingTrivia(token: previous, newTrivia: [])]), ] ) ], @@ -1008,7 +1008,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { message: ReplaceTokensFixIt(replaceTokens: [singleQuote], replacement: node.openQuote), changes: [ .makeMissing(singleQuote, transferTrivia: false), - .makePresent(node.openQuote, leadingTrivia: singleQuote.leadingTrivia ?? []), + .makePresent(node.openQuote, leadingTrivia: singleQuote.leadingTrivia), .makeMissing(node.unexpectedBetweenSegmentsAndCloseQuote, transferTrivia: false), .makePresent(node.closeQuote, trailingTrivia: node.unexpectedBetweenSegmentsAndCloseQuote?.trailingTrivia ?? []), ] diff --git a/Sources/SwiftSyntax/Raw/RawSyntax.swift b/Sources/SwiftSyntax/Raw/RawSyntax.swift index 736a61cd722..e757df91896 100644 --- a/Sources/SwiftSyntax/Raw/RawSyntax.swift +++ b/Sources/SwiftSyntax/Raw/RawSyntax.swift @@ -474,12 +474,12 @@ extension RawSyntax { } } - func formLeadingTrivia() -> Trivia? { - firstToken(viewMode: .sourceAccurate)?.formLeadingTrivia() + func formLeadingTrivia() -> Trivia { + firstToken(viewMode: .sourceAccurate)?.formLeadingTrivia() ?? [] } - func formTrailingTrivia() -> Trivia? { - lastToken(viewMode: .sourceAccurate)?.formTrailingTrivia() + func formTrailingTrivia() -> Trivia { + lastToken(viewMode: .sourceAccurate)?.formTrailingTrivia() ?? [] } } diff --git a/Sources/SwiftSyntax/Syntax.swift b/Sources/SwiftSyntax/Syntax.swift index ef1c2789f51..c648c486228 100644 --- a/Sources/SwiftSyntax/Syntax.swift +++ b/Sources/SwiftSyntax/Syntax.swift @@ -471,24 +471,24 @@ public extension SyntaxProtocol { /// The leading trivia of this syntax node. Leading trivia is attached to /// the first token syntax contained by this node. Without such token, this /// property will return nil. - var leadingTrivia: Trivia? { + var leadingTrivia: Trivia { get { return raw.formLeadingTrivia() } set { - self = Self(Syntax(data.withLeadingTrivia(newValue ?? [], arena: SyntaxArena())))! + self = Self(Syntax(data.withLeadingTrivia(newValue, arena: SyntaxArena())))! } } /// The trailing trivia of this syntax node. Trailing trivia is attached to /// the last token syntax contained by this node. Without such token, this /// property will return nil. - var trailingTrivia: Trivia? { + var trailingTrivia: Trivia { get { return raw.formTrailingTrivia() } set { - self = Self(Syntax(data.withTrailingTrivia(newValue ?? [], arena: SyntaxArena())))! + self = Self(Syntax(data.withTrailingTrivia(newValue, arena: SyntaxArena())))! } } @@ -510,7 +510,7 @@ public extension SyntaxProtocol { /// When isImplicit is true, the syntax node doesn't include any /// underlying tokens, e.g. an empty CodeBlockItemList. var isImplicit: Bool { - return leadingTrivia == nil + return raw.isEmpty } /// The textual byte length of this node exluding leading and trailing trivia. @@ -636,10 +636,10 @@ public extension SyntaxProtocol { if let token = Syntax(self).as(TokenSyntax.self) { target.write(String(describing: token.tokenKind)) if includeTrivia { - if let leadingTrivia = leadingTrivia, !leadingTrivia.isEmpty { + if !leadingTrivia.isEmpty { target.write(" leadingTrivia=\(leadingTrivia.debugDescription)") } - if let trailingTrivia = trailingTrivia, !trailingTrivia.isEmpty { + if !trailingTrivia.isEmpty { target.write(" trailingTrivia=\(trailingTrivia.debugDescription)") } } diff --git a/Sources/SwiftSyntax/generated/Trivia.swift b/Sources/SwiftSyntax/generated/Trivia.swift index f02b17ca6b0..304e4496141 100644 --- a/Sources/SwiftSyntax/generated/Trivia.swift +++ b/Sources/SwiftSyntax/generated/Trivia.swift @@ -165,11 +165,6 @@ public struct Trivia { self.pieces = Array(pieces) } - /// Creates Trivia with no pieces. - public static var zero: Trivia { - return Trivia(pieces: []) - } - /// Whether the Trivia contains no pieces. public var isEmpty: Bool { pieces.isEmpty diff --git a/Sources/_SwiftSyntaxTestSupport/SyntaxProtocol+Initializer.swift b/Sources/_SwiftSyntaxTestSupport/SyntaxProtocol+Initializer.swift index 65a6f4d1a21..a0b88d8d0ae 100644 --- a/Sources/_SwiftSyntaxTestSupport/SyntaxProtocol+Initializer.swift +++ b/Sources/_SwiftSyntaxTestSupport/SyntaxProtocol+Initializer.swift @@ -23,10 +23,10 @@ private class InitializerExprFormat: BasicFormat { self.visit($0).as(SyntaxType.self)! } formattedChildren = formattedChildren.map { - if $0.leadingTrivia?.first?.isNewline == true { + if $0.leadingTrivia.first?.isNewline == true { return $0 } else { - return $0.with(\.leadingTrivia, indentedNewline + ($0.leadingTrivia ?? [])) + return $0.with(\.leadingTrivia, indentedNewline + $0.leadingTrivia) } } indentationLevel -= 1 diff --git a/Tests/SwiftSyntaxBuilderTest/StringInterpolationTests.swift b/Tests/SwiftSyntaxBuilderTest/StringInterpolationTests.swift index bc0106bd26e..35e7af26829 100644 --- a/Tests/SwiftSyntaxBuilderTest/StringInterpolationTests.swift +++ b/Tests/SwiftSyntaxBuilderTest/StringInterpolationTests.swift @@ -230,8 +230,8 @@ final class StringInterpolationTests: XCTestCase { class Rewriter: SyntaxRewriter { override func visit(_ node: FunctionDeclSyntax) -> DeclSyntax { let newFunc = DeclSyntax("func newName() {}") - .with(\.leadingTrivia, node.leadingTrivia!) - .with(\.trailingTrivia, node.trailingTrivia!) + .with(\.leadingTrivia, node.leadingTrivia) + .with(\.trailingTrivia, node.trailingTrivia) return DeclSyntax(newFunc) } } diff --git a/Tests/SwiftSyntaxMacrosTest/MacroSystemTests.swift b/Tests/SwiftSyntaxMacrosTest/MacroSystemTests.swift index fea0bea81a1..be58c905e93 100644 --- a/Tests/SwiftSyntaxMacrosTest/MacroSystemTests.swift +++ b/Tests/SwiftSyntaxMacrosTest/MacroSystemTests.swift @@ -75,10 +75,7 @@ public struct ColorLiteralMacro: ExpressionMacro { with: "_colorLiteralRed" ) let initSyntax: ExprSyntax = ".init(\(argList))" - if let leadingTrivia = macro.leadingTrivia { - return initSyntax.with(\.leadingTrivia, leadingTrivia) - } - return initSyntax + return initSyntax.with(\.leadingTrivia, macro.leadingTrivia) } } @@ -95,10 +92,7 @@ public struct FileLiteralMacro: ExpressionMacro { with: "fileReferenceLiteralResourceName" ) let initSyntax: ExprSyntax = ".init(\(argList))" - if let leadingTrivia = macro.leadingTrivia { - return initSyntax.with(\.leadingTrivia, leadingTrivia) - } - return initSyntax + return initSyntax.with(\.leadingTrivia, macro.leadingTrivia) } } @@ -115,10 +109,7 @@ public struct ImageLiteralMacro: ExpressionMacro { with: "imageLiteralResourceName" ) let initSyntax: ExprSyntax = ".init(\(argList))" - if let leadingTrivia = macro.leadingTrivia { - return initSyntax.with(\.leadingTrivia, leadingTrivia) - } - return initSyntax + return initSyntax.with(\.leadingTrivia, macro.leadingTrivia) } } @@ -134,11 +125,7 @@ public struct ColumnMacro: ExpressionMacro { else { throw CustomError.message("can't find location for macro") } - - if let leadingTrivia = macro.leadingTrivia { - return sourceLoc.column.with(\.leadingTrivia, leadingTrivia) - } - return sourceLoc.column + return sourceLoc.column.with(\.leadingTrivia, macro.leadingTrivia) } } @@ -154,11 +141,7 @@ public struct FileIDMacro: ExpressionMacro { else { throw CustomError.message("can't find location for macro") } - - if let leadingTrivia = macro.leadingTrivia { - return sourceLoc.file.with(\.leadingTrivia, leadingTrivia) - } - return sourceLoc.file + return sourceLoc.file.with(\.leadingTrivia, macro.leadingTrivia) } } diff --git a/Tests/SwiftSyntaxParserTest/AbsolutePositionTests.swift b/Tests/SwiftSyntaxParserTest/AbsolutePositionTests.swift index d73d06f6809..f6b8ab181ef 100644 --- a/Tests/SwiftSyntaxParserTest/AbsolutePositionTests.swift +++ b/Tests/SwiftSyntaxParserTest/AbsolutePositionTests.swift @@ -149,14 +149,14 @@ public class AbsolutePositionTests: XCTestCase { public func testTrivias() { let idx = 5 let root = self.createSourceFile(idx + 1) - XCTAssertEqual(3, root.leadingTrivia!.count) - XCTAssertEqual(0, root.trailingTrivia!.count) + XCTAssertEqual(3, root.leadingTrivia.count) + XCTAssertEqual(0, root.trailingTrivia.count) let state = root.statements[idx] - XCTAssertEqual(3, state.leadingTrivia!.count) - XCTAssertEqual(2, state.trailingTrivia!.count) + XCTAssertEqual(3, state.leadingTrivia.count) + XCTAssertEqual(2, state.trailingTrivia.count) XCTAssertEqual( state.byteSize, - state.leadingTrivia!.byteSize + state.trailingTrivia!.byteSize + state.leadingTrivia.byteSize + state.trailingTrivia.byteSize + state.byteSizeAfterTrimmingTrivia ) XCTAssertFalse(root.statements.isImplicit) diff --git a/Tests/SwiftSyntaxParserTest/SyntaxVisitorTests.swift b/Tests/SwiftSyntaxParserTest/SyntaxVisitorTests.swift index d047cb954cc..31e202f7d11 100644 --- a/Tests/SwiftSyntaxParserTest/SyntaxVisitorTests.swift +++ b/Tests/SwiftSyntaxParserTest/SyntaxVisitorTests.swift @@ -122,7 +122,7 @@ public class SyntaxVisitorTests: XCTestCase { public func testRewriteTrivia() { class TriviaRemover: SyntaxRewriter { override func visit(_ token: TokenSyntax) -> TokenSyntax { - return token.with(\.trailingTrivia, .zero) + return token.with(\.trailingTrivia, []) } }