Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert UseLetInEveryBoundCaseVariable to be a formatter #926

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Documentation/RuleDocumentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,10 @@ For example, `case let .identifier(x, y)` is forbidden. Use

Lint: `case let .identifier(...)` will yield a lint error.

`UseLetInEveryBoundCaseVariable` is a linter-only rule.
Format: `case let .identifier(x, y)` will be replaced by
`case .identifier(let x, let y)`.

`UseLetInEveryBoundCaseVariable` rule can format your code automatically.

### UseShorthandTypeNames

Expand Down
27 changes: 19 additions & 8 deletions Sources/SwiftFormat/Core/Pipelines+Generated.swift
Original file line number Diff line number Diff line change
Expand Up @@ -225,10 +225,12 @@ class LintPipeline: SyntaxVisitor {
}

override func visit(_ node: ForStmtSyntax) -> SyntaxVisitorContinueKind {
visitIfEnabled(UseLetInEveryBoundCaseVariable.visit, for: node)
visitIfEnabled(UseWhereClausesInForLoops.visit, for: node)
return .visitChildren
}
override func visitPost(_ node: ForStmtSyntax) {
onVisitPost(rule: UseLetInEveryBoundCaseVariable.self, for: node)
onVisitPost(rule: UseWhereClausesInForLoops.self, for: node)
}

Expand Down Expand Up @@ -386,6 +388,14 @@ class LintPipeline: SyntaxVisitor {
onVisitPost(rule: NoPlaygroundLiterals.self, for: node)
}

override func visit(_ node: MatchingPatternConditionSyntax) -> SyntaxVisitorContinueKind {
visitIfEnabled(UseLetInEveryBoundCaseVariable.visit, for: node)
return .visitChildren
}
override func visitPost(_ node: MatchingPatternConditionSyntax) {
onVisitPost(rule: UseLetInEveryBoundCaseVariable.self, for: node)
}

override func visit(_ node: MemberBlockItemListSyntax) -> SyntaxVisitorContinueKind {
visitIfEnabled(DoNotUseSemicolons.visit, for: node)
return .visitChildren
Expand Down Expand Up @@ -510,6 +520,14 @@ class LintPipeline: SyntaxVisitor {
onVisitPost(rule: UseTripleSlashForDocumentationComments.self, for: node)
}

override func visit(_ node: SwitchCaseItemSyntax) -> SyntaxVisitorContinueKind {
visitIfEnabled(UseLetInEveryBoundCaseVariable.visit, for: node)
return .visitChildren
}
override func visitPost(_ node: SwitchCaseItemSyntax) {
onVisitPost(rule: UseLetInEveryBoundCaseVariable.self, for: node)
}

override func visit(_ node: SwitchCaseLabelSyntax) -> SyntaxVisitorContinueKind {
visitIfEnabled(NoLabelsInCasePatterns.visit, for: node)
return .visitChildren
Expand Down Expand Up @@ -568,14 +586,6 @@ class LintPipeline: SyntaxVisitor {
onVisitPost(rule: UseTripleSlashForDocumentationComments.self, for: node)
}

override func visit(_ node: ValueBindingPatternSyntax) -> SyntaxVisitorContinueKind {
visitIfEnabled(UseLetInEveryBoundCaseVariable.visit, for: node)
return .visitChildren
}
override func visitPost(_ node: ValueBindingPatternSyntax) {
onVisitPost(rule: UseLetInEveryBoundCaseVariable.self, for: node)
}

override func visit(_ node: VariableDeclSyntax) -> SyntaxVisitorContinueKind {
visitIfEnabled(AllPublicDeclarationsHaveDocumentation.visit, for: node)
visitIfEnabled(AlwaysUseLowerCamelCase.visit, for: node)
Expand Down Expand Up @@ -627,6 +637,7 @@ extension FormatPipeline {
node = ReturnVoidInsteadOfEmptyTuple(context: context).rewrite(node)
node = UseEarlyExits(context: context).rewrite(node)
node = UseExplicitNilCheckInConditions(context: context).rewrite(node)
node = UseLetInEveryBoundCaseVariable(context: context).rewrite(node)
node = UseShorthandTypeNames(context: context).rewrite(node)
node = UseSingleLinePropertyGetter(context: context).rewrite(node)
node = UseTripleSlashForDocumentationComments(context: context).rewrite(node)
Expand Down
165 changes: 147 additions & 18 deletions Sources/SwiftFormat/Rules/UseLetInEveryBoundCaseVariable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,54 +18,183 @@ import SwiftSyntax
/// `case .identifier(let x, let y)` instead.
///
/// Lint: `case let .identifier(...)` will yield a lint error.
///
/// Format: `case let .identifier(x, y)` will be replaced by
/// `case .identifier(let x, let y)`.
@_spi(Rules)
public final class UseLetInEveryBoundCaseVariable: SyntaxLintRule {
public final class UseLetInEveryBoundCaseVariable: SyntaxFormatRule {
public override func visit(_ node: MatchingPatternConditionSyntax) -> MatchingPatternConditionSyntax {
if let (replacement, specifier) = distributeLetVarThroughPattern(node.pattern) {
diagnose(.useLetInBoundCaseVariables(specifier), on: node.pattern)

var result = node
result.pattern = PatternSyntax(replacement)
return result
}

return super.visit(node)
}

public override func visit(_ node: SwitchCaseItemSyntax) -> SwitchCaseItemSyntax {
if let (replacement, specifier) = distributeLetVarThroughPattern(node.pattern) {
diagnose(.useLetInBoundCaseVariables(specifier), on: node.pattern)

var result = node
result.pattern = PatternSyntax(replacement)
return result
}

return super.visit(node)
}

public override func visit(_ node: ForStmtSyntax) -> StmtSyntax {
guard node.caseKeyword != nil else {
return super.visit(node)
}

if let (replacement, specifier) = distributeLetVarThroughPattern(node.pattern) {
diagnose(.useLetInBoundCaseVariables(specifier), on: node.pattern)

var result = node
result.pattern = PatternSyntax(replacement)
return StmtSyntax(result)
}

return super.visit(node)
}
}

extension UseLetInEveryBoundCaseVariable {
private enum OptionalPatternKind {
case chained
case forced
}

public override func visit(_ node: ValueBindingPatternSyntax) -> SyntaxVisitorContinueKind {
// Diagnose a pattern binding if it is a function call and the callee is a member access
// expression (e.g., `case let .x(y)` or `case let T.x(y)`).
if canDistributeLetVarThroughPattern(node.pattern) {
diagnose(.useLetInBoundCaseVariables, on: node)
/// Wraps the given expression in the optional chaining and/or force
/// unwrapping expressions, as described by the specified stack.
private func restoreOptionalChainingAndForcing(
_ expr: ExprSyntax,
patternStack: [(OptionalPatternKind, Trivia)]
) -> ExprSyntax {
var patternStack = patternStack
var result = expr

// As we unwind the stack, wrap the expression in optional chaining
// or force unwrap expressions.
while let (kind, trivia) = patternStack.popLast() {
if kind == .chained {
result = ExprSyntax(
OptionalChainingExprSyntax(
expression: result,
trailingTrivia: trivia
)
)
} else {
result = ExprSyntax(
ForceUnwrapExprSyntax(
expression: result,
trailingTrivia: trivia
)
)
}
}
return .visitChildren

return result
}

/// Returns true if the given pattern is one that allows a `let/var` to be distributed
/// through to subpatterns.
private func canDistributeLetVarThroughPattern(_ pattern: PatternSyntax) -> Bool {
guard let exprPattern = pattern.as(ExpressionPatternSyntax.self) else { return false }
/// Returns a rewritten version of the given pattern if bindings can be moved
/// into bound cases.
///
/// - Parameter pattern: The pattern to rewrite.
/// - Returns: An optional tuple with the rewritten pattern and the binding
/// specifier used in `pattern`, for use in the diagnostic. If `pattern`
/// doesn't qualify for distributing the binding, then the result is `nil`.
private func distributeLetVarThroughPattern(
_ pattern: PatternSyntax
) -> (ExpressionPatternSyntax, TokenSyntax)? {
guard let bindingPattern = pattern.as(ValueBindingPatternSyntax.self),
let exprPattern = bindingPattern.pattern.as(ExpressionPatternSyntax.self)
else { return nil }

// Grab the `let` or `var` used in the binding pattern.
let specifier = bindingPattern.bindingSpecifier
let identifierBinder = BindIdentifiersRewriter(bindingSpecifier: specifier)

// Drill down into any optional patterns that we encounter (e.g., `case let .foo(x)?`).
var patternStack: [(OptionalPatternKind, Trivia)] = []
var expression = exprPattern.expression
while true {
if let optionalExpr = expression.as(OptionalChainingExprSyntax.self) {
expression = optionalExpr.expression
patternStack.append((.chained, optionalExpr.questionMark.trailingTrivia))
} else if let forcedExpr = expression.as(ForceUnwrapExprSyntax.self) {
expression = forcedExpr.expression
patternStack.append((.forced, forcedExpr.exclamationMark.trailingTrivia))
} else {
break
}
}

// Enum cases are written as function calls on member access expressions. The arguments
// are the associated values, so the `let/var` can be distributed into those.
if let functionCall = expression.as(FunctionCallExprSyntax.self),
if var functionCall = expression.as(FunctionCallExprSyntax.self),
functionCall.calledExpression.is(MemberAccessExprSyntax.self)
{
return true
var result = exprPattern
let newArguments = identifierBinder.rewrite(functionCall.arguments)
functionCall.arguments = newArguments.as(LabeledExprListSyntax.self)!
result.expression = restoreOptionalChainingAndForcing(
ExprSyntax(functionCall),
patternStack: patternStack
)
return (result, specifier)
}

// A tuple expression can have the `let/var` distributed into the elements.
if expression.is(TupleExprSyntax.self) {
return true
if var tupleExpr = expression.as(TupleExprSyntax.self) {
var result = exprPattern
let newElements = identifierBinder.rewrite(tupleExpr.elements)
tupleExpr.elements = newElements.as(LabeledExprListSyntax.self)!
result.expression = restoreOptionalChainingAndForcing(
ExprSyntax(tupleExpr),
patternStack: patternStack
)
return (result, specifier)
}

// Otherwise, we're not sure this is a pattern we can distribute through.
return false
return nil
}
}

extension Finding.Message {
fileprivate static let useLetInBoundCaseVariables: Finding.Message =
"move this 'let' keyword inside the 'case' pattern, before each of the bound variables"
fileprivate static func useLetInBoundCaseVariables(
_ specifier: TokenSyntax
) -> Finding.Message {
"move this '\(specifier.text)' keyword inside the 'case' pattern, before each of the bound variables"
}
}

/// A syntax rewriter that converts identifier patterns to bindings
/// with the given specifier.
private final class BindIdentifiersRewriter: SyntaxRewriter {
var bindingSpecifier: TokenSyntax

init(bindingSpecifier: TokenSyntax) {
self.bindingSpecifier = bindingSpecifier
}

override func visit(_ node: PatternExprSyntax) -> ExprSyntax {
guard let identifier = node.pattern.as(IdentifierPatternSyntax.self) else {
return super.visit(node)
}

let binding = ValueBindingPatternSyntax(
bindingSpecifier: bindingSpecifier,
pattern: identifier
)
var result = node
result.pattern = PatternSyntax(binding)
return ExprSyntax(result)
}
}
Loading
Loading