Skip to content

Commit

Permalink
Refactor closingParenOnSameLine options
Browse files Browse the repository at this point in the history
  • Loading branch information
nicklockwood committed Jun 10, 2024
1 parent 1a1c749 commit 25be105
Show file tree
Hide file tree
Showing 11 changed files with 104 additions and 94 deletions.
2 changes: 1 addition & 1 deletion Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -2629,7 +2629,7 @@ Option | Description
`--wrapparameters` | Wrap func params: "before-first", "after-first", "preserve"
`--wrapcollections` | Wrap array/dict: "before-first", "after-first", "preserve"
`--closingparen` | Closing paren position: "balanced" (default) or "same-line"
`--callsiteparen` | Closing paren at callsite: "inherit" (default) or "same-line"
`--callsiteparen` | Closing paren at call sites: "balanced" or "same-line"
`--wrapreturntype` | Wrap return type: "if-multiline", "preserve" (default)
`--wrapconditions` | Wrap conditions: "before-first", "after-first", "preserve"
`--wraptypealiases` | Wrap typealiases: "before-first", "after-first", "preserve"
Expand Down
10 changes: 8 additions & 2 deletions Sources/FormattingHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,13 @@ extension Formatter {
keepParameterLabelsOnSameLine(startOfScope: i,
endOfScope: &endOfScope)

if options.closingCallSiteParenOnSameLine, isFunctionCall(at: i) {
let closingParenOnSameLine: Bool
switch options.callSiteClosingParenPosition {
case .balanced: closingParenOnSameLine = false
case .sameLine: closingParenOnSameLine = true
case .default: closingParenOnSameLine = options.closingParenPosition == .sameLine
}
if closingParenOnSameLine, isFunctionCall(at: i) {
removeLinebreakBeforeEndOfScope(at: &endOfScope)
} else if endOfScopeOnSameLine {
removeLinebreakBeforeEndOfScope(at: &endOfScope)
Expand Down Expand Up @@ -556,7 +562,7 @@ extension Formatter {
return
}

endOfScopeOnSameLine = options.closingParenOnSameLine
endOfScopeOnSameLine = options.closingParenPosition == .sameLine
isParameters = isParameterList(at: i)
if isParameters, options.wrapParameters != .default {
mode = options.wrapParameters
Expand Down
20 changes: 12 additions & 8 deletions Sources/Inference.swift
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ private struct Inference {
options.wrapCollections = formatter.wrapMode(for: "[")
}

let closingParenOnSameLine = OptionInferrer { formatter, options in
let closingParenPosition = OptionInferrer { formatter, options in
var functionCallSameLine = 0, functionCallBalanced = 0
var functionDeclarationSameLine = 0, functionDeclarationBalanced = 0

Expand Down Expand Up @@ -407,20 +407,24 @@ private struct Inference {
}
}

// Decide on closingCallSiteParenOnSameLine
// Decide on callSiteClosingParenPosition
if functionCallSameLine > functionCallBalanced && functionDeclarationBalanced > functionDeclarationSameLine {
options.closingCallSiteParenOnSameLine = true
options.callSiteClosingParenPosition = .sameLine
} else {
options.closingCallSiteParenOnSameLine = false
options.callSiteClosingParenPosition = .balanced
}

// If closingCallSiteParenOnSameLine is true, trust only the declarations to infer closingParenOnSameLine
if options.closingCallSiteParenOnSameLine {
options.closingParenOnSameLine = functionDeclarationSameLine > functionDeclarationBalanced
// If callSiteClosingParenPosition is sameLine, trust only the declarations to infer closingParenPosition
if options.callSiteClosingParenPosition == .sameLine {
options.closingParenPosition = functionDeclarationSameLine > functionDeclarationBalanced ? .sameLine : .balanced
} else {
let balanced = functionDeclarationBalanced + functionCallBalanced
let sameLine = functionDeclarationSameLine + functionCallSameLine
options.closingParenOnSameLine = sameLine > balanced
options.closingParenPosition = sameLine > balanced ? .sameLine : .balanced
}

if options.closingParenPosition == options.callSiteClosingParenPosition {
options.callSiteClosingParenPosition = .default
}
}

Expand Down
14 changes: 5 additions & 9 deletions Sources/OptionDescriptor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -496,21 +496,17 @@ struct _Descriptors {
help: "Wrap ternary operators: \"default\", \"before-operators\"",
keyPath: \.wrapTernaryOperators
)
let closingParenOnSameLine = OptionDescriptor(
let closingParenPosition = OptionDescriptor(
argumentName: "closingparen",
displayName: "Closing Paren Position",
help: "Closing paren position: \"balanced\" (default) or \"same-line\"",
keyPath: \.closingParenOnSameLine,
trueValues: ["same-line"],
falseValues: ["balanced"]
keyPath: \.closingParenPosition
)
let closingCallSiteParenOnSameLine = OptionDescriptor(
let callSiteClosingParenPosition = OptionDescriptor(
argumentName: "callsiteparen",
displayName: "Call Site Closing Paren",
help: "Closing paren at callsite: \"inherit\" (default) or \"same-line\"",
keyPath: \.closingCallSiteParenOnSameLine,
trueValues: ["same-line"],
falseValues: ["inherit"]
help: "Closing paren at call sites: \"balanced\" or \"same-line\"",
keyPath: \.callSiteClosingParenPosition
)
let uppercaseHex = OptionDescriptor(
argumentName: "hexliteralcase",
Expand Down
19 changes: 13 additions & 6 deletions Sources/Options.swift
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,13 @@ public enum NilInitType: String, CaseIterable {
case insert
}

/// Placement for closing paren in a function call or definition
public enum ClosingParenPosition: String, CaseIterable {
case balanced
case sameLine = "same-line"
case `default`
}

/// Configuration options for formatting. These aren't actually used by the
/// Formatter class itself, but it makes them available to the format rules.
public struct FormatOptions: CustomStringConvertible {
Expand All @@ -575,8 +582,8 @@ public struct FormatOptions: CustomStringConvertible {
public var wrapCollections: WrapMode
public var wrapTypealiases: WrapMode
public var wrapEnumCases: WrapEnumCases
public var closingParenOnSameLine: Bool
public var closingCallSiteParenOnSameLine: Bool
public var closingParenPosition: ClosingParenPosition
public var callSiteClosingParenPosition: ClosingParenPosition
public var wrapReturnType: WrapReturnType
public var wrapConditions: WrapMode
public var wrapTernaryOperators: TernaryOperatorWrapMode
Expand Down Expand Up @@ -689,8 +696,8 @@ public struct FormatOptions: CustomStringConvertible {
wrapCollections: WrapMode = .preserve,
wrapTypealiases: WrapMode = .preserve,
wrapEnumCases: WrapEnumCases = .always,
closingParenOnSameLine: Bool = false,
closingCallSiteParenOnSameLine: Bool = false,
closingParenPosition: ClosingParenPosition = .balanced,
callSiteClosingParenPosition: ClosingParenPosition = .default,
wrapReturnType: WrapReturnType = .preserve,
wrapConditions: WrapMode = .preserve,
wrapTernaryOperators: TernaryOperatorWrapMode = .default,
Expand Down Expand Up @@ -793,8 +800,8 @@ public struct FormatOptions: CustomStringConvertible {
self.wrapCollections = wrapCollections
self.wrapTypealiases = wrapTypealiases
self.wrapEnumCases = wrapEnumCases
self.closingParenOnSameLine = closingParenOnSameLine
self.closingCallSiteParenOnSameLine = closingCallSiteParenOnSameLine
self.closingParenPosition = closingParenPosition
self.callSiteClosingParenPosition = callSiteClosingParenPosition
self.wrapReturnType = wrapReturnType
self.wrapConditions = wrapConditions
self.wrapTernaryOperators = wrapTernaryOperators
Expand Down
6 changes: 3 additions & 3 deletions Tests/InferenceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -292,18 +292,18 @@ class InferenceTests: XCTestCase {
XCTAssertEqual(options.wrapCollections, .afterFirst)
}

// MARK: closingParenOnSameLine
// MARK: closingParenPosition

func testInferParenOnSameLine() {
let input = "func foo(\n bar: Int,\n baz: String) {\n}\nfunc foo(\n bar: Int,\n baz: String)"
let options = inferFormatOptions(from: tokenize(input))
XCTAssertTrue(options.closingParenOnSameLine)
XCTAssertEqual(options.closingParenPosition, .sameLine)
}

func testInferParenOnNextLine() {
let input = "func foo(\n bar: Int,\n baz: String) {\n}\nfunc foo(\n bar: Int,\n baz: String\n)"
let options = inferFormatOptions(from: tokenize(input))
XCTAssertFalse(options.closingParenOnSameLine)
XCTAssertEqual(options.closingParenPosition, .balanced)
}

// MARK: uppercaseHex
Expand Down
2 changes: 1 addition & 1 deletion Tests/MetadataTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ class MetadataTests: XCTestCase {
case .identifier("wrapCollectionsAndArguments"):
referencedOptions += [
Descriptors.wrapArguments, Descriptors.wrapParameters, Descriptors.wrapCollections,
Descriptors.closingParenOnSameLine, Descriptors.closingCallSiteParenOnSameLine,
Descriptors.closingParenPosition, Descriptors.callSiteClosingParenPosition,
Descriptors.linebreak, Descriptors.truncateBlankLines,
Descriptors.indent, Descriptors.tabWidth, Descriptors.smartTabs, Descriptors.maxWidth,
Descriptors.assetLiteralWidth, Descriptors.wrapReturnType, Descriptors.wrapEffects,
Expand Down
4 changes: 2 additions & 2 deletions Tests/RulesTests+Braces.swift
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ class BracesTests: RulesTests {
}
"""

let options = FormatOptions(wrapArguments: .beforeFirst, closingParenOnSameLine: true)
let options = FormatOptions(wrapArguments: .beforeFirst, closingParenPosition: .sameLine)
testFormatting(for: input, rules: [FormatRules.braces, FormatRules.wrapMultilineStatementBraces], options: options)
}

Expand All @@ -557,7 +557,7 @@ class BracesTests: RulesTests {
}
"""

let options = FormatOptions(wrapArguments: .beforeFirst, closingParenOnSameLine: true)
let options = FormatOptions(wrapArguments: .beforeFirst, closingParenPosition: .sameLine)
testFormatting(for: input, rules: [FormatRules.braces, FormatRules.wrapMultilineStatementBraces], options: options)
}
}
31 changes: 14 additions & 17 deletions Tests/RulesTests+Indentation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ class IndentTests: RulesTests {
.build()
}
"""
let options = FormatOptions(closingParenOnSameLine: true)
let options = FormatOptions(closingParenPosition: .sameLine)
testFormatting(for: input, rule: FormatRules.indent, options: options)
}

Expand Down Expand Up @@ -372,7 +372,7 @@ class IndentTests: RulesTests {
self?.viewportLoggingRegistry.logViewportSessionEnd(with: viewportLoggingContext)
}
"""
let options = FormatOptions(closingParenOnSameLine: true)
let options = FormatOptions(closingParenPosition: .sameLine)
testFormatting(for: input, rule: FormatRules.indent, options: options)
}

Expand Down Expand Up @@ -483,10 +483,7 @@ class IndentTests: RulesTests {
"one"
}
"""
let options = FormatOptions(
wrapArguments: .afterFirst,
closingParenOnSameLine: true
)
let options = FormatOptions(wrapArguments: .afterFirst, closingParenPosition: .sameLine)
testFormatting(for: input, rule: FormatRules.indent, options: options)
}

Expand Down Expand Up @@ -1469,7 +1466,7 @@ class IndentTests: RulesTests {
},
baz: baz)
"""
let options = FormatOptions(closingParenOnSameLine: true)
let options = FormatOptions(closingParenPosition: .sameLine)
testFormatting(for: input, rule: FormatRules.indent, options: options)
}

Expand Down Expand Up @@ -1528,7 +1525,7 @@ class IndentTests: RulesTests {
}
}
"""
let options = FormatOptions(wrapArguments: .disabled, closingParenOnSameLine: false)
let options = FormatOptions(wrapArguments: .disabled, closingParenPosition: .balanced)
testFormatting(for: input, rule: FormatRules.indent, options: options,
exclude: ["wrapConditionalBodies"])
}
Expand All @@ -1543,7 +1540,7 @@ class IndentTests: RulesTests {
}
}
"""
let options = FormatOptions(wrapArguments: .disabled, closingParenOnSameLine: true)
let options = FormatOptions(wrapArguments: .disabled, closingParenPosition: .sameLine)
testFormatting(for: input, rule: FormatRules.indent, options: options,
exclude: ["wrapConditionalBodies", "wrapMultilineStatementBraces"])
}
Expand All @@ -1559,7 +1556,7 @@ class IndentTests: RulesTests {
}
}
"""
let options = FormatOptions(wrapArguments: .disabled, closingParenOnSameLine: true)
let options = FormatOptions(wrapArguments: .disabled, closingParenPosition: .sameLine)
testFormatting(for: input, rule: FormatRules.indent, options: options,
exclude: ["wrapConditionalBodies", "wrapMultilineStatementBraces"])
}
Expand All @@ -1575,7 +1572,7 @@ class IndentTests: RulesTests {
}
}
"""
let options = FormatOptions(wrapArguments: .disabled, closingParenOnSameLine: true)
let options = FormatOptions(wrapArguments: .disabled, closingParenPosition: .sameLine)
testFormatting(for: input, rule: FormatRules.indent, options: options,
exclude: ["wrapMultilineStatementBraces"])
}
Expand All @@ -1588,7 +1585,7 @@ class IndentTests: RulesTests {
cancelBlock()
}
"""
let options = FormatOptions(wrapArguments: .disabled, closingParenOnSameLine: true)
let options = FormatOptions(wrapArguments: .disabled, closingParenPosition: .sameLine)
testFormatting(for: input, rule: FormatRules.indent, options: options)
}

Expand All @@ -1604,7 +1601,7 @@ class IndentTests: RulesTests {
}
}
"""
let options = FormatOptions(wrapArguments: .disabled, closingParenOnSameLine: true)
let options = FormatOptions(wrapArguments: .disabled, closingParenPosition: .sameLine)
testFormatting(for: input, rule: FormatRules.indent, options: options,
exclude: ["braces", "wrapConditionalBodies"])
}
Expand All @@ -1616,7 +1613,7 @@ class IndentTests: RulesTests {
print("and a trailing closure")
}
"""
let options = FormatOptions(wrapArguments: .disabled, closingParenOnSameLine: true)
let options = FormatOptions(wrapArguments: .disabled, closingParenPosition: .sameLine)
testFormatting(for: input, rule: FormatRules.indent, options: options,
exclude: ["wrapConditionalBodies"])
}
Expand Down Expand Up @@ -3706,7 +3703,7 @@ class IndentTests: RulesTests {
bar _: String)
async throws -> String {}
"""
let options = FormatOptions(closingParenOnSameLine: true)
let options = FormatOptions(closingParenPosition: .sameLine)
testFormatting(for: input, rule: FormatRules.indent, options: options)
}

Expand All @@ -3717,7 +3714,7 @@ class IndentTests: RulesTests {
bar _: String)
async throws(Foo) -> String {}
"""
let options = FormatOptions(closingParenOnSameLine: true)
let options = FormatOptions(closingParenPosition: .sameLine)
testFormatting(for: input, rule: FormatRules.indent, options: options)
}

Expand Down Expand Up @@ -3767,7 +3764,7 @@ class IndentTests: RulesTests {
async _: String)
-> String {}
"""
let options = FormatOptions(closingParenOnSameLine: true)
let options = FormatOptions(closingParenPosition: .sameLine)
testFormatting(for: input, rule: FormatRules.indent, options: options)
}

Expand Down
4 changes: 2 additions & 2 deletions Tests/RulesTests+Redundancy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8695,7 +8695,7 @@ class RedundancyTests: RulesTests {
bar: bar)
"""

let options = FormatOptions(wrapArguments: .beforeFirst, closingParenOnSameLine: true)
let options = FormatOptions(wrapArguments: .beforeFirst, closingParenPosition: .sameLine)
testFormatting(for: input, [output],
rules: [FormatRules.redundantClosure, FormatRules.wrapArguments],
options: options)
Expand All @@ -8714,7 +8714,7 @@ class RedundancyTests: RulesTests {
bar: bar)
"""

let options = FormatOptions(wrapArguments: .afterFirst, closingParenOnSameLine: true)
let options = FormatOptions(wrapArguments: .afterFirst, closingParenPosition: .sameLine)
testFormatting(for: input, [output],
rules: [FormatRules.redundantClosure, FormatRules.wrapArguments],
options: options)
Expand Down
Loading

0 comments on commit 25be105

Please sign in to comment.