Skip to content

Commit

Permalink
Fixed some bugs in redundantSelf rule
Browse files Browse the repository at this point in the history
  • Loading branch information
@nicklockwood
nicklockwood committed Mar 13, 2017
1 parent ce75350 commit 8c45a16
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 11 deletions.
52 changes: 41 additions & 11 deletions Sources/Rules.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1730,7 +1730,7 @@ extension FormatRules {
case "Self" where formatter.last(.nonSpaceOrCommentOrLinebreak, before: i) != .delimiter(":"):
// TODO: check for other cases where it's safe to use unescaped
break
case "get", "set":
case "get", "set", "willSet", "didSet":
guard formatter.last(.nonSpaceOrCommentOrLinebreak, before: i) != .startOfScope("{") else {
return
}
Expand Down Expand Up @@ -1760,7 +1760,7 @@ extension FormatRules {

/// Remove redundant self keyword
public class func redundantSelf(_ formatter: Formatter) {
func processBody(at index: inout Int, localNames: Set<String>) {
func processBody(at index: inout Int, localNames: Set<String>, isTypeRoot: Bool) {
var localNames = localNames
var scopeStack = [Token]()
var lastKeyword = ""
Expand All @@ -1774,15 +1774,16 @@ extension FormatRules {
case .keyword("extension"), .keyword("class"), .keyword("struct"), .keyword("enum"):
guard let scopeStart = formatter.index(of: .startOfScope("{"), after: index) else { return }
index = scopeStart + 1
processBody(at: &index, localNames: ["init"])
processBody(at: &index, localNames: ["init"], isTypeRoot: true)
case .keyword("var"), .keyword("let"):
index += 1
let lazy = (lastKeyword == "lazy")
lastKeyword = token.string
loop: while let token = formatter.token(at: index) {
switch token {
case .identifier:
let name = token.unescaped()
if name != "_" {
if name != "_", !(isTypeRoot && scopeStack.isEmpty) {
localNames.insert(name)
}
guard let nextIndex = formatter.index(of: .delimiter(","), after: index) else {
Expand All @@ -1794,15 +1795,44 @@ extension FormatRules {
}
index += 1
}
guard lazy else { break }
loop: while let nextIndex =
formatter.index(of: .nonSpaceOrCommentOrLinebreak, after: index) {
switch formatter.tokens[nextIndex] {
case .keyword("as"), .keyword("is"), .keyword("try"):
break
case .keyword:
break loop
default:
break
}
index = nextIndex
}
case let .keyword(name):
lastKeyword = name
case .startOfScope("("):
scopeStack.append(token)
case .startOfScope(":") where ["switch", "where", "case"].contains(lastKeyword),
.startOfScope("{") where [
"for", "in", "where", "if", "else", "while",
"repeat", "do", "catch", "switch",
].contains(lastKeyword):
case .startOfScope("{") where lastKeyword == "catch":
lastKeyword = ""
scopeStack.append(token)
localNames.insert("error") // Implicit error argument
case .startOfScope("{") where lastKeyword == "in":
lastKeyword = ""
scopeStack.append(token)
guard let keywordIndex = formatter.index(of: .keyword, before: index),
let prevKeywordIndex = formatter.index(of: .keyword, before: keywordIndex),
let prevKeywordToken = formatter.token(at: prevKeywordIndex),
case .keyword("for") = prevKeywordToken else { return }
for token in formatter.tokens[prevKeywordIndex + 1 ..< keywordIndex] {
if case let .identifier(name) = token, name != "_" {
localNames.insert(token.unescaped())
}
}
case .startOfScope("{") where [
"for", "where", "if", "else", "while",
"repeat", "do", "switch",
].contains(lastKeyword), .startOfScope(":"):
lastKeyword = ""
scopeStack.append(token)
case .startOfScope:
index += 1
Expand Down Expand Up @@ -1884,10 +1914,10 @@ extension FormatRules {
return
}
index = bodyStartIndex + 1
processBody(at: &index, localNames: localNames)
processBody(at: &index, localNames: localNames, isTypeRoot: false)
}
var index = 0
processBody(at: &index, localNames: ["init"])
processBody(at: &index, localNames: ["init"], isTypeRoot: false)
}

/// Replace unused arguments with an underscore
Expand Down
70 changes: 70 additions & 0 deletions Tests/RulesTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3584,6 +3584,76 @@ class RulesTests: XCTestCase {
XCTAssertEqual(try format(input + "\n", rules: FormatRules.default), output + "\n")
}

func testRemoveSelfInsideClassInit() {
let input = "class Foo {\n var bar = 5\n init() { self.bar = 6 }\n}"
let output = "class Foo {\n var bar = 5\n init() { bar = 6 }\n}"
XCTAssertEqual(try format(input, rules: [FormatRules.redundantSelf]), output)
XCTAssertEqual(try format(input + "\n", rules: FormatRules.default), output + "\n")
}

func testNoRemoveSelfInClosureInsideIf() {
let input = "if foo { bar { self.baz() } }"
let output = "if foo { bar { self.baz() } }"
XCTAssertEqual(try format(input, rules: [FormatRules.redundantSelf]), output)
XCTAssertEqual(try format(input + "\n", rules: FormatRules.default), output + "\n")
}

func testNoRemoveSelfForErrorInCatch() {
let input = "do {} catch { self.error = error }"
let output = "do {} catch { self.error = error }"
XCTAssertEqual(try format(input, rules: [FormatRules.redundantSelf]), output)
XCTAssertEqual(try format(input + "\n", rules: FormatRules.default), output + "\n")
}

func testNoRemoveSelfForNewValueInSet() {
let input = "var foo: Int { set { self.newValue = newValue } get { return 0 } }"
let output = "var foo: Int { set { self.newValue = newValue } get { return 0 } }"
XCTAssertEqual(try format(input, rules: [FormatRules.redundantSelf]), output)
XCTAssertEqual(try format(input + "\n", rules: FormatRules.default), output + "\n")
}

func testNoRemoveSelfForNewValueInWillSet() {
let input = "var foo: Int { willSet { self.newValue = newValue } }"
let output = "var foo: Int { willSet { self.newValue = newValue } }"
XCTAssertEqual(try format(input, rules: [FormatRules.redundantSelf]), output)
XCTAssertEqual(try format(input + "\n", rules: FormatRules.default), output + "\n")
}

func testNoRemoveSelfForOldValueInDidSet() {
let input = "var foo: Int { didSet { self.oldValue = oldValue } }"
let output = "var foo: Int { didSet { self.oldValue = oldValue } }"
XCTAssertEqual(try format(input, rules: [FormatRules.redundantSelf]), output)
XCTAssertEqual(try format(input + "\n", rules: FormatRules.default), output + "\n")
}

func testNoRemoveSelfForIndexVarInFor() {
let input = "for foo in bar { self.foo = foo }"
let output = "for foo in bar { self.foo = foo }"
XCTAssertEqual(try format(input, rules: [FormatRules.redundantSelf]), output)
XCTAssertEqual(try format(input + "\n", rules: FormatRules.default), output + "\n")
}

func testNoRemoveSelfForKeyValueTupleInFor() {
let input = "for (foo, bar) in baz { self.foo = foo; self.bar = bar }"
let output = "for (foo, bar) in baz { self.foo = foo; self.bar = bar }"
XCTAssertEqual(try format(input, rules: [FormatRules.redundantSelf]), output)
XCTAssertEqual(try format(input + "\n", rules: FormatRules.default), output + "\n")
}

func testNoRemoveSelfFromLazyVar() {
let input = "lazy var foo = self.bar"
let output = "lazy var foo = self.bar"
XCTAssertEqual(try format(input, rules: [FormatRules.redundantSelf]), output)
XCTAssertEqual(try format(input + "\n", rules: FormatRules.default), output + "\n")
}

func testNoRemoveSelfFromLazyVarClosure() {
let input = "lazy var foo = { self.bar }()"
let output = "lazy var foo = { self.bar }()"
XCTAssertEqual(try format(input, rules: [FormatRules.redundantSelf]), output)
XCTAssertEqual(try format(input + "\n", rules: FormatRules.default), output + "\n")
}

// MARK: unusedArguments

// closures
Expand Down

0 comments on commit 8c45a16

Please sign in to comment.