Skip to content

Commit 7edd85f

Browse files
committed
[SwiftRefactor] Fix handling of leading trivia for new elements/arguments
When inserting a new element to an array or new argument to a call let's only copy the indentation of the previous element instead of whole leading trivia that includes comments.
1 parent a317415 commit 7edd85f

File tree

2 files changed

+106
-6
lines changed

2 files changed

+106
-6
lines changed

Sources/SwiftRefactor/PackageManifest/SyntaxEditUtils.swift

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,12 @@ extension LabeledExprListSyntax {
135135
if position > startIndex {
136136
let priorArgument = arguments[positionIdx - 1]
137137

138-
// Our leading trivia will be based on the prior argument's leading
139-
// trivia.
140-
leadingTrivia = priorArgument.leadingTrivia
138+
// Our indentation will be based on the prior argument's.
139+
if priorArgument.leadingTrivia.hasNewlines {
140+
leadingTrivia = .newline + (priorArgument.firstToken(viewMode: .sourceAccurate)?.indentationOfLine ?? Trivia())
141+
} else {
142+
leadingTrivia = priorArgument.leadingTrivia.indentation(isOnNewline: false) ?? Trivia()
143+
}
141144

142145
// If the prior argument is missing a trailing comma, add one.
143146
if priorArgument.trailingComma == nil {
@@ -221,9 +224,13 @@ extension ArrayExprSyntax {
221224
let trailingTrivia: Trivia
222225
let leftSquareTrailingTrivia: Trivia
223226
if let last = elements.last {
224-
// The leading trivia of the new element should match that of the
227+
// The leading indentation of the new element should match that of the
225228
// last element.
226-
leadingTrivia = last.leadingTrivia.onlyLastLine
229+
if last.leadingTrivia.hasNewlines {
230+
leadingTrivia = .newline + (last.firstToken(viewMode: .sourceAccurate)?.indentationOfLine ?? Trivia())
231+
} else {
232+
leadingTrivia = last.leadingTrivia.indentation(isOnNewline: false) ?? Trivia()
233+
}
227234

228235
// Add a trailing comma to the last element if it isn't already
229236
// there.

Tests/SwiftRefactorTest/ManifestEditTests.swift

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,72 @@ final class ManifestEditTests: XCTestCase {
9797
)
9898
)
9999
)
100+
101+
try assertManifestRefactor(
102+
"""
103+
// swift-tools-version: 5.5
104+
let package = Package(
105+
name: "packages",
106+
dependencies: [
107+
/* test */ .package(url: "https://github.com/swiftlang/swift-syntax.git", from: "510.0.1")
108+
]
109+
)
110+
""",
111+
expectedManifest: """
112+
// swift-tools-version: 5.5
113+
let package = Package(
114+
name: "packages",
115+
dependencies: [
116+
/* test */ .package(url: "https://github.com/swiftlang/swift-syntax.git", from: "510.0.1"),
117+
.package(url: "https://github.com/apple/swift-system.git", exact: "510.0.0"),
118+
]
119+
)
120+
""",
121+
provider: AddPackageDependency.self,
122+
context: .init(
123+
dependency: .sourceControl(
124+
.init(
125+
identity: PackageIdentity("swift-system"),
126+
location: .remote(Self.swiftSystemURL),
127+
requirement: .exact(SemanticVersion("510.0.0"))
128+
)
129+
)
130+
)
131+
)
132+
133+
try assertManifestRefactor(
134+
"""
135+
// swift-tools-version: 5.5
136+
let package = Package(
137+
name: "packages",
138+
dependencies: [
139+
/* test */
140+
.package(url: "https://github.com/swiftlang/swift-syntax.git", from: "510.0.1")
141+
]
142+
)
143+
""",
144+
expectedManifest: """
145+
// swift-tools-version: 5.5
146+
let package = Package(
147+
name: "packages",
148+
dependencies: [
149+
/* test */
150+
.package(url: "https://github.com/swiftlang/swift-syntax.git", from: "510.0.1"),
151+
.package(url: "https://github.com/apple/swift-system.git", exact: "510.0.0"),
152+
]
153+
)
154+
""",
155+
provider: AddPackageDependency.self,
156+
context: .init(
157+
dependency: .sourceControl(
158+
.init(
159+
identity: PackageIdentity("swift-system"),
160+
location: .remote(Self.swiftSystemURL),
161+
requirement: .exact(SemanticVersion("510.0.0"))
162+
)
163+
)
164+
)
165+
)
100166
}
101167

102168
func testAddPackageDependencyExistingAppended() throws {
@@ -160,6 +226,33 @@ final class ManifestEditTests: XCTestCase {
160226
)
161227
)
162228
)
229+
230+
try assertManifestRefactor(
231+
"""
232+
// swift-tools-version: 5.5
233+
let package = Package(
234+
name: "packages",
235+
dependencies: [ /*test*/ .package(url: "https://github.com/swiftlang/swift-syntax.git", from: "510.0.1") ]
236+
)
237+
""",
238+
expectedManifest: """
239+
// swift-tools-version: 5.5
240+
let package = Package(
241+
name: "packages",
242+
dependencies: [ /*test*/ .package(url: "https://github.com/swiftlang/swift-syntax.git", from: "510.0.1"), .package(url: "https://github.com/apple/swift-system.git", from: "510.0.0"),]
243+
)
244+
""",
245+
provider: AddPackageDependency.self,
246+
context: .init(
247+
dependency: .sourceControl(
248+
.init(
249+
identity: PackageIdentity("swift-system"),
250+
location: .remote(Self.swiftSystemURL),
251+
requirement: .rangeFrom(SemanticVersion("510.0.0"))
252+
)
253+
)
254+
)
255+
)
163256
}
164257

165258
func testAddPackageDependencyExistingEmpty() throws {
@@ -450,7 +543,7 @@ final class ManifestEditTests: XCTestCase {
450543
.target(name: "TargetLib"),
451544
.byName(name: "MyLib"),
452545
]
453-
),
546+
)
454547
)
455548
)
456549
}

0 commit comments

Comments
 (0)