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
swiftlang · Apr 5, 2023 · 8fe7a2a · 8fe7a2a
2 parents 3f0ed45 + 1ce24a9
commit 8fe7a2a
Show file tree

Hide file tree

Showing 9 changed files with 149 additions and 19 deletions.
diff --git a/Sources/SwiftFormat/Pipelines+Generated.swift b/Sources/SwiftFormat/Pipelines+Generated.swift
@@ -55,6 +55,11 @@ class LintPipeline: SyntaxVisitor {
     return .visitChildren
   }
 
+  override func visit(_ node: ClosureParameterSyntax) -> SyntaxVisitorContinueKind {
+    visitIfEnabled(NoLeadingUnderscores.visit, for: node)
+    return .visitChildren
+  }
+
   override func visit(_ node: ClosureSignatureSyntax) -> SyntaxVisitorContinueKind {
     visitIfEnabled(AlwaysUseLowerCamelCase.visit, for: node)
     visitIfEnabled(ReturnVoidInsteadOfEmptyTuple.visit, for: node)
@@ -92,6 +97,11 @@ class LintPipeline: SyntaxVisitor {
     return .visitChildren
   }
 
+  override func visit(_ node: EnumCaseParameterSyntax) -> SyntaxVisitorContinueKind {
+    visitIfEnabled(NoLeadingUnderscores.visit, for: node)
+    return .visitChildren
+  }
+
   override func visit(_ node: EnumDeclSyntax) -> SyntaxVisitorContinueKind {
     visitIfEnabled(BeginDocumentationCommentWithOneLineSummary.visit, for: node)
     visitIfEnabled(DontRepeatTypeInStaticProperties.visit, for: node)

diff --git a/Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift b/Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift
@@ -1122,7 +1122,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
       // When it's parenthesized, the input is a `ParameterClauseSyntax`. Otherwise, it's a
       // `ClosureParamListSyntax`. The parenthesized version is wrapped in open/close breaks so that
       // the parens create an extra level of indentation.
-      if let parameterClause = input.as(ParameterClauseSyntax.self) {
+      if let parameterClause = input.as(ClosureParameterClauseSyntax.self) {
         // Whether we should prioritize keeping ") throws -> <return_type>" together. We can only do
         // this if the closure has arguments.
         let keepOutputTogether =
@@ -1141,7 +1141,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
           after(input.lastToken, tokens: .close)
         }
 
-        arrangeParameterClause(parameterClause, forcesBreakBeforeRightParen: true)
+        arrangeClosureParameterClause(parameterClause, forcesBreakBeforeRightParen: true)
       } else {
         // Group around the arguments, but don't use open/close breaks because there are no parens
         // to create a new scope.
@@ -1245,6 +1245,30 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
     return .visitChildren
   }
 
+  override func visit(_ node: ClosureParameterClauseSyntax) -> SyntaxVisitorContinueKind {
+    // Prioritize keeping ") throws -> <return_type>" together. We can only do this if the function
+    // has arguments.
+    if !node.parameterList.isEmpty && config.prioritizeKeepingFunctionOutputTogether {
+      // Due to visitation order, this .open corresponds to a .close added in FunctionDeclSyntax
+      // or SubscriptDeclSyntax.
+      before(node.rightParen, tokens: .open)
+    }
+
+    return .visitChildren
+  }
+
+  override func visit(_ node: EnumCaseParameterClauseSyntax) -> SyntaxVisitorContinueKind {
+    // Prioritize keeping ") throws -> <return_type>" together. We can only do this if the function
+    // has arguments.
+    if !node.parameterList.isEmpty && config.prioritizeKeepingFunctionOutputTogether {
+      // Due to visitation order, this .open corresponds to a .close added in FunctionDeclSyntax
+      // or SubscriptDeclSyntax.
+      before(node.rightParen, tokens: .open)
+    }
+
+    return .visitChildren
+  }
+
   override func visit(_ node: ParameterClauseSyntax) -> SyntaxVisitorContinueKind {
     // Prioritize keeping ") throws -> <return_type>" together. We can only do this if the function
     // has arguments.
@@ -1257,6 +1281,37 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
     return .visitChildren
   }
 
+  override func visit(_ node: ClosureParameterSyntax) -> SyntaxVisitorContinueKind {
+    before(node.firstToken, tokens: .open)
+    arrangeAttributeList(node.attributes)
+    before(
+      node.secondName,
+      tokens: .break(.continue, newlines: .elective(ignoresDiscretionary: true)))
+    after(node.colon, tokens: .break)
+
+    if let trailingComma = node.trailingComma {
+      after(trailingComma, tokens: .close, .break(.same))
+    } else {
+      after(node.lastToken, tokens: .close)
+    }
+    return .visitChildren
+  }
+
+  override func visit(_ node: EnumCaseParameterSyntax) -> SyntaxVisitorContinueKind {
+    before(node.firstToken, tokens: .open)
+    before(
+      node.secondName,
+      tokens: .break(.continue, newlines: .elective(ignoresDiscretionary: true)))
+    after(node.colon, tokens: .break)
+
+    if let trailingComma = node.trailingComma {
+      after(trailingComma, tokens: .close, .break(.same))
+    } else {
+      after(node.lastToken, tokens: .close)
+    }
+    return .visitChildren
+  }
+
   override func visit(_ node: FunctionParameterSyntax) -> SyntaxVisitorContinueKind {
     before(node.firstToken, tokens: .open)
     arrangeAttributeList(node.attributes)
@@ -1413,7 +1468,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
     after(node.trailingComma, tokens: .break)
 
     if let associatedValue = node.associatedValue {
-      arrangeParameterClause(associatedValue, forcesBreakBeforeRightParen: false)
+      arrangeEnumCaseParameterClause(associatedValue, forcesBreakBeforeRightParen: false)
     }
 
     return .visitChildren
@@ -2588,6 +2643,42 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
     return contentsIterator.next() == nil && !commentPrecedesRightBrace
   }
 
+  /// Applies formatting to a collection of parameters for a decl.
+  ///
+  /// - Parameters:
+  ///    - parameters: A node that contains the parameters that can be passed to a decl when its
+  ///      called.
+  ///    - forcesBreakBeforeRightParen: Whether a break should be required before the right paren
+  ///      when the right paren is on a different line than the corresponding left paren.
+  private func arrangeClosureParameterClause(
+    _ parameters: ClosureParameterClauseSyntax, forcesBreakBeforeRightParen: Bool
+  ) {
+    guard !parameters.parameterList.isEmpty else { return }
+
+    after(parameters.leftParen, tokens: .break(.open, size: 0), .open(argumentListConsistency()))
+    before(
+      parameters.rightParen,
+      tokens: .break(.close(mustBreak: forcesBreakBeforeRightParen), size: 0), .close)
+  }
+
+  /// Applies formatting to a collection of enum case parameters for a decl.
+  ///
+  /// - Parameters:
+  ///    - parameters: A node that contains the parameters that can be passed to a decl when its
+  ///      called.
+  ///    - forcesBreakBeforeRightParen: Whether a break should be required before the right paren
+  ///      when the right paren is on a different line than the corresponding left paren.
+  private func arrangeEnumCaseParameterClause(
+    _ parameters: EnumCaseParameterClauseSyntax, forcesBreakBeforeRightParen: Bool
+  ) {
+    guard !parameters.parameterList.isEmpty else { return }
+
+    after(parameters.leftParen, tokens: .break(.open, size: 0), .open(argumentListConsistency()))
+    before(
+      parameters.rightParen,
+      tokens: .break(.close(mustBreak: forcesBreakBeforeRightParen), size: 0), .close)
+  }
+
   /// Applies formatting to a collection of parameters for a decl.
   ///
   /// - Parameters:

diff --git a/Sources/SwiftFormatRules/AlwaysUseLowerCamelCase.swift b/Sources/SwiftFormatRules/AlwaysUseLowerCamelCase.swift
@@ -73,7 +73,16 @@ public final class AlwaysUseLowerCamelCase: SyntaxLintRule {
           diagnoseLowerCamelCaseViolations(
             param.name, allowUnderscores: false, description: identifierDescription(for: node))
         }
-      } else if let parameterClause = input.as(ParameterClauseSyntax.self) {
+      } else if let parameterClause = input.as(ClosureParameterClauseSyntax.self) {
+        for param in parameterClause.parameterList {
+          diagnoseLowerCamelCaseViolations(
+            param.firstName, allowUnderscores: false, description: identifierDescription(for: node))
+          if let secondName = param.secondName {
+            diagnoseLowerCamelCaseViolations(
+              secondName, allowUnderscores: false, description: identifierDescription(for: node))
+          }
+        }
+      } else if let parameterClause = input.as(EnumCaseParameterClauseSyntax.self) {
         for param in parameterClause.parameterList {
           if let firstName = param.firstName {
             diagnoseLowerCamelCaseViolations(
@@ -84,6 +93,15 @@ public final class AlwaysUseLowerCamelCase: SyntaxLintRule {
               secondName, allowUnderscores: false, description: identifierDescription(for: node))
           }
         }
+      } else if let parameterClause = input.as(ParameterClauseSyntax.self) {
+        for param in parameterClause.parameterList {
+          diagnoseLowerCamelCaseViolations(
+            param.firstName, allowUnderscores: false, description: identifierDescription(for: node))
+          if let secondName = param.secondName {
+            diagnoseLowerCamelCaseViolations(
+              secondName, allowUnderscores: false, description: identifierDescription(for: node))
+          }
+        }
       }
     }
     return .visitChildren
@@ -106,10 +124,8 @@ public final class AlwaysUseLowerCamelCase: SyntaxLintRule {
     for param in node.signature.input.parameterList {
       // These identifiers aren't described using `identifierDescription(for:)` because no single
       // node can disambiguate the argument label from the parameter name.
-      if let label = param.firstName {
-        diagnoseLowerCamelCaseViolations(
-          label, allowUnderscores: false, description: "argument label")
-      }
+      diagnoseLowerCamelCaseViolations(
+        param.firstName, allowUnderscores: false, description: "argument label")
       if let paramName = param.secondName {
         diagnoseLowerCamelCaseViolations(
           paramName, allowUnderscores: false, description: "function parameter")

diff --git a/Sources/SwiftFormatRules/AmbiguousTrailingClosureOverload.swift b/Sources/SwiftFormatRules/AmbiguousTrailingClosureOverload.swift
@@ -41,7 +41,7 @@ public final class AmbiguousTrailingClosureOverload: SyntaxLintRule {
     for fn in functions {
       let params = fn.signature.input.parameterList
       guard let firstParam = params.firstAndOnly else { continue }
-      guard let type = firstParam.type, type.is(FunctionTypeSyntax.self) else { continue }
+      guard firstParam.type.is(FunctionTypeSyntax.self) else { continue }
       if let mods = fn.modifiers, mods.has(modifier: "static") || mods.has(modifier: "class") {
         staticOverloads[fn.identifier.text, default: []].append(fn)
       } else {

diff --git a/Sources/SwiftFormatRules/FunctionDeclSyntax+Convenience.swift b/Sources/SwiftFormatRules/FunctionDeclSyntax+Convenience.swift
@@ -16,7 +16,7 @@ extension FunctionDeclSyntax {
   /// Constructs a name for a function that includes parameter labels, i.e. `foo(_:bar:)`.
   var fullDeclName: String {
     let params = signature.input.parameterList.map { param in
-      "\(param.firstName?.text ?? "_"):"
+      "\(param.firstName.text):"
     }
     return "\(identifier.text)(\(params.joined()))"
   }

diff --git a/Sources/SwiftFormatRules/NoLeadingUnderscores.swift b/Sources/SwiftFormatRules/NoLeadingUnderscores.swift
@@ -56,7 +56,15 @@ public final class NoLeadingUnderscores: SyntaxLintRule {
     return .visitChildren
   }
 
-  public override func visit(_ node: FunctionParameterSyntax) -> SyntaxVisitorContinueKind {
+  public override func visit(_ node: ClosureParameterSyntax) -> SyntaxVisitorContinueKind {
+    // If both names are provided, we want to check `secondName`, which will be the parameter name
+    // (in that case, `firstName` is the label). If only one name is present, then it is recorded in
+    // `firstName`, and it is both the label and the parameter name.
+    diagnoseIfNameStartsWithUnderscore(node.secondName ?? node.firstName)
+    return .visitChildren
+  }
+
+  public override func visit(_ node: EnumCaseParameterSyntax) -> SyntaxVisitorContinueKind {
     // If both names are provided, we want to check `secondName`, which will be the parameter name
     // (in that case, `firstName` is the label). If only one name is present, then it is recorded in
     // `firstName`, and it is both the label and the parameter name.
@@ -66,6 +74,14 @@ public final class NoLeadingUnderscores: SyntaxLintRule {
     return .visitChildren
   }
 
+  public override func visit(_ node: FunctionParameterSyntax) -> SyntaxVisitorContinueKind {
+    // If both names are provided, we want to check `secondName`, which will be the parameter name
+    // (in that case, `firstName` is the label). If only one name is present, then it is recorded in
+    // `firstName`, and it is both the label and the parameter name.
+    diagnoseIfNameStartsWithUnderscore(node.secondName ?? node.firstName)
+    return .visitChildren
+  }
+
   public override func visit(_ node: GenericParameterSyntax) -> SyntaxVisitorContinueKind {
     diagnoseIfNameStartsWithUnderscore(node.name)
     return .visitChildren

diff --git a/Sources/SwiftFormatRules/UseSynthesizedInitializer.swift b/Sources/SwiftFormatRules/UseSynthesizedInitializer.swift
@@ -106,8 +106,7 @@ public final class UseSynthesizedInitializer: SyntaxLintRule {
     guard parameters.count == properties.count else { return false }
     for (idx, parameter) in parameters.enumerated() {
 
-      guard let paramId = parameter.firstName, parameter.secondName == nil else { return false }
-      guard let paramType = parameter.type else { return false }
+      guard parameter.secondName == nil else { return false }
 
       let property = properties[idx]
       let propertyId = property.firstIdentifier
@@ -124,9 +123,9 @@ public final class UseSynthesizedInitializer: SyntaxLintRule {
         return false
       }
 
-      if propertyId.identifier.text != paramId.text
+      if propertyId.identifier.text != parameter.firstName.text
         || propertyType.description.trimmingCharacters(
-          in: .whitespaces) != paramType.description.trimmingCharacters(in: .whitespacesAndNewlines)
+          in: .whitespaces) != parameter.type.description.trimmingCharacters(in: .whitespacesAndNewlines)
       { return false }
     }
     return true

diff --git a/Sources/SwiftFormatRules/ValidateDocumentationComments.swift b/Sources/SwiftFormatRules/ValidateDocumentationComments.swift
@@ -139,9 +139,7 @@ fileprivate func funcParametersIdentifiers(in paramList: FunctionParameterListSy
     // If there is a label and an identifier, then the identifier (`secondName`) is the name that
     // should be documented. Otherwise, the label and identifier are the same, occupying
     // `firstName`.
-    guard let parameterIdentifier = parameter.secondName ?? parameter.firstName else {
-      continue
-    }
+    let parameterIdentifier = parameter.secondName ?? parameter.firstName
     funcParameters.append(parameterIdentifier.text)
   }
   return funcParameters

diff --git a/Sources/generate-pipeline/RuleCollector.swift b/Sources/generate-pipeline/RuleCollector.swift
@@ -126,7 +126,7 @@ final class RuleCollector {
         guard let function = member.decl.as(FunctionDeclSyntax.self) else { continue }
         guard function.identifier.text == "visit" else { continue }
         let params = function.signature.input.parameterList
-        guard let firstType = params.firstAndOnly?.type?.as(SimpleTypeIdentifierSyntax.self) else {
+        guard let firstType = params.firstAndOnly?.type.as(SimpleTypeIdentifierSyntax.self) else {
           continue
         }
         visitedNodes.append(firstType.name.text)