Skip to content

Commit

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

This reverts commit b66f7c7.
  • Loading branch information
jpsim committed Feb 26, 2021
1 parent 9182eda commit 3e04acd
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 @@ -102,6 +102,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 _0CustomRulesOnly: String { Dir.level0.stringByAppendingPathComponent("custom_rules_only.yml") }
static var _2: String { Dir.level2.stringByAppendingPathComponent(Configuration.defaultFileName) }
Expand Down Expand Up @@ -64,7 +64,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 _0CustomRulesOnly: Configuration { Configuration(configurationFiles: [Yml._0CustomRulesOnly]) }
static var _2: Configuration { Configuration(configurationFiles: [Yml._2]) }
Expand Down
40 changes: 38 additions & 2 deletions Tests/SwiftLintFrameworkTests/ConfigurationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,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 @@ -190,6 +192,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 @@ -281,7 +317,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 @@ -292,7 +328,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 3e04acd

Please sign in to comment.