diff --git a/Sources/FormattingHelpers.swift b/Sources/FormattingHelpers.swift index eb52ca8f7..19361c8ea 100644 --- a/Sources/FormattingHelpers.swift +++ b/Sources/FormattingHelpers.swift @@ -1214,43 +1214,25 @@ extension Formatter { return isSingleStatement && isOnlyStatement } - var index = startOfBody + 1 - while index < endOfScopeIndex { - switch tokens[index] { - case .startOfScope("("), .startOfScope("//"), .startOfScope("/*"), - .startOfScope where tokens[index].isStringDelimiter: - break - case .startOfScope("{") where isStartOfClosure(at: index): - index = endOfScope(at: index) ?? index - case .startOfScope: - // any other statement-forming scope (e.g. guard, #if) - // within the main body, that isn't itself a closure - return false - case .keyword("return"): - // any return statement within the main body that isn't at the very beginning of the body - if self.index(of: .nonSpaceOrCommentOrLinebreak, before: index) != startOfBody { - return false - } - case .delimiter(";"): - // if there are any semicolons within the scope but not at the end of a line - let nextTokenIndex = self.index(of: .nonSpaceOrCommentOrLinebreak, after: index) ?? index - if startOfLine(at: index) == startOfLine(at: nextTokenIndex) { - return false - } - case .operator("=", _), .keyword("fallthrough"): - return false - case .endOfScope(")"): - // if there is a method call immediately followed an identifier - if next(.nonSpaceOrCommentOrLinebreak, after: index)?.isIdentifier == true { - return false - } - default: - break - } - index += 1 + // The body should contain exactly one expression. + // We can confirm this by parsing the body with `parseExpressionRange`, + // and checking that the token after that expression is just the end of the scope. + guard var firstTokenInBody = index(of: .nonSpaceOrCommentOrLinebreak, after: startOfBody) else { + return false } - return true + // Skip over any optional `return` keyword + if tokens[firstTokenInBody] == .keyword("return") { + guard let tokenAfterReturnKeyword = index(of: .nonSpaceOrCommentOrLinebreak, after: firstTokenInBody) else { return false } + firstTokenInBody = tokenAfterReturnKeyword + } + guard let expressionRange = parseExpressionRange(startingAt: firstTokenInBody), + let nextIndexAfterExpression = index(of: .nonSpaceOrCommentOrLinebreak, after: expressionRange.upperBound) + else { + return false + } + + return nextIndexAfterExpression == endOfScopeIndex } /// The token before the body of the scope following the given `startOfScopeIndex`. diff --git a/Sources/ParsingHelpers.swift b/Sources/ParsingHelpers.swift index 211a84f29..96b255704 100644 --- a/Sources/ParsingHelpers.swift +++ b/Sources/ParsingHelpers.swift @@ -1272,6 +1272,160 @@ extension Formatter { return nil } + /// Parses the expression starting at the given index. + /// + /// A full list of expression types are available here: + /// https://docs.swift.org/swift-book/documentation/the-swift-programming-language/expressions/ + /// + /// Can be any of: + /// - `identifier` + /// - `1` (integer literal) + /// - `1.0` (double literal) + /// - `"foo"` (string literal) + /// - `(...)` (tuple) + /// - `[...]` (array or dictionary) + /// - `{ ... }` (closure) + /// - `#selector(...)` / macro invocations + /// - Any value can be preceded by a prefix operator + /// - Any value can be preceded by `try`, `try?`, `try!`, or `await` + /// - Any value can be followed by a postfix operator + /// - Any value can be followed by an infix operator plus a right-hand-side expression. + /// - Any value can be followed by an arbitrary number of method calls `(...)` or subscripts `[...]`. + /// - Any value can be followed by a `.identifier` + func parseExpressionRange(startingAt startIndex: Int) -> ClosedRange? { + // Any expression can start with a prefix operator, or `await` + if tokens[startIndex].isOperator(ofType: .prefix) || tokens[startIndex].string == "await", + let nextTokenIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: startIndex), + let followingExpression = parseExpressionRange(startingAt: nextTokenIndex) + { + return startIndex ... followingExpression.upperBound + } + + // Any value can be preceded by `try` + if tokens[startIndex].string == "try" { + guard var nextTokenAfterTry = index(of: .nonSpaceOrCommentOrLinebreak, after: startIndex) else { return nil } + + // `try` can either be by itself, or followed by `?` or `!` (`try`, `try?`, or `try!`). + // If present, skip the operator. + if tokens[nextTokenAfterTry] == .operator("?", .postfix) + || tokens[nextTokenAfterTry] == .operator("!", .postfix) + { + guard let nextTokenAfterTryOperator = index(of: .nonSpaceOrCommentOrLinebreak, after: nextTokenAfterTry) else { return nil } + nextTokenAfterTry = nextTokenAfterTryOperator + } + + if let startOfFollowingExpressionIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: nextTokenAfterTry), + let followingExpression = parseExpressionRange(startingAt: startOfFollowingExpressionIndex) + { + return startIndex ... followingExpression.upperBound + } + } + + // Parse the base of any potential method call or chain, + // which is always a simple identifier or a simple literal. + var endOfExpression: Int + switch tokens[startIndex] { + case .identifier, .number: + endOfExpression = startIndex + + case .startOfScope: + // All types of scopes (tuples, arrays, closures, strings) are considered expressions + // _except_ for conditional complication blocks. + if ["#if", "#elseif", "#else"].contains(tokens[startIndex].string) { + return nil + } + + guard let endOfScope = endOfScope(at: startIndex) else { return nil } + endOfExpression = endOfScope + + case let .keyword(keyword) where keyword.hasPrefix("#"): + // #selector() and macro expansions like #macro() are parsed into keyword tokens. + endOfExpression = startIndex + + default: + return nil + } + + while let nextTokenIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: endOfExpression), + let nextToken = token(at: nextTokenIndex) + { + switch nextToken { + // Any expression can be followed by an arbitrary number of method calls `(...)` or subscripts `[...]`. + case .startOfScope("("), .startOfScope("["): + // If there's a linebreak between an expression and a paren or subscript, + // then it's not parsed as a method call and is actually a separate expression + if tokens[endOfExpression ..< nextTokenIndex].contains(where: \.isLinebreak) { + return startIndex ... endOfExpression + } + + guard let endOfScope = endOfScope(at: nextTokenIndex) else { return nil } + endOfExpression = endOfScope + + /// Any value can be followed by a `.identifier` + case .delimiter("."): + guard let nextIdentifierIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: nextTokenIndex), + tokens[nextIdentifierIndex].isIdentifier + else { return startIndex ... endOfExpression } + + endOfExpression = nextIdentifierIndex + + /// Any value can be followed by a postfix operator + case .operator(_, .postfix): + endOfExpression = nextTokenIndex + + /// Any value can be followed by an infix operator, plus another expression + /// - However, the assignment operator (`=`) is special and _isn't_ an expression + case let .operator(operatorString, .infix) where operatorString != "=": + guard let nextTokenIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: nextTokenIndex), + let nextExpression = parseExpressionRange(startingAt: nextTokenIndex) + else { return startIndex ... endOfExpression } + + endOfExpression = nextExpression.upperBound + + /// Any value can be followed by `is`, `as`, `as?`, or `as?`, plus another expression + case .keyword("is"), .keyword("as"): + guard var nextTokenAfterKeyword = index(of: .nonSpaceOrCommentOrLinebreak, after: nextTokenIndex) else { return nil } + + // `as` can either be by itself, or followed by `?` or `!` (`as`, `as?`, or `as!`). + // If present, skip the operator. + if tokens[nextTokenAfterKeyword] == .operator("?", .postfix) + || tokens[nextTokenAfterKeyword] == .operator("!", .postfix) + { + guard let nextTokenAfterOperator = index(of: .nonSpaceOrCommentOrLinebreak, after: nextTokenAfterKeyword) else { return nil } + nextTokenAfterKeyword = nextTokenAfterOperator + } + + guard let followingExpression = parseExpressionRange(startingAt: nextTokenAfterKeyword) else { + return startIndex ... endOfExpression + } + + endOfExpression = followingExpression.upperBound + + /// Any value can be followed by a trailing closure + case .startOfScope("{"): + guard let endOfScope = endOfScope(at: nextTokenIndex) else { return nil } + endOfExpression = endOfScope + + /// Some values can be followed by a labeled trailing closure, + /// like (expression) trailingClosure: { ... } + case .identifier: + guard let colonIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: nextTokenIndex), + tokens[colonIndex] == .delimiter(":"), + let startOfClosureIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: colonIndex), + tokens[startOfClosureIndex] == .startOfScope("{"), + let endOfClosureScope = endOfScope(at: startOfClosureIndex) + else { return startIndex ... endOfExpression } + + endOfExpression = endOfClosureScope + + default: + return startIndex ... endOfExpression + } + } + + return startIndex ... endOfExpression + } + struct ImportRange: Comparable { var module: String var range: Range diff --git a/Tests/ParsingHelpersTests.swift b/Tests/ParsingHelpersTests.swift index e4b6e492e..21e89af14 100644 --- a/Tests/ParsingHelpersTests.swift +++ b/Tests/ParsingHelpersTests.swift @@ -1771,4 +1771,189 @@ class ParsingHelpersTests: XCTestCase { XCTAssertEqual(formatter.endOfDeclaration(atDeclarationKeyword: 24), 39) // let defaultCacheAge XCTAssertEqual(formatter.endOfDeclaration(atDeclarationKeyword: 43), 112) // func requestStrategy } + + // MARK: - parseExpressionRange + + func testParseIndividualExpressions() { + XCTAssert(isSingleExpression(#"Foo("bar")"#)) + XCTAssert(isSingleExpression(#"foo.bar"#)) + XCTAssert(isSingleExpression(#"foo .bar"#)) + XCTAssert(isSingleExpression(#"foo["bar"]("baaz")"#)) + XCTAssert(isSingleExpression(#"foo().bar().baaz[]().bar"#)) + XCTAssert(isSingleExpression(#"foo?.bar?().baaz!.quux ?? """#)) + XCTAssert(isSingleExpression(#"1"#)) + XCTAssert(isSingleExpression(#"10.0"#)) + XCTAssert(isSingleExpression(#"10000"#)) + XCTAssert(isSingleExpression(#"-24.0"#)) + XCTAssert(isSingleExpression(#"3.14e2"#)) + XCTAssert(isSingleExpression(#"1 + 2"#)) + XCTAssert(isSingleExpression(#"-0.05 * 10"#)) + XCTAssert(isSingleExpression(#"0...10"#)) + XCTAssert(isSingleExpression(#"0..<20"#)) + XCTAssert(isSingleExpression(#"0 ... array.indices.last"#)) + XCTAssert(isSingleExpression(#"true"#)) + XCTAssert(isSingleExpression(#"false"#)) + XCTAssert(isSingleExpression(#"!boolean"#)) + XCTAssert(isSingleExpression(#"boolean || !boolean && boolean"#)) + XCTAssert(isSingleExpression(#"boolean ? value : value"#)) + XCTAssert(isSingleExpression(#"foo"#)) + XCTAssert(isSingleExpression(#""foo""#)) + XCTAssert(isSingleExpression(##"#"raw string"#"##)) + XCTAssert(isSingleExpression(###"##"raw string"##"###)) + XCTAssert(isSingleExpression(#"["foo", "bar"]"#)) + XCTAssert(isSingleExpression(#"["foo": bar]"#)) + XCTAssert(isSingleExpression(#"(tuple: "foo", bar: "baaz")"#)) + XCTAssert(isSingleExpression(#"foo.bar { "baaz"}"#)) + XCTAssert(isSingleExpression(#"foo.bar({ "baaz" })"#)) + XCTAssert(isSingleExpression(#"foo.bar() { "baaz" }"#)) + XCTAssert(isSingleExpression(#"foo.bar { "baaz" } anotherTrailingClosure: { "quux" }"#)) + XCTAssert(isSingleExpression(#"try foo()"#)) + XCTAssert(isSingleExpression(#"try! foo()"#)) + XCTAssert(isSingleExpression(#"try? foo()"#)) + XCTAssert(isSingleExpression(#"try await foo()"#)) + XCTAssert(isSingleExpression(#"foo is Foo"#)) + XCTAssert(isSingleExpression(#"foo as Foo"#)) + XCTAssert(isSingleExpression(#"foo as? Foo"#)) + XCTAssert(isSingleExpression(#"foo as! Foo"#)) + XCTAssert(isSingleExpression(#"foo ? bar : baaz"#)) + XCTAssert(isSingleExpression(#".implicitMember"#)) + XCTAssert(isSingleExpression(#"\Foo.explicitKeypath"#)) + XCTAssert(isSingleExpression(#"\.inferredKeypath"#)) + XCTAssert(isSingleExpression(#"#selector(Foo.bar)"#)) + XCTAssert(isSingleExpression(#"#macro()"#)) + XCTAssert(isSingleExpression(#"#outerMacro(12, #innerMacro(34), "some text")"#)) + + XCTAssert(isSingleExpression(""" + foo + .bar + """)) + + XCTAssert(isSingleExpression(""" + foo? + .bar?() + .baaz![0] + """)) + + XCTAssert(isSingleExpression(#""" + """ + multi-line string + """ + """#)) + + XCTAssert(isSingleExpression(##""" + #""" + raw multi-line string + """# + """##)) + + XCTAssertFalse(isSingleExpression(#"foo = bar"#)) + XCTAssertFalse(isSingleExpression(#"foo = "foo"#)) + XCTAssertFalse(isSingleExpression(#"10 20 30"#)) + XCTAssertFalse(isSingleExpression(#"foo bar"#)) + XCTAssertFalse(isSingleExpression(#"foo? bar"#)) + + XCTAssertFalse(isSingleExpression(""" + foo + () // if you have a linebreak before a method call, its parsed as a tuple + """)) + + XCTAssertFalse(isSingleExpression(""" + foo + [0] // if you have a linebreak before a subscript, its invalid + """)) + + XCTAssertFalse(isSingleExpression(""" + #if DEBUG + foo + #else + bar + #endif + """)) + } + + func testParseMultipleSingleLineExpressions() { + let input = """ + foo + foo?.bar().baaz() + 24 + !foo + methodCall() + foo ?? bar ?? baaz + """ + + // Each line is a single expression + let expectedExpressions = input.components(separatedBy: "\n") + XCTAssertEqual(parseExpressions(input), expectedExpressions) + } + + func testParseMultipleLineExpressions() { + let input = """ + [ + "foo", + "bar" + ].map { + $0.uppercased() + } + + foo?.bar().methodCall( + foo: foo, + bar: bar) + + foo.multipleTrailingClosure { + print("foo") + } anotherTrailingClosure: { + print("bar") + } + """ + + let expectedExpressions = [ + """ + [ + "foo", + "bar" + ].map { + $0.uppercased() + } + """, + """ + foo?.bar().methodCall( + foo: foo, + bar: bar) + """, + """ + foo.multipleTrailingClosure { + print("foo") + } anotherTrailingClosure: { + print("bar") + } + """, + ] + + XCTAssertEqual(parseExpressions(input), expectedExpressions) + } + + func isSingleExpression(_ string: String) -> Bool { + let formatter = Formatter(tokenize(string)) + guard let expressionRange = formatter.parseExpressionRange(startingAt: 0) else { return false } + return expressionRange.upperBound == formatter.tokens.indices.last! + } + + func parseExpressions(_ string: String) -> [String] { + let formatter = Formatter(tokenize(string)) + var expressions = [String]() + + var parseIndex = 0 + while let expressionRange = formatter.parseExpressionRange(startingAt: parseIndex) { + let expression = formatter.tokens[expressionRange].map { $0.string }.joined() + expressions.append(expression) + + if let nextExpressionIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: expressionRange.upperBound) { + parseIndex = nextExpressionIndex + } else { + return expressions + } + } + + return expressions + } } diff --git a/Tests/RulesTests+Syntax.swift b/Tests/RulesTests+Syntax.swift index 7e68584b9..5a9414172 100644 --- a/Tests/RulesTests+Syntax.swift +++ b/Tests/RulesTests+Syntax.swift @@ -3574,6 +3574,81 @@ class SyntaxTests: RulesTests { testFormatting(for: input, rule: FormatRules.conditionalAssignment, options: options) } + func testDoesntConvertMultiStatementIfStatementWithStringLiteral() { + let input = """ + let text: String + if conditionOne { + text = "Hello World!" + doSomeStuffHere() + } else { + text = "Goodbye!" + } + """ + + let options = FormatOptions(swiftVersion: "5.9") + testFormatting(for: input, rule: FormatRules.conditionalAssignment, options: options) + } + + func testDoesntConvertMultiStatementIfStatementWithCollectionLiteral() { + let input = """ + let text: [String] + if conditionOne { + text = [] + doSomeStuffHere() + } else { + text = ["Goodbye!"] + } + """ + + let options = FormatOptions(swiftVersion: "5.9") + testFormatting(for: input, rule: FormatRules.conditionalAssignment, options: options) + } + + func testDoesntConvertMultiStatementIfStatementWithIntLiteral() { + let input = """ + let number: Int? + if conditionOne { + number = 5 + doSomeStuffHere() + } else { + number = 10 + } + """ + + let options = FormatOptions(swiftVersion: "5.9") + testFormatting(for: input, rule: FormatRules.conditionalAssignment, options: options) + } + + func testDoesntConvertMultiStatementIfStatementWithNilLiteral() { + let input = """ + let number: Int? + if conditionOne { + number = nil + doSomeStuffHere() + } else { + number = 10 + } + """ + + let options = FormatOptions(swiftVersion: "5.9") + testFormatting(for: input, rule: FormatRules.conditionalAssignment, options: options) + } + + func testDoesntConvertMultiStatementIfStatementWithOtherProperty() { + let input = """ + let number: Int? + if conditionOne { + number = someOtherProperty + doSomeStuffHere() + } else { + number = 10 + } + """ + + let options = FormatOptions(swiftVersion: "5.9") + testFormatting(for: input, rule: FormatRules.conditionalAssignment, options: options) + } + // MARK: - preferForLoop func testConvertSimpleForEachToForLoop() {