diff --git a/Sources/SwiftFormatConfiguration/Configuration.swift b/Sources/SwiftFormatConfiguration/Configuration.swift index f698a398..f5caaaab 100644 --- a/Sources/SwiftFormatConfiguration/Configuration.swift +++ b/Sources/SwiftFormatConfiguration/Configuration.swift @@ -36,6 +36,7 @@ public struct Configuration: Codable, Equatable { case indentSwitchCaseLabels case rules case spacesAroundRangeFormationOperators + case noAssignmentInExpressions } /// The version of this configuration. @@ -147,6 +148,9 @@ public struct Configuration: Codable, Equatable { /// `...` and `..<`. public var spacesAroundRangeFormationOperators = false + /// Contains exceptions for the `NoAssignmentInExpressions` rule. + public var noAssignmentInExpressions = NoAssignmentInExpressionsConfiguration() + /// Constructs a Configuration with all default values. public init() { self.version = highestSupportedConfigurationVersion @@ -208,6 +212,10 @@ public struct Configuration: Codable, Equatable { ?? FileScopedDeclarationPrivacyConfiguration() self.indentSwitchCaseLabels = try container.decodeIfPresent(Bool.self, forKey: .indentSwitchCaseLabels) ?? false + self.noAssignmentInExpressions = + try container.decodeIfPresent( + NoAssignmentInExpressionsConfiguration.self, forKey: .noAssignmentInExpressions) + ?? NoAssignmentInExpressionsConfiguration() // If the `rules` key is not present at all, default it to the built-in set // so that the behavior is the same as if the configuration had been @@ -238,6 +246,7 @@ public struct Configuration: Codable, Equatable { spacesAroundRangeFormationOperators, forKey: .spacesAroundRangeFormationOperators) try container.encode(fileScopedDeclarationPrivacy, forKey: .fileScopedDeclarationPrivacy) try container.encode(indentSwitchCaseLabels, forKey: .indentSwitchCaseLabels) + try container.encode(noAssignmentInExpressions, forKey: .noAssignmentInExpressions) try container.encode(rules, forKey: .rules) } @@ -287,3 +296,15 @@ public struct FileScopedDeclarationPrivacyConfiguration: Codable, Equatable { /// private access. public var accessLevel: AccessLevel = .private } + +/// Configuration for the `NoAssignmentInExpressions` rule. +public struct NoAssignmentInExpressionsConfiguration: Codable, Equatable { + /// A list of function names where assignments are allowed to be embedded in expressions that are + /// passed as parameters to that function. + public var allowedFunctions: [String] = [ + // Allow `XCTAssertNoThrow` because `XCTAssertNoThrow(x = try ...)` is clearer about intent than + // `x = try XCTUnwrap(try? ...)` or force-unwrapped if you need to use the value `x` later on + // in the test. + "XCTAssertNoThrow" + ] +} diff --git a/Sources/SwiftFormatRules/NoAssignmentInExpressions.swift b/Sources/SwiftFormatRules/NoAssignmentInExpressions.swift index 0ae58473..7384c5f3 100644 --- a/Sources/SwiftFormatRules/NoAssignmentInExpressions.swift +++ b/Sources/SwiftFormatRules/NoAssignmentInExpressions.swift @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +import SwiftFormatConfiguration import SwiftFormatCore import SwiftSyntax @@ -27,7 +28,10 @@ public final class NoAssignmentInExpressions: SyntaxFormatRule { public override func visit(_ node: InfixOperatorExprSyntax) -> ExprSyntax { // Diagnose any assignment that isn't directly a child of a `CodeBlockItem` (which would be the // case if it was its own statement). - if isAssignmentExpression(node) && !isStandaloneAssignmentStatement(node) { + if isAssignmentExpression(node) + && !isStandaloneAssignmentStatement(node) + && !isInAllowedFunction(node) + { diagnose(.moveAssignmentToOwnStatement, on: node) } return ExprSyntax(node) @@ -131,6 +135,30 @@ public final class NoAssignmentInExpressions: SyntaxFormatRule { } return parent.is(CodeBlockItemSyntax.self) } + + /// Returns true if the infix operator expression is in the (non-closure) parameters of an allowed + /// function call. + private func isInAllowedFunction(_ node: InfixOperatorExprSyntax) -> Bool { + let allowedFunctions = context.configuration.noAssignmentInExpressions.allowedFunctions + // Walk up the tree until we find a FunctionCallExprSyntax, and if the name matches, return + // true. However, stop early if we hit a CodeBlockItemSyntax first; this would represent a + // closure context where we *don't* want the exception to apply (for example, in + // `someAllowedFunction(a, b) { return c = d }`, the `c = d` is a descendent of a function call + // but we want it to be evaluated in its own context. + var node = Syntax(node) + while let parent = node.parent { + node = parent + if node.is(CodeBlockItemSyntax.self) { + break + } + if let functionCallExpr = node.as(FunctionCallExprSyntax.self), + allowedFunctions.contains(functionCallExpr.calledExpression.trimmedDescription) + { + return true + } + } + return false + } } extension Finding.Message { diff --git a/Tests/SwiftFormatRulesTests/NoAssignmentInExpressionsTests.swift b/Tests/SwiftFormatRulesTests/NoAssignmentInExpressionsTests.swift index dfeeefbf..6ba554a0 100644 --- a/Tests/SwiftFormatRulesTests/NoAssignmentInExpressionsTests.swift +++ b/Tests/SwiftFormatRulesTests/NoAssignmentInExpressionsTests.swift @@ -180,4 +180,35 @@ final class NoAssignmentInExpressionsTests: LintOrFormatRuleTestCase { ) XCTAssertNotDiagnosed(.moveAssignmentToOwnStatement) } + + func testAssignmentExpressionsInAllowedFunctions() { + XCTAssertFormatting( + NoAssignmentInExpressions.self, + input: """ + // These should not diagnose. + XCTAssertNoThrow(a = try b()) + XCTAssertNoThrow { a = try b() } + XCTAssertNoThrow({ a = try b() }) + someRegularFunction({ a = b }) + someRegularFunction { a = b } + + // This should be diagnosed. + someRegularFunction(a = b) + """, + expected: """ + // These should not diagnose. + XCTAssertNoThrow(a = try b()) + XCTAssertNoThrow { a = try b() } + XCTAssertNoThrow({ a = try b() }) + someRegularFunction({ a = b }) + someRegularFunction { a = b } + + // This should be diagnosed. + someRegularFunction(a = b) + """ + ) + XCTAssertDiagnosed(.moveAssignmentToOwnStatement, line: 9, column: 21) + // Make sure no other expressions were diagnosed. + XCTAssertNotDiagnosed(.moveAssignmentToOwnStatement) + } }