Resolve booleans via a bool primitive, not string parsing #8
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| // | ||
| // ConfigValueReading+Bool.swift | ||
| // ConfigKeyKit | ||
| // | ||
| // Created by Leo Dion. | ||
| // Copyright © 2026 BrightDigit. | ||
| // | ||
| // Permission is hereby granted, free of charge, to any person | ||
| // obtaining a copy of this software and associated documentation | ||
| // files (the "Software"), to deal in the Software without | ||
| // restriction, including without limitation the rights to use, | ||
| // copy, modify, merge, publish, distribute, sublicense, and/or | ||
| // sell copies of the Software, and to permit persons to whom the | ||
| // Software is furnished to do so, subject to the following | ||
| // conditions: | ||
| // | ||
| // The above copyright notice and this permission notice shall be | ||
| // included in all copies or substantial portions of the Software. | ||
| // | ||
| // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, | ||
| // EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES | ||
| // OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND | ||
| // NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT | ||
| // HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, | ||
| // WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING | ||
| // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR | ||
| // OTHER DEALINGS IN THE SOFTWARE. | ||
| // | ||
|
|
||
| internal import Foundation | ||
|
|
||
| // swiftlint:disable discouraged_optional_boolean | ||
| extension ConfigValueReading { | ||
| /// Parses a boolean from the reader's string value. | ||
| /// | ||
| /// `true` / `1` / `yes` and `false` / `0` / `no` are recognized, case-insensitively. | ||
| /// Anything else — including an empty value — yields `nil`, so resolution falls through | ||
| /// to the next source and ultimately to the key's default. An unrecognized value must | ||
| /// not be treated as `false`: a typo would then silently *disable* a flag rather than | ||
| /// being ignored. | ||
| public func bool( | ||
| forKey key: Key, | ||
| isSecret: Bool, | ||
| fileID: String, | ||
| line: UInt | ||
| ) -> Bool? { | ||
| guard | ||
| let value = string(forKey: key, isSecret: isSecret, fileID: fileID, line: line) | ||
| else { | ||
| return nil | ||
| } | ||
| switch value.lowercased().trimmingCharacters(in: .whitespaces) { | ||
| case "true", "1", "yes": | ||
| return true | ||
| case "false", "0", "no": | ||
| return false | ||
| default: | ||
| return nil | ||
| } | ||
| } | ||
| } | ||
| // swiftlint:enable discouraged_optional_boolean | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| // | ||
| // ConfigValueReadingTests.swift | ||
| // ConfigKeyKit | ||
| // | ||
| // Created by Leo Dion. | ||
| // Copyright © 2026 BrightDigit. | ||
| // | ||
| // Permission is hereby granted, free of charge, to any person | ||
| // obtaining a copy of this software and associated documentation | ||
| // files (the "Software"), to deal in the Software without | ||
| // restriction, including without limitation the rights to use, | ||
| // copy, modify, merge, publish, distribute, sublicense, and/or | ||
| // sell copies of the Software, and to permit persons to whom the | ||
| // Software is furnished to do so, subject to the following | ||
| // conditions: | ||
| // | ||
| // The above copyright notice and this permission notice shall be | ||
| // included in all copies or substantial portions of the Software. | ||
| // | ||
| // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, | ||
| // EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES | ||
| // OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND | ||
| // NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT | ||
| // HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, | ||
| // WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING | ||
| // FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR | ||
| // OTHER DEALINGS IN THE SOFTWARE. | ||
| // | ||
|
|
||
| import Testing | ||
|
|
||
| @testable import ConfigKeyKit | ||
|
Comment on lines
+30
to
+32
There was a problem hiding this comment. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- applicable repository conventions ---'
find /tmp/coderabbit-repo-knowledge/brightdigit-configkeykit-2b58d020 -type f -name '*.md' -print | sort
printf '%s\n' '--- package feature declarations ---'
rg -n -C 3 'InternalImportsByDefault|MemberImportVisibility|swiftLanguageModes|SwiftSetting|swiftSettings|Swift 6' Package.swift Sources Tests 2>/dev/null || true
printf '%s\n' '--- affected imports ---'
for f in \
Tests/ConfigKeyKitTests/ConfigValueReadingBoolTests.swift \
Tests/ConfigKeyKitTests/MockConfigValueReader.swift \
Tests/ConfigKeyKitTests/StringOnlyConfigValueReader.swift
do
printf '\n--- %s ---\n' "$f"
sed -n '1,45p' "$f"
doneRepository: brightdigit/ConfigKeyKit Length of output: 8874 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- package convention ---'
cat /tmp/coderabbit-repo-knowledge/brightdigit-configkeykit-2b58d020/conventions/package-swift.md
printf '%s\n' '--- repository-wide convention ---'
cat /tmp/coderabbit-repo-knowledge/brightdigit-configkeykit-2b58d020/conventions/repo-wide.md
printf '%s\n' '--- Package.swift ---'
sed -n '1,55p' Package.swift
printf '%s\n' '--- import forms in test sources ---'
rg -n '(^| )(`@testable` )?(public |internal )?import ' Tests/ConfigKeyKitTests -g '*.swift' | head -120Repository: brightdigit/ConfigKeyKit Length of output: 6855 🌐 Web query:
💡 Result: SE-0409, titled "Access-level modifiers on import declarations," introduces the ability to specify access levels (such as Citations:
🌐 Web query:
💡 Result: The query refers to Swift Evolution Proposal SE-0409, titled Access-level modifiers on import declarations [1]. SE-0409 introduced the ability to apply access-level modifiers (such as internal, package, and public) directly to import statements [1]. This allows developers to control the visibility of imported modules, preventing them from leaking into the public API of the importing module [1][2]. Key details regarding the interaction between access-level imports and Citations:
🏁 Script executed: #!/bin/bash
set -e
curl -fsSL https://raw.githubusercontent.com/swiftlang/swift-evolution/main/proposals/0409-access-level-on-imports.md \
| rg -n -C 5 '`@testable`|ImportDeclaration|internal import|access.level|access level'Repository: brightdigit/ConfigKeyKit Length of output: 17503 🏁 Script executed: #!/bin/bash
set -e
if ! command -v swiftc >/dev/null 2>&1; then
echo 'swiftc unavailable'
exit 0
fi
swiftc --version
for form in \
'internal import Swift' \
'`@testable` import Swift' \
'`@testable` internal import Swift' \
'internal `@testable` import Swift'
do
printf '\n--- %s ---\n' "$form"
printf '%s\n' "$form" | swiftc -typecheck -enable-upcoming-feature InternalImportsByDefault - 2>&1 || true
doneRepository: brightdigit/ConfigKeyKit Length of output: 181 🌐 Web query:
💡 Result: The valid syntax for enabling access to internal entities in a module for testing purposes is Citations:
Use explicit access on all affected imports.
📍 Affects 3 files
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| /// Boolean resolution across sources. | ||
| /// | ||
| /// Split from the main suite because booleans are the one type whose resolution differs | ||
| /// per reader: one with a native boolean accessor (``MockConfigValueReader``, as | ||
| /// `ConfigReader` is) sees a valueless command-line flag, while one supplying only | ||
| /// strings (``StringOnlyConfigValueReader``) falls back to the protocol's parsing. | ||
| @Suite("ConfigValueReading: booleans") | ||
| internal struct ConfigValueReadingBoolTests { | ||
| @Test("Required bool: CLI flag presence is true") | ||
| internal func boolCLIPresence() throws { | ||
| let boolKey = ConfigKey("verbose", envPrefix: "BRIGHTDIGIT", default: false) | ||
| let cli = try #require(boolKey.key(for: .commandLine)) | ||
| let reader = MockConfigValueReader(bools: [cli: true]) | ||
| #expect(reader.read(boolKey) == true) | ||
| } | ||
|
|
||
| @Test("Required bool: an explicit CLI false is honored, not overridden by presence") | ||
| internal func boolCLIExplicitFalse() throws { | ||
| let boolKey = ConfigKey("verbose", envPrefix: "BRIGHTDIGIT", default: true) | ||
| let cli = try #require(boolKey.key(for: .commandLine)) | ||
| #expect(MockConfigValueReader(bools: [cli: false]).read(boolKey) == false) | ||
| } | ||
|
|
||
| @Test( | ||
| "Required bool: ENV truthy strings, via the string-parsing default", | ||
| arguments: [ | ||
| ("true", true), ("1", true), ("YES", true), ("yes", true), | ||
| ("false", false), ("0", false), ("no", false), ("NO", false), | ||
| ] | ||
| ) | ||
| internal func boolENVParsing(value: String, expected: Bool) throws { | ||
| let boolKey = ConfigKey("verbose", envPrefix: "BRIGHTDIGIT", default: false) | ||
| let env = try #require(boolKey.key(for: .environment)) | ||
| let reader = StringOnlyConfigValueReader(strings: [env: value]) | ||
| #expect(reader.read(boolKey) == expected) | ||
| } | ||
|
|
||
| @Test( | ||
| "Required bool: an unrecognized value is ignored, never coerced to false", | ||
| arguments: ["banana", "on", "off", "ture", "2"] | ||
| ) | ||
| internal func boolUnrecognizedFallsThrough(value: String) throws { | ||
| // Regression: these used to resolve as `false`, so a typo silently *disabled* a | ||
| // flag whose default was `true` instead of being ignored. | ||
| let boolKey = ConfigKey("verbose", envPrefix: "BRIGHTDIGIT", default: true) | ||
| let env = try #require(boolKey.key(for: .environment)) | ||
| #expect(StringOnlyConfigValueReader(strings: [env: value]).read(boolKey) == true) | ||
|
|
||
| let optionalKey = OptionalConfigKey<Bool>("verbose", envPrefix: "BRIGHTDIGIT") | ||
| let optionalEnv = try #require(optionalKey.key(for: .environment)) | ||
| #expect(StringOnlyConfigValueReader(strings: [optionalEnv: value]).read(optionalKey) == nil) | ||
| } | ||
|
|
||
| @Test("Required bool: default when absent") | ||
| internal func boolDefault() { | ||
| let boolKey = ConfigKey("verbose", envPrefix: "BRIGHTDIGIT", default: true) | ||
| #expect(MockConfigValueReader().read(boolKey) == true) | ||
| } | ||
|
|
||
| @Test("Optional bool: CLI presence true, ENV truthy, nil when absent") | ||
| internal func optionalBool() throws { | ||
| let boolKey = OptionalConfigKey<Bool>("verbose", envPrefix: "BRIGHTDIGIT") | ||
| let cli = try #require(boolKey.key(for: .commandLine)) | ||
| let env = try #require(boolKey.key(for: .environment)) | ||
| #expect(MockConfigValueReader(bools: [cli: true]).read(boolKey) == true) | ||
| #expect(StringOnlyConfigValueReader(strings: [env: "yes"]).read(boolKey) == true) | ||
| #expect(StringOnlyConfigValueReader(strings: [env: "false"]).read(boolKey) == false) | ||
| #expect(MockConfigValueReader().read(boolKey) == nil) | ||
| } | ||
|
|
||
| @Test("Required bool honors sourcePriority: ENV value wins over CLI flag when reversed") | ||
| internal func boolReversedPriority() throws { | ||
| let boolKey = ConfigKey("verbose", envPrefix: "BRIGHTDIGIT", default: false) | ||
| let cli = try #require(boolKey.key(for: .commandLine)) | ||
| let env = try #require(boolKey.key(for: .environment)) | ||
| // CLI flag present (true) and ENV explicitly "false": precedence decides. | ||
| let forward = MockConfigValueReader(bools: [cli: true, env: false]) | ||
| #expect(forward.read(boolKey) == true) | ||
| let reversed = MockConfigValueReader( | ||
| bools: [cli: true, env: false], | ||
| sourcePriority: [.environment, .commandLine] | ||
| ) | ||
| #expect(reversed.read(boolKey) == false) | ||
| } | ||
|
|
||
| @Test("Required bool: empty ENV is treated as absent, default used") | ||
| internal func boolEmptyENVUsesDefault() throws { | ||
| let boolKey = ConfigKey("verbose", envPrefix: "BRIGHTDIGIT", default: true) | ||
| let env = try #require(boolKey.key(for: .environment)) | ||
| #expect(StringOnlyConfigValueReader(strings: [env: ""]).read(boolKey) == true) | ||
| #expect(StringOnlyConfigValueReader(strings: [env: " "]).read(boolKey) == true) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: brightdigit/ConfigKeyKit
Length of output: 6804
🏁 Script executed:
Repository: brightdigit/ConfigKeyKit
Length of output: 6684
Declare access on the extension.
extension ConfigValueReadinghas implicit access atSources/ConfigKeyKit/ConfigValueReading+Bool.swift:33. Add an explicit access modifier to satisfy the enabledexplicit_aclandexplicit_top_level_aclrules.🤖 Prompt for AI Agents
Source: Coding guidelines