Skip to content

Commit

Permalink
Fix issue that prevented the inclusion of a configuration file from a…
Browse files Browse the repository at this point in the history
… parent folder (#3491)

* Add (currently failing) test for #3485

* [#3485] Add missing rootDir adjustment for included / excluded paths

* Add changelog entry

* Adjust tests for relative included / excluded paths
  • Loading branch information
fredpi authored and sethfri committed Feb 24, 2021
1 parent 8f216b8 commit ef09c50
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 11 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@

* Fix typos in configuration options for `file_name` rule.
[advantis](https://github.com/advantis)

* Fix issue that prevented the inclusion of a configuration file from a parent folder.
[Frederick Pietschmann](https://github.com/fredpi)
[#3485](https://github.com/realm/SwiftLint/issues/3485)

* Fix violation location and misplaced corrections for some function
references in `explicit_self` rule.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,15 @@ internal extension Configuration {
dict: firstConfigurationData.configurationDict,
enableAllRules: enableAllRules
)
firstConfiguration.fileGraph = FileGraph(rootDirectory: firstConfigurationData.rootDirectory)

// Set the config's rootDirectory to rootDirectory (+ adjust included / excluded paths that relate to it).
// firstConfigurationData.rootDirectory may be different from rootDirectory,
// e. g. when ../file.yml is passed as the first config
firstConfiguration.fileGraph = FileGraph(rootDirectory: rootDirectory)
firstConfiguration.makeIncludedAndExcludedPaths(
relativeTo: rootDirectory,
previousBasePath: firstConfigurationData.rootDirectory
)

// Build succeeding configurations
return try configurationData.reduce(firstConfiguration) {
Expand Down
25 changes: 19 additions & 6 deletions Source/SwiftLintFramework/Models/Configuration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ public struct Configuration {

// MARK: Public Instance
/// The paths that should be included when linting
public let includedPaths: [String]
public private(set) var includedPaths: [String]

/// The paths that should be excluded when linting
public let excludedPaths: [String]
public private(set) var excludedPaths: [String]

/// The style to use when indenting Swift source code.
public let indentation: IndentationStyle
Expand Down Expand Up @@ -46,7 +46,8 @@ public struct Configuration {

/// The root directory is the directory that included & excluded paths relate to.
/// By default, the root directory is the current working directory,
/// but in some merging algorithms it is used differently.
/// but during some merging algorithms it may be used differently.
/// The rootDirectory also serves as the stopping point when searching for nested configs along the file hierarchy.
public var rootDirectory: String { fileGraph.rootDirectory }

/// The rules mode used for this configuration.
Expand Down Expand Up @@ -190,11 +191,11 @@ public struct Configuration {
let configurationFiles = configurationFiles.isEmpty ? [Configuration.defaultFileName] : configurationFiles
defer { basedOnCustomConfigurationFiles = hasCustomConfigurationFiles }

let rootDirectory = FileManager.default.currentDirectoryPath.bridge().absolutePathStandardized()
let currentWorkingDirectory = FileManager.default.currentDirectoryPath.bridge().absolutePathStandardized()
let rulesMode: RulesMode = enableAllRules ? .allEnabled : .default(disabled: [], optIn: [])

// Try obtaining cached config
let cacheIdentifier = "\(rootDirectory) - \(configurationFiles)"
let cacheIdentifier = "\(currentWorkingDirectory) - \(configurationFiles)"
if let cachedConfig = Configuration.getCached(forIdentifier: cacheIdentifier) {
self.init(copying: cachedConfig)
return
Expand All @@ -204,7 +205,7 @@ public struct Configuration {
do {
var fileGraph = FileGraph(
commandLineChildConfigs: configurationFiles,
rootDirectory: rootDirectory,
rootDirectory: currentWorkingDirectory,
ignoreParentAndChildConfigs: ignoreParentAndChildConfigs
)
let resultingConfiguration = try fileGraph.resultingConfiguration(enableAllRules: enableAllRules)
Expand Down Expand Up @@ -237,8 +238,20 @@ public struct Configuration {
}
}
}

// MARK: - Methods: Internal
mutating func makeIncludedAndExcludedPaths(relativeTo newBasePath: String, previousBasePath: String) {
includedPaths = includedPaths.map {
$0.bridge().absolutePathRepresentation(rootDirectory: previousBasePath).path(relativeTo: newBasePath)
}

excludedPaths = excludedPaths.map {
$0.bridge().absolutePathRepresentation(rootDirectory: previousBasePath).path(relativeTo: newBasePath)
}
}
}

// MARK: - FileGraphInitializationResult
private enum FileGraphInitializationResult {
case initialImplicitFileNotFound
case error(message: String)
Expand Down
4 changes: 2 additions & 2 deletions Tests/SwiftLintFrameworkTests/ConfigurationTests+Mock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ internal extension ConfigurationTests {
// MARK: YAML File Paths
enum Yml {
static var _0: String { Dir.level0.stringByAppendingPathComponent(Configuration.defaultFileName) }
static var _0CustomPath: String { Dir.level0.stringByAppendingPathComponent("custom.yml") }
static var _0Custom: String { Dir.level0.stringByAppendingPathComponent("custom.yml") }
static var _0CustomRules: String { Dir.level0.stringByAppendingPathComponent("custom_rules.yml") }
static var _2: String { Dir.level2.stringByAppendingPathComponent(Configuration.defaultFileName) }
static var _2CustomRules: String { Dir.level2.stringByAppendingPathComponent("custom_rules.yml") }
Expand All @@ -59,7 +59,7 @@ internal extension ConfigurationTests {
// MARK: Configurations
enum Config {
static var _0: Configuration { Configuration(configurationFiles: []) }
static var _0CustomPath: Configuration { Configuration(configurationFiles: [Yml._0CustomPath]) }
static var _0Custom: Configuration { Configuration(configurationFiles: [Yml._0Custom]) }
static var _0CustomRules: Configuration { Configuration(configurationFiles: [Yml._0CustomRules]) }
static var _2: Configuration { Configuration(configurationFiles: [Yml._2]) }
static var _2CustomRules: Configuration { Configuration(configurationFiles: [Yml._2CustomRules]) }
Expand Down
40 changes: 38 additions & 2 deletions Tests/SwiftLintFrameworkTests/ConfigurationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import XCTest

private let optInRules = primaryRuleList.list.filter({ $0.1.init() is OptInRule }).map({ $0.0 })

// swiftlint:disable file_length type_body_length

class ConfigurationTests: XCTestCase {
// MARK: Setup & Teardown
private var previousWorkingDir: String!
Expand Down Expand Up @@ -168,6 +170,40 @@ class ConfigurationTests: XCTestCase {
)
}

func testIncludedExcludedRelativeLocationLevel1() {
FileManager.default.changeCurrentDirectoryPath(Mock.Dir.level1)

// The included path "File.swift" should be put relative to the configuration file
// (~> Resources/ProjectMock/File.swift) and not relative to the path where
// SwiftLint is run from (~> Resources/ProjectMock/Level1/File.swift)
let configuration = Configuration(configurationFiles: ["../custom_included_excluded.yml"])
let actualIncludedPath = configuration.includedPaths.first!.bridge()
.absolutePathRepresentation(rootDirectory: configuration.rootDirectory)
let desiredIncludedPath = "File1.swift".absolutePathRepresentation(rootDirectory: Mock.Dir.level0)
let actualExcludedPath = configuration.excludedPaths.first!.bridge()
.absolutePathRepresentation(rootDirectory: configuration.rootDirectory)
let desiredExcludedPath = "File2.swift".absolutePathRepresentation(rootDirectory: Mock.Dir.level0)

XCTAssertEqual(actualIncludedPath, desiredIncludedPath)
XCTAssertEqual(actualExcludedPath, desiredExcludedPath)
}

func testIncludedExcludedRelativeLocationLevel0() {
// Same as testIncludedPathRelatedToConfigurationFileLocationLevel1(),
// but run from the directory the config file resides in
FileManager.default.changeCurrentDirectoryPath(Mock.Dir.level0)
let configuration = Configuration(configurationFiles: ["custom_included_excluded.yml"])
let actualIncludedPath = configuration.includedPaths.first!.bridge()
.absolutePathRepresentation(rootDirectory: configuration.rootDirectory)
let desiredIncludedPath = "File1.swift".absolutePathRepresentation(rootDirectory: Mock.Dir.level0)
let actualExcludedPath = configuration.excludedPaths.first!.bridge()
.absolutePathRepresentation(rootDirectory: configuration.rootDirectory)
let desiredExcludedPath = "File2.swift".absolutePathRepresentation(rootDirectory: Mock.Dir.level0)

XCTAssertEqual(actualIncludedPath, desiredIncludedPath)
XCTAssertEqual(actualExcludedPath, desiredExcludedPath)
}

private class TestFileManager: LintableFileManager {
func filesToLint(inPath path: String, rootDirectory: String? = nil) -> [String] {
switch path {
Expand Down Expand Up @@ -259,7 +295,7 @@ class ConfigurationTests: XCTestCase {
func testCustomConfiguration() {
let file = SwiftLintFile(path: Mock.Swift._0)!
XCTAssertNotEqual(Mock.Config._0.configuration(for: file),
Mock.Config._0CustomPath.configuration(for: file))
Mock.Config._0Custom.configuration(for: file))
}

func testConfigurationWithSwiftFileAsRoot() {
Expand All @@ -270,7 +306,7 @@ class ConfigurationTests: XCTestCase {
}

func testConfigurationWithSwiftFileAsRootAndCustomConfiguration() {
let configuration = Configuration(configurationFiles: [Mock.Yml._0CustomPath])
let configuration = Mock.Config._0Custom

let file = SwiftLintFile(path: Mock.Swift._0)!
XCTAssertEqual(configuration.configuration(for: file), configuration)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
included:
- "File1.swift"

excluded:
- "File2.swift"

0 comments on commit ef09c50

Please sign in to comment.