Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include the output paths for clang files in the sources property of a BuildDescription #8332

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Include the output paths for clang files in the sources property of a BuildDescription #8332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions Sources/SourceKitLSPAPI/BuildDescription.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,19 @@ public enum BuildTargetCompiler {
case clang
}

/// Information about a source file that belongs to a target.
public struct SourceItem {
/// The URL of the source file itself.
public let sourceFile: URL

/// If the file has a unique output path (eg. for clang files), the output paths. `nil` for eg. Swift targets,
/// which don't have unique output paths for each file.
public let outputFile: URL?
}

public protocol BuildTarget {
/// Source files in the target
var sources: [URL] { get }
var sources: [SourceItem] { get }

/// Header files in the target
var headers: [URL] { get }
Expand All @@ -61,8 +71,6 @@ public protocol BuildTarget {

var isTestTarget: Bool { get }

var outputPaths: [URL] { get throws }

func compileArguments(for fileURL: URL) throws -> [String]
}

Expand All @@ -77,11 +85,13 @@ private struct WrappedClangTargetBuildDescription: BuildTarget {
self.isTestTarget = description.isTestTarget
}

public var sources: [URL] {
public var sources: [SourceItem] {
guard let compilePaths = try? description.compilePaths() else {
return []
}
return compilePaths.map(\.source.asURL)
return compilePaths.map {
SourceItem(sourceFile: $0.source.asURL, outputFile: $0.object.asURL)
}
}

public var headers: [URL] {
Expand Down Expand Up @@ -117,12 +127,6 @@ private struct WrappedClangTargetBuildDescription: BuildTarget {
return description.destination == .host ? .host : .target
}

var outputPaths: [URL] {
get throws {
return try description.compilePaths().map(\.object.asURL)
}
}

public func compileArguments(for fileURL: URL) throws -> [String] {
let filePath = try resolveSymlinks(try Basics.AbsolutePath(validating: fileURL.path))
let commandLine = try description.emitCommandLine(for: filePath)
Expand Down Expand Up @@ -152,8 +156,10 @@ private struct WrappedSwiftTargetBuildDescription: BuildTarget {
return description.destination == .host ? .host : .target
}

var sources: [URL] {
return description.sources.map(\.asURL)
var sources: [SourceItem] {
return description.sources.map {
return SourceItem(sourceFile: $0.asURL, outputFile: nil)
}
}

var headers: [URL] { [] }
Expand Down
8 changes: 5 additions & 3 deletions Sources/SourceKitLSPAPI/PluginTargetBuildDescription.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@ struct PluginTargetBuildDescription: BuildTarget {
self.isPartOfRootPackage = isPartOfRootPackage
}

var sources: [URL] {
return target.sources.paths.map(\.asURL)
var sources: [SourceItem] {
return target.sources.paths.map {
SourceItem(sourceFile: $0.asURL, outputFile: nil)
}
}

var headers: [URL] { [] }
Expand Down Expand Up @@ -74,7 +76,7 @@ struct PluginTargetBuildDescription: BuildTarget {
// FIXME: This is very odd and we should clean this up by merging `ManifestLoader` and `DefaultPluginScriptRunner` again.
var args = ManifestLoader.interpreterFlags(for: self.toolsVersion, toolchain: toolchain)
// Note: we ignore the `fileURL` here as the expectation is that we get a commandline for the entire target in case of Swift. Plugins are always assumed to only consist of Swift files.
args += try sources.map { try $0.filePath }
args += try sources.map { try $0.sourceFile.filePath }
return args
}
}
Expand Down
6 changes: 3 additions & 3 deletions Tests/SourceKitLSPAPITests/SourceKitLSPAPITests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,8 @@ final class SourceKitLSPAPITests: XCTestCase {

let target = try XCTUnwrap(description.getBuildTarget(for: XCTUnwrap(graph.module(for: "lib")), destination: .target))
XCTAssertEqual(target.compiler, .clang)
XCTAssertEqual(try target.outputPaths.count, 1)
XCTAssertEqual(try target.outputPaths.last?.lastPathComponent, "lib.cpp.o")
XCTAssertEqual(target.sources.count, 1)
XCTAssertEqual(target.sources.last?.outputFile?.lastPathComponent, "lib.cpp.o")
}
}

Expand All @@ -384,7 +384,7 @@ extension SourceKitLSPAPI.BuildDescription {
XCTAssertEqual(buildTarget.ignored, ignoredFiles, "build target \(targetName) contains unexpected ignored files")
XCTAssertEqual(buildTarget.others, otherFiles, "build target \(targetName) contains unexpected other files")

guard let source = buildTarget.sources.first else {
guard let source = buildTarget.sources.first?.sourceFile else {
XCTFail("build target \(targetName) contains no source files")
return false
}
Expand Down