From 69a61b514aad5ec10151bf337cfcd45041c2c415 Mon Sep 17 00:00:00 2001 From: Nate Cook Date: Sun, 16 Feb 2025 13:08:45 -0600 Subject: [PATCH 1/2] Make NameSpecification and its element ExpressibleByStringLiteral This simplifies declaring alternate names by allowing just a string representation of an option or flag name. The string is parsed to split on spaces, and then validates that each name has either a one or two dash prefix and is ASCII only. Existing style: @Flag(name: [.customLong("hex-output"), .customShort("x")]) var hexadecimalOutput = false New style: @Flag(name: "--hex-output -x") var hexadecimalOutput = false --- Examples/math/Math.swift | 2 +- .../NameSpecification.swift | 80 ++++++++++++++ .../NameSpecificationTests.swift | 100 ++++++++++++++++++ 3 files changed, 181 insertions(+), 1 deletion(-) diff --git a/Examples/math/Math.swift b/Examples/math/Math.swift index 9703a5af5..0169fe24c 100644 --- a/Examples/math/Math.swift +++ b/Examples/math/Math.swift @@ -35,7 +35,7 @@ struct Math: ParsableCommand { struct Options: ParsableArguments { @Flag( - name: [.customLong("hex-output"), .customShort("x")], + name: "--hex-output -x", help: "Use hexadecimal notation for the result.") var hexadecimalOutput = false diff --git a/Sources/ArgumentParser/Parsable Properties/NameSpecification.swift b/Sources/ArgumentParser/Parsable Properties/NameSpecification.swift index df061304e..0b9b151b0 100644 --- a/Sources/ArgumentParser/Parsable Properties/NameSpecification.swift +++ b/Sources/ArgumentParser/Parsable Properties/NameSpecification.swift @@ -19,6 +19,7 @@ public struct NameSpecification: ExpressibleByArrayLiteral { case customLong(_ name: String, withSingleDash: Bool) case short case customShort(_ char: Character, allowingJoined: Bool) + case invalidLiteral(literal: String, message: String) } internal var base: Representation @@ -78,7 +79,16 @@ public struct NameSpecification: ExpressibleByArrayLiteral { ) -> Element { self.init(base: .customShort(char, allowingJoined: allowingJoined)) } + + /// An invalid literal, for a later diagnostic. + internal static func invalidLiteral( + literal str: String, + message: String + ) -> Element { + self.init(base: .invalidLiteral(literal: str, message: message)) + } } + var elements: [Element] public init(_ sequence: S) where S: Sequence, Element == S.Element { @@ -92,6 +102,70 @@ public struct NameSpecification: ExpressibleByArrayLiteral { extension NameSpecification: Sendable {} +extension NameSpecification.Element: + ExpressibleByStringLiteral, ExpressibleByStringInterpolation +{ + public init(stringLiteral string: String) { + // Check for spaces + guard !string.contains(where: { $0 == " " }) else { + self = .invalidLiteral( + literal: string, + message: "Can't use spaces in a name.") + return + } + // Check for non-ascii chars + guard string.allSatisfy({ $0.isASCII }) else { + self = .invalidLiteral( + literal: string, + message: "Must use only ASCII characters.") + return + } + + let dashPrefixCount = string.prefix(while: { $0 == "-" }).count + switch (dashPrefixCount, string.count) { + case (0, _): + self = .invalidLiteral( + literal: string, + message: "Need one or two prefix dashes.") + case (1, 1), (2, 2): + self = .invalidLiteral( + literal: string, + message: "Need at least one character after the dash prefix.") + case (1, 2): + // swift-format-ignore: NeverForceUnwrap + // The case match validates the length. + self = .customShort(string.dropFirst().first!) + case (1, _): + self = .customLong(String(string.dropFirst()), withSingleDash: true) + case (2, _): + self = .customLong(String(string.dropFirst(2))) + default: + self = .invalidLiteral( + literal: string, + message: "Can't have more than a two-dash prefix.") + } + } +} + +extension NameSpecification: + ExpressibleByStringLiteral, ExpressibleByStringInterpolation +{ + public init(stringLiteral string: String) { + guard !string.isEmpty else { + self = [ + .invalidLiteral( + literal: string, + message: "Can't use the empty string as a name.") + ] + return + } + + self.elements = string.split(separator: " ").map { + Element(stringLiteral: String($0)) + } + } +} + extension NameSpecification { /// Use the property's name converted to lowercase with words separated by /// hyphens. @@ -171,6 +245,10 @@ extension NameSpecification.Element { : .long(name) case .customShort(let name, let allowingJoined): return .short(name, allowingJoined: allowingJoined) + case .invalidLiteral(let literal, let message): + configurationFailure( + "Invalid literal name '\(literal)' for property '\(key.name)': \(message)" + .wrapped(to: 70)) } } } @@ -202,6 +280,8 @@ extension FlagInversion { let modifiedElement = NameSpecification.Element.customLong( modifiedName, withSingleDash: withSingleDash) return modifiedElement.name(for: key) + case .invalidLiteral: + fatalError("Invalid literals are diagnosed previously") } } } diff --git a/Tests/ArgumentParserUnitTests/NameSpecificationTests.swift b/Tests/ArgumentParserUnitTests/NameSpecificationTests.swift index 471e5a89e..47d6fad57 100644 --- a/Tests/ArgumentParserUnitTests/NameSpecificationTests.swift +++ b/Tests/ArgumentParserUnitTests/NameSpecificationTests.swift @@ -21,6 +21,10 @@ extension NameSpecificationTests { func testFlagNames_withNoPrefix() { let key = InputKey(name: "index", parent: nil) + XCTAssertEqual( + FlagInversion.prefixedNo.enableDisableNamePair( + for: key, name: .long + ).1, [.long("no-index")]) XCTAssertEqual( FlagInversion.prefixedNo.enableDisableNamePair( for: key, name: .customLong("foo") @@ -37,6 +41,12 @@ extension NameSpecificationTests { FlagInversion.prefixedNo.enableDisableNamePair( for: key, name: .customLong("fooBarBaz") ).1, [.long("noFooBarBaz")]) + + // Short names don't work in combination + XCTAssertEqual( + FlagInversion.prefixedNo.enableDisableNamePair( + for: key, name: .short + ).1, []) } func testFlagNames_withEnableDisablePrefix() { @@ -83,6 +93,12 @@ extension NameSpecificationTests { FlagInversion.prefixedEnableDisable.enableDisableNamePair( for: key, name: .customLong("fooBarBaz") ).1, [.long("disableFooBarBaz")]) + + // Short names don't work in combination + XCTAssertEqual( + FlagInversion.prefixedEnableDisable.enableDisableNamePair( + for: key, name: .short + ).1, []) } } @@ -111,6 +127,19 @@ private func Assert( } } +// swift-format-ignore: AlwaysUseLowerCamelCase +private func AssertInvalid( + nameSpecification: NameSpecification, + file: StaticString = #filePath, + line: UInt = #line +) { + XCTAssert( + nameSpecification.elements.contains(where: { + if case .invalidLiteral = $0.base { return true } else { return false } + }), "Expected invalid name.", + file: file, line: line) +} + // swift-format-ignore: AlwaysUseLowerCamelCase // https://github.com/apple/swift-argument-parser/issues/710 extension NameSpecificationTests { @@ -144,4 +173,75 @@ extension NameSpecificationTests { nameSpecification: .customLong("baz", withSingleDash: true), key: "foo", makeNames: [.longWithSingleDash("baz")]) } + + func testMakeNames_shortLiteral() { + Assert(nameSpecification: "-x", key: "foo", makeNames: [.short("x")]) + Assert(nameSpecification: ["-x"], key: "foo", makeNames: [.short("x")]) + } + + func testMakeNames_longLiteral() { + Assert(nameSpecification: "--foo", key: "foo", makeNames: [.long("foo")]) + Assert(nameSpecification: ["--foo"], key: "foo", makeNames: [.long("foo")]) + Assert( + nameSpecification: "--foo-bar-baz", key: "foo", + makeNames: [.long("foo-bar-baz")]) + Assert( + nameSpecification: "--fooBarBAZ", key: "foo", + makeNames: [.long("fooBarBAZ")]) + } + + func testMakeNames_longWithSingleDashLiteral() { + Assert( + nameSpecification: "-foo", key: "foo", + makeNames: [.longWithSingleDash("foo")]) + Assert( + nameSpecification: ["-foo"], key: "foo", + makeNames: [.longWithSingleDash("foo")]) + Assert( + nameSpecification: "-foo-bar-baz", key: "foo", + makeNames: [.longWithSingleDash("foo-bar-baz")]) + Assert( + nameSpecification: "-fooBarBAZ", key: "foo", + makeNames: [.longWithSingleDash("fooBarBAZ")]) + } + + func testMakeNames_combinedLiteral() { + Assert( + nameSpecification: "-x -y --zilch", key: "foo", + makeNames: [.short("x"), .short("y"), .long("zilch")]) + Assert( + nameSpecification: " -x -y ", key: "foo", + makeNames: [.short("x"), .short("y")]) + Assert( + nameSpecification: ["-x", "-y", "--zilch"], key: "foo", + makeNames: [.short("x"), .short("y"), .long("zilch")]) + } + + func testMakeNames_literalFailures() { + // Empty string + AssertInvalid(nameSpecification: "") + // No dash prefix + AssertInvalid(nameSpecification: "x") + // Dash prefix only + AssertInvalid(nameSpecification: "-") + AssertInvalid(nameSpecification: "--") + AssertInvalid(nameSpecification: "---") + // Triple dash + AssertInvalid(nameSpecification: "---x") + // Non-ASCII + AssertInvalid(nameSpecification: "--café") + + // Repeating as elements + AssertInvalid(nameSpecification: [""]) + AssertInvalid(nameSpecification: ["x"]) + AssertInvalid(nameSpecification: ["-"]) + AssertInvalid(nameSpecification: ["--"]) + AssertInvalid(nameSpecification: ["---"]) + AssertInvalid(nameSpecification: ["---x"]) + AssertInvalid(nameSpecification: ["--café"]) + + // Spaces in _elements_, not the top level literal + AssertInvalid(nameSpecification: ["-x -y -z"]) + AssertInvalid(nameSpecification: ["-x", "-y", " -z"]) + } } From dc7ec065fe476e20f580bf51d637260a6182ca56 Mon Sep 17 00:00:00 2001 From: Nate Cook Date: Wed, 5 Mar 2025 09:06:22 -0600 Subject: [PATCH 2/2] Limit string-expressible names to valid chars --- .../NameSpecification.swift | 4 ++-- .../Utilities/StringExtensions.swift | 21 +++++++++++++++++++ .../NameSpecificationTests.swift | 3 ++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/Sources/ArgumentParser/Parsable Properties/NameSpecification.swift b/Sources/ArgumentParser/Parsable Properties/NameSpecification.swift index 3cb625100..4379ff4e9 100644 --- a/Sources/ArgumentParser/Parsable Properties/NameSpecification.swift +++ b/Sources/ArgumentParser/Parsable Properties/NameSpecification.swift @@ -114,10 +114,10 @@ extension NameSpecification.Element: return } // Check for non-ascii chars - guard string.allSatisfy({ $0.isASCII }) else { + guard string.allSatisfy({ $0.isValidForName }) else { self = .invalidLiteral( literal: string, - message: "Must use only ASCII characters.") + message: "Must use only letters, numbers, underscores, or dashes.") return } diff --git a/Sources/ArgumentParser/Utilities/StringExtensions.swift b/Sources/ArgumentParser/Utilities/StringExtensions.swift index 0e2076727..095228896 100644 --- a/Sources/ArgumentParser/Utilities/StringExtensions.swift +++ b/Sources/ArgumentParser/Utilities/StringExtensions.swift @@ -256,3 +256,24 @@ extension StringProtocol where SubSequence == Substring { isEmpty ? nil : self } } + +extension Character { + /// Returns a Boolean value indicating whether this character is valid for the + /// command-line name of an option or flag. + /// + /// Only ASCII letters, numbers, dashes, and the underscore are valid name + /// characters. + var isValidForName: Bool { + guard isASCII, let firstScalar = unicodeScalars.first else { return false } + switch firstScalar.value { + case 0x41...0x5A, // uppercase + 0x61...0x7A, // lowercase + 0x30...0x39, // numbers + 0x5F, // underscore + 0x2D: // dash + return true + default: + return false + } + } +} diff --git a/Tests/ArgumentParserUnitTests/NameSpecificationTests.swift b/Tests/ArgumentParserUnitTests/NameSpecificationTests.swift index c1ae8e435..600e46379 100644 --- a/Tests/ArgumentParserUnitTests/NameSpecificationTests.swift +++ b/Tests/ArgumentParserUnitTests/NameSpecificationTests.swift @@ -228,8 +228,9 @@ extension NameSpecificationTests { AssertInvalid(nameSpecification: "---") // Triple dash AssertInvalid(nameSpecification: "---x") - // Non-ASCII + // Invalid characters AssertInvalid(nameSpecification: "--café") + AssertInvalid(nameSpecification: "--c!f!") // Repeating as elements AssertInvalid(nameSpecification: [""])