Skip to content

Commit 8e9c19a

Browse files
ahoppenahoppen
committed
Don’t fatalError when constructing DocumentURI from an invalid URL
1 parent 3e51f70 commit 8e9c19a

File tree

10 files changed

+87
-62
lines changed

10 files changed

+87
-62
lines changed

‎Sources/LanguageServerProtocol/Requests/IndexedRenameRequest.swift

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ extension IndexedRenameRequest: Codable {
7272
)
7373
self.oldName = try container.decode(String.self, forKey: IndexedRenameRequest.CodingKeys.oldName)
7474
self.newName = try container.decode(String.self, forKey: IndexedRenameRequest.CodingKeys.newName)
75-
self.positions = try container.decode([String: [Position]].self, forKey: .positions).mapKeys(DocumentURI.init)
75+
self.positions = try container.decode([String: [Position]].self, forKey: .positions).compactMapKeys {
76+
try? DocumentURI(string: $0)
77+
}
7678
}
7779

7880
public func encode(to encoder: Encoder) throws {
@@ -81,13 +83,22 @@ extension IndexedRenameRequest: Codable {
8183
try container.encode(self.textDocument, forKey: IndexedRenameRequest.CodingKeys.textDocument)
8284
try container.encode(self.oldName, forKey: IndexedRenameRequest.CodingKeys.oldName)
8385
try container.encode(self.newName, forKey: IndexedRenameRequest.CodingKeys.newName)
84-
try container.encode(self.positions.mapKeys(\.stringValue), forKey: IndexedRenameRequest.CodingKeys.positions)
86+
try container.encode(
87+
self.positions.compactMapKeys { $0.stringValue },
88+
forKey: IndexedRenameRequest.CodingKeys.positions
89+
)
8590

8691
}
8792
}
8893

8994
fileprivate extension Dictionary {
90-
func mapKeys<NewKeyType: Hashable>(_ transform: (Key) -> NewKeyType) -> [NewKeyType: Value] {
91-
return [NewKeyType: Value](uniqueKeysWithValues: self.map { (transform($0.key), $0.value) })
95+
func compactMapKeys<NewKeyType: Hashable>(_ transform: (Key) -> NewKeyType?) -> [NewKeyType: Value] {
96+
let newKeysAndValues = self.compactMap { (key, value) -> (NewKeyType, Value)? in
97+
guard let newKey = transform(key) else {
98+
return nil
99+
}
100+
return (newKey, value)
101+
}
102+
return [NewKeyType: Value](uniqueKeysWithValues: newKeysAndValues)
92103
}
93104
}

‎Sources/LanguageServerProtocol/SupportTypes/DocumentURI.swift

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,14 @@
1212

1313
import Foundation
1414

15+
struct FailedToConstructDocumentURIFromStringError: Error, CustomStringConvertible {
16+
let string: String
17+
18+
var description: String {
19+
return "Failed to construct DocumentURI from '\(string)'"
20+
}
21+
}
22+
1523
public struct DocumentURI: Codable, Hashable, Sendable {
1624
/// The URL that store the URIs value
1725
private let storage: URL
@@ -55,9 +63,9 @@ public struct DocumentURI: Codable, Hashable, Sendable {
5563

5664
/// Construct a DocumentURI from the given URI string, automatically parsing
5765
/// it either as a URL or an opaque URI.
58-
public init(string: String) {
66+
public init(string: String) throws {
5967
guard let url = URL(string: string) else {
60-
fatalError("Failed to construct DocumentURI from '\(string)'")
68+
throw FailedToConstructDocumentURIFromStringError(string: string)
6169
}
6270
self.init(url)
6371
}
@@ -68,7 +76,7 @@ public struct DocumentURI: Codable, Hashable, Sendable {
6876
}
6977

7078
public init(from decoder: Decoder) throws {
71-
self.init(string: try decoder.singleValueContainer().decode(String.self))
79+
try self.init(string: decoder.singleValueContainer().decode(String.self))
7280
}
7381

7482
/// Equality check to handle escape sequences in file URLs.

‎Sources/LanguageServerProtocol/SupportTypes/TextDocumentIdentifier.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ extension TextDocumentIdentifier: LSPAnyCodable {
2626
guard case .string(let uriString)? = dictionary[CodingKeys.uri.stringValue] else {
2727
return nil
2828
}
29-
self.uri = DocumentURI(string: uriString)
29+
guard let uri = try? DocumentURI(string: uriString) else {
30+
return nil
31+
}
32+
self.uri = uri
3033
}
3134

3235
public func encodeToLSPAny() -> LSPAny {

‎Sources/LanguageServerProtocol/SupportTypes/WorkspaceEdit.swift

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ extension WorkspaceEdit: Codable {
4949
if let changesDict = try container.decodeIfPresent([String: [TextEdit]].self, forKey: .changes) {
5050
var changes = [DocumentURI: [TextEdit]]()
5151
for change in changesDict {
52-
let uri = DocumentURI(string: change.key)
52+
let uri = try DocumentURI(string: change.key)
5353
changes[uri] = change.value
5454
}
5555
self.changes = changes
@@ -311,8 +311,10 @@ extension WorkspaceEdit: LSPAnyCodable {
311311
}
312312
var dictionary = [DocumentURI: [TextEdit]]()
313313
for (key, value) in lspDict {
314-
let uri = DocumentURI(string: key)
315-
guard let edits = [TextEdit](fromLSPArray: value) else {
314+
guard
315+
let uri = try? DocumentURI(string: key),
316+
let edits = [TextEdit](fromLSPArray: value)
317+
else {
316318
return nil
317319
}
318320
dictionary[uri] = edits

‎Sources/SourceKitLSP/SourceKitLSPServer.swift

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2104,14 +2104,12 @@ extension SourceKitLSPServer {
21042104
private nonisolated func extractCallHierarchyItemData(_ rawData: LSPAny?) -> (uri: DocumentURI, usr: String)? {
21052105
guard case let .dictionary(data) = rawData,
21062106
case let .string(uriString) = data["uri"],
2107-
case let .string(usr) = data["usr"]
2107+
case let .string(usr) = data["usr"],
2108+
let uri = orLog("DocumentURI for call hierarchy item", { try DocumentURI(string: uriString) })
21082109
else {
21092110
return nil
21102111
}
2111-
return (
2112-
uri: DocumentURI(string: uriString),
2113-
usr: usr
2114-
)
2112+
return (uri: uri, usr: usr)
21152113
}
21162114

21172115
func incomingCalls(_ req: CallHierarchyIncomingCallsRequest) async throws -> [CallHierarchyIncomingCall]? {
@@ -2289,14 +2287,12 @@ extension SourceKitLSPServer {
22892287
private nonisolated func extractTypeHierarchyItemData(_ rawData: LSPAny?) -> (uri: DocumentURI, usr: String)? {
22902288
guard case let .dictionary(data) = rawData,
22912289
case let .string(uriString) = data["uri"],
2292-
case let .string(usr) = data["usr"]
2290+
case let .string(usr) = data["usr"],
2291+
let uri = orLog("DocumentURI for type hierarchy item", { try DocumentURI(string: uriString) })
22932292
else {
22942293
return nil
22952294
}
2296-
return (
2297-
uri: DocumentURI(string: uriString),
2298-
usr: usr
2299-
)
2295+
return (uri: uri, usr: usr)
23002296
}
23012297

23022298
func supertypes(_ req: TypeHierarchySupertypesRequest) async throws -> [TypeHierarchyItem]? {

‎Tests/LanguageServerProtocolTests/CodingTests.swift

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import XCTest
1616

1717
final class CodingTests: XCTestCase {
1818

19-
func testValueCoding() {
19+
func testValueCoding() throws {
2020
let url = URL(fileURLWithPath: "/foo.swift")
2121
let uri = DocumentURI(url)
2222
// The \\/\\/\\/ is escaping file:// + /foo.swift, which is silly but allowed by json.
@@ -252,7 +252,7 @@ final class CodingTests: XCTestCase {
252252
checkCoding(DiagnosticCode.string("hi"), json: "\"hi\"")
253253

254254
checkCoding(
255-
CodeDescription(href: DocumentURI(string: "file:///some/path")),
255+
CodeDescription(href: try DocumentURI(string: "file:///some/path")),
256256
json: """
257257
{
258258
"href" : "file:\\/\\/\\/some\\/path"
@@ -816,7 +816,7 @@ final class CodingTests: XCTestCase {
816816
checkCoding(ProgressToken.string("foobar"), json: #""foobar""#)
817817
}
818818

819-
func testDocumentDiagnosticReport() {
819+
func testDocumentDiagnosticReport() throws {
820820
checkCoding(
821821
DocumentDiagnosticReport.full(RelatedFullDocumentDiagnosticReport(items: [])),
822822
json: """
@@ -848,7 +848,7 @@ final class CodingTests: XCTestCase {
848848
resultId: "myResults",
849849
items: [],
850850
relatedDocuments: [
851-
DocumentURI(string: "file:///some/path"): DocumentDiagnosticReport.unchanged(
851+
try DocumentURI(string: "file:///some/path"): DocumentDiagnosticReport.unchanged(
852852
RelatedUnchangedDocumentDiagnosticReport(resultId: "myOtherResults")
853853
)
854854
]
@@ -887,7 +887,7 @@ final class CodingTests: XCTestCase {
887887
RelatedUnchangedDocumentDiagnosticReport(
888888
resultId: "myResults",
889889
relatedDocuments: [
890-
DocumentURI(string: "file:///some/path"): DocumentDiagnosticReport.unchanged(
890+
try DocumentURI(string: "file:///some/path"): DocumentDiagnosticReport.unchanged(
891891
RelatedUnchangedDocumentDiagnosticReport(resultId: "myOtherResults")
892892
)
893893
]
@@ -1033,10 +1033,10 @@ final class CodingTests: XCTestCase {
10331033
)
10341034
}
10351035

1036-
func testWorkspaceDocumentDiagnosticReport() {
1036+
func testWorkspaceDocumentDiagnosticReport() throws {
10371037
checkCoding(
10381038
WorkspaceDocumentDiagnosticReport.full(
1039-
WorkspaceFullDocumentDiagnosticReport(items: [], uri: DocumentURI(string: "file:///some/path"))
1039+
WorkspaceFullDocumentDiagnosticReport(items: [], uri: try DocumentURI(string: "file:///some/path"))
10401040
),
10411041
json: #"""
10421042
{
@@ -1051,7 +1051,10 @@ final class CodingTests: XCTestCase {
10511051

10521052
checkCoding(
10531053
WorkspaceDocumentDiagnosticReport.unchanged(
1054-
WorkspaceUnchangedDocumentDiagnosticReport(resultId: "myResults", uri: DocumentURI(string: "file:///some/path"))
1054+
WorkspaceUnchangedDocumentDiagnosticReport(
1055+
resultId: "myResults",
1056+
uri: try DocumentURI(string: "file:///some/path")
1057+
)
10551058
),
10561059
json: #"""
10571060
{
@@ -1063,14 +1066,14 @@ final class CodingTests: XCTestCase {
10631066
)
10641067
}
10651068

1066-
func testWorkspaceSymbolItem() {
1069+
func testWorkspaceSymbolItem() throws {
10671070
checkCoding(
10681071
WorkspaceSymbolItem.symbolInformation(
10691072
SymbolInformation(
10701073
name: "mySym",
10711074
kind: .constant,
10721075
location: Location(
1073-
uri: DocumentURI(string: "file:///some/path"),
1076+
uri: try DocumentURI(string: "file:///some/path"),
10741077
range: Position(line: 3, utf16index: 14)..<Position(line: 4, utf16index: 3)
10751078
)
10761079
)
@@ -1101,7 +1104,9 @@ final class CodingTests: XCTestCase {
11011104
WorkspaceSymbol(
11021105
name: "mySym",
11031106
kind: .boolean,
1104-
location: WorkspaceSymbol.WorkspaceSymbolLocation.uri(.init(uri: DocumentURI(string: "file:///some/path")))
1107+
location: WorkspaceSymbol.WorkspaceSymbolLocation.uri(
1108+
.init(uri: try DocumentURI(string: "file:///some/path"))
1109+
)
11051110
)
11061111
),
11071112
json: #"""
@@ -1116,9 +1121,9 @@ final class CodingTests: XCTestCase {
11161121
)
11171122
}
11181123

1119-
func testWorkspapceSymbolLocation() {
1124+
func testWorkspapceSymbolLocation() throws {
11201125
checkCoding(
1121-
WorkspaceSymbol.WorkspaceSymbolLocation.uri(.init(uri: DocumentURI(string: "file:///some/path"))),
1126+
WorkspaceSymbol.WorkspaceSymbolLocation.uri(.init(uri: try DocumentURI(string: "file:///some/path"))),
11221127
json: #"""
11231128
{
11241129
"uri" : "file:\/\/\/some\/path"
@@ -1129,7 +1134,7 @@ final class CodingTests: XCTestCase {
11291134
checkCoding(
11301135
WorkspaceSymbol.WorkspaceSymbolLocation.location(
11311136
Location(
1132-
uri: DocumentURI(string: "file:///some/path"),
1137+
uri: try DocumentURI(string: "file:///some/path"),
11331138
range: Position(line: 3, utf16index: 14)..<Position(line: 4, utf16index: 3)
11341139
)
11351140
),

‎Tests/LanguageServerProtocolTests/LanguageServerProtocolTests.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ final class LanguageServerProtocolTests: XCTestCase {
3636
XCTAssertEqual(Language.objective_cpp.xflagHeader, "objective-c++-header")
3737
}
3838

39-
func testURLEscaping() {
39+
func testURLEscaping() throws {
4040
let fileURI = DocumentURI(URL(fileURLWithPath: "/folder/[email protected]", isDirectory: false))
41-
let encodedURI = DocumentURI(string: "file:///folder/image%403x.png")
41+
let encodedURI = try DocumentURI(string: "file:///folder/image%403x.png")
4242
XCTAssertEqual(encodedURI, fileURI)
4343
XCTAssertEqual(encodedURI.hashValue, fileURI.hashValue)
4444
}

‎Tests/SKCoreTests/BuildServerBuildSystemTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ final class BuildServerBuildSystemTests: XCTestCase {
8787

8888
let fileUrl = URL(fileURLWithPath: "/some/file/path")
8989
let expectation = XCTestExpectation(description: "target changed")
90-
let targetIdentifier = BuildTargetIdentifier(uri: DocumentURI(string: "build://target/a"))
90+
let targetIdentifier = BuildTargetIdentifier(uri: try DocumentURI(string: "build://target/a"))
9191
let buildSystemDelegate = TestDelegate(targetExpectations: [
9292
BuildTargetEvent(
9393
target: targetIdentifier,

‎Tests/SKCoreTests/BuildSystemManagerTests.swift

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@ import XCTest
1919

2020
final class BuildSystemManagerTests: XCTestCase {
2121

22-
func testMainFiles() async {
23-
let a = DocumentURI(string: "bsm:a")
24-
let b = DocumentURI(string: "bsm:b")
25-
let c = DocumentURI(string: "bsm:c")
26-
let d = DocumentURI(string: "bsm:d")
22+
func testMainFiles() async throws {
23+
let a = try DocumentURI(string: "bsm:a")
24+
let b = try DocumentURI(string: "bsm:b")
25+
let c = try DocumentURI(string: "bsm:c")
26+
let d = try DocumentURI(string: "bsm:d")
2727

2828
let mainFiles = ManualMainFilesProvider(
2929
[
@@ -88,7 +88,7 @@ final class BuildSystemManagerTests: XCTestCase {
8888
}
8989

9090
func testSettingsMainFile() async throws {
91-
let a = DocumentURI(string: "bsm:a.swift")
91+
let a = try try DocumentURI(string: "bsm:a.swift")
9292
let mainFiles = ManualMainFilesProvider([a: [a]])
9393
let bs = ManualBuildSystem()
9494
let bsm = await BuildSystemManager(
@@ -111,7 +111,7 @@ final class BuildSystemManagerTests: XCTestCase {
111111
}
112112

113113
func testSettingsMainFileInitialNil() async throws {
114-
let a = DocumentURI(string: "bsm:a.swift")
114+
let a = try DocumentURI(string: "bsm:a.swift")
115115
let mainFiles = ManualMainFilesProvider([a: [a]])
116116
let bs = ManualBuildSystem()
117117
let bsm = await BuildSystemManager(
@@ -132,7 +132,7 @@ final class BuildSystemManagerTests: XCTestCase {
132132
}
133133

134134
func testSettingsMainFileWithFallback() async throws {
135-
let a = DocumentURI(string: "bsm:a.swift")
135+
let a = try DocumentURI(string: "bsm:a.swift")
136136
let mainFiles = ManualMainFilesProvider([a: [a]])
137137
let bs = ManualBuildSystem()
138138
let fallback = FallbackBuildSystem(buildSetup: .default)
@@ -161,8 +161,8 @@ final class BuildSystemManagerTests: XCTestCase {
161161
}
162162

163163
func testSettingsMainFileInitialIntersect() async throws {
164-
let a = DocumentURI(string: "bsm:a.swift")
165-
let b = DocumentURI(string: "bsm:b.swift")
164+
let a = try DocumentURI(string: "bsm:a.swift")
165+
let b = try DocumentURI(string: "bsm:b.swift")
166166
let mainFiles = ManualMainFilesProvider([a: [a], b: [b]])
167167
let bs = ManualBuildSystem()
168168
let bsm = await BuildSystemManager(
@@ -201,8 +201,8 @@ final class BuildSystemManagerTests: XCTestCase {
201201
}
202202

203203
func testSettingsMainFileUnchanged() async throws {
204-
let a = DocumentURI(string: "bsm:a.swift")
205-
let b = DocumentURI(string: "bsm:b.swift")
204+
let a = try DocumentURI(string: "bsm:a.swift")
205+
let b = try DocumentURI(string: "bsm:b.swift")
206206
let mainFiles = ManualMainFilesProvider([a: [a], b: [b]])
207207
let bs = ManualBuildSystem()
208208
let bsm = await BuildSystemManager(
@@ -231,9 +231,9 @@ final class BuildSystemManagerTests: XCTestCase {
231231
}
232232

233233
func testSettingsHeaderChangeMainFile() async throws {
234-
let h = DocumentURI(string: "bsm:header.h")
235-
let cpp1 = DocumentURI(string: "bsm:main.cpp")
236-
let cpp2 = DocumentURI(string: "bsm:other.cpp")
234+
let h = try DocumentURI(string: "bsm:header.h")
235+
let cpp1 = try DocumentURI(string: "bsm:main.cpp")
236+
let cpp2 = try DocumentURI(string: "bsm:other.cpp")
237237
let mainFiles = ManualMainFilesProvider(
238238
[
239239
h: [cpp1],
@@ -286,9 +286,9 @@ final class BuildSystemManagerTests: XCTestCase {
286286
}
287287

288288
func testSettingsOneMainTwoHeader() async throws {
289-
let h1 = DocumentURI(string: "bsm:header1.h")
290-
let h2 = DocumentURI(string: "bsm:header2.h")
291-
let cpp = DocumentURI(string: "bsm:main.cpp")
289+
let h1 = try DocumentURI(string: "bsm:header1.h")
290+
let h2 = try DocumentURI(string: "bsm:header2.h")
291+
let cpp = try DocumentURI(string: "bsm:main.cpp")
292292
let mainFiles = ManualMainFilesProvider(
293293
[
294294
h1: [cpp],
@@ -333,9 +333,9 @@ final class BuildSystemManagerTests: XCTestCase {
333333
}
334334

335335
func testSettingsChangedAfterUnregister() async throws {
336-
let a = DocumentURI(string: "bsm:a.swift")
337-
let b = DocumentURI(string: "bsm:b.swift")
338-
let c = DocumentURI(string: "bsm:c.swift")
336+
let a = try DocumentURI(string: "bsm:a.swift")
337+
let b = try DocumentURI(string: "bsm:b.swift")
338+
let c = try DocumentURI(string: "bsm:c.swift")
339339
let mainFiles = ManualMainFilesProvider([a: [a], b: [b], c: [c]])
340340
let bs = ManualBuildSystem()
341341
let bsm = await BuildSystemManager(
@@ -376,7 +376,7 @@ final class BuildSystemManagerTests: XCTestCase {
376376
}
377377

378378
func testDependenciesUpdated() async throws {
379-
let a = DocumentURI(string: "bsm:a.swift")
379+
let a = try DocumentURI(string: "bsm:a.swift")
380380
let mainFiles = ManualMainFilesProvider([a: [a]])
381381

382382
let bs = ManualBuildSystem()

0 commit comments

Comments
 (0)