Skip to content
New issue

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

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

Already on GitHub? # to your account

Add support for compilation databases #2962

Merged
merged 14 commits into from
Dec 3, 2019
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
[PaulTaykalo](https://github.com/PaulTaykalo)
[#2953](https://github.com/realm/SwiftLint/issues/2953)

* Support compilation databases for `swiftlint analayze`.
[kastiglione](https://github.com/kastiglione)
[#2962](https://github.com/realm/SwiftLint/issues/2962)

## 0.37.0: Double Load

#### Breaking
Expand Down
11 changes: 7 additions & 4 deletions Source/swiftlint/Commands/AnalyzeCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,20 @@ struct AnalyzeOptions: OptionsProtocol {
let enableAllRules: Bool
let autocorrect: Bool
let compilerLogPath: String
let compileCommands: String

// swiftlint:disable line_length
static func create(_ path: String) -> (_ configurationFile: String) -> (_ strict: Bool) -> (_ lenient: Bool) -> (_ forceExclude: Bool) -> (_ useScriptInputFiles: Bool) -> (_ benchmark: Bool) -> (_ reporter: String) -> (_ quiet: Bool) -> (_ enableAllRules: Bool) -> (_ autocorrect: Bool) -> (_ compilerLogPath: String) -> (_ paths: [String]) -> AnalyzeOptions {
return { configurationFile in { strict in { lenient in { forceExclude in { useScriptInputFiles in { benchmark in { reporter in { quiet in { enableAllRules in { autocorrect in { compilerLogPath in { paths in
static func create(_ path: String) -> (_ configurationFile: String) -> (_ strict: Bool) -> (_ lenient: Bool) -> (_ forceExclude: Bool) -> (_ useScriptInputFiles: Bool) -> (_ benchmark: Bool) -> (_ reporter: String) -> (_ quiet: Bool) -> (_ enableAllRules: Bool) -> (_ autocorrect: Bool) -> (_ compilerLogPath: String) -> (_ compileCommands: String) -> (_ paths: [String]) -> AnalyzeOptions {
return { configurationFile in { strict in { lenient in { forceExclude in { useScriptInputFiles in { benchmark in { reporter in { quiet in { enableAllRules in { autocorrect in { compilerLogPath in { compileCommands in { paths in
let allPaths: [String]
if !path.isEmpty {
allPaths = [path]
} else {
allPaths = paths
}
return self.init(paths: allPaths, configurationFile: configurationFile, strict: strict, lenient: lenient, forceExclude: forceExclude, useScriptInputFiles: useScriptInputFiles, benchmark: benchmark, reporter: reporter, quiet: quiet, enableAllRules: enableAllRules, autocorrect: autocorrect, compilerLogPath: compilerLogPath)
return self.init(paths: allPaths, configurationFile: configurationFile, strict: strict, lenient: lenient, forceExclude: forceExclude, useScriptInputFiles: useScriptInputFiles, benchmark: benchmark, reporter: reporter, quiet: quiet, enableAllRules: enableAllRules, autocorrect: autocorrect, compilerLogPath: compilerLogPath, compileCommands: compileCommands)
// swiftlint:enable line_length
}}}}}}}}}}}}
}}}}}}}}}}}}}
}

static func evaluate(_ mode: CommandMode) -> Result<AnalyzeOptions, CommandantError<CommandantError<()>>> {
Expand All @@ -86,6 +87,8 @@ struct AnalyzeOptions: OptionsProtocol {
usage: "correct violations whenever possible")
<*> mode <| Option(key: "compiler-log-path", defaultValue: "",
usage: "the path of the full xcodebuild log to use when linting AnalyzerRules")
<*> mode <| Option(key: "compile-commands", defaultValue: "",
usage: "the path of a compilation database to use when linting AnalyzerRules")
// This should go last to avoid eating other args
<*> mode <| pathsArgument(action: "analyze")
}
Expand Down
64 changes: 32 additions & 32 deletions Source/swiftlint/Helpers/CompilerArgumentsExtractor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,37 @@ struct CompilerArgumentsExtractor {

return parseCLIArguments(compilerInvocation)
}

/**
Filters compiler arguments from `xcodebuild` to something that SourceKit/Clang will accept.

- parameter args: Compiler arguments, as parsed from `xcodebuild`.

- returns: Filtered compiler arguments.
*/
static func filterCompilerArguments(_ args: [String]) -> [String] {
var args = args
args.append(contentsOf: ["-D", "DEBUG"])
var shouldContinueToFilterArguments = true
while shouldContinueToFilterArguments {
(args, shouldContinueToFilterArguments) = partiallyFilter(arguments: args)
}
return args.filter {
![
"-parseable-output",
"-incremental",
"-serialize-diagnostics",
"-emit-dependencies"
].contains($0)
}.map {
if $0 == "-O" {
return "-Onone"
} else if $0 == "-DNDEBUG=1" {
return "-DDEBUG=1"
}
return $0
}
}
}

// MARK: - Private
Expand Down Expand Up @@ -66,7 +97,7 @@ private func parseCLIArguments(_ string: String) -> [String] {
_ = scanner.scanString("\"")
didStart.toggle()
}
return filter(arguments:
return CompilerArgumentsExtractor.filterCompilerArguments(
str.trimmingCharacters(in: .whitespaces)
.replacingOccurrences(of: "\\ ", with: escapedSpacePlaceholder)
.components(separatedBy: " ")
Expand All @@ -92,37 +123,6 @@ private func partiallyFilter(arguments args: [String]) -> ([String], Bool) {
return (args, true)
}

/**
Filters compiler arguments from `xcodebuild` to something that SourceKit/Clang will accept.

- parameter args: Compiler arguments, as parsed from `xcodebuild`.

- returns: Filtered compiler arguments.
*/
private func filter(arguments args: [String]) -> [String] {
var args = args
args.append(contentsOf: ["-D", "DEBUG"])
var shouldContinueToFilterArguments = true
while shouldContinueToFilterArguments {
(args, shouldContinueToFilterArguments) = partiallyFilter(arguments: args)
}
return args.filter {
![
"-parseable-output",
"-incremental",
"-serialize-diagnostics",
"-emit-dependencies"
].contains($0)
}.map {
if $0 == "-O" {
return "-Onone"
} else if $0 == "-DNDEBUG=1" {
return "-DDEBUG=1"
}
return $0
}
}

private extension Array where Element == String {
/// Return the full list of compiler arguments, replacing any response files with their contents.
var expandingResponseFiles: [String] {
Expand Down
3 changes: 3 additions & 0 deletions Source/swiftlint/Helpers/LintOrAnalyzeCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ struct LintOrAnalyzeOptions {
let enableAllRules: Bool
let autocorrect: Bool
let compilerLogPath: String
let compileCommands: String

init(_ options: LintOptions) {
mode = .lint
Expand All @@ -160,6 +161,7 @@ struct LintOrAnalyzeOptions {
enableAllRules = options.enableAllRules
autocorrect = false
compilerLogPath = ""
compileCommands = ""
}

init(_ options: AnalyzeOptions) {
Expand All @@ -179,6 +181,7 @@ struct LintOrAnalyzeOptions {
enableAllRules = options.enableAllRules
autocorrect = options.autocorrect
compilerLogPath = options.compilerLogPath
compileCommands = options.compileCommands
}

var verb: String {
Expand Down
120 changes: 90 additions & 30 deletions Source/swiftlint/Helpers/LintableFilesVisitor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,29 @@ import Foundation
import SourceKittenFramework
import SwiftLintFramework

typealias File = String
typealias Arguments = [String]

enum CompilerInvocations {
case buildLog(compilerInvocations: [String])
case compilationDatabase(compileCommands: [File: Arguments])

func arguments(forFile path: String?) -> [String] {
return path.flatMap { path in
switch self {
case let .buildLog(compilerInvocations):
return CompilerArgumentsExtractor
.compilerArgumentsForFile(path, compilerInvocations: compilerInvocations)
case let .compilationDatabase(compileCommands):
return compileCommands[path]
}
} ?? []
}
}

enum LintOrAnalyzeModeWithCompilerArguments {
case lint
case analyze(allCompilerInvocations: [String])
case analyze(allCompilerInvocations: CompilerInvocations)
}

struct LintableFilesVisitor {
Expand Down Expand Up @@ -35,7 +55,7 @@ struct LintableFilesVisitor {
}

private init(paths: [String], action: String, useSTDIN: Bool, quiet: Bool, useScriptInputFiles: Bool,
forceExclude: Bool, cache: LinterCache?, compilerLogContents: String,
forceExclude: Bool, cache: LinterCache?, compilerInvocations: CompilerInvocations?,
block: @escaping (CollectedLinter) -> Void) {
self.paths = paths
self.action = action
Expand All @@ -45,46 +65,42 @@ struct LintableFilesVisitor {
self.forceExclude = forceExclude
self.cache = cache
self.parallel = true
if compilerLogContents.isEmpty {
self.mode = .lint
if let compilerInvocations = compilerInvocations {
self.mode = .analyze(allCompilerInvocations: compilerInvocations)
} else {
let allCompilerInvocations = CompilerArgumentsExtractor
.allCompilerInvocations(compilerLogs: compilerLogContents)
self.mode = .analyze(allCompilerInvocations: allCompilerInvocations)
self.mode = .lint
}
self.block = block
}

static func create(_ options: LintOrAnalyzeOptions, cache: LinterCache?, block: @escaping (CollectedLinter) -> Void)
-> Result<LintableFilesVisitor, CommandantError<()>> {
let compilerLogContents: String
let compilerInvocations: CompilerInvocations?
if options.mode == .lint {
compilerLogContents = ""
} else if let logContents = LintableFilesVisitor.compilerLogContents(logPath: options.compilerLogPath),
!logContents.isEmpty {
compilerLogContents = logContents
compilerInvocations = nil
} else {
return .failure(
.usageError(description: "Could not read compiler log at path: '\(options.compilerLogPath)'")
)
switch loadCompilerInvocations(options) {
case let .success(invocations):
compilerInvocations = invocations
case let .failure(error):
return .failure(error)
}
}

let visitor = LintableFilesVisitor(paths: options.paths, action: options.verb.bridge().capitalized,
useSTDIN: options.useSTDIN, quiet: options.quiet,
useScriptInputFiles: options.useScriptInputFiles,
forceExclude: options.forceExclude, cache: cache,
compilerLogContents: compilerLogContents, block: block)
compilerInvocations: compilerInvocations, block: block)
return .success(visitor)
}

func shouldSkipFile(atPath path: String?) -> Bool {
switch self.mode {
case .lint:
return false
case let .analyze(allCompilerInvocations):
let compilerArguments = path.flatMap {
CompilerArgumentsExtractor.compilerArgumentsForFile($0, compilerInvocations: allCompilerInvocations)
} ?? []
case let .analyze(compilerInvocations):
let compilerArguments = compilerInvocations.arguments(forFile: path)
return compilerArguments.isEmpty
}
}
Expand All @@ -93,25 +109,69 @@ struct LintableFilesVisitor {
switch self.mode {
case .lint:
return Linter(file: file, configuration: configuration, cache: cache)
case let .analyze(allCompilerInvocations):
let compilerArguments = file.path.flatMap {
CompilerArgumentsExtractor.compilerArgumentsForFile($0, compilerInvocations: allCompilerInvocations)
} ?? []
case let .analyze(compilerInvocations):
let compilerArguments = compilerInvocations.arguments(forFile: file.path)
return Linter(file: file, configuration: configuration, compilerArguments: compilerArguments)
}
}

private static func compilerLogContents(logPath: String) -> String? {
if logPath.isEmpty {
return nil
private static func loadCompilerInvocations(_ options: LintOrAnalyzeOptions)
-> Result<CompilerInvocations, CommandantError<()>> {
if !options.compilerLogPath.isEmpty {
let path = options.compilerLogPath
guard let compilerInvocations = self.loadLogCompilerInvocations(path) else {
return .failure(
.usageError(description: "Could not read compiler log at path: '\(path)'")
)
}

return .success(.buildLog(compilerInvocations: compilerInvocations))
} else if !options.compileCommands.isEmpty {
let path = options.compileCommands
guard let compileCommands = self.loadCompileCommands(path) else {
return .failure(
.usageError(description: "Could not read compilation database at path: '\(path)'")
)
}

return .success(.compilationDatabase(compileCommands: compileCommands))
}

if let data = FileManager.default.contents(atPath: logPath),
return .failure(.usageError(description: "Could not read compiler invocations"))
}

private static func loadLogCompilerInvocations(_ path: String) -> [String]? {
if let data = FileManager.default.contents(atPath: path),
let logContents = String(data: data, encoding: .utf8) {
return logContents
if logContents.isEmpty {
return nil
}

return CompilerArgumentsExtractor.allCompilerInvocations(compilerLogs: logContents)
}

print("couldn't read log file at path '\(logPath)'")
return nil
}

private static func loadCompileCommands(_ path: String) -> [String: [String]]? {
guard let jsonContents = FileManager.default.contents(atPath: path),
let object = try? JSONSerialization.jsonObject(with: jsonContents),
let compileDB = object as? [[String: Any]] else {
return nil
}

// Convert the compilation database to a dictionary, with source files as keys and compiler arguments as values.
Copy link
Contributor Author

@kastiglione kastiglione Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A note about this dictionary. It could use quite a lot of redundant memory for large modules. For each file in a module, there will be a single compiler invocation (which covers each file in the module). For example:

swiftc --yada --yada File1.swift File2.swift ... FileN.swift 

This command line will be copied N times, one for each path in the module. In theory, the same array could be shared for each file in the module. Array deduping (or string interning) would reduce the memory here.

Just wanted to make a note of this.

//
// Compilation databases are an array of dictionaries. Each dict has "file" and "arguments" keys.
return compileDB.reduce(into: [:]) { (commands: inout [File: Arguments], entry: [String: Any]) in
if let file = entry["file"] as? String, var arguments = entry["arguments"] as? [String] {
// Compilation databases include the compiler, but it's left out when sending to SourceKit.
if arguments.first == "swiftc" {
arguments.removeFirst()
}

commands[file] = CompilerArgumentsExtractor.filterCompilerArguments(arguments)
}
}
}
}