Skip to content

Commit

Permalink
Add redundantProperty rule
Browse files Browse the repository at this point in the history
  • Loading branch information
calda authored and nicklockwood committed Jun 11, 2024
1 parent 25fd24d commit 6d1d64f
Show file tree
Hide file tree
Showing 8 changed files with 391 additions and 25 deletions.
19 changes: 19 additions & 0 deletions Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
* [markTypes](#markTypes)
* [noExplicitOwnership](#noExplicitOwnership)
* [organizeDeclarations](#organizeDeclarations)
* [redundantProperty](#redundantProperty)
* [sortSwitchCases](#sortSwitchCases)
* [wrapConditionalBodies](#wrapConditionalBodies)
* [wrapEnumCases](#wrapEnumCases)
Expand Down Expand Up @@ -1863,6 +1864,24 @@ Remove redundant pattern matching parameter syntax.
</details>
<br/>

## redundantProperty

Simplifies redundant property definitions that are immediately returned.

<details>
<summary>Examples</summary>

```diff
func foo() -> Foo {
- let foo = Foo()
- return foo
+ return Foo()
}
```

</details>
<br/>

## redundantRawValues

Remove redundant raw string values for enum cases.
Expand Down
10 changes: 10 additions & 0 deletions Sources/Examples.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1887,4 +1887,14 @@ private struct Examples {
}
```
"""#

let redundantProperty = """
```diff
func foo() -> Foo {
- let foo = Foo()
- return foo
+ return Foo()
}
```
"""
}
90 changes: 85 additions & 5 deletions Sources/ParsingHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1435,17 +1435,24 @@ extension Formatter {
/// - `[...]` (array or dictionary)
/// - `{ ... }` (closure)
/// - `#selector(...)` / macro invocations
/// - An `if/switch` expression (only allowed if this is the only expression in
/// a code block or if following an assignment `=` operator).
/// - 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 `(...)`, subscripts `[...]`, or generic arguments `<...>`.
/// - Any value can be followed by a `.identifier`
func parseExpressionRange(startingAt startIndex: Int) -> ClosedRange<Int>? {
func parseExpressionRange(
startingAt startIndex: Int,
allowConditionalExpressions: Bool = false
)
-> ClosedRange<Int>?
{
// 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)
let followingExpression = parseExpressionRange(startingAt: nextTokenIndex, allowConditionalExpressions: allowConditionalExpressions)
{
return startIndex ... followingExpression.upperBound
}
Expand All @@ -1461,7 +1468,7 @@ extension Formatter {
nextTokenAfterTry = nextTokenAfterTryOperator
}

if let followingExpression = parseExpressionRange(startingAt: nextTokenAfterTry) {
if let followingExpression = parseExpressionRange(startingAt: nextTokenAfterTry, allowConditionalExpressions: allowConditionalExpressions) {
return startIndex ... followingExpression.upperBound
}
}
Expand All @@ -1487,10 +1494,16 @@ extension Formatter {
// #selector() and macro expansions like #macro() are parsed into keyword tokens.
endOfExpression = startIndex

case .keyword("if"), .keyword("switch"):
guard allowConditionalExpressions,
let conditionalBranches = conditionalBranches(at: startIndex),
let lastBranch = conditionalBranches.last
else { return nil }
endOfExpression = lastBranch.endOfBranch

default:
return nil
}

while let nextTokenIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: endOfExpression),
let nextToken = token(at: nextTokenIndex)
{
Expand All @@ -1507,7 +1520,7 @@ extension Formatter {
endOfExpression = endOfScope

/// Any value can be followed by a `.identifier`
case .delimiter("."):
case .delimiter("."), .operator(".", _):
guard let nextIdentifierIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: nextTokenIndex),
tokens[nextIdentifierIndex].isIdentifier
else { return startIndex ... endOfExpression }
Expand Down Expand Up @@ -1585,6 +1598,73 @@ extension Formatter {
}
}

/// A property of the format `(let|var) identifier: Type = expression`.
/// - `: Type` and `= expression` elements are optional
struct PropertyDeclaration {
let introducerIndex: Int
let identifier: String
let identifierIndex: Int
let type: (colonIndex: Int, name: String, range: ClosedRange<Int>)?
let value: (assignmentIndex: Int, expressionRange: ClosedRange<Int>)?

var range: ClosedRange<Int> {
if let value = value {
return introducerIndex ... value.expressionRange.upperBound
} else if let type = type {
return introducerIndex ... type.range.upperBound
} else {
return introducerIndex ... identifierIndex
}
}
}

/// Parses a property of the format `(let|var) identifier: Type = expression`
/// starting at the given introducer index (the `let` / `var` keyword).
func parsePropertyDeclaration(atIntroducerIndex introducerIndex: Int) -> PropertyDeclaration? {
assert(["let", "var"].contains(tokens[introducerIndex].string))

guard let propertyIdentifierIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: introducerIndex),
let propertyIdentifier = token(at: propertyIdentifierIndex),
propertyIdentifier.isIdentifier
else { return nil }

var typeInformation: (colonIndex: Int, name: String, range: ClosedRange<Int>)?

if let colonIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: propertyIdentifierIndex),
tokens[colonIndex] == .delimiter(":"),
let startOfTypeIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: colonIndex),
let type = parseType(at: startOfTypeIndex)
{
typeInformation = (
colonIndex: colonIndex,
name: type.name,
range: type.range
)
}

let endOfTypeOrIdentifier = typeInformation?.range.upperBound ?? propertyIdentifierIndex
var valueInformation: (assignmentIndex: Int, expressionRange: ClosedRange<Int>)?

if let assignmentIndex = index(of: .nonSpaceOrCommentOrLinebreak, after: endOfTypeOrIdentifier),
tokens[assignmentIndex] == .operator("=", .infix),
let startOfExpression = index(of: .nonSpaceOrCommentOrLinebreak, after: assignmentIndex),
let expressionRange = parseExpressionRange(startingAt: startOfExpression, allowConditionalExpressions: true)
{
valueInformation = (
assignmentIndex: assignmentIndex,
expressionRange: expressionRange
)
}

return PropertyDeclaration(
introducerIndex: introducerIndex,
identifier: propertyIdentifier.string,
identifierIndex: propertyIdentifierIndex,
type: typeInformation,
value: valueInformation
)
}

/// Shared import rules implementation
func parseImports() -> [[ImportRange]] {
var importStack = [[ImportRange]]()
Expand Down
40 changes: 40 additions & 0 deletions Sources/Rules.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7973,4 +7973,44 @@ public struct _FormatRules {
}
}
}

public let redundantProperty = FormatRule(
help: "Simplifies redundant property definitions that are immediately returned.",
disabledByDefault: true
) { formatter in
formatter.forEach(.keyword) { introducerIndex, introducerToken in
// Find properties like `let identifier = value` followed by `return identifier`
guard ["let", "var"].contains(introducerToken.string),
let property = formatter.parsePropertyDeclaration(atIntroducerIndex: introducerIndex),
let (assignmentIndex, expressionRange) = property.value,
let returnIndex = formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: expressionRange.upperBound),
formatter.tokens[returnIndex] == .keyword("return"),
let returnedValueIndex = formatter.index(of: .nonSpaceOrComment, after: returnIndex),
let returnedExpression = formatter.parseExpressionRange(startingAt: returnedValueIndex, allowConditionalExpressions: true),
formatter.tokens[returnedExpression] == [.identifier(property.identifier)]
else { return }

let returnRange = formatter.startOfLine(at: returnIndex) ... formatter.endOfLine(at: returnedExpression.upperBound)
let propertyRange = introducerIndex ... expressionRange.upperBound

guard !propertyRange.overlaps(returnRange) else { return }

// Remove the line with the `return identifier` statement.
formatter.removeTokens(in: returnRange)

// If there's nothing but whitespace between the end of the expression
// and the return statement, we can remove all of it. But if there's a comment,
// we should preserve it.
let rangeBetweenExpressionAndReturn = (expressionRange.upperBound + 1) ..< (returnRange.lowerBound - 1)
if formatter.tokens[rangeBetweenExpressionAndReturn].allSatisfy(\.isSpaceOrLinebreak) {
formatter.removeTokens(in: rangeBetweenExpressionAndReturn)
}

// Replace the `let identifier = value` with `return value`
formatter.replaceTokens(
in: introducerIndex ..< expressionRange.lowerBound,
with: [.keyword("return"), .space(" ")]
)
}
}
}
30 changes: 28 additions & 2 deletions Tests/ParsingHelpersTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1873,7 +1873,10 @@ class ParsingHelpersTests: XCTestCase {
// MARK: - parseExpressionRange

func testParseIndividualExpressions() {
XCTAssert(isSingleExpression(#"Foo()"#))
XCTAssert(isSingleExpression(#"Foo("bar")"#))
XCTAssert(isSingleExpression(#"Foo.init()"#))
XCTAssert(isSingleExpression(#"Foo.init("bar")"#))
XCTAssert(isSingleExpression(#"foo.bar"#))
XCTAssert(isSingleExpression(#"foo .bar"#))
XCTAssert(isSingleExpression(#"foo["bar"]("baaz")"#))
Expand Down Expand Up @@ -1927,6 +1930,29 @@ class ParsingHelpersTests: XCTestCase {
XCTAssert(isSingleExpression(#"try await { try await printAsyncThrows(foo) }()"#))
XCTAssert(isSingleExpression(#"Foo<Bar>()"#))
XCTAssert(isSingleExpression(#"Foo<Bar, Baaz>(quux: quux)"#))
XCTAssert(!isSingleExpression(#"if foo { "foo" } else { "bar" }"#))

XCTAssert(isSingleExpression(
#"if foo { "foo" } else { "bar" }"#,
allowConditionalExpressions: true
))

XCTAssert(isSingleExpression("""
if foo {
"foo"
} else {
"bar"
}
""", allowConditionalExpressions: true))

XCTAssert(isSingleExpression("""
switch foo {
case true:
"foo"
case false:
"bar"
}
""", allowConditionalExpressions: true))

XCTAssert(isSingleExpression("""
foo
Expand Down Expand Up @@ -2037,9 +2063,9 @@ class ParsingHelpersTests: XCTestCase {
XCTAssertEqual(parseExpressions(input), expectedExpressions)
}

func isSingleExpression(_ string: String) -> Bool {
func isSingleExpression(_ string: String, allowConditionalExpressions: Bool = false) -> Bool {
let formatter = Formatter(tokenize(string))
guard let expressionRange = formatter.parseExpressionRange(startingAt: 0) else { return false }
guard let expressionRange = formatter.parseExpressionRange(startingAt: 0, allowConditionalExpressions: allowConditionalExpressions) else { return false }
return expressionRange.upperBound == formatter.tokens.indices.last!
}

Expand Down
8 changes: 4 additions & 4 deletions Tests/RulesTests+Indentation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ class IndentTests: RulesTests {
}
"""
testFormatting(for: input, rule: FormatRules.indent,
exclude: ["braces", "wrapMultilineStatementBraces"])
exclude: ["braces", "wrapMultilineStatementBraces", "redundantProperty"])
}

func testIndentLineAfterIndentedInlineClosure() {
Expand All @@ -460,7 +460,7 @@ class IndentTests: RulesTests {
return viewController
}
"""
testFormatting(for: input, rule: FormatRules.indent)
testFormatting(for: input, rule: FormatRules.indent, exclude: ["redundantProperty"])
}

func testIndentLineAfterNonIndentedClosure() {
Expand All @@ -473,7 +473,7 @@ class IndentTests: RulesTests {
return viewController
}
"""
testFormatting(for: input, rule: FormatRules.indent)
testFormatting(for: input, rule: FormatRules.indent, exclude: ["redundantProperty"])
}

func testIndentMultilineStatementDoesntFailToTerminate() {
Expand Down Expand Up @@ -3885,7 +3885,7 @@ class IndentTests: RulesTests {
}
"""

testFormatting(for: input, output, rule: FormatRules.indent)
testFormatting(for: input, output, rule: FormatRules.indent, exclude: ["redundantProperty"])
}

func testIndentNestedSwitchExpressionAssignment() {
Expand Down
Loading

0 comments on commit 6d1d64f

Please sign in to comment.