Skip to content

Commit

Permalink
Improve support for multi-platform projects
Browse files Browse the repository at this point in the history
In response to #462, a check was added to detect index stores containing multiple units for any given source file, and raise an error. The ultimate cause of the issue was the use of a custom `--index-store` path and Xcode being left open on a CI machine. Still, the check seemed reasonable to include to prevent others running into the same issue.

Some users then began running into this new error on projects that contained multi-platform targets, such as those with watchOS extensions. To workaround this, a mechanism was added that attempted to identify which index store units should be indexed by computing a target triple from the output of `xcodebuild -showBuildSettings`. Unfortunately, this approach did not work in some cases as `-showBuildSettings` does not respect the `-destination` argument, resulting in incorrect computed target triples.

There doesn't appear to be a simple way to compute a target triple ahead of time, and thus any potentially fixes will further increase the complexity of an already complex mechanism. Given that the original reason for #462 was caused by a user's particular setup, rather than an inherent issue with the index store, this change reverts all checks around the nature of units contained in the index store. I think this is a reasonable trade-off, as multi-platform projects are likely more command than the situations like the one faced in #462.

A new warning has been added when using `--index-store` to inform users that Xcode should not write to the index store while Periphery runs.
  • Loading branch information
ileitch committed Oct 30, 2023
1 parent 96e5d85 commit 6319894
Show file tree
Hide file tree
Showing 26 changed files with 393 additions and 219 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
##### Bug Fixes

- Fix redundant public accessibility analysis for protocol members declared in extensions that are referenced cross-module where the protocol itself is not.
- Remove checks causing errors when scanning multi-platform projects.

## 2.16.0 (2023-09-27)

Expand Down
10 changes: 7 additions & 3 deletions Sources/Frontend/Scan.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@ final class Scan {
}

func perform(project: Project) throws -> [ScanResult] {
if !configuration.indexStorePath.isEmpty, !configuration.skipBuild {
logger.warn("The '--index-store-path' option implies '--skip-build', specify it to silence this warning")
configuration.skipBuild = true
if !configuration.indexStorePath.isEmpty {
logger.warn("When using the '--index-store-path' option please ensure that Xcode is not running. False-positives can occur if Xcode writes to the index store while Periphery is running.")

if !configuration.skipBuild {
logger.warn("The '--index-store-path' option implies '--skip-build', specify it to silence this warning.")
configuration.skipBuild = true
}
}

if configuration.verbose {
Expand Down
4 changes: 1 addition & 3 deletions Sources/PeripheryKit/Indexer/IndexTarget.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ import Foundation

public struct IndexTarget: Hashable {
public let name: String
public var triple: String?

public init(name: String, triple: String? = nil) {
public init(name: String) {
self.name = name
self.triple = triple
}
}
32 changes: 2 additions & 30 deletions Sources/PeripheryKit/Indexer/JobPool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ struct JobPool<T> {
}
}

func map<R>(_ block: @escaping (T) throws -> R) throws -> [R] {
func flatMap<R>(_ block: @escaping (T) throws -> [R]) throws -> [R] {
var error: Error?
var results: [R] = []
let lock = UnfairLock()
Expand All @@ -36,7 +36,7 @@ struct JobPool<T> {
let result = try block(job)

lock.perform {
results.append(result)
results.append(contentsOf: result)
}
} catch let e {
error = e
Expand All @@ -49,32 +49,4 @@ struct JobPool<T> {

return results
}

func compactMap<R>(_ block: @escaping (T) throws -> R?) throws -> [R] {
var error: Error?
var results: [R] = []
let lock = UnfairLock()

DispatchQueue.concurrentPerform(iterations: jobs.count) { idx in
guard error == nil else { return }

do {
let job = jobs[idx]
if let result = try block(job) {
lock.perform {
results.append(result)
}
}
} catch let e {
error = e
}
}

if let error = error {
throw error
}

return results
}

}
100 changes: 44 additions & 56 deletions Sources/PeripheryKit/Indexer/SwiftIndexer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,47 +32,28 @@ public final class SwiftIndexer: Indexer {
let (includedFiles, excludedFiles) = filterIndexExcluded(from: allSourceFiles)
excludedFiles.forEach { self.logger.debug("Excluding \($0.string)") }

let stores = try JobPool(jobs: indexStorePaths)
.map { [logger] indexStorePath in
let unitsByFile = try JobPool(jobs: indexStorePaths)
.flatMap { [logger, currentFilePath] indexStorePath in
logger.debug("Reading \(indexStorePath)")
let indexStore = try IndexStore.open(store: URL(fileURLWithPath: indexStorePath.string), lib: .open())
return (indexStore, indexStore.units(includeSystem: false))
}

let units = stores.reduce(into: [(IndexStore, IndexStoreUnit)]()) { result, tuple in
let (indexStore, units) = tuple
units.forEach { result.append((indexStore, $0)) }
}

let unitsByFile = try JobPool(jobs: units)
.compactMap { [logger, sourceFiles, currentFilePath] (indexStore, unit) -> (FilePath, IndexStore, IndexStoreUnit)? in
guard let filePath = try indexStore.mainFilePath(for: unit) else { return nil }
let units = indexStore.units(includeSystem: false)

let file = FilePath.makeAbsolute(filePath, relativeTo: currentFilePath)
return try units.compactMap { unit -> (FilePath, IndexStore, IndexStoreUnit)? in
guard let filePath = try indexStore.mainFilePath(for: unit) else { return nil }

if includedFiles.contains(file) {
// Ignore units built for other architectures/platforms.
let validTargetTriples = sourceFiles[file]?.compactMapSet { $0.triple } ?? []
let file = FilePath.makeAbsolute(filePath, relativeTo: currentFilePath)

if validTargetTriples.isEmpty {
if includedFiles.contains(file) {
return (file, indexStore, unit)
} else {
if let unitTargetTriple = try indexStore.target(for: unit) {
if validTargetTriples.contains(unitTargetTriple) {
return (file, indexStore, unit)
}
} else {
logger.warn("No unit target triple for: \(file)")
}
}
}

return nil
return nil
}
}
.reduce(into: [FilePath: [(IndexStore, IndexStoreUnit)]](), { result, tuple in
let (file, indexStore, unit) = tuple
result[file, default: []].append((indexStore, unit))
})
.reduce(into: [FilePath: [(IndexStore, IndexStoreUnit)]](), { result, tuple in
let (file, indexStore, unit) = tuple
result[file, default: []].append((indexStore, unit))
})

let indexedFiles = Set(unitsByFile.keys)
let unindexedFiles = allSourceFiles.subtracting(excludedFiles).subtracting(indexedFiles)
Expand All @@ -83,21 +64,9 @@ public final class SwiftIndexer: Indexer {
throw PeripheryError.unindexedTargetsError(targets: targets, indexStorePaths: indexStorePaths)
}

let jobs = try unitsByFile.map { (file, units) -> Job in
let modules = try units.reduce(into: Set<String>()) { (set, tuple) in
let (indexStore, unit) = tuple
if let name = try indexStore.moduleName(for: unit) {
let (didInsert, _) = set.insert(name)
if !didInsert {
let targets = try units.compactMapSet { try indexStore.target(for: $0.1) }
throw PeripheryError.conflictingIndexUnitsError(file: file, module: name, unitTargets: targets)
}
}
}
let sourceFile = SourceFile(path: file, modules: modules)

let jobs = unitsByFile.map { (file, units) -> Job in
return Job(
file: sourceFile,
file: file,
units: units,
graph: graph,
logger: logger,
Expand All @@ -113,7 +82,7 @@ public final class SwiftIndexer: Indexer {
try job.phaseOne()
}

phaseOneLogger.debug("\(job.file.path) (\(elapsed)s)")
phaseOneLogger.debug("\(job.file) (\(elapsed)s)")
}

logger.endInterval(phaseOneInterval)
Expand All @@ -126,7 +95,7 @@ public final class SwiftIndexer: Indexer {
try job.phaseTwo()
}

phaseTwoLogger.debug("\(job.file.path) (\(elapsed)s)")
phaseTwoLogger.debug("\(job.file) (\(elapsed)s)")
}

logger.endInterval(phaseTwoInterval)
Expand All @@ -135,15 +104,16 @@ public final class SwiftIndexer: Indexer {
// MARK: - Private

private class Job {
let file: SourceFile
let file: FilePath

private let units: [(IndexStore, IndexStoreUnit)]
private let graph: SourceGraph
private let logger: ContextualLogger
private let configuration: Configuration
private var sourceFile: SourceFile?

required init(
file: SourceFile,
file: FilePath,
units: [(IndexStore, IndexStoreUnit)],
graph: SourceGraph,
logger: ContextualLogger,
Expand Down Expand Up @@ -203,7 +173,7 @@ public final class SwiftIndexer: Indexer {

try indexStore.forEachOccurrences(for: record, language: .swift) { occurrence in
guard let usr = occurrence.symbol.usr,
let location = transformLocation(occurrence.location)
let location = try transformLocation(occurrence.location)
else { return true }

if !occurrence.roles.intersection([.definition, .declaration]).isEmpty {
Expand Down Expand Up @@ -262,13 +232,14 @@ public final class SwiftIndexer: Indexer {

/// Phase two associates latent references, and performs other actions that depend on the completed source graph.
func phaseTwo() throws {
let multiplexingSyntaxVisitor = try MultiplexingSyntaxVisitor(file: file)
let sourceFile = try getSourceFile()
let multiplexingSyntaxVisitor = try MultiplexingSyntaxVisitor(file: sourceFile)
let declarationSyntaxVisitor = multiplexingSyntaxVisitor.add(DeclarationSyntaxVisitor.self)
let importSyntaxVisitor = multiplexingSyntaxVisitor.add(ImportSyntaxVisitor.self)

multiplexingSyntaxVisitor.visit()

file.importStatements = importSyntaxVisitor.importStatements
sourceFile.importStatements = importSyntaxVisitor.importStatements

associateLatentReferences()
associateDanglingReferences()
Expand All @@ -277,12 +248,29 @@ public final class SwiftIndexer: Indexer {
applyCommentCommands(using: multiplexingSyntaxVisitor)
}

// MARK: - Private

private var declarations: [Declaration] = []
private var childDeclsByParentUsr: [String: Set<Declaration>] = [:]
private var referencesByUsr: [String: Set<Reference>] = [:]
private var danglingReferences: [Reference] = []
private var varParameterUsrs: Set<String> = []

private func getSourceFile() throws -> SourceFile {
if let sourceFile { return sourceFile }

let modules = try units.reduce(into: Set<String>()) { (set, tuple) in
let (indexStore, unit) = tuple
if let name = try indexStore.moduleName(for: unit) {
set.insert(name)
}
}

let sourceFile = SourceFile(path: file, modules: modules)
self.sourceFile = sourceFile
return sourceFile
}

private func establishDeclarationHierarchy() {
graph.withLock {
for (parent, decls) in childDeclsByParentUsr {
Expand Down Expand Up @@ -446,7 +434,7 @@ public final class SwiftIndexer: Indexer {

let analyzer = UnusedParameterAnalyzer()
let paramsByFunction = analyzer.analyze(
file: file,
file: syntaxVisitor.sourceFile,
syntax: syntaxVisitor.syntax,
locationConverter: syntaxVisitor.locationConverter,
parseProtocols: true)
Expand Down Expand Up @@ -661,8 +649,8 @@ public final class SwiftIndexer: Indexer {
return refs
}

private func transformLocation(_ input: IndexStoreOccurrence.Location) -> SourceLocation? {
return SourceLocation(file: file, line: Int(input.line), column: Int(input.column))
private func transformLocation(_ input: IndexStoreOccurrence.Location) throws -> SourceLocation? {
return SourceLocation(file: try getSourceFile(), line: Int(input.line), column: Int(input.column))
}

private func transformDeclarationKind(_ kind: IndexStoreSymbol.Kind, _ subKind: IndexStoreSymbol.SubKind) -> Declaration.Kind? {
Expand Down
2 changes: 2 additions & 0 deletions Sources/PeripheryKit/Syntax/MultiplexingSyntaxVisitor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,15 @@ extension PeripherySyntaxVisitor {
}

final class MultiplexingSyntaxVisitor: SyntaxVisitor {
let sourceFile: SourceFile
let syntax: SourceFileSyntax
let locationConverter: SourceLocationConverter
let sourceLocationBuilder: SourceLocationBuilder

private var visitors: [PeripherySyntaxVisitor] = []

required init(file: SourceFile) throws {
self.sourceFile = file
let source = try String(contentsOf: file.path.url)
self.syntax = Parser.parse(source: source)
self.locationConverter = SourceLocationConverter(fileName: file.path.string, tree: syntax)
Expand Down
1 change: 1 addition & 0 deletions Sources/PeripheryKit/Syntax/UnusedParameterAnalyzer.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Foundation
import SwiftSyntax
import SystemPackage

final class UnusedParameterAnalyzer {
private enum UsageType {
Expand Down
7 changes: 0 additions & 7 deletions Sources/Shared/Extensions/Sequence+Extension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,4 @@ public extension Sequence {
}
}
}

func mapDict<Key, Value>(_ transform: (Element) throws -> (Key, Value)) rethrows -> Dictionary<Key, Value> {
try reduce(into: .init()) { result, element in
let pair = try transform(element)
result[pair.0] = pair.1
}
}
}
4 changes: 0 additions & 4 deletions Sources/Shared/Logger.swift
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,6 @@ public struct ContextualLogger {
logger.debug("[\(context)] \(text)")
}

public func warn(_ text: String) {
logger.warn("[\(context)] \(text)")
}

public func beginInterval(_ name: StaticString) -> SignpostInterval {
logger.beginInterval(name)
}
Expand Down
11 changes: 0 additions & 11 deletions Sources/Shared/PeripheryError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ public enum PeripheryError: Error, LocalizedError, CustomStringConvertible {
case unindexedTargetsError(targets: Set<String>, indexStorePaths: [FilePath])
case jsonDeserializationError(error: Error, json: String)
case indexStoreNotFound(derivedDataPath: String)
case conflictingIndexUnitsError(file: FilePath, module: String, unitTargets: Set<String>)
case invalidTargetTriple(target: String, arch: String, vendor: String, osVersion: String)

public var errorDescription: String? {
switch self {
Expand Down Expand Up @@ -67,15 +65,6 @@ public enum PeripheryError: Error, LocalizedError, CustomStringConvertible {
return "JSON deserialization failed: \(describe(error))\nJSON:\n\(json)"
case let .indexStoreNotFound(derivedDataPath):
return "Failed to find index datastore at path: \(derivedDataPath)"
case let .conflictingIndexUnitsError(file, module, targets):
var parts = ["Found conflicting index store units for '\(file)' in module '\(module)'."]
if targets.count > 1 {
parts.append("The units have conflicting build targets: \(targets.sorted().joined(separator: ", ")).")
}
parts.append("If you passed the '--index-store-path' option, ensure that Xcode is not open with a project that may write to this index store while Periphery is running.")
return parts.joined(separator: " ")
case let .invalidTargetTriple(target, arch, vendor, osVersion):
return "Failed to construct triple for target '\(target)': \(arch), \(vendor), \(osVersion)"
}
}

Expand Down
15 changes: 1 addition & 14 deletions Sources/XcodeSupport/XcodeProjectDriver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,19 +113,6 @@ public final class XcodeProjectDriver {

extension XcodeProjectDriver: ProjectDriver {
public func build() throws {
// Copy target triples to the targets. The triple is used by the indexer to ignore index store units built for
// other architectures/platforms.
let targetTriples = try xcodebuild.buildSettings(targets: targets)
.mapDict { action in
(action.target, try action.makeTargetTriple())
}

for target in targets {
if let triple = targetTriples[target.name] {
target.triple = triple
}
}

guard !configuration.skipBuild else { return }

if configuration.cleanBuild {
Expand Down Expand Up @@ -164,7 +151,7 @@ extension XcodeProjectDriver: ProjectDriver {

for target in targets {
target.files(kind: .swift).forEach {
let indexTarget = IndexTarget(name: target.name, triple: target.triple)
let indexTarget = IndexTarget(name: target.name)
sourceFiles[$0, default: []].insert(indexTarget)
}
}
Expand Down
Loading

0 comments on commit 6319894

Please sign in to comment.