Skip to content

Commit 8fe7a2a

Browse files
authored
Merge pull request #495 from ahoppen/ahoppen/split-function-parameter
Adjustments to split `FunctionParameterSyntax` into multiple nodes for function parameters, closure parameters and enum parameters
2 parents 3f0ed45 + 1ce24a9 commit 8fe7a2a

9 files changed

+149
-19
lines changed

Sources/SwiftFormat/Pipelines+Generated.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ class LintPipeline: SyntaxVisitor {
5555
return .visitChildren
5656
}
5757

58+
override func visit(_ node: ClosureParameterSyntax) -> SyntaxVisitorContinueKind {
59+
visitIfEnabled(NoLeadingUnderscores.visit, for: node)
60+
return .visitChildren
61+
}
62+
5863
override func visit(_ node: ClosureSignatureSyntax) -> SyntaxVisitorContinueKind {
5964
visitIfEnabled(AlwaysUseLowerCamelCase.visit, for: node)
6065
visitIfEnabled(ReturnVoidInsteadOfEmptyTuple.visit, for: node)
@@ -92,6 +97,11 @@ class LintPipeline: SyntaxVisitor {
9297
return .visitChildren
9398
}
9499

100+
override func visit(_ node: EnumCaseParameterSyntax) -> SyntaxVisitorContinueKind {
101+
visitIfEnabled(NoLeadingUnderscores.visit, for: node)
102+
return .visitChildren
103+
}
104+
95105
override func visit(_ node: EnumDeclSyntax) -> SyntaxVisitorContinueKind {
96106
visitIfEnabled(BeginDocumentationCommentWithOneLineSummary.visit, for: node)
97107
visitIfEnabled(DontRepeatTypeInStaticProperties.visit, for: node)

Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift

Lines changed: 94 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,7 +1122,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
11221122
// When it's parenthesized, the input is a `ParameterClauseSyntax`. Otherwise, it's a
11231123
// `ClosureParamListSyntax`. The parenthesized version is wrapped in open/close breaks so that
11241124
// the parens create an extra level of indentation.
1125-
if let parameterClause = input.as(ParameterClauseSyntax.self) {
1125+
if let parameterClause = input.as(ClosureParameterClauseSyntax.self) {
11261126
// Whether we should prioritize keeping ") throws -> <return_type>" together. We can only do
11271127
// this if the closure has arguments.
11281128
let keepOutputTogether =
@@ -1141,7 +1141,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
11411141
after(input.lastToken, tokens: .close)
11421142
}
11431143

1144-
arrangeParameterClause(parameterClause, forcesBreakBeforeRightParen: true)
1144+
arrangeClosureParameterClause(parameterClause, forcesBreakBeforeRightParen: true)
11451145
} else {
11461146
// Group around the arguments, but don't use open/close breaks because there are no parens
11471147
// to create a new scope.
@@ -1245,6 +1245,30 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
12451245
return .visitChildren
12461246
}
12471247

1248+
override func visit(_ node: ClosureParameterClauseSyntax) -> SyntaxVisitorContinueKind {
1249+
// Prioritize keeping ") throws -> <return_type>" together. We can only do this if the function
1250+
// has arguments.
1251+
if !node.parameterList.isEmpty && config.prioritizeKeepingFunctionOutputTogether {
1252+
// Due to visitation order, this .open corresponds to a .close added in FunctionDeclSyntax
1253+
// or SubscriptDeclSyntax.
1254+
before(node.rightParen, tokens: .open)
1255+
}
1256+
1257+
return .visitChildren
1258+
}
1259+
1260+
override func visit(_ node: EnumCaseParameterClauseSyntax) -> SyntaxVisitorContinueKind {
1261+
// Prioritize keeping ") throws -> <return_type>" together. We can only do this if the function
1262+
// has arguments.
1263+
if !node.parameterList.isEmpty && config.prioritizeKeepingFunctionOutputTogether {
1264+
// Due to visitation order, this .open corresponds to a .close added in FunctionDeclSyntax
1265+
// or SubscriptDeclSyntax.
1266+
before(node.rightParen, tokens: .open)
1267+
}
1268+
1269+
return .visitChildren
1270+
}
1271+
12481272
override func visit(_ node: ParameterClauseSyntax) -> SyntaxVisitorContinueKind {
12491273
// Prioritize keeping ") throws -> <return_type>" together. We can only do this if the function
12501274
// has arguments.
@@ -1257,6 +1281,37 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
12571281
return .visitChildren
12581282
}
12591283

1284+
override func visit(_ node: ClosureParameterSyntax) -> SyntaxVisitorContinueKind {
1285+
before(node.firstToken, tokens: .open)
1286+
arrangeAttributeList(node.attributes)
1287+
before(
1288+
node.secondName,
1289+
tokens: .break(.continue, newlines: .elective(ignoresDiscretionary: true)))
1290+
after(node.colon, tokens: .break)
1291+
1292+
if let trailingComma = node.trailingComma {
1293+
after(trailingComma, tokens: .close, .break(.same))
1294+
} else {
1295+
after(node.lastToken, tokens: .close)
1296+
}
1297+
return .visitChildren
1298+
}
1299+
1300+
override func visit(_ node: EnumCaseParameterSyntax) -> SyntaxVisitorContinueKind {
1301+
before(node.firstToken, tokens: .open)
1302+
before(
1303+
node.secondName,
1304+
tokens: .break(.continue, newlines: .elective(ignoresDiscretionary: true)))
1305+
after(node.colon, tokens: .break)
1306+
1307+
if let trailingComma = node.trailingComma {
1308+
after(trailingComma, tokens: .close, .break(.same))
1309+
} else {
1310+
after(node.lastToken, tokens: .close)
1311+
}
1312+
return .visitChildren
1313+
}
1314+
12601315
override func visit(_ node: FunctionParameterSyntax) -> SyntaxVisitorContinueKind {
12611316
before(node.firstToken, tokens: .open)
12621317
arrangeAttributeList(node.attributes)
@@ -1413,7 +1468,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
14131468
after(node.trailingComma, tokens: .break)
14141469

14151470
if let associatedValue = node.associatedValue {
1416-
arrangeParameterClause(associatedValue, forcesBreakBeforeRightParen: false)
1471+
arrangeEnumCaseParameterClause(associatedValue, forcesBreakBeforeRightParen: false)
14171472
}
14181473

14191474
return .visitChildren
@@ -2588,6 +2643,42 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
25882643
return contentsIterator.next() == nil && !commentPrecedesRightBrace
25892644
}
25902645

2646+
/// Applies formatting to a collection of parameters for a decl.
2647+
///
2648+
/// - Parameters:
2649+
/// - parameters: A node that contains the parameters that can be passed to a decl when its
2650+
/// called.
2651+
/// - forcesBreakBeforeRightParen: Whether a break should be required before the right paren
2652+
/// when the right paren is on a different line than the corresponding left paren.
2653+
private func arrangeClosureParameterClause(
2654+
_ parameters: ClosureParameterClauseSyntax, forcesBreakBeforeRightParen: Bool
2655+
) {
2656+
guard !parameters.parameterList.isEmpty else { return }
2657+
2658+
after(parameters.leftParen, tokens: .break(.open, size: 0), .open(argumentListConsistency()))
2659+
before(
2660+
parameters.rightParen,
2661+
tokens: .break(.close(mustBreak: forcesBreakBeforeRightParen), size: 0), .close)
2662+
}
2663+
2664+
/// Applies formatting to a collection of enum case parameters for a decl.
2665+
///
2666+
/// - Parameters:
2667+
/// - parameters: A node that contains the parameters that can be passed to a decl when its
2668+
/// called.
2669+
/// - forcesBreakBeforeRightParen: Whether a break should be required before the right paren
2670+
/// when the right paren is on a different line than the corresponding left paren.
2671+
private func arrangeEnumCaseParameterClause(
2672+
_ parameters: EnumCaseParameterClauseSyntax, forcesBreakBeforeRightParen: Bool
2673+
) {
2674+
guard !parameters.parameterList.isEmpty else { return }
2675+
2676+
after(parameters.leftParen, tokens: .break(.open, size: 0), .open(argumentListConsistency()))
2677+
before(
2678+
parameters.rightParen,
2679+
tokens: .break(.close(mustBreak: forcesBreakBeforeRightParen), size: 0), .close)
2680+
}
2681+
25912682
/// Applies formatting to a collection of parameters for a decl.
25922683
///
25932684
/// - Parameters:

Sources/SwiftFormatRules/AlwaysUseLowerCamelCase.swift

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,16 @@ public final class AlwaysUseLowerCamelCase: SyntaxLintRule {
7373
diagnoseLowerCamelCaseViolations(
7474
param.name, allowUnderscores: false, description: identifierDescription(for: node))
7575
}
76-
} else if let parameterClause = input.as(ParameterClauseSyntax.self) {
76+
} else if let parameterClause = input.as(ClosureParameterClauseSyntax.self) {
77+
for param in parameterClause.parameterList {
78+
diagnoseLowerCamelCaseViolations(
79+
param.firstName, allowUnderscores: false, description: identifierDescription(for: node))
80+
if let secondName = param.secondName {
81+
diagnoseLowerCamelCaseViolations(
82+
secondName, allowUnderscores: false, description: identifierDescription(for: node))
83+
}
84+
}
85+
} else if let parameterClause = input.as(EnumCaseParameterClauseSyntax.self) {
7786
for param in parameterClause.parameterList {
7887
if let firstName = param.firstName {
7988
diagnoseLowerCamelCaseViolations(
@@ -84,6 +93,15 @@ public final class AlwaysUseLowerCamelCase: SyntaxLintRule {
8493
secondName, allowUnderscores: false, description: identifierDescription(for: node))
8594
}
8695
}
96+
} else if let parameterClause = input.as(ParameterClauseSyntax.self) {
97+
for param in parameterClause.parameterList {
98+
diagnoseLowerCamelCaseViolations(
99+
param.firstName, allowUnderscores: false, description: identifierDescription(for: node))
100+
if let secondName = param.secondName {
101+
diagnoseLowerCamelCaseViolations(
102+
secondName, allowUnderscores: false, description: identifierDescription(for: node))
103+
}
104+
}
87105
}
88106
}
89107
return .visitChildren
@@ -106,10 +124,8 @@ public final class AlwaysUseLowerCamelCase: SyntaxLintRule {
106124
for param in node.signature.input.parameterList {
107125
// These identifiers aren't described using `identifierDescription(for:)` because no single
108126
// node can disambiguate the argument label from the parameter name.
109-
if let label = param.firstName {
110-
diagnoseLowerCamelCaseViolations(
111-
label, allowUnderscores: false, description: "argument label")
112-
}
127+
diagnoseLowerCamelCaseViolations(
128+
param.firstName, allowUnderscores: false, description: "argument label")
113129
if let paramName = param.secondName {
114130
diagnoseLowerCamelCaseViolations(
115131
paramName, allowUnderscores: false, description: "function parameter")

Sources/SwiftFormatRules/AmbiguousTrailingClosureOverload.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public final class AmbiguousTrailingClosureOverload: SyntaxLintRule {
4141
for fn in functions {
4242
let params = fn.signature.input.parameterList
4343
guard let firstParam = params.firstAndOnly else { continue }
44-
guard let type = firstParam.type, type.is(FunctionTypeSyntax.self) else { continue }
44+
guard firstParam.type.is(FunctionTypeSyntax.self) else { continue }
4545
if let mods = fn.modifiers, mods.has(modifier: "static") || mods.has(modifier: "class") {
4646
staticOverloads[fn.identifier.text, default: []].append(fn)
4747
} else {

Sources/SwiftFormatRules/FunctionDeclSyntax+Convenience.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ extension FunctionDeclSyntax {
1616
/// Constructs a name for a function that includes parameter labels, i.e. `foo(_:bar:)`.
1717
var fullDeclName: String {
1818
let params = signature.input.parameterList.map { param in
19-
"\(param.firstName?.text ?? "_"):"
19+
"\(param.firstName.text):"
2020
}
2121
return "\(identifier.text)(\(params.joined()))"
2222
}

Sources/SwiftFormatRules/NoLeadingUnderscores.swift

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,15 @@ public final class NoLeadingUnderscores: SyntaxLintRule {
5656
return .visitChildren
5757
}
5858

59-
public override func visit(_ node: FunctionParameterSyntax) -> SyntaxVisitorContinueKind {
59+
public override func visit(_ node: ClosureParameterSyntax) -> SyntaxVisitorContinueKind {
60+
// If both names are provided, we want to check `secondName`, which will be the parameter name
61+
// (in that case, `firstName` is the label). If only one name is present, then it is recorded in
62+
// `firstName`, and it is both the label and the parameter name.
63+
diagnoseIfNameStartsWithUnderscore(node.secondName ?? node.firstName)
64+
return .visitChildren
65+
}
66+
67+
public override func visit(_ node: EnumCaseParameterSyntax) -> SyntaxVisitorContinueKind {
6068
// If both names are provided, we want to check `secondName`, which will be the parameter name
6169
// (in that case, `firstName` is the label). If only one name is present, then it is recorded in
6270
// `firstName`, and it is both the label and the parameter name.
@@ -66,6 +74,14 @@ public final class NoLeadingUnderscores: SyntaxLintRule {
6674
return .visitChildren
6775
}
6876

77+
public override func visit(_ node: FunctionParameterSyntax) -> SyntaxVisitorContinueKind {
78+
// If both names are provided, we want to check `secondName`, which will be the parameter name
79+
// (in that case, `firstName` is the label). If only one name is present, then it is recorded in
80+
// `firstName`, and it is both the label and the parameter name.
81+
diagnoseIfNameStartsWithUnderscore(node.secondName ?? node.firstName)
82+
return .visitChildren
83+
}
84+
6985
public override func visit(_ node: GenericParameterSyntax) -> SyntaxVisitorContinueKind {
7086
diagnoseIfNameStartsWithUnderscore(node.name)
7187
return .visitChildren

Sources/SwiftFormatRules/UseSynthesizedInitializer.swift

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,7 @@ public final class UseSynthesizedInitializer: SyntaxLintRule {
106106
guard parameters.count == properties.count else { return false }
107107
for (idx, parameter) in parameters.enumerated() {
108108

109-
guard let paramId = parameter.firstName, parameter.secondName == nil else { return false }
110-
guard let paramType = parameter.type else { return false }
109+
guard parameter.secondName == nil else { return false }
111110

112111
let property = properties[idx]
113112
let propertyId = property.firstIdentifier
@@ -124,9 +123,9 @@ public final class UseSynthesizedInitializer: SyntaxLintRule {
124123
return false
125124
}
126125

127-
if propertyId.identifier.text != paramId.text
126+
if propertyId.identifier.text != parameter.firstName.text
128127
|| propertyType.description.trimmingCharacters(
129-
in: .whitespaces) != paramType.description.trimmingCharacters(in: .whitespacesAndNewlines)
128+
in: .whitespaces) != parameter.type.description.trimmingCharacters(in: .whitespacesAndNewlines)
130129
{ return false }
131130
}
132131
return true

Sources/SwiftFormatRules/ValidateDocumentationComments.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,7 @@ fileprivate func funcParametersIdentifiers(in paramList: FunctionParameterListSy
139139
// If there is a label and an identifier, then the identifier (`secondName`) is the name that
140140
// should be documented. Otherwise, the label and identifier are the same, occupying
141141
// `firstName`.
142-
guard let parameterIdentifier = parameter.secondName ?? parameter.firstName else {
143-
continue
144-
}
142+
let parameterIdentifier = parameter.secondName ?? parameter.firstName
145143
funcParameters.append(parameterIdentifier.text)
146144
}
147145
return funcParameters

Sources/generate-pipeline/RuleCollector.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ final class RuleCollector {
126126
guard let function = member.decl.as(FunctionDeclSyntax.self) else { continue }
127127
guard function.identifier.text == "visit" else { continue }
128128
let params = function.signature.input.parameterList
129-
guard let firstType = params.firstAndOnly?.type?.as(SimpleTypeIdentifierSyntax.self) else {
129+
guard let firstType = params.firstAndOnly?.type.as(SimpleTypeIdentifierSyntax.self) else {
130130
continue
131131
}
132132
visitedNodes.append(firstType.name.text)

0 commit comments

Comments
 (0)