diff --git a/CHANGELOG.md b/CHANGELOG.md index dc7fad79b7a..66b636a04b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,15 @@ ### Bug Fixes +* Honor `parent_config` and `child_config` keys in nested `.swiftlint.yml` + files. Previously these keys were silently dropped when the file was + discovered as a nested config during linting, causing per-directory + configs that pointed to a shared rule set (a common pattern for monorepos + and package fleets) to fall back to the parent's rules with no warning. + Nested configs are now resolved through the same `parent_config` / + `child_config` graph as the main config, with per-graph cycle detection. + [#3540](https://github.com/realm/SwiftLint/issues/3540) + * Fix `literal_expression_end_indentation` autocorrection deleting source code when the closing bracket of a multiline literal shares a line with the end of a multiline last element (e.g. `...))]`). The corrector assumed everything diff --git a/README.md b/README.md index 615db35016a..56a2b01dad4 100644 --- a/README.md +++ b/README.md @@ -1060,8 +1060,12 @@ main configuration). `.swiftlint.yml` files are only considered as a nested configuration if they have not been used to build the main configuration already (e. g. by having been referenced via something like `child_config: Folder/.swiftlint.yml`). -Also, `parent_config`/`child_config` specifications of nested configurations -are getting ignored because there's no sense to that. + +Nested configurations may use `parent_config` and `child_config` references the +same way as the main configuration — this is useful when several directories +should share a delta on top of the same upstream rule set, for example a +central `tests.yml` referenced from every `Tests/.swiftlint.yml` via +`parent_config:`. If one (or more) SwiftLint file(s) are explicitly specified via the `--config` parameter, that configuration will be treated as an override, no matter whether diff --git a/Source/SwiftLintFramework/Configuration/Configuration+Merging.swift b/Source/SwiftLintFramework/Configuration/Configuration+Merging.swift index f85bb282536..5742c371a66 100644 --- a/Source/SwiftLintFramework/Configuration/Configuration+Merging.swift +++ b/Source/SwiftLintFramework/Configuration/Configuration+Merging.swift @@ -95,10 +95,13 @@ extension Configuration { // Use self merged with the nested config that was found // iff that nested config has not already been used to build the main config - // Ignore parent_config / child_config specifications of nested configs + // Resolve parent_config / child_config of nested configs the same way as for the main + // config. Cycle detection is scoped per nested file by FileGraph, and the + // `!fileGraph.includesFile(atPath:)` guard above prevents re-merging a file already in + // the main config chain. var childConfiguration = Configuration( configurationFiles: [configurationSearchPath], - ignoreParentAndChildConfigs: true + ignoreParentAndChildConfigs: false ) childConfiguration.fileGraph = FileGraph(rootDirectory: directory) config = merged(withChild: childConfiguration, rootDirectory: rootDirectory) diff --git a/Tests/FileSystemAccessTests/ConfigurationTests+Mock.swift b/Tests/FileSystemAccessTests/ConfigurationTests+Mock.swift index a57bd06e302..11d01792780 100644 --- a/Tests/FileSystemAccessTests/ConfigurationTests+Mock.swift +++ b/Tests/FileSystemAccessTests/ConfigurationTests+Mock.swift @@ -25,6 +25,10 @@ enum Mock { static var childConfigCycle4: URL { level0.appending(path: "ChildConfig/Cycle4/") } static var parentConfigTest1: URL { level0.appending(path: "ParentConfig/Test1/") } static var parentConfigTest2: URL { level0.appending(path: "ParentConfig/Test2/") } + static var nestedParentConfig: URL { level0.appending(path: "NestedParentConfig/Test/") } + static var nestedParentConfigSub: URL { nestedParentConfig.appending(path: "Sub/") } + static var nestedParentConfigChild: URL { nestedParentConfig.appending(path: "Child/") } + static var nestedParentConfigChain: URL { nestedParentConfig.appending(path: "Chain/") } static var parentConfigCycle1: URL { level0.appending(path: "ParentConfig/Cycle1/") } static var parentConfigCycle2: URL { level0.appending(path: "ParentConfig/Cycle2/") } static var parentConfigCycle3: URL { level0.appending(path: "ParentConfig/Cycle3/") } @@ -56,6 +60,9 @@ enum Mock { } static var _3: URL { Dir.level3.appending(path: Configuration.defaultFileName) } static var nested: URL { Dir.nested.appending(path: Configuration.defaultFileName) } + static var nestedParentConfig: URL { + Dir.nestedParentConfig.appending(path: Configuration.defaultFileName) + } } // MARK: Swift File Paths @@ -65,6 +72,9 @@ enum Mock { static var _2: URL { Dir.level2.appending(path: "Level2.swift") } static var _3: URL { Dir.level3.appending(path: "Level3.swift") } static var nestedSub: URL { Dir.nestedSub.appending(path: "Sub.swift") } + static var nestedParentConfigSub: URL { Dir.nestedParentConfigSub.appending(path: "Sub.swift") } + static var nestedParentConfigChild: URL { Dir.nestedParentConfigChild.appending(path: "Child.swift") } + static var nestedParentConfigChain: URL { Dir.nestedParentConfigChain.appending(path: "Chain.swift") } } // MARK: Configurations @@ -84,5 +94,8 @@ enum Mock { } static var _3: Configuration { Configuration(configurationFiles: [Yml._3]) } static var nested: Configuration { Configuration(configurationFiles: [Yml.nested]) } + static var nestedParentConfig: Configuration { + Configuration(configurationFiles: [Yml.nestedParentConfig]) + } } } diff --git a/Tests/FileSystemAccessTests/ConfigurationTests.swift b/Tests/FileSystemAccessTests/ConfigurationTests.swift index 50a7aa5359b..e6fb1ea6be8 100644 --- a/Tests/FileSystemAccessTests/ConfigurationTests.swift +++ b/Tests/FileSystemAccessTests/ConfigurationTests.swift @@ -354,6 +354,7 @@ final class ConfigurationTests: SwiftLintTestCase { // swiftlint:disable:this ty excludeByPrefix: false) let filenames = paths.map(\.lastPathComponent).sorted() let expectedFilenames = [ + "Chain.swift", "Child.swift", "DirectoryLevel1.swift", "Level0.swift", "Level1.swift", "Level2.swift", "Level3.swift", "Main.swift", "Sub.swift", @@ -542,6 +543,7 @@ final class ConfigurationTests: SwiftLintTestCase { // swiftlint:disable:this ty "ChildConfig".url(), "ParentConfig".url(), "NestedConfig".url(), + "NestedParentConfig".url(), ] ) let paths = configuration.lintablePaths(inPath: URL.cwd, @@ -560,6 +562,7 @@ final class ConfigurationTests: SwiftLintTestCase { // swiftlint:disable:this ty "ChildConfig".url(), "ParentConfig".url(), "NestedConfig".url(), + "NestedParentConfig".url(), ] ) let paths = configuration.lintablePaths(inPath: URL.cwd, diff --git a/Tests/FileSystemAccessTests/GlobTests.swift b/Tests/FileSystemAccessTests/GlobTests.swift index 743f435e904..51c7fcf2f63 100644 --- a/Tests/FileSystemAccessTests/GlobTests.swift +++ b/Tests/FileSystemAccessTests/GlobTests.swift @@ -83,6 +83,9 @@ final class GlobTests: SwiftLintTestCase { "Level1/Level2/Level3/Level3.swift", "NestedConfig/Test/Main.swift", "NestedConfig/Test/Sub/Sub.swift", + "NestedParentConfig/Test/Chain/Chain.swift", + "NestedParentConfig/Test/Child/Child.swift", + "NestedParentConfig/Test/Sub/Sub.swift", ].map { mockPath.appending(path: $0) } let files = Glob.resolveGlob(mockPath.appending(path: "**/*.swift")) diff --git a/Tests/FileSystemAccessTests/MultipleConfigurationsTests.swift b/Tests/FileSystemAccessTests/MultipleConfigurationsTests.swift index 56a3a4281cf..9abd93e44a8 100644 --- a/Tests/FileSystemAccessTests/MultipleConfigurationsTests.swift +++ b/Tests/FileSystemAccessTests/MultipleConfigurationsTests.swift @@ -258,15 +258,54 @@ final class MultipleConfigurationsTests: SwiftLintTestCase { ) } - func testParentConfigIsIgnoredAsNestedConfiguration() { - // If a configuration has already been used to build the main config, - // it should not again be regarded as a nested config + func testNestedConfigAlreadyInMainGraphIsSkipped() { + // If a configuration file has already been used to build the main config (e.g. as a + // parent_config), discovering the same file as a nested config during linting must not + // re-merge it; the per-file configuration must equal the main config unchanged. XCTAssertEqual( Mock.Config.nested.configuration(for: SwiftLintFile(path: Mock.Swift.nestedSub)!), Mock.Config.nested ) } + // MARK: - Nested Configs with parent_config / child_config (issue #3540) + func testNestedConfigHonorsParentConfig() { + // A nested .swiftlint.yml may reference a parent_config. The resolved per-file + // configuration must equal "main merged with the parent's contents on top". + XCTAssert(FileManager.default.changeCurrentDirectoryPath(Mock.Dir.nestedParentConfig.filepath)) + let mainConfig = Configuration(configurationFiles: []) + let actual = mainConfig.configuration(for: SwiftLintFile(path: Mock.Swift.nestedParentConfigSub)!) + let expected = Configuration( + configurationFiles: [".swiftlint.yml".url(), "refinements.yml".url()] + ) + assertEqualExceptForFileGraph(actual, expected) + } + + func testNestedConfigHonorsChildConfig() { + // A nested .swiftlint.yml may reference a child_config. The child wins over the + // nested file itself, then the combined result is merged on top of the main config. + XCTAssert(FileManager.default.changeCurrentDirectoryPath(Mock.Dir.nestedParentConfig.filepath)) + let mainConfig = Configuration(configurationFiles: []) + let actual = mainConfig.configuration(for: SwiftLintFile(path: Mock.Swift.nestedParentConfigChild)!) + let expected = Configuration( + configurationFiles: [".swiftlint.yml".url(), "child_refinements.yml".url()] + ) + assertEqualExceptForFileGraph(actual, expected) + } + + func testNestedConfigParentChain() { + // A nested .swiftlint.yml whose parent_config itself has a parent_config: the full + // chain must be merged in order (deepest ancestor first) before being applied on top + // of the main config. + XCTAssert(FileManager.default.changeCurrentDirectoryPath(Mock.Dir.nestedParentConfig.filepath)) + let mainConfig = Configuration(configurationFiles: []) + let actual = mainConfig.configuration(for: SwiftLintFile(path: Mock.Swift.nestedParentConfigChain)!) + let expected = Configuration( + configurationFiles: [".swiftlint.yml".url(), "chain_base.yml".url(), "chain_refinements.yml".url()] + ) + assertEqualExceptForFileGraph(actual, expected) + } + // MARK: - Child & Parent Configs func testValidChildConfig() { guard !isRunningWithBazel else { diff --git a/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/.swiftlint.yml b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/.swiftlint.yml new file mode 100644 index 00000000000..1c65b0389e1 --- /dev/null +++ b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/.swiftlint.yml @@ -0,0 +1,3 @@ +line_length: 80 +disabled_rules: + - todo diff --git a/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Chain/.swiftlint.yml b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Chain/.swiftlint.yml new file mode 100644 index 00000000000..b0e9b6cdf2e --- /dev/null +++ b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Chain/.swiftlint.yml @@ -0,0 +1 @@ +parent_config: ../chain_refinements.yml diff --git a/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Chain/Chain.swift b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Chain/Chain.swift new file mode 100644 index 00000000000..ec5b5427bbe --- /dev/null +++ b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Chain/Chain.swift @@ -0,0 +1 @@ +// Empty placeholder for nested-config lookup tests. diff --git a/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Child/.swiftlint.yml b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Child/.swiftlint.yml new file mode 100644 index 00000000000..d5e9b0aa1a1 --- /dev/null +++ b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Child/.swiftlint.yml @@ -0,0 +1 @@ +child_config: ../child_refinements.yml diff --git a/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Child/Child.swift b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Child/Child.swift new file mode 100644 index 00000000000..ec5b5427bbe --- /dev/null +++ b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Child/Child.swift @@ -0,0 +1 @@ +// Empty placeholder for nested-config lookup tests. diff --git a/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Sub/.swiftlint.yml b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Sub/.swiftlint.yml new file mode 100644 index 00000000000..35fc63913d9 --- /dev/null +++ b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Sub/.swiftlint.yml @@ -0,0 +1 @@ +parent_config: ../refinements.yml diff --git a/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Sub/Sub.swift b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Sub/Sub.swift new file mode 100644 index 00000000000..ec5b5427bbe --- /dev/null +++ b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/Sub/Sub.swift @@ -0,0 +1 @@ +// Empty placeholder for nested-config lookup tests. diff --git a/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/chain_base.yml b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/chain_base.yml new file mode 100644 index 00000000000..6ebb7e411a1 --- /dev/null +++ b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/chain_base.yml @@ -0,0 +1 @@ +line_length: 500 diff --git a/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/chain_refinements.yml b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/chain_refinements.yml new file mode 100644 index 00000000000..046ddb0d6d3 --- /dev/null +++ b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/chain_refinements.yml @@ -0,0 +1,2 @@ +parent_config: chain_base.yml +line_length: 300 diff --git a/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/child_refinements.yml b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/child_refinements.yml new file mode 100644 index 00000000000..75749613fc1 --- /dev/null +++ b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/child_refinements.yml @@ -0,0 +1 @@ +line_length: 250 diff --git a/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/refinements.yml b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/refinements.yml new file mode 100644 index 00000000000..ce39befc6d7 --- /dev/null +++ b/Tests/FileSystemAccessTests/Resources/ProjectMock/NestedParentConfig/Test/refinements.yml @@ -0,0 +1 @@ +line_length: 200