Skip to content

Commit f7dba31

Browse files
committed
Infer build settings from a sibling file if current file doesn’t have build settings
1 parent abf98b9 commit f7dba31

File tree

2 files changed

+106
-48
lines changed

2 files changed

+106
-48
lines changed

Sources/BuildServerIntegration/BuildServerManager.swift

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1119,6 +1119,64 @@ package actor BuildServerManager: QueueBasedMessageHandler {
11191119
return settings
11201120
}
11211121

1122+
/// Try finding a source file with the same language as `document` in the same directory as `document` and patch its
1123+
/// build settings to provide more accurate fallback settings than the generic fallback settings.
1124+
private func fallbackBuildSettingsInferredFromSiblingFile(
1125+
of document: DocumentURI,
1126+
target explicitlyRequestedTarget: BuildTargetIdentifier?,
1127+
language: Language?,
1128+
fallbackAfterTimeout: Bool
1129+
) async throws -> FileBuildSettings? {
1130+
guard let documentFileURL = document.fileURL else {
1131+
return nil
1132+
}
1133+
let directory = documentFileURL.deletingLastPathComponent()
1134+
guard let language = language ?? Language(inferredFromFileExtension: document) else {
1135+
return nil
1136+
}
1137+
let siblingFile = try await self.sourceFilesAndDirectories().files.compactMap { (uri, info) -> DocumentURI? in
1138+
guard info.isBuildable, uri.fileURL?.deletingLastPathComponent() == directory else {
1139+
return nil
1140+
}
1141+
if let explicitlyRequestedTarget, !info.targets.contains(explicitlyRequestedTarget) {
1142+
return nil
1143+
}
1144+
// Only consider build settings from sibling files that appear to have the same language. In theory, we might skip
1145+
// valid sibling files because of this since non-standard file extension might be mapped to `language` by the
1146+
// build server, but this is a good first check to avoid requesting build settings for too many documents. And
1147+
// since all of this is fallback-logic, skipping over possibly valid files is not a correctness issue.
1148+
guard let siblingLanguage = Language(inferredFromFileExtension: uri), siblingLanguage == language else {
1149+
return nil
1150+
}
1151+
return uri
1152+
}.sorted(by: { $0.pseudoPath < $1.pseudoPath }).first
1153+
1154+
guard let siblingFile else {
1155+
return nil
1156+
}
1157+
1158+
let siblingSettings = await self.buildSettingsInferredFromMainFile(
1159+
for: siblingFile,
1160+
target: explicitlyRequestedTarget,
1161+
language: language,
1162+
fallbackAfterTimeout: fallbackAfterTimeout,
1163+
allowInferenceFromSiblingFile: false
1164+
)
1165+
guard var siblingSettings, !siblingSettings.isFallback else {
1166+
return nil
1167+
}
1168+
siblingSettings.isFallback = true
1169+
switch language.semanticKind {
1170+
case .swift:
1171+
siblingSettings.compilerArguments += [try documentFileURL.filePath]
1172+
case .clang:
1173+
siblingSettings = siblingSettings.patching(newFile: document, originalFile: siblingFile)
1174+
case nil:
1175+
return nil
1176+
}
1177+
return siblingSettings
1178+
}
1179+
11221180
/// Returns the build settings for the given document.
11231181
///
11241182
/// If the document doesn't have builds settings by itself, eg. because it is a C header file, the build settings will
@@ -1135,7 +1193,8 @@ package actor BuildServerManager: QueueBasedMessageHandler {
11351193
for document: DocumentURI,
11361194
target explicitlyRequestedTarget: BuildTargetIdentifier? = nil,
11371195
language: Language?,
1138-
fallbackAfterTimeout: Bool
1196+
fallbackAfterTimeout: Bool,
1197+
allowInferenceFromSiblingFile: Bool = true
11391198
) async -> FileBuildSettings? {
11401199
func mainFileAndSettings(
11411200
basedOn document: DocumentURI
@@ -1171,6 +1230,19 @@ package actor BuildServerManager: QueueBasedMessageHandler {
11711230
fallbackAfterTimeout: fallbackAfterTimeout
11721231
)
11731232
}
1233+
if settings?.isFallback ?? true, allowInferenceFromSiblingFile {
1234+
let settingsFromSibling = await orLog("Inferring build settings from sibling file") {
1235+
try await self.fallbackBuildSettingsInferredFromSiblingFile(
1236+
of: document,
1237+
target: explicitlyRequestedTarget,
1238+
language: language,
1239+
fallbackAfterTimeout: fallbackAfterTimeout
1240+
)
1241+
}
1242+
if let settingsFromSibling {
1243+
return (mainFile, settingsFromSibling)
1244+
}
1245+
}
11741246
guard let settings else {
11751247
return nil
11761248
}

Tests/SourceKitLSPTests/SwiftPMIntegrationTests.swift

Lines changed: 33 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -111,80 +111,66 @@ final class SwiftPMIntegrationTests: XCTestCase {
111111
enableBackgroundIndexing: true
112112
)
113113

114+
// First, create a new in-memory file and verify that we get some basic functionality for it
115+
114116
let newFileUrl = project.scratchDirectory
115117
.appending(components: "Sources", "MyLibrary", "Other.swift")
116118
let newFileUri = DocumentURI(newFileUrl)
117119

118120
let newFileContents = """
119121
func baz(l: Lib) {
120122
l.2️⃣foo()
123+
#warning("A manual warning")
121124
}
122125
"""
123-
try await extractMarkers(newFileContents).textWithoutMarkers.writeWithRetry(to: newFileUrl)
124-
125-
// Check that we don't get cross-file code completion before we send a `DidChangeWatchedFilesNotification` to make
126-
// sure we didn't include the file in the initial retrieval of build settings.
127-
let (oldFileUri, oldFilePositions) = try project.openDocument("Lib.swift")
128126
let newFilePositions = project.testClient.openDocument(newFileContents, uri: newFileUri)
129127

130-
let completionsBeforeDidChangeNotification = try await project.testClient.send(
128+
try await extractMarkers(newFileContents).textWithoutMarkers.writeWithRetry(to: newFileUrl)
129+
let completionsBeforeSave = try await project.testClient.send(
131130
CompletionRequest(textDocument: TextDocumentIdentifier(newFileUri), position: newFilePositions["2️⃣"])
132131
)
133-
XCTAssertEqual(completionsBeforeDidChangeNotification.items, [])
132+
XCTAssertEqual(Set(completionsBeforeSave.items.map(\.label)), ["foo()", "self"])
133+
134+
// We shouldn't get diagnostics for the new file yet since we still consider the build settings inferred from a
135+
// sibling file fallback settings.
136+
let diagnosticsBeforeSave = try await project.testClient.send(
137+
DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(newFileUri))
138+
)
139+
XCTAssertEqual(diagnosticsBeforeSave.fullReport?.items, [])
140+
141+
let (oldFileUri, oldFilePositions) = try project.openDocument("Lib.swift")
142+
// Check that we don't get completions for `baz` (defined in the new file) in the old file yet because the new file
143+
// is not part of the package manifest yet.
144+
let oldFileCompletionsBeforeSave = try await project.testClient.send(
145+
CompletionRequest(textDocument: TextDocumentIdentifier(oldFileUri), position: oldFilePositions["1️⃣"])
146+
)
147+
XCTAssert(!oldFileCompletionsBeforeSave.items.contains(where: { $0.label == "baz(l: Lib)" }))
148+
149+
// Now save the file to disk, which adds it to the package graph, which should enable more functionality.
134150

135-
// Send a `DidChangeWatchedFilesNotification` and verify that we now get cross-file code completion.
151+
try await extractMarkers(newFileContents).textWithoutMarkers.writeWithRetry(to: newFileUrl)
136152
project.testClient.send(
137153
DidChangeWatchedFilesNotification(changes: [
138154
FileEvent(uri: newFileUri, type: .created)
139155
])
140156
)
141-
142157
// Ensure that the DidChangeWatchedFilesNotification is handled before we continue.
143158
try await project.testClient.send(SynchronizeRequest(index: true))
144159

145-
let completions = try await project.testClient.send(
160+
// Check that we still get completions in the new file, now get diagnostics in the new file and also see functions
161+
// from the new file in the old file
162+
let completionsAfterSave = try await project.testClient.send(
146163
CompletionRequest(textDocument: TextDocumentIdentifier(newFileUri), position: newFilePositions["2️⃣"])
147164
)
148-
149-
XCTAssertEqual(
150-
completions.items.clearingUnstableValues,
151-
[
152-
CompletionItem(
153-
label: "foo()",
154-
kind: .method,
155-
detail: "Void",
156-
deprecated: false,
157-
sortText: nil,
158-
filterText: "foo()",
159-
insertText: "foo()",
160-
insertTextFormat: .plain,
161-
textEdit: .textEdit(
162-
TextEdit(range: Range(newFilePositions["2️⃣"]), newText: "foo()")
163-
)
164-
),
165-
CompletionItem(
166-
label: "self",
167-
kind: .keyword,
168-
detail: "Lib",
169-
deprecated: false,
170-
sortText: nil,
171-
filterText: "self",
172-
insertText: "self",
173-
insertTextFormat: .plain,
174-
textEdit: .textEdit(
175-
TextEdit(range: Range(newFilePositions["2️⃣"]), newText: "self")
176-
)
177-
),
178-
]
165+
XCTAssertEqual(Set(completionsAfterSave.items.map(\.label)), ["foo()", "self"])
166+
let diagnosticsAfterSave = try await project.testClient.send(
167+
DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(newFileUri))
179168
)
180-
181-
// Check that we get code completion for `baz` (defined in the new file) in the old file.
182-
// I.e. check that the existing file's build settings have been updated to include the new file.
183-
184-
let oldFileCompletions = try await project.testClient.send(
169+
XCTAssertEqual(diagnosticsAfterSave.fullReport?.items.map(\.message), ["A manual warning"])
170+
let oldFileCompletionsAfterSave = try await project.testClient.send(
185171
CompletionRequest(textDocument: TextDocumentIdentifier(oldFileUri), position: oldFilePositions["1️⃣"])
186172
)
187-
XCTAssert(oldFileCompletions.items.contains(where: { $0.label == "baz(l: Lib)" }))
173+
assertContains(oldFileCompletionsAfterSave.items.map(\.label), "baz(l: Lib)")
188174
}
189175

190176
func testNestedPackage() async throws {

0 commit comments

Comments
 (0)