Compare commits

...

7 Commits

Author SHA1 Message Date
JP Simard ddcde3faf1
fixup! Fall back to file system traversal if config files changed 2021-03-08 20:49:26 -05:00
JP Simard adc92db7b6
Fall back to file system traversal if config files changed 2021-03-08 18:02:54 -05:00
JP Simard b5421db0f5
Move specifying git revision from config to CLI 2021-03-08 18:02:53 -05:00
JP Simard 204c32481e
Fix typos 2021-03-08 18:00:47 -05:00
JP Simard 6b59bf0d3e
Docs & dead code 2021-03-08 18:00:47 -05:00
JP Simard e996f50c05
Lint all files changed compared to the merge-base
Between HEAD and the stable git revision. Also include untracked files.
2021-03-08 18:00:47 -05:00
JP Simard 6483fc142a
Add "stable git revision" to configuration
This allows specifying a git revision or "commit-ish" that is considered
stable. If specified, SwifLint will attempt to query the git repository
index for files changed since that revision, using only those files as
input as opposed to traversing the file system to collect lintable
files.
2021-03-08 18:00:47 -05:00
13 changed files with 294 additions and 74 deletions

View File

@ -11,7 +11,12 @@
#### Enhancements
* None.
* Add `--stable-git-revision` option to the lint command. This allows
specifying a git revision or "commit-ish" that is considered stable.
If specified, SwiftLint will attempt to query the git repository index
for files changed since that revision, using only those files as input
as opposed to traversing the file system to collect lintable files.
[jpsim](https://github.com/jpsim)
#### Bug Fixes

View File

@ -8,12 +8,15 @@ extension Configuration {
/// file.
/// - parameter forceExclude: Whether or not excludes defined in this configuration should be applied even if
/// `path` is an exact match.
/// - parameter fileManager: The lintable file manager to use to search for lintable files.
/// - parameter excludeByPrefix: Whether or not uses excluding by prefix algorithm.
///
/// - returns: Files to lint.
public func lintableFiles(inPath path: String, forceExclude: Bool,
fileManager: LintableFileManager,
excludeByPrefix: Bool = false) -> [SwiftLintFile] {
return lintablePaths(inPath: path, forceExclude: forceExclude, excludeByPrefix: excludeByPrefix)
return lintablePaths(inPath: path, forceExclude: forceExclude, excludeByPrefix: excludeByPrefix,
fileManager: fileManager)
.compactMap(SwiftLintFile.init(pathDeferringReading:))
}
@ -31,7 +34,7 @@ extension Configuration {
inPath path: String,
forceExclude: Bool,
excludeByPrefix: Bool = false,
fileManager: LintableFileManager = FileManager.default
fileManager: LintableFileManager
) -> [String] {
// If path is a file and we're not forcing excludes, skip filtering with excluded/included paths
if path.isFile && !forceExclude { return [path] }
@ -53,7 +56,7 @@ extension Configuration {
///
/// - returns: The input paths after removing the excluded paths.
public func filterExcludedPaths(
fileManager: LintableFileManager = FileManager.default,
fileManager: LintableFileManager,
in paths: [String]...
) -> [String] {
let allPaths = paths.flatMap { $0 }

View File

@ -1,45 +0,0 @@
import Foundation
/// An interface for enumerating files that can be linted by SwiftLint.
public protocol LintableFileManager {
/// Returns all files that can be linted in the specified path. If the path is relative, it will be appended to the
/// specified root path, or currentt working directory if no root directory is specified.
///
/// - parameter path: The path in which lintable files should be found.
/// - parameter rootDirectory: The parent directory for the specified path. If none is provided, the current working
/// directory will be used.
///
/// - returns: Files to lint.
func filesToLint(inPath path: String, rootDirectory: String?) -> [String]
/// Returns the date when the file at the specified path was last modified. Returns `nil` if the file cannot be
/// found or its last modification date cannot be determined.
///
/// - parameter path: The file whose modification date should be determined.
///
/// - returns: A date, if one was determined.
func modificationDate(forFileAtPath path: String) -> Date?
}
extension FileManager: LintableFileManager {
public func filesToLint(inPath path: String, rootDirectory: String? = nil) -> [String] {
let absolutePath = path.bridge()
.absolutePathRepresentation(rootDirectory: rootDirectory ?? currentDirectoryPath).bridge()
.standardizingPath
// if path is a file, it won't be returned in `enumerator(atPath:)`
if absolutePath.bridge().isSwiftFile() && absolutePath.isFile {
return [absolutePath]
}
return subpaths(atPath: absolutePath)?.parallelCompactMap { element -> String? in
guard element.bridge().isSwiftFile() else { return nil }
let absoluteElementPath = absolutePath.bridge().appendingPathComponent(element)
return absoluteElementPath.isFile ? absoluteElementPath : nil
} ?? []
}
public func modificationDate(forFileAtPath path: String) -> Date? {
return (try? attributesOfItem(atPath: path))?[.modificationDate] as? Date
}
}

View File

@ -0,0 +1,61 @@
import Foundation
/// Namespace for utilities to execute a child process.
enum Exec {
/// The result of running the child process.
struct Results {
/// The process's exit status.
let terminationStatus: Int32
/// The data from stdout and optionally stderr.
let data: Data
/// The `data` reinterpreted as a string with whitespace trimmed; `nil` for the empty string.
var string: String? {
let encoded = String(data: data, encoding: .utf8) ?? ""
let trimmed = encoded.trimmingCharacters(in: .whitespacesAndNewlines)
return trimmed.isEmpty ? nil : trimmed
}
}
/**
Run a command with arguments and return its output and exit status.
- parameter command: Absolute path of the command to run.
- parameter arguments: Arguments to pass to the command.
- parameter currentDirectory: Current directory for the command. By default
the parent process's current directory.
*/
static func run(_ command: String,
_ arguments: [String] = [],
currentDirectory: String = FileManager.default.currentDirectoryPath) -> Results {
let process = Process()
process.arguments = arguments
let pipe = Pipe()
process.standardOutput = pipe
do {
#if canImport(Darwin)
if #available(macOS 10.13, *) {
process.executableURL = URL(fileURLWithPath: command)
process.currentDirectoryURL = URL(fileURLWithPath: currentDirectory)
try process.run()
} else {
process.launchPath = command
process.currentDirectoryPath = currentDirectory
process.launch()
}
#else
process.executableURL = URL(fileURLWithPath: command)
process.currentDirectoryURL = URL(fileURLWithPath: currentDirectory)
try process.run()
#endif
} catch {
return Results(terminationStatus: -1, data: Data())
}
let file = pipe.fileHandleForReading
let data = file.readDataToEndOfFile()
process.waitUntilExit()
return Results(terminationStatus: process.terminationStatus, data: data)
}
}

View File

@ -0,0 +1,151 @@
import Foundation
// MARK: - Protocol
/// An interface for enumerating files that can be linted by SwiftLint.
public protocol LintableFileManager {
/// Returns all files that can be linted in the specified path. If the path is relative, it will be appended to the
/// specified root path, or currentt working directory if no root directory is specified.
///
/// - parameter path: The path in which lintable files should be found.
/// - parameter rootDirectory: The parent directory for the specified path. If none is provided, the current working
/// directory will be used.
///
/// - returns: Files to lint.
func filesToLint(inPath path: String, rootDirectory: String?) -> [String]
/// Returns the date when the file at the specified path was last modified. Returns `nil` if the file cannot be
/// found or its last modification date cannot be determined.
///
/// - parameter path: The file whose modification date should be determined.
///
/// - returns: A date, if one was determined.
func modificationDate(forFileAtPath path: String) -> Date?
}
// MARK: - FileManager Conformance
extension FileManager: LintableFileManager {
public func filesToLint(inPath path: String, rootDirectory: String? = nil) -> [String] {
let absolutePath = path.bridge()
.absolutePathRepresentation(rootDirectory: rootDirectory ?? currentDirectoryPath).bridge()
.standardizingPath
// if path is a file, it won't be returned in `enumerator(atPath:)`
if absolutePath.bridge().isSwiftFile() && absolutePath.isFile {
return [absolutePath]
}
return subpaths(atPath: absolutePath)?.parallelCompactMap { element -> String? in
guard element.bridge().isSwiftFile() else { return nil }
let absoluteElementPath = absolutePath.bridge().appendingPathComponent(element)
return absoluteElementPath.isFile ? absoluteElementPath : nil
} ?? []
}
public func modificationDate(forFileAtPath path: String) -> Date? {
return (try? attributesOfItem(atPath: path))?[.modificationDate] as? Date
}
}
// MARK: - GitLintableFileManager
/// A producer of lintable files that compares against a stable git revision.
public class GitLintableFileManager {
private let stableRevision: String
private let explicitConfigurationPaths: [String]
/// Creates a `GitLintableFileManager` with the specified stable revision.
///
/// - parameter stableRevision: The stable git revision to compare lintable files against.
/// - parameter explicitConfigurationPaths: The explicit configuration file paths specified by the user.
public init(stableRevision: String, explicitConfigurationPaths: [String]) {
self.stableRevision = stableRevision
self.explicitConfigurationPaths = explicitConfigurationPaths
}
}
extension GitLintableFileManager: LintableFileManager {
/// Returns all files that can be linted in the specified path. If the path is relative, it will be appended to the
/// specified root path, or current working directory if no root directory is specified.
///
/// - parameter path: The path in which lintable files should be found.
/// - parameter rootDirectory: The parent directory for the specified path. If none is provided, the current working
/// directory will be used.
///
/// - returns: Files to lint.
public func filesToLint(inPath path: String, rootDirectory: String? = nil) -> [String] {
func git(_ args: [String]) -> [String]? {
let out = Exec.run("/usr/bin/env", ["git"] + args)
if out.terminationStatus == 0 {
return out.string?.components(separatedBy: .newlines) ?? []
} else {
return nil
}
}
func fallback(reason: GitFallbackReason) -> [String] {
queuedPrintError(
"\(reason.description) from specified stable git revision. Falling back to file system traversal."
)
return FileManager.default.filesToLint(inPath: path, rootDirectory: rootDirectory)
}
guard let mergeBase = git(["merge-base", stableRevision, "HEAD"])?.first else {
return fallback(reason: .mergeBaseNotFound)
}
func allFilesChanged(filters: [String]) -> [String]? {
guard let changed = git(["diff", "--name-only", "--diff-filter=AMRCU", mergeBase, "--", path] + filters),
let untracked = git(["ls-files", "--others", "--exclude-standard", "--", path] + filters)
else {
return nil
}
return changed + untracked
}
if let configurationFiles = allFilesChanged(filters: ["'*.swiftlint.yml'"] + explicitConfigurationPaths),
!configurationFiles.isEmpty {
return fallback(reason: .configChanged)
}
guard let filesToLint = allFilesChanged(filters: ["'*.swift'"]) else {
return fallback(reason: .filesChangedNotFound)
}
return filesToLint.compactMap { relativePath in
relativePath.bridge()
.absolutePathRepresentation(
rootDirectory: rootDirectory ?? FileManager.default.currentDirectoryPath
)
.bridge()
.standardizingPath
}
}
/// Returns the date when the file at the specified path was last modified. Returns `nil` if the file cannot be
/// found or its last modification date cannot be determined.
///
/// - parameter path: The file whose modification date should be determined.
///
/// - returns: A date, if one was determined.
public func modificationDate(forFileAtPath path: String) -> Date? {
return FileManager.default.modificationDate(forFileAtPath: path)
}
}
private enum GitFallbackReason {
case mergeBaseNotFound, configChanged, filesChangedNotFound
var description: String {
switch self {
case .mergeBaseNotFound:
return "Merge base not found"
case .configChanged:
return "Configuration files changed"
case .filesChangedNotFound:
return "Could not get changed files"
}
}
}

View File

@ -84,7 +84,7 @@ public struct Configuration {
/// Creates a Configuration by copying an existing configuration.
///
/// - parameter copying: The existing configuration to copy.
/// - parameter copying: The existing configuration to copy.
internal init(copying configuration: Configuration) {
rulesWrapper = configuration.rulesWrapper
fileGraph = configuration.fileGraph

View File

@ -44,7 +44,8 @@ extension SwiftLint {
enableAllRules: false,
autocorrect: common.fix,
compilerLogPath: compilerLogPath,
compileCommands: compileCommands
compileCommands: compileCommands,
stableGitRevision: nil
)
let result = LintOrAnalyzeCommand.run(options)

View File

@ -18,6 +18,14 @@ extension SwiftLint {
var noCache = false
@Flag(help: "Run all rules, even opt-in and disabled ones, ignoring `only_rules`.")
var enableAllRules = false
@Option(help:
"""
A git revision or "commit-ish" that is considered stable. If specified, SwiftLint will attempt to query \
the git repository index for files changed since that revision, using only those files as input as opposed \
to traversing the file system to collect lintable files.
"""
)
var stableGitRevision: String?
@Argument(help: pathsArgumentDescription(for: .lint))
var paths = [String]()
@ -48,7 +56,8 @@ extension SwiftLint {
enableAllRules: enableAllRules,
autocorrect: common.fix,
compilerLogPath: nil,
compileCommands: nil
compileCommands: nil,
stableGitRevision: stableGitRevision
)
let result = LintOrAnalyzeCommand.run(options)
switch result {

View File

@ -212,6 +212,16 @@ extension Configuration {
}
fileprivate func getFiles(with visitor: LintableFilesVisitor) -> Result<[SwiftLintFile], SwiftLintError> {
let fileManager: LintableFileManager
if let stableGitRevision = visitor.stableGitRevision {
fileManager = GitLintableFileManager(
stableRevision: stableGitRevision,
explicitConfigurationPaths: visitor.explicitConfigurationPaths
)
} else {
fileManager = FileManager.default
}
if visitor.useSTDIN {
let stdinData = FileHandle.standardInput.readDataToEndOfFile()
if let stdinString = String(data: stdinData, encoding: .utf8) {
@ -228,7 +238,7 @@ extension Configuration {
let scriptInputPaths = files.compactMap { $0.path }
let filesToLint = visitor.useExcludingByPrefix
? filterExcludedPathsByPrefix(in: scriptInputPaths)
: filterExcludedPaths(in: scriptInputPaths)
: filterExcludedPaths(fileManager: fileManager, in: scriptInputPaths)
return filesToLint.map(SwiftLintFile.init(pathDeferringReading:))
}
}
@ -240,10 +250,14 @@ extension Configuration {
filesInfo = "at paths \(visitor.paths.joined(separator: ", "))"
}
queuedPrintError("\(visitor.action) Swift files \(filesInfo)")
let gitInfo = visitor.stableGitRevision
.map { " that have changed compared to \($0)" }
?? ""
queuedPrintError("\(visitor.action) Swift files \(filesInfo)\(gitInfo)")
}
return .success(visitor.paths.flatMap {
self.lintableFiles(inPath: $0, forceExclude: visitor.forceExclude,
self.lintableFiles(inPath: $0, forceExclude: visitor.forceExclude, fileManager: fileManager,
excludeByPrefix: visitor.useExcludingByPrefix)
})
}
@ -251,12 +265,13 @@ extension Configuration {
func visitLintableFiles(options: LintOrAnalyzeOptions, cache: LinterCache? = nil, storage: RuleStorage,
visitorBlock: @escaping (CollectedLinter) -> Void)
-> Result<[SwiftLintFile], SwiftLintError> {
return LintableFilesVisitor.create(options,
cache: cache,
allowZeroLintableFiles: allowZeroLintableFiles,
block: visitorBlock).flatMap({ visitor in
visitLintableFiles(with: visitor, storage: storage)
})
return LintableFilesVisitor.create(
options,
cache: cache,
allowZeroLintableFiles: allowZeroLintableFiles || options.stableGitRevision != nil,
block: visitorBlock
)
.flatMap { visitLintableFiles(with: $0, storage: storage) }
}
// MARK: LintOrAnalyze Command

View File

@ -193,6 +193,7 @@ struct LintOrAnalyzeOptions {
let autocorrect: Bool
let compilerLogPath: String?
let compileCommands: String?
let stableGitRevision: String?
var verb: String {
if autocorrect {

View File

@ -48,13 +48,16 @@ struct LintableFilesVisitor {
let cache: LinterCache?
let parallel: Bool
let allowZeroLintableFiles: Bool
let stableGitRevision: String?
let explicitConfigurationPaths: [String]
let mode: LintOrAnalyzeModeWithCompilerArguments
let block: (CollectedLinter) -> Void
init(paths: [String], action: String, useSTDIN: Bool,
quiet: Bool, useScriptInputFiles: Bool, forceExclude: Bool, useExcludingByPrefix: Bool,
cache: LinterCache?, parallel: Bool,
allowZeroLintableFiles: Bool, block: @escaping (CollectedLinter) -> Void) {
allowZeroLintableFiles: Bool, stableGitRevision: String?, explicitConfigurationPaths: [String],
block: @escaping (CollectedLinter) -> Void) {
self.paths = resolveParamsFiles(args: paths)
self.action = action
self.useSTDIN = useSTDIN
@ -66,13 +69,16 @@ struct LintableFilesVisitor {
self.parallel = parallel
self.mode = .lint
self.allowZeroLintableFiles = allowZeroLintableFiles
self.stableGitRevision = stableGitRevision
self.explicitConfigurationPaths = explicitConfigurationPaths
self.block = block
}
private init(paths: [String], action: String, useSTDIN: Bool, quiet: Bool,
useScriptInputFiles: Bool, forceExclude: Bool, useExcludingByPrefix: Bool,
cache: LinterCache?, compilerInvocations: CompilerInvocations?,
allowZeroLintableFiles: Bool, block: @escaping (CollectedLinter) -> Void) {
allowZeroLintableFiles: Bool, stableGitRevision: String?, explicitConfigurationPaths: [String],
block: @escaping (CollectedLinter) -> Void) {
self.paths = resolveParamsFiles(args: paths)
self.action = action
self.useSTDIN = useSTDIN
@ -89,6 +95,8 @@ struct LintableFilesVisitor {
}
self.block = block
self.allowZeroLintableFiles = allowZeroLintableFiles
self.stableGitRevision = stableGitRevision
self.explicitConfigurationPaths = explicitConfigurationPaths
}
static func create(_ options: LintOrAnalyzeOptions,
@ -116,7 +124,10 @@ struct LintableFilesVisitor {
useExcludingByPrefix: options.useExcludingByPrefix,
cache: cache,
compilerInvocations: compilerInvocations,
allowZeroLintableFiles: allowZeroLintableFiles, block: block)
allowZeroLintableFiles: allowZeroLintableFiles,
stableGitRevision: options.stableGitRevision,
explicitConfigurationPaths: options.configurationFiles,
block: block)
return .success(visitor)
}
}

View File

@ -280,7 +280,8 @@ class ConfigurationTests: XCTestCase {
}
func testLintablePaths() {
let paths = Configuration.default.lintablePaths(inPath: Mock.Dir.level0, forceExclude: false)
let paths = Configuration.default.lintablePaths(inPath: Mock.Dir.level0, forceExclude: false,
fileManager: FileManager.default)
let filenames = paths.map { $0.bridge().lastPathComponent }.sorted()
let expectedFilenames = [
"DirectoryLevel1.swift",
@ -297,7 +298,8 @@ class ConfigurationTests: XCTestCase {
excludedPaths: [Mock.Dir.level3.stringByAppendingPathComponent("*.swift")]
)
XCTAssertEqual(configuration.lintablePaths(inPath: "", forceExclude: false), [])
XCTAssertEqual(configuration.lintablePaths(inPath: "", forceExclude: false, fileManager: FileManager.default),
[])
}
// MARK: - Testing Configuration Equality
@ -385,7 +387,8 @@ extension ConfigurationTests {
"Level1/Level2/Level3"])
let paths = configuration.lintablePaths(inPath: Mock.Dir.level0,
forceExclude: false,
excludeByPrefix: true)
excludeByPrefix: true,
fileManager: FileManager.default)
let filenames = paths.map { $0.bridge().lastPathComponent }
XCTAssertEqual(filenames, ["Level2.swift"])
}
@ -395,7 +398,8 @@ extension ConfigurationTests {
let configuration = Configuration(excludedPaths: ["Level1/Level2/Level3/Level3.swift"])
let paths = configuration.lintablePaths(inPath: "Level1/Level2/Level3/Level3.swift",
forceExclude: true,
excludeByPrefix: true)
excludeByPrefix: true,
fileManager: FileManager.default)
XCTAssertEqual([], paths)
}
@ -405,7 +409,8 @@ extension ConfigurationTests {
excludedPaths: ["Level1/Level1.swift"])
let paths = configuration.lintablePaths(inPath: "Level1",
forceExclude: true,
excludeByPrefix: true)
excludeByPrefix: true,
fileManager: FileManager.default)
let filenames = paths.map { $0.bridge().lastPathComponent }.sorted()
XCTAssertEqual(["Level2.swift", "Level3.swift"], filenames)
}
@ -419,7 +424,8 @@ extension ConfigurationTests {
)
let paths = configuration.lintablePaths(inPath: ".",
forceExclude: true,
excludeByPrefix: true)
excludeByPrefix: true,
fileManager: FileManager.default)
let filenames = paths.map { $0.bridge().lastPathComponent }.sorted()
XCTAssertEqual(["Level0.swift", "Level1.swift"], filenames)
}
@ -433,7 +439,8 @@ extension ConfigurationTests {
)
let paths = configuration.lintablePaths(inPath: ".",
forceExclude: true,
excludeByPrefix: true)
excludeByPrefix: true,
fileManager: FileManager.default)
let filenames = paths.map { $0.bridge().lastPathComponent }
XCTAssertEqual(["Level0.swift"], filenames)
}
@ -445,7 +452,8 @@ extension ConfigurationTests {
excludedPaths: ["Level1/**/*.swift", "Level1/**/**/*.swift"])
let paths = configuration.lintablePaths(inPath: "Level1",
forceExclude: false,
excludeByPrefix: true)
excludeByPrefix: true,
fileManager: FileManager.default)
let filenames = paths.map { $0.bridge().lastPathComponent }.sorted()
XCTAssertEqual(filenames, ["Level1.swift"])
}

View File

@ -15,7 +15,7 @@ private let config: Configuration = {
class IntegrationTests: XCTestCase {
func testSwiftLintLints() {
// This is as close as we're ever going to get to a self-hosting linter.
let swiftFiles = config.lintableFiles(inPath: "", forceExclude: false)
let swiftFiles = config.lintableFiles(inPath: "", forceExclude: false, fileManager: FileManager.default)
XCTAssert(swiftFiles.contains(where: { #file == $0.path }), "current file should be included")
let storage = RuleStorage()
@ -30,7 +30,7 @@ class IntegrationTests: XCTestCase {
}
func testSwiftLintAutoCorrects() {
let swiftFiles = config.lintableFiles(inPath: "", forceExclude: false)
let swiftFiles = config.lintableFiles(inPath: "", forceExclude: false, fileManager: FileManager.default)
let storage = RuleStorage()
let corrections = swiftFiles.parallelFlatMap {
Linter(file: $0, configuration: config).collect(into: storage).correct(using: storage)