Skip to content

Commit

Permalink
Flip --shortoptionals default to except-properties
Browse files Browse the repository at this point in the history
  • Loading branch information
@nicklockwood
nicklockwood committed Nov 11, 2023
1 parent 1639d00 commit 5ec0a94
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 27 deletions.
2 changes: 1 addition & 1 deletion Rules.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2383,7 +2383,7 @@ Prefer shorthand syntax for Arrays, Dictionaries and Optionals.

Option | Description
--- | ---
`--shortoptionals` | Use ? for optionals "always" (default) or "except-properties"

This comment has been minimized.

Sign in to view

Copy link
@calda

calda Nov 13, 2023

Collaborator

@nicklockwood -- just curious, why change the default here? Is there a problem with converting properties from the Optional<T> syntax to T??

This comment has been minimized.

Sign in to view

Copy link
@nicklockwood

nicklockwood Nov 13, 2023

Author Owner

It makes them optional in the initializer, which makes it easy to forget to set them as the compiler won't warn you if you add a new optional property and don't update all the call sites.
`--shortoptionals` | Use ? for optionals "always" or "except-properties" (default)

<details>
<summary>Examples</summary>
Expand Down
2 changes: 1 addition & 1 deletion Sources/OptionDescriptor.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ struct _Descriptors {
let shortOptionals = OptionDescriptor(
argumentName: "shortoptionals",
displayName: "Short Optional Syntax",
help: "Use ? for optionals \"always\" (default) or \"except-properties\"",
help: "Use ? for optionals \"always\" or \"except-properties\" (default)",
keyPath: \.shortOptionals
)
let markTypes = OptionDescriptor(
Expand Down
2 changes: 1 addition & 1 deletion Sources/Options.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,7 @@ public struct FormatOptions: CustomStringConvertible {
noSpaceOperators: Set<String> = [],
noWrapOperators: Set<String> = [],
modifierOrder: [String] = [],
shortOptionals: OptionalsMode = .always,
shortOptionals: OptionalsMode = .exceptProperties,
funcAttributes: AttributeMode = .preserve,
typeAttributes: AttributeMode = .preserve,
varAttributes: AttributeMode = .preserve,
Expand Down
58 changes: 34 additions & 24 deletions Tests/RulesTests+Syntax.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1656,40 +1656,51 @@ class SyntaxTests: RulesTests {

// optionals

func testOptionalPropertyTypeNotConvertedToSugarByDefault() {
let input = "var bar: Optional<String>"
testFormatting(for: input, rule: FormatRules.typeSugar)
}

func testOptionalTypeConvertedToSugar() {
let input = "var foo: Optional<String>"
let output = "var foo: String?"
testFormatting(for: input, output, rule: FormatRules.typeSugar)
let options = FormatOptions(shortOptionals: .always)
testFormatting(for: input, output, rule: FormatRules.typeSugar, options: options)
}

func testSwiftOptionalTypeConvertedToSugar() {
let input = "var foo: Swift.Optional<String>"
let output = "var foo: String?"
testFormatting(for: input, output, rule: FormatRules.typeSugar)
let options = FormatOptions(shortOptionals: .always)
testFormatting(for: input, output, rule: FormatRules.typeSugar, options: options)
}

func testOptionalClosureParenthesizedConvertedToSugar() {
let input = "var foo: Optional<(Int) -> String>"
let output = "var foo: ((Int) -> String)?"
testFormatting(for: input, output, rule: FormatRules.typeSugar)
let options = FormatOptions(shortOptionals: .always)
testFormatting(for: input, output, rule: FormatRules.typeSugar, options: options)
}

func testOptionalTupleWrappedInParensConvertedToSugar() {
let input = "let foo: Optional<(foo: Int, bar: String)>"
let output = "let foo: (foo: Int, bar: String)?"
testFormatting(for: input, output, rule: FormatRules.typeSugar)
let options = FormatOptions(shortOptionals: .always)
testFormatting(for: input, output, rule: FormatRules.typeSugar, options: options)
}

func testOptionalComposedProtocolWrappedInParensConvertedToSugar() {
let input = "let foo: Optional<UIView & Foo>"
let output = "let foo: (UIView & Foo)?"
testFormatting(for: input, output, rule: FormatRules.typeSugar)
let options = FormatOptions(shortOptionals: .always)
testFormatting(for: input, output, rule: FormatRules.typeSugar, options: options)
}

func testSwiftOptionalClosureParenthesizedConvertedToSugar() {
let input = "var foo: Swift.Optional<(Int) -> String>"
let output = "var foo: ((Int) -> String)?"
testFormatting(for: input, output, rule: FormatRules.typeSugar)
let options = FormatOptions(shortOptionals: .always)
testFormatting(for: input, output, rule: FormatRules.typeSugar, options: options)
}

func testStrippingSwiftNamespaceInOptionalTypeWhenConvertedToSugar() {
Expand All @@ -1701,7 +1712,8 @@ class SyntaxTests: RulesTests {
func testStrippingSwiftNamespaceDoesNotStripPreviousSwiftNamespaceReferences() {
let input = "let a: Swift.String = Optional<String>"
let output = "let a: Swift.String = String?"
testFormatting(for: input, output, rule: FormatRules.typeSugar)
let options = FormatOptions(shortOptionals: .always)
testFormatting(for: input, output, rule: FormatRules.typeSugar, options: options)
}

func testOptionalTypeInsideCaseConvertedToSugar() {
Expand Down Expand Up @@ -1730,65 +1742,63 @@ class SyntaxTests: RulesTests {
testFormatting(for: input, output, rule: FormatRules.typeSugar)
}

// shortOptionals = exceptProperties

func testPropertyTypeNotConvertedToSugar() {
let input = "var foo: Optional<String>"
let options = FormatOptions(shortOptionals: .exceptProperties)
testFormatting(for: input, rule: FormatRules.typeSugar, options: options)
}

// swift parser bug

func testAvoidSwiftParserBugWithClosuresInsideArrays() {
let input = "var foo = Array<(_ image: Data?) -> Void>()"
testFormatting(for: input, rule: FormatRules.typeSugar)
testFormatting(for: input, rule: FormatRules.typeSugar, options: FormatOptions(shortOptionals: .always))
}

func testAvoidSwiftParserBugWithClosuresInsideDictionaries() {
let input = "var foo = Dictionary<String, (_ image: Data?) -> Void>()"
testFormatting(for: input, rule: FormatRules.typeSugar)
testFormatting(for: input, rule: FormatRules.typeSugar, options: FormatOptions(shortOptionals: .always))
}

func testAvoidSwiftParserBugWithClosuresInsideOptionals() {
let input = "var foo = Optional<(_ image: Data?) -> Void>()"
testFormatting(for: input, rule: FormatRules.typeSugar)
testFormatting(for: input, rule: FormatRules.typeSugar, options: FormatOptions(shortOptionals: .always))
}

func testDontOverApplyBugWorkaround() {
let input = "var foo: Array<(_ image: Data?) -> Void>"
let output = "var foo: [(_ image: Data?) -> Void]"
testFormatting(for: input, output, rule: FormatRules.typeSugar)
let options = FormatOptions(shortOptionals: .always)
testFormatting(for: input, output, rule: FormatRules.typeSugar, options: options)
}

func testDontOverApplyBugWorkaround2() {
let input = "var foo: Dictionary<String, (_ image: Data?) -> Void>"
let output = "var foo: [String: (_ image: Data?) -> Void]"
testFormatting(for: input, output, rule: FormatRules.typeSugar)
let options = FormatOptions(shortOptionals: .always)
testFormatting(for: input, output, rule: FormatRules.typeSugar, options: options)
}

func testDontOverApplyBugWorkaround3() {
let input = "var foo: Optional<(_ image: Data?) -> Void>"
let output = "var foo: ((_ image: Data?) -> Void)?"
testFormatting(for: input, output, rule: FormatRules.typeSugar)
let options = FormatOptions(shortOptionals: .always)
testFormatting(for: input, output, rule: FormatRules.typeSugar, options: options)
}

func testDontOverApplyBugWorkaround4() {
let input = "var foo = Array<(image: Data?) -> Void>()"
let output = "var foo = [(image: Data?) -> Void]()"
testFormatting(for: input, output, rule: FormatRules.typeSugar)
let options = FormatOptions(shortOptionals: .always)
testFormatting(for: input, output, rule: FormatRules.typeSugar, options: options)
}

func testDontOverApplyBugWorkaround5() {
let input = "var foo = Array<(Data?) -> Void>()"
let output = "var foo = [(Data?) -> Void]()"
testFormatting(for: input, output, rule: FormatRules.typeSugar)
let options = FormatOptions(shortOptionals: .always)
testFormatting(for: input, output, rule: FormatRules.typeSugar, options: options)
}

func testDontOverApplyBugWorkaround6() {
let input = "var foo = Dictionary<Int, Array<(_ image: Data?) -> Void>>()"
let output = "var foo = [Int: Array<(_ image: Data?) -> Void>]()"
testFormatting(for: input, output, rule: FormatRules.typeSugar)
let options = FormatOptions(shortOptionals: .always)
testFormatting(for: input, output, rule: FormatRules.typeSugar, options: options)
}

// MARK: - preferKeyPath
Expand Down

0 comments on commit 5ec0a94

Please sign in to comment.