From 0cdd80d305bffc4654ec613ab3e76fca0d8c2a16 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Mon, 19 May 2025 19:00:33 +0900 Subject: [PATCH 01/23] Add validation to ensure settings.configs values are dictionaries to prevent misuse --- Sources/ProjectSpec/Project.swift | 12 +++++++++++- Sources/ProjectSpec/Settings.swift | 18 +++++++++++++++++- Sources/ProjectSpec/SpecParsingError.swift | 3 +++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/Sources/ProjectSpec/Project.swift b/Sources/ProjectSpec/Project.swift index 72daf0324..50183c2ed 100644 --- a/Sources/ProjectSpec/Project.swift +++ b/Sources/ProjectSpec/Project.swift @@ -173,7 +173,17 @@ extension Project { let jsonDictionary = Project.resolveProject(jsonDictionary: jsonDictionary) name = try jsonDictionary.json(atKeyPath: "name") - settings = jsonDictionary.json(atKeyPath: "settings") ?? .empty + + do { + settings = try jsonDictionary.json(atKeyPath: "settings") + } catch let specParsingError as SpecParsingError { + // Rethrow SpecParsingError to prevent misuse of settings.configs + throw specParsingError + } catch { + // Ignore any errors other than SpecParsingError + settings = .empty + } + settingGroups = jsonDictionary.json(atKeyPath: "settingGroups") ?? jsonDictionary.json(atKeyPath: "settingPresets") ?? [:] let configs: [String: String] = jsonDictionary.json(atKeyPath: "configs") ?? [:] diff --git a/Sources/ProjectSpec/Settings.swift b/Sources/ProjectSpec/Settings.swift index f59006a72..68888dc02 100644 --- a/Sources/ProjectSpec/Settings.swift +++ b/Sources/ProjectSpec/Settings.swift @@ -28,7 +28,23 @@ public struct Settings: Equatable, JSONObjectConvertible, CustomStringConvertibl groups = jsonDictionary.json(atKeyPath: "groups") ?? jsonDictionary.json(atKeyPath: "presets") ?? [] let buildSettingsDictionary: JSONDictionary = jsonDictionary.json(atKeyPath: "base") ?? [:] buildSettings = buildSettingsDictionary - configSettings = jsonDictionary.json(atKeyPath: "configs") ?? [:] + + guard let configSettings = jsonDictionary["configs"] as? JSONDictionary else { + configSettings = [:] + return + } + + let invalidConfigKeys = configSettings.reduce(into: Set()) { partialResult, config in + if !(config.value is JSONDictionary) { + partialResult.insert(config.key) + } + } + + guard invalidConfigKeys.isEmpty else { + throw SpecParsingError.invalidConfigsFormat(keys: invalidConfigKeys) + } + + self.configSettings = try jsonDictionary.json(atKeyPath: "configs") } else { buildSettings = jsonDictionary configSettings = [:] diff --git a/Sources/ProjectSpec/SpecParsingError.swift b/Sources/ProjectSpec/SpecParsingError.swift index b875eadce..2511aba71 100644 --- a/Sources/ProjectSpec/SpecParsingError.swift +++ b/Sources/ProjectSpec/SpecParsingError.swift @@ -15,6 +15,7 @@ public enum SpecParsingError: Error, CustomStringConvertible { case unknownBreakpointActionType(String) case unknownBreakpointActionConveyanceType(String) case unknownBreakpointActionSoundName(String) + case invalidConfigsFormat(keys: Set) public var description: String { switch self { @@ -46,6 +47,8 @@ public enum SpecParsingError: Error, CustomStringConvertible { return "Unknown Breakpoint Action conveyance type: \(type)" case let .unknownBreakpointActionSoundName(name): return "Unknown Breakpoint Action sound name: \(name)" + case let .invalidConfigsFormat(keys): + return "The value for \"\(keys.sorted().joined(separator: ", "))\" in configs must be a dictionary" } } } From 58cdf9b2e6834fb891ee7a490c75660fe254cc19 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Mon, 19 May 2025 19:04:55 +0900 Subject: [PATCH 02/23] Add tests for invalid settings.configs value formats --- .../invalid_configs_value_non_dict.yml | 8 +++++ .../InvalidConfigsFormatTests.swift | 35 +++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 Tests/Fixtures/invalid_configs_value_non_dict.yml create mode 100644 Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift diff --git a/Tests/Fixtures/invalid_configs_value_non_dict.yml b/Tests/Fixtures/invalid_configs_value_non_dict.yml new file mode 100644 index 000000000..dca38fc81 --- /dev/null +++ b/Tests/Fixtures/invalid_configs_value_non_dict.yml @@ -0,0 +1,8 @@ +name: InvalidConfigsNonDict + +settings: + configs: + invalid_key0: value0 + debug: + valid_key: value1 + invalid_key1: value2 diff --git a/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift new file mode 100644 index 000000000..ad1a639b5 --- /dev/null +++ b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift @@ -0,0 +1,35 @@ +import ProjectSpec +import Testing +import TestSupport +import PathKit + +struct InvalidConfigsFormatTests { + @Test("throws invalidConfigsFormat error for non-dictionary configs entries") + func testNonDictionaryConfigsEntries() throws { + let path = fixturePath + "invalid_configs_value_non_dict.yml" + let expectedError = SpecParsingError.invalidConfigsFormat(keys: ["invalid_key0", "invalid_key1"]) + + #expect(throws: EquatableErrorBox(expectedError)) { + try perform(path: path) + } + } + + private func perform(path: Path) throws { + do { + _ = try Project(path: path) + } catch let error as SpecParsingError { + throw EquatableErrorBox(error) + } catch { + throw error + } + } + + // SpecParsingError does not conform to Equatable, so we wrap its description here + private struct EquatableErrorBox: Error, Equatable { + let description: String + + init(_ error: E) { + self.description = error.description + } + } +} From 5c164b2fee1cb31933e87fa62fcbc54e29c74560 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Tue, 20 May 2025 11:57:50 +0900 Subject: [PATCH 03/23] Replaced with filter and split into a function --- Sources/ProjectSpec/Settings.swift | 34 ++++++++++++++++-------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/Sources/ProjectSpec/Settings.swift b/Sources/ProjectSpec/Settings.swift index 68888dc02..f8f7a16d2 100644 --- a/Sources/ProjectSpec/Settings.swift +++ b/Sources/ProjectSpec/Settings.swift @@ -29,22 +29,7 @@ public struct Settings: Equatable, JSONObjectConvertible, CustomStringConvertibl let buildSettingsDictionary: JSONDictionary = jsonDictionary.json(atKeyPath: "base") ?? [:] buildSettings = buildSettingsDictionary - guard let configSettings = jsonDictionary["configs"] as? JSONDictionary else { - configSettings = [:] - return - } - - let invalidConfigKeys = configSettings.reduce(into: Set()) { partialResult, config in - if !(config.value is JSONDictionary) { - partialResult.insert(config.key) - } - } - - guard invalidConfigKeys.isEmpty else { - throw SpecParsingError.invalidConfigsFormat(keys: invalidConfigKeys) - } - - self.configSettings = try jsonDictionary.json(atKeyPath: "configs") + self.configSettings = try Self.validateMappingStyleInConfig(jsonDictionary: jsonDictionary) } else { buildSettings = jsonDictionary configSettings = [:] @@ -52,6 +37,23 @@ public struct Settings: Equatable, JSONObjectConvertible, CustomStringConvertibl } } + private static func validateMappingStyleInConfig(jsonDictionary: JSONDictionary) throws -> [String: Settings] { + guard let configSettings = jsonDictionary["configs"] as? JSONDictionary else { + return [:] + } + + let invalidConfigKeys = Set( + configSettings.filter { !($0.value is JSONDictionary) } + .map { $0.key } + ) + + guard invalidConfigKeys.isEmpty else { + throw SpecParsingError.invalidConfigsFormat(keys: invalidConfigKeys) + } + + return try jsonDictionary.json(atKeyPath: "configs") + } + public static func == (lhs: Settings, rhs: Settings) -> Bool { NSDictionary(dictionary: lhs.buildSettings).isEqual(to: rhs.buildSettings) && lhs.configSettings == rhs.configSettings && From d90866faf112301a13315226e4bf20f0360551f7 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Tue, 20 May 2025 12:00:52 +0900 Subject: [PATCH 04/23] Rename invalidConfigsFormat to invalidConfigsMappingFormat --- Sources/ProjectSpec/Settings.swift | 2 +- Sources/ProjectSpec/SpecParsingError.swift | 4 ++-- Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/ProjectSpec/Settings.swift b/Sources/ProjectSpec/Settings.swift index f8f7a16d2..7f78ba096 100644 --- a/Sources/ProjectSpec/Settings.swift +++ b/Sources/ProjectSpec/Settings.swift @@ -48,7 +48,7 @@ public struct Settings: Equatable, JSONObjectConvertible, CustomStringConvertibl ) guard invalidConfigKeys.isEmpty else { - throw SpecParsingError.invalidConfigsFormat(keys: invalidConfigKeys) + throw SpecParsingError.invalidConfigsMappingFormat(keys: invalidConfigKeys) } return try jsonDictionary.json(atKeyPath: "configs") diff --git a/Sources/ProjectSpec/SpecParsingError.swift b/Sources/ProjectSpec/SpecParsingError.swift index 2511aba71..8bd5a86e5 100644 --- a/Sources/ProjectSpec/SpecParsingError.swift +++ b/Sources/ProjectSpec/SpecParsingError.swift @@ -15,7 +15,7 @@ public enum SpecParsingError: Error, CustomStringConvertible { case unknownBreakpointActionType(String) case unknownBreakpointActionConveyanceType(String) case unknownBreakpointActionSoundName(String) - case invalidConfigsFormat(keys: Set) + case invalidConfigsMappingFormat(keys: Set) public var description: String { switch self { @@ -47,7 +47,7 @@ public enum SpecParsingError: Error, CustomStringConvertible { return "Unknown Breakpoint Action conveyance type: \(type)" case let .unknownBreakpointActionSoundName(name): return "Unknown Breakpoint Action sound name: \(name)" - case let .invalidConfigsFormat(keys): + case let .invalidConfigsMappingFormat(keys): return "The value for \"\(keys.sorted().joined(separator: ", "))\" in configs must be a dictionary" } } diff --git a/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift index ad1a639b5..7d79cc51a 100644 --- a/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift +++ b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift @@ -3,11 +3,11 @@ import Testing import TestSupport import PathKit -struct InvalidConfigsFormatTests { - @Test("throws invalidConfigsFormat error for non-dictionary configs entries") +struct invalidConfigsMappingFormatTests { + @Test("throws invalidConfigsMappingFormat error for non-dictionary configs entries") func testNonDictionaryConfigsEntries() throws { let path = fixturePath + "invalid_configs_value_non_dict.yml" - let expectedError = SpecParsingError.invalidConfigsFormat(keys: ["invalid_key0", "invalid_key1"]) + let expectedError = SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"]) #expect(throws: EquatableErrorBox(expectedError)) { try perform(path: path) From eac4831d6496bf3b45b387687cd8f9f17e24d784 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Tue, 20 May 2025 12:08:38 +0900 Subject: [PATCH 05/23] Add comments to explain invalid fixture --- Tests/Fixtures/invalid_configs_value_non_dict.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Tests/Fixtures/invalid_configs_value_non_dict.yml b/Tests/Fixtures/invalid_configs_value_non_dict.yml index dca38fc81..de11f0301 100644 --- a/Tests/Fixtures/invalid_configs_value_non_dict.yml +++ b/Tests/Fixtures/invalid_configs_value_non_dict.yml @@ -1,5 +1,8 @@ name: InvalidConfigsNonDict +# This fixture is used to test the validation that ensures all values under `settings.configs` +# are mappings. `invalid_key0` and `invalid_key1` are not mapping, +# which should cause a SpecParsingError.invalidConfigsMappingFormat to be thrown. settings: configs: invalid_key0: value0 From 8d90c4a690925426bde88ce7db749788c6ba5e89 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Tue, 20 May 2025 12:10:50 +0900 Subject: [PATCH 06/23] Rename test fixture --- ...value_non_dict.yml => invalid_configs_value_non_mapping.yml} | 2 +- Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename Tests/Fixtures/{invalid_configs_value_non_dict.yml => invalid_configs_value_non_mapping.yml} (90%) diff --git a/Tests/Fixtures/invalid_configs_value_non_dict.yml b/Tests/Fixtures/invalid_configs_value_non_mapping.yml similarity index 90% rename from Tests/Fixtures/invalid_configs_value_non_dict.yml rename to Tests/Fixtures/invalid_configs_value_non_mapping.yml index de11f0301..3e687d2e2 100644 --- a/Tests/Fixtures/invalid_configs_value_non_dict.yml +++ b/Tests/Fixtures/invalid_configs_value_non_mapping.yml @@ -1,4 +1,4 @@ -name: InvalidConfigsNonDict +name: InvalidConfigsValueNonMapping # This fixture is used to test the validation that ensures all values under `settings.configs` # are mappings. `invalid_key0` and `invalid_key1` are not mapping, diff --git a/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift index 7d79cc51a..8b504ff23 100644 --- a/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift +++ b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift @@ -6,7 +6,7 @@ import PathKit struct invalidConfigsMappingFormatTests { @Test("throws invalidConfigsMappingFormat error for non-dictionary configs entries") func testNonDictionaryConfigsEntries() throws { - let path = fixturePath + "invalid_configs_value_non_dict.yml" + let path = fixturePath + "invalid_configs_value_non_mapping.yml" let expectedError = SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"]) #expect(throws: EquatableErrorBox(expectedError)) { From 340b8e64c3d5751325f6d69b58145fe72ac7cf15 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Tue, 20 May 2025 13:20:47 +0900 Subject: [PATCH 07/23] Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 059ad11b3..a3ed1916f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Next Version +### Fixed +- Added validation to ensure that all values in `settings.configs` are mappings. Previously, passing non-mapping values did not raise an error, making it difficult to detect misconfigurations. Now, `SpecParsingError.invalidConfigsMappingFormat` is thrown if misused. #1547 @Ryu0118 + ## 2.43.0 ### Added From 1b32082e1bae597699a11b191ca063ed03f9aee5 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Tue, 20 May 2025 13:26:08 +0900 Subject: [PATCH 08/23] Correct grammer --- Sources/ProjectSpec/Project.swift | 4 ++-- Sources/ProjectSpec/SpecParsingError.swift | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/ProjectSpec/Project.swift b/Sources/ProjectSpec/Project.swift index 50183c2ed..8f31e4d85 100644 --- a/Sources/ProjectSpec/Project.swift +++ b/Sources/ProjectSpec/Project.swift @@ -177,10 +177,10 @@ extension Project { do { settings = try jsonDictionary.json(atKeyPath: "settings") } catch let specParsingError as SpecParsingError { - // Rethrow SpecParsingError to prevent misuse of settings.configs + // Re-throw `SpecParsingError` to prevent the misuse of settings.configs. throw specParsingError } catch { - // Ignore any errors other than SpecParsingError + // Ignore all errors except `SpecParsingError` settings = .empty } diff --git a/Sources/ProjectSpec/SpecParsingError.swift b/Sources/ProjectSpec/SpecParsingError.swift index 8bd5a86e5..57db99f2e 100644 --- a/Sources/ProjectSpec/SpecParsingError.swift +++ b/Sources/ProjectSpec/SpecParsingError.swift @@ -48,7 +48,7 @@ public enum SpecParsingError: Error, CustomStringConvertible { case let .unknownBreakpointActionSoundName(name): return "Unknown Breakpoint Action sound name: \(name)" case let .invalidConfigsMappingFormat(keys): - return "The value for \"\(keys.sorted().joined(separator: ", "))\" in configs must be a dictionary" + return "Invalid format: The value for \"\(keys.sorted().joined(separator: ", "))\" in `configs` must be mapping format" } } } From 70bf93378c58517300250e1bb1db2d52b65cdfa4 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Tue, 20 May 2025 14:16:53 +0900 Subject: [PATCH 09/23] Use KeyPath instead of closure --- Sources/ProjectSpec/Settings.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/ProjectSpec/Settings.swift b/Sources/ProjectSpec/Settings.swift index 7f78ba096..514ced75a 100644 --- a/Sources/ProjectSpec/Settings.swift +++ b/Sources/ProjectSpec/Settings.swift @@ -44,7 +44,7 @@ public struct Settings: Equatable, JSONObjectConvertible, CustomStringConvertibl let invalidConfigKeys = Set( configSettings.filter { !($0.value is JSONDictionary) } - .map { $0.key } + .map(\.key) ) guard invalidConfigKeys.isEmpty else { From d9ca22543ae8262dfce43ff6752f64c7ae667ea2 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Tue, 20 May 2025 14:18:40 +0900 Subject: [PATCH 10/23] Rename validateMappingStyleInConfig to extractValidConfigs --- Sources/ProjectSpec/Settings.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/ProjectSpec/Settings.swift b/Sources/ProjectSpec/Settings.swift index 514ced75a..eb808c606 100644 --- a/Sources/ProjectSpec/Settings.swift +++ b/Sources/ProjectSpec/Settings.swift @@ -37,7 +37,7 @@ public struct Settings: Equatable, JSONObjectConvertible, CustomStringConvertibl } } - private static func validateMappingStyleInConfig(jsonDictionary: JSONDictionary) throws -> [String: Settings] { + private static func extractValidConfigs(from jsonDictionary: JSONDictionary) throws -> [String: Settings] { guard let configSettings = jsonDictionary["configs"] as? JSONDictionary else { return [:] } From 53968a501fa579e120b68f4aa2c309c81c9adeb6 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Tue, 20 May 2025 14:26:31 +0900 Subject: [PATCH 11/23] Add a document comment for extractValidConfigs(from:) --- Sources/ProjectSpec/Settings.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Sources/ProjectSpec/Settings.swift b/Sources/ProjectSpec/Settings.swift index eb808c606..b3b366a86 100644 --- a/Sources/ProjectSpec/Settings.swift +++ b/Sources/ProjectSpec/Settings.swift @@ -29,7 +29,7 @@ public struct Settings: Equatable, JSONObjectConvertible, CustomStringConvertibl let buildSettingsDictionary: JSONDictionary = jsonDictionary.json(atKeyPath: "base") ?? [:] buildSettings = buildSettingsDictionary - self.configSettings = try Self.validateMappingStyleInConfig(jsonDictionary: jsonDictionary) + self.configSettings = try Self.extractValidConfigs(from: jsonDictionary) } else { buildSettings = jsonDictionary configSettings = [:] @@ -37,6 +37,9 @@ public struct Settings: Equatable, JSONObjectConvertible, CustomStringConvertibl } } + /// Extracts and validates the `configs` mapping from the given JSON dictionary. + /// - Parameter jsonDictionary: The JSON dictionary to extract `configs` from. + /// - Returns: A dictionary mapping configuration names to `Settings` objects. private static func extractValidConfigs(from jsonDictionary: JSONDictionary) throws -> [String: Settings] { guard let configSettings = jsonDictionary["configs"] as? JSONDictionary else { return [:] From c56b0724dc930e13f77e6ab9781ca593c272133f Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Tue, 20 May 2025 14:46:23 +0900 Subject: [PATCH 12/23] Use old testing api and remove EquatableErrorBox --- .../InvalidConfigsFormatTests.swift | 25 +++---------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift index 8b504ff23..4f58f185d 100644 --- a/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift +++ b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift @@ -9,27 +9,10 @@ struct invalidConfigsMappingFormatTests { let path = fixturePath + "invalid_configs_value_non_mapping.yml" let expectedError = SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"]) - #expect(throws: EquatableErrorBox(expectedError)) { - try perform(path: path) - } - } - - private func perform(path: Path) throws { - do { - _ = try Project(path: path) - } catch let error as SpecParsingError { - throw EquatableErrorBox(error) - } catch { - throw error - } - } - - // SpecParsingError does not conform to Equatable, so we wrap its description here - private struct EquatableErrorBox: Error, Equatable { - let description: String - - init(_ error: E) { - self.description = error.description + #expect { + try Project(path: path) + } throws: { actualError in + (actualError as any CustomStringConvertible).description == expectedError.description } } } From d65fc9ef8be93a7f41d592780fa6332a782e768b Mon Sep 17 00:00:00 2001 From: Ryu <87907656+Ryu0118@users.noreply.github.com> Date: Tue, 20 May 2025 14:49:28 +0900 Subject: [PATCH 13/23] Rename test case to use "mapping" instead of "dictionary" --- Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift index 4f58f185d..185a4edb6 100644 --- a/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift +++ b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift @@ -4,8 +4,8 @@ import TestSupport import PathKit struct invalidConfigsMappingFormatTests { - @Test("throws invalidConfigsMappingFormat error for non-dictionary configs entries") - func testNonDictionaryConfigsEntries() throws { + @Test("throws invalidConfigsMappingFormat error for non-mapping configs entries") + func testNonMappingConfigsEntries() throws { let path = fixturePath + "invalid_configs_value_non_mapping.yml" let expectedError = SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"]) From cb2f605a90541e869939778d96ce83bef1f0cd18 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Tue, 20 May 2025 17:56:33 +0900 Subject: [PATCH 14/23] Add ValidSettingsExtractor to encapsulate the logic for converting a dictionary to Settings --- .../ProjectSpec/ValidSettingsExtractor.swift | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 Sources/ProjectSpec/ValidSettingsExtractor.swift diff --git a/Sources/ProjectSpec/ValidSettingsExtractor.swift b/Sources/ProjectSpec/ValidSettingsExtractor.swift new file mode 100644 index 000000000..7690562ed --- /dev/null +++ b/Sources/ProjectSpec/ValidSettingsExtractor.swift @@ -0,0 +1,18 @@ +import Foundation +import JSONUtilities + +struct ValidSettingsExtractor { + let jsonDictionary: JSONDictionary + + func extract() throws -> Settings { + do { + return try jsonDictionary.json(atKeyPath: "settings") + } catch let specParsingError as SpecParsingError { + // Re-throw `SpecParsingError` to prevent the misuse of settings.configs. + throw specParsingError + } catch { + // Ignore all errors except `SpecParsingError` + return .empty + } + } +} From 54081ce67329d4b769b669ce8930682e99c91d68 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Tue, 20 May 2025 17:57:24 +0900 Subject: [PATCH 15/23] Add settings validation for both Target and AggregateTarget --- Sources/ProjectSpec/AggregateTarget.swift | 2 +- Sources/ProjectSpec/Project.swift | 10 +--------- Sources/ProjectSpec/Target.swift | 2 +- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/Sources/ProjectSpec/AggregateTarget.swift b/Sources/ProjectSpec/AggregateTarget.swift index 1a3225c40..def018f3f 100644 --- a/Sources/ProjectSpec/AggregateTarget.swift +++ b/Sources/ProjectSpec/AggregateTarget.swift @@ -60,7 +60,7 @@ extension AggregateTarget: NamedJSONDictionaryConvertible { public init(name: String, jsonDictionary: JSONDictionary) throws { self.name = jsonDictionary.json(atKeyPath: "name") ?? name targets = jsonDictionary.json(atKeyPath: "targets") ?? [] - settings = jsonDictionary.json(atKeyPath: "settings") ?? .empty + settings = try ValidSettingsExtractor(jsonDictionary: jsonDictionary).extract() configFiles = jsonDictionary.json(atKeyPath: "configFiles") ?? [:] buildScripts = jsonDictionary.json(atKeyPath: "buildScripts") ?? [] buildToolPlugins = jsonDictionary.json(atKeyPath: "buildToolPlugins") ?? [] diff --git a/Sources/ProjectSpec/Project.swift b/Sources/ProjectSpec/Project.swift index 8f31e4d85..b034b1329 100644 --- a/Sources/ProjectSpec/Project.swift +++ b/Sources/ProjectSpec/Project.swift @@ -174,15 +174,7 @@ extension Project { name = try jsonDictionary.json(atKeyPath: "name") - do { - settings = try jsonDictionary.json(atKeyPath: "settings") - } catch let specParsingError as SpecParsingError { - // Re-throw `SpecParsingError` to prevent the misuse of settings.configs. - throw specParsingError - } catch { - // Ignore all errors except `SpecParsingError` - settings = .empty - } + settings = try ValidSettingsExtractor(jsonDictionary: jsonDictionary).extract() settingGroups = jsonDictionary.json(atKeyPath: "settingGroups") ?? jsonDictionary.json(atKeyPath: "settingPresets") ?? [:] diff --git a/Sources/ProjectSpec/Target.swift b/Sources/ProjectSpec/Target.swift index 3b9f2c535..8e99569cc 100644 --- a/Sources/ProjectSpec/Target.swift +++ b/Sources/ProjectSpec/Target.swift @@ -316,7 +316,7 @@ extension Target: NamedJSONDictionaryConvertible { deploymentTarget = nil } - settings = jsonDictionary.json(atKeyPath: "settings") ?? .empty + settings = try ValidSettingsExtractor(jsonDictionary: jsonDictionary).extract() configFiles = jsonDictionary.json(atKeyPath: "configFiles") ?? [:] if let source: String = jsonDictionary.json(atKeyPath: "sources") { sources = [TargetSource(path: source)] From 24040e75d4b5e0d9c47adbc6c5555904bc36c3db Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Tue, 20 May 2025 17:58:13 +0900 Subject: [PATCH 16/23] Add tests for invalid settings.configs in Target and AggregateTarget --- ...gs_value_non_mapping_aggregate_targets.yml | 24 ++++++++++++++ ...lid_configs_value_non_mapping_settings.yml | 11 +++++++ ...alid_configs_value_non_mapping_targets.yml | 15 +++++++++ .../invalid_configs_value_non_mapping.yml | 11 ------- .../InvalidConfigsFormatTests.swift | 32 +++++++++++++++++-- 5 files changed, 79 insertions(+), 14 deletions(-) create mode 100644 Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_aggregate_targets.yml create mode 100644 Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_settings.yml create mode 100644 Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_targets.yml delete mode 100644 Tests/Fixtures/invalid_configs_value_non_mapping.yml diff --git a/Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_aggregate_targets.yml b/Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_aggregate_targets.yml new file mode 100644 index 000000000..293e384df --- /dev/null +++ b/Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_aggregate_targets.yml @@ -0,0 +1,24 @@ +name: InvalidConfigsValueNonMappingAggregateTargets + +# This fixture tests validation of `settings.configs` under an aggregate target. +# Here, `invalid_key0` and `invalid_key1` are scalar values (not mappings), +# so parsing should throw SpecParsingError.invalidConfigsMappingFormat. +targets: + valid_target1: + type: application + platform: iOS + valid_target2: + type: application + platform: iOS + +aggregateTargets: + invalid_target: + targets: + - valid_target1 + - valid_target2 + settings: + configs: + invalid_key0: value0 + debug: + valid_key: value1 + invalid_key1: value2 diff --git a/Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_settings.yml b/Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_settings.yml new file mode 100644 index 000000000..431804b70 --- /dev/null +++ b/Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_settings.yml @@ -0,0 +1,11 @@ +name: InvalidConfigsValueNonMappingSettings + +# This fixture tests validation of `settings.configs` at the top level. +# Here, `invalid_key0` and `invalid_key1` are scalar values (not mappings), +# so parsing should throw SpecParsingError.invalidConfigsMappingFormat. +settings: + configs: + invalid_key0: value0 + debug: + valid_key: value1 + invalid_key1: value2 diff --git a/Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_targets.yml b/Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_targets.yml new file mode 100644 index 000000000..84a803bce --- /dev/null +++ b/Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_targets.yml @@ -0,0 +1,15 @@ +name: InvalidConfigsValueNonMappingTargets + +# This fixture tests nested validation of `settings.configs` under a target. +# Here, `invalid_key0` and `invalid_key1` are scalar values (not mappings), +# so parsing should throw SpecParsingError.invalidConfigsMappingFormat. +targets: + invalid_target: + type: application + platform: iOS + settings: + configs: + invalid_key0: value0 + debug: + valid_key: value1 + invalid_key1: value2 diff --git a/Tests/Fixtures/invalid_configs_value_non_mapping.yml b/Tests/Fixtures/invalid_configs_value_non_mapping.yml deleted file mode 100644 index 3e687d2e2..000000000 --- a/Tests/Fixtures/invalid_configs_value_non_mapping.yml +++ /dev/null @@ -1,11 +0,0 @@ -name: InvalidConfigsValueNonMapping - -# This fixture is used to test the validation that ensures all values under `settings.configs` -# are mappings. `invalid_key0` and `invalid_key1` are not mapping, -# which should cause a SpecParsingError.invalidConfigsMappingFormat to be thrown. -settings: - configs: - invalid_key0: value0 - debug: - valid_key: value1 - invalid_key1: value2 diff --git a/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift index 185a4edb6..006671356 100644 --- a/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift +++ b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift @@ -4,9 +4,35 @@ import TestSupport import PathKit struct invalidConfigsMappingFormatTests { - @Test("throws invalidConfigsMappingFormat error for non-mapping configs entries") - func testNonMappingConfigsEntries() throws { - let path = fixturePath + "invalid_configs_value_non_mapping.yml" + let invalidConfigsFixturePath: Path = fixturePath + "invalid_configs" + + @Test("throws invalidConfigsMappingFormat for non-mapping settings.configs entries at root level") + func testNonMappingSettingsConfigsEntries() throws { + let path = invalidConfigsFixturePath + "invalid_configs_value_non_mapping_settings.yml" + let expectedError = SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"]) + + #expect { + try Project(path: path) + } throws: { actualError in + (actualError as any CustomStringConvertible).description == expectedError.description + } + } + + @Test("throws invalidConfigsMappingFormat for non-mapping settings.configs entries in a target") + func testNonMappingTargetsConfigsEntries() throws { + let path = invalidConfigsFixturePath + "invalid_configs_value_non_mapping_targets.yml" + let expectedError = SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"]) + + #expect { + try Project(path: path) + } throws: { actualError in + (actualError as any CustomStringConvertible).description == expectedError.description + } + } + + @Test("throws invalidConfigsMappingFormat for non-mapping settings.configs entries in an aggregate target") + func testNonMappingAggregateTargetsConfigsEntries() throws { + let path = invalidConfigsFixturePath + "invalid_configs_value_non_mapping_aggregate_targets.yml" let expectedError = SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"]) #expect { From 9dc840575cfed5139c33f3c6a356bd51605bcede Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Tue, 20 May 2025 18:25:09 +0900 Subject: [PATCH 17/23] Add document comments for ValidSettingsExtractor --- Sources/ProjectSpec/ValidSettingsExtractor.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Sources/ProjectSpec/ValidSettingsExtractor.swift b/Sources/ProjectSpec/ValidSettingsExtractor.swift index 7690562ed..563ba78fa 100644 --- a/Sources/ProjectSpec/ValidSettingsExtractor.swift +++ b/Sources/ProjectSpec/ValidSettingsExtractor.swift @@ -1,9 +1,13 @@ import Foundation import JSONUtilities +/// A helper for extracting and validating the `Settings` object from a JSON dictionary. struct ValidSettingsExtractor { let jsonDictionary: JSONDictionary + /// Attempts to extract and parse the `Settings` from the dictionary. + /// + /// - Returns: A valid `Settings` object func extract() throws -> Settings { do { return try jsonDictionary.json(atKeyPath: "settings") From ca9ed3e7f91ce18e5dbd129882ff768000dc6082 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Mon, 26 May 2025 11:24:16 +0900 Subject: [PATCH 18/23] Rename ValidSettingsExtractor to BuildSettingsExtractor --- Sources/ProjectSpec/AggregateTarget.swift | 2 +- ...alidSettingsExtractor.swift => BuildSettingsExtractor.swift} | 2 +- Sources/ProjectSpec/Project.swift | 2 +- Sources/ProjectSpec/Target.swift | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename Sources/ProjectSpec/{ValidSettingsExtractor.swift => BuildSettingsExtractor.swift} (95%) diff --git a/Sources/ProjectSpec/AggregateTarget.swift b/Sources/ProjectSpec/AggregateTarget.swift index def018f3f..6cebe30d5 100644 --- a/Sources/ProjectSpec/AggregateTarget.swift +++ b/Sources/ProjectSpec/AggregateTarget.swift @@ -60,7 +60,7 @@ extension AggregateTarget: NamedJSONDictionaryConvertible { public init(name: String, jsonDictionary: JSONDictionary) throws { self.name = jsonDictionary.json(atKeyPath: "name") ?? name targets = jsonDictionary.json(atKeyPath: "targets") ?? [] - settings = try ValidSettingsExtractor(jsonDictionary: jsonDictionary).extract() + settings = try BuildSettingsParser(jsonDictionary: jsonDictionary).extract() configFiles = jsonDictionary.json(atKeyPath: "configFiles") ?? [:] buildScripts = jsonDictionary.json(atKeyPath: "buildScripts") ?? [] buildToolPlugins = jsonDictionary.json(atKeyPath: "buildToolPlugins") ?? [] diff --git a/Sources/ProjectSpec/ValidSettingsExtractor.swift b/Sources/ProjectSpec/BuildSettingsExtractor.swift similarity index 95% rename from Sources/ProjectSpec/ValidSettingsExtractor.swift rename to Sources/ProjectSpec/BuildSettingsExtractor.swift index 563ba78fa..bd0cd7189 100644 --- a/Sources/ProjectSpec/ValidSettingsExtractor.swift +++ b/Sources/ProjectSpec/BuildSettingsExtractor.swift @@ -2,7 +2,7 @@ import Foundation import JSONUtilities /// A helper for extracting and validating the `Settings` object from a JSON dictionary. -struct ValidSettingsExtractor { +struct BuildSettingsParser { let jsonDictionary: JSONDictionary /// Attempts to extract and parse the `Settings` from the dictionary. diff --git a/Sources/ProjectSpec/Project.swift b/Sources/ProjectSpec/Project.swift index b034b1329..ecf9738a3 100644 --- a/Sources/ProjectSpec/Project.swift +++ b/Sources/ProjectSpec/Project.swift @@ -174,7 +174,7 @@ extension Project { name = try jsonDictionary.json(atKeyPath: "name") - settings = try ValidSettingsExtractor(jsonDictionary: jsonDictionary).extract() + settings = try BuildSettingsParser(jsonDictionary: jsonDictionary).extract() settingGroups = jsonDictionary.json(atKeyPath: "settingGroups") ?? jsonDictionary.json(atKeyPath: "settingPresets") ?? [:] diff --git a/Sources/ProjectSpec/Target.swift b/Sources/ProjectSpec/Target.swift index 8e99569cc..5788df285 100644 --- a/Sources/ProjectSpec/Target.swift +++ b/Sources/ProjectSpec/Target.swift @@ -316,7 +316,7 @@ extension Target: NamedJSONDictionaryConvertible { deploymentTarget = nil } - settings = try ValidSettingsExtractor(jsonDictionary: jsonDictionary).extract() + settings = try BuildSettingsParser(jsonDictionary: jsonDictionary).extract() configFiles = jsonDictionary.json(atKeyPath: "configFiles") ?? [:] if let source: String = jsonDictionary.json(atKeyPath: "sources") { sources = [TargetSource(path: source)] From 6b9cc59e533862943567dfb480d273289751f791 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Mon, 26 May 2025 12:18:26 +0900 Subject: [PATCH 19/23] Add settings validation for settingGroups --- Sources/ProjectSpec/BuildSettingsExtractor.swift | 12 ++++++++++++ Sources/ProjectSpec/Project.swift | 6 +++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/Sources/ProjectSpec/BuildSettingsExtractor.swift b/Sources/ProjectSpec/BuildSettingsExtractor.swift index bd0cd7189..138b71c8d 100644 --- a/Sources/ProjectSpec/BuildSettingsExtractor.swift +++ b/Sources/ProjectSpec/BuildSettingsExtractor.swift @@ -19,4 +19,16 @@ struct BuildSettingsParser { return .empty } } + + func extractSettingGroups(withDefault defaultGroups: [String: Settings]) throws -> [String: Settings] { + do { + return try jsonDictionary.json(atKeyPath: "settingGroups", invalidItemBehaviour: .fail) + } catch let specParsingError as SpecParsingError { + // Re-throw `SpecParsingError` to prevent the misuse of settings.configs. + throw specParsingError + } catch { + // Ignore all errors except `SpecParsingError` + return defaultGroups + } + } } diff --git a/Sources/ProjectSpec/Project.swift b/Sources/ProjectSpec/Project.swift index ecf9738a3..4403e25de 100644 --- a/Sources/ProjectSpec/Project.swift +++ b/Sources/ProjectSpec/Project.swift @@ -171,13 +171,13 @@ extension Project { self.basePath = basePath let jsonDictionary = Project.resolveProject(jsonDictionary: jsonDictionary) + let buildSettingsParser = BuildSettingsParser(jsonDictionary: jsonDictionary) name = try jsonDictionary.json(atKeyPath: "name") - settings = try BuildSettingsParser(jsonDictionary: jsonDictionary).extract() + settings = try buildSettingsParser.extract() - settingGroups = jsonDictionary.json(atKeyPath: "settingGroups") - ?? jsonDictionary.json(atKeyPath: "settingPresets") ?? [:] + settingGroups = try buildSettingsParser.extractSettingGroups(withDefault: jsonDictionary.json(atKeyPath: "settingPresets") ?? [:]) let configs: [String: String] = jsonDictionary.json(atKeyPath: "configs") ?? [:] self.configs = configs.isEmpty ? Config.defaultConfigs : configs.map { Config(name: $0, type: ConfigType(rawValue: $1)) }.sorted { $0.name < $1.name } From 6f7592d1914bdb8edea61de26cb38e235c9ba16b Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Mon, 26 May 2025 12:19:05 +0900 Subject: [PATCH 20/23] Add tests for settingGroups --- ...nfigs_value_non_mapping_setting_groups.yml | 19 +++++++ .../InvalidConfigsFormatTests.swift | 57 +++++++++---------- 2 files changed, 47 insertions(+), 29 deletions(-) create mode 100644 Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_setting_groups.yml diff --git a/Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_setting_groups.yml b/Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_setting_groups.yml new file mode 100644 index 000000000..90822c24f --- /dev/null +++ b/Tests/Fixtures/invalid_configs/invalid_configs_value_non_mapping_setting_groups.yml @@ -0,0 +1,19 @@ +name: InvalidConfigsValueNonMappingSettingGroups + +# This fixture tests validation of `settings.configs` under an aggregate target. +# Here, `invalid_key0` and `invalid_key1` are scalar values (not mappings), +# so parsing should throw SpecParsingError.invalidConfigsMappingFormat. +settingGroups: + invalid_preset: + configs: + invalid_key0: value0 + debug: + valid_key: value1 + invalid_key1: value2 +targets: + invalid_target: + type: application + platform: iOS + settings: + groups: + - invalid_preset diff --git a/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift index 006671356..e10236088 100644 --- a/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift +++ b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift @@ -4,41 +4,40 @@ import TestSupport import PathKit struct invalidConfigsMappingFormatTests { - let invalidConfigsFixturePath: Path = fixturePath + "invalid_configs" - - @Test("throws invalidConfigsMappingFormat for non-mapping settings.configs entries at root level") - func testNonMappingSettingsConfigsEntries() throws { - let path = invalidConfigsFixturePath + "invalid_configs_value_non_mapping_settings.yml" - let expectedError = SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"]) - - #expect { - try Project(path: path) - } throws: { actualError in - (actualError as any CustomStringConvertible).description == expectedError.description - } + struct InvalidConfigsTestArguments { + var fixturePath: Path + var expectedError: SpecParsingError } - @Test("throws invalidConfigsMappingFormat for non-mapping settings.configs entries in a target") - func testNonMappingTargetsConfigsEntries() throws { - let path = invalidConfigsFixturePath + "invalid_configs_value_non_mapping_targets.yml" - let expectedError = SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"]) - - #expect { - try Project(path: path) - } throws: { actualError in - (actualError as any CustomStringConvertible).description == expectedError.description - } + private static var testArguments: [InvalidConfigsTestArguments] { + let invalidConfigsFixturePath: Path = fixturePath + "invalid_configs" + return [ + InvalidConfigsTestArguments( + fixturePath: invalidConfigsFixturePath + "invalid_configs_value_non_mapping_settings.yml", + expectedError: SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"]) + ), + InvalidConfigsTestArguments( + fixturePath: invalidConfigsFixturePath + "invalid_configs_value_non_mapping_targets.yml", + expectedError: SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"]) + ), + InvalidConfigsTestArguments( + fixturePath: invalidConfigsFixturePath + "invalid_configs_value_non_mapping_aggregate_targets.yml", + expectedError: SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"]) + ), + InvalidConfigsTestArguments( + fixturePath: invalidConfigsFixturePath + "invalid_configs_value_non_mapping_setting_groups.yml", + expectedError: SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"]) + ) + ] } - @Test("throws invalidConfigsMappingFormat for non-mapping settings.configs entries in an aggregate target") - func testNonMappingAggregateTargetsConfigsEntries() throws { - let path = invalidConfigsFixturePath + "invalid_configs_value_non_mapping_aggregate_targets.yml" - let expectedError = SpecParsingError.invalidConfigsMappingFormat(keys: ["invalid_key0", "invalid_key1"]) - + @Test("throws invalidConfigsMappingFormat for non-mapping configs entries", arguments: testArguments) + func testInvalidConfigsMappingFormat(_ arguments: InvalidConfigsTestArguments) throws { #expect { - try Project(path: path) + try Project(path: arguments.fixturePath) } throws: { actualError in - (actualError as any CustomStringConvertible).description == expectedError.description + (actualError as any CustomStringConvertible).description + == arguments.expectedError.description } } } From 79d78ca2e80b4776d8063e0a66c6df2ed170c310 Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Mon, 26 May 2025 12:59:42 +0900 Subject: [PATCH 21/23] Rename extract to parse --- Sources/ProjectSpec/AggregateTarget.swift | 2 +- Sources/ProjectSpec/BuildSettingsExtractor.swift | 4 ++-- Sources/ProjectSpec/Project.swift | 4 ++-- Sources/ProjectSpec/Target.swift | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/ProjectSpec/AggregateTarget.swift b/Sources/ProjectSpec/AggregateTarget.swift index 6cebe30d5..9bea7098c 100644 --- a/Sources/ProjectSpec/AggregateTarget.swift +++ b/Sources/ProjectSpec/AggregateTarget.swift @@ -60,7 +60,7 @@ extension AggregateTarget: NamedJSONDictionaryConvertible { public init(name: String, jsonDictionary: JSONDictionary) throws { self.name = jsonDictionary.json(atKeyPath: "name") ?? name targets = jsonDictionary.json(atKeyPath: "targets") ?? [] - settings = try BuildSettingsParser(jsonDictionary: jsonDictionary).extract() + settings = try BuildSettingsParser(jsonDictionary: jsonDictionary).parse() configFiles = jsonDictionary.json(atKeyPath: "configFiles") ?? [:] buildScripts = jsonDictionary.json(atKeyPath: "buildScripts") ?? [] buildToolPlugins = jsonDictionary.json(atKeyPath: "buildToolPlugins") ?? [] diff --git a/Sources/ProjectSpec/BuildSettingsExtractor.swift b/Sources/ProjectSpec/BuildSettingsExtractor.swift index 138b71c8d..6efbf60a8 100644 --- a/Sources/ProjectSpec/BuildSettingsExtractor.swift +++ b/Sources/ProjectSpec/BuildSettingsExtractor.swift @@ -8,7 +8,7 @@ struct BuildSettingsParser { /// Attempts to extract and parse the `Settings` from the dictionary. /// /// - Returns: A valid `Settings` object - func extract() throws -> Settings { + func parse() throws -> Settings { do { return try jsonDictionary.json(atKeyPath: "settings") } catch let specParsingError as SpecParsingError { @@ -20,7 +20,7 @@ struct BuildSettingsParser { } } - func extractSettingGroups(withDefault defaultGroups: [String: Settings]) throws -> [String: Settings] { + func parseSettingGroups(withDefault defaultGroups: [String: Settings]) throws -> [String: Settings] { do { return try jsonDictionary.json(atKeyPath: "settingGroups", invalidItemBehaviour: .fail) } catch let specParsingError as SpecParsingError { diff --git a/Sources/ProjectSpec/Project.swift b/Sources/ProjectSpec/Project.swift index 4403e25de..a90ca2a34 100644 --- a/Sources/ProjectSpec/Project.swift +++ b/Sources/ProjectSpec/Project.swift @@ -175,9 +175,9 @@ extension Project { name = try jsonDictionary.json(atKeyPath: "name") - settings = try buildSettingsParser.extract() + settings = try buildSettingsParser.parse() - settingGroups = try buildSettingsParser.extractSettingGroups(withDefault: jsonDictionary.json(atKeyPath: "settingPresets") ?? [:]) + settingGroups = try buildSettingsParser.parseSettingGroups(withDefault: jsonDictionary.json(atKeyPath: "settingPresets") ?? [:]) let configs: [String: String] = jsonDictionary.json(atKeyPath: "configs") ?? [:] self.configs = configs.isEmpty ? Config.defaultConfigs : configs.map { Config(name: $0, type: ConfigType(rawValue: $1)) }.sorted { $0.name < $1.name } diff --git a/Sources/ProjectSpec/Target.swift b/Sources/ProjectSpec/Target.swift index 5788df285..781994f42 100644 --- a/Sources/ProjectSpec/Target.swift +++ b/Sources/ProjectSpec/Target.swift @@ -316,7 +316,7 @@ extension Target: NamedJSONDictionaryConvertible { deploymentTarget = nil } - settings = try BuildSettingsParser(jsonDictionary: jsonDictionary).extract() + settings = try BuildSettingsParser(jsonDictionary: jsonDictionary).parse() configFiles = jsonDictionary.json(atKeyPath: "configFiles") ?? [:] if let source: String = jsonDictionary.json(atKeyPath: "sources") { sources = [TargetSource(path: source)] From 03bcec17e407ef6ebe9170f7d79cef9c6c56ec2f Mon Sep 17 00:00:00 2001 From: Ryu0118 Date: Mon, 26 May 2025 13:05:22 +0900 Subject: [PATCH 22/23] Refactor --- Sources/ProjectSpec/BuildSettingsExtractor.swift | 9 ++++++--- Sources/ProjectSpec/Project.swift | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Sources/ProjectSpec/BuildSettingsExtractor.swift b/Sources/ProjectSpec/BuildSettingsExtractor.swift index 6efbf60a8..1dfeb93de 100644 --- a/Sources/ProjectSpec/BuildSettingsExtractor.swift +++ b/Sources/ProjectSpec/BuildSettingsExtractor.swift @@ -20,15 +20,18 @@ struct BuildSettingsParser { } } - func parseSettingGroups(withDefault defaultGroups: [String: Settings]) throws -> [String: Settings] { + /// Attempts to extract and parse setting groups from the dictionary with fallback defaults. + /// + /// - Returns: Parsed setting groups or default groups if parsing fails + func parseSettingGroups() throws -> [String: Settings] { do { return try jsonDictionary.json(atKeyPath: "settingGroups", invalidItemBehaviour: .fail) } catch let specParsingError as SpecParsingError { - // Re-throw `SpecParsingError` to prevent the misuse of settings.configs. + // Re-throw `SpecParsingError` to prevent the misuse of settingGroups. throw specParsingError } catch { // Ignore all errors except `SpecParsingError` - return defaultGroups + return jsonDictionary.json(atKeyPath: "settingPresets") ?? [:] } } } diff --git a/Sources/ProjectSpec/Project.swift b/Sources/ProjectSpec/Project.swift index a90ca2a34..70fb9bc33 100644 --- a/Sources/ProjectSpec/Project.swift +++ b/Sources/ProjectSpec/Project.swift @@ -176,8 +176,8 @@ extension Project { name = try jsonDictionary.json(atKeyPath: "name") settings = try buildSettingsParser.parse() + settingGroups = try buildSettingsParser.parseSettingGroups() - settingGroups = try buildSettingsParser.parseSettingGroups(withDefault: jsonDictionary.json(atKeyPath: "settingPresets") ?? [:]) let configs: [String: String] = jsonDictionary.json(atKeyPath: "configs") ?? [:] self.configs = configs.isEmpty ? Config.defaultConfigs : configs.map { Config(name: $0, type: ConfigType(rawValue: $1)) }.sorted { $0.name < $1.name } From a792bcc8740448a789fcf3211f64f0563b79de71 Mon Sep 17 00:00:00 2001 From: Yonas Kolb Date: Sat, 7 Jun 2025 00:19:04 +1000 Subject: [PATCH 23/23] Update Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift --- Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift index e10236088..96a7540d1 100644 --- a/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift +++ b/Tests/ProjectSpecTests/InvalidConfigsFormatTests.swift @@ -3,7 +3,7 @@ import Testing import TestSupport import PathKit -struct invalidConfigsMappingFormatTests { +struct InvalidConfigsMappingFormatTests { struct InvalidConfigsTestArguments { var fixturePath: Path var expectedError: SpecParsingError