From 15701ea7db2ea50aeffa2be7451befef52ca86a6 Mon Sep 17 00:00:00 2001 From: Nick Lockwood Date: Mon, 24 Aug 2020 10:58:32 +0100 Subject: [PATCH] Reduce spurious change tracking --- Sources/CommandLine.swift | 10 +++++----- Sources/Formatter.swift | 6 +++--- Sources/Rules.swift | 14 +++++--------- Tests/FormatterTests.swift | 11 +++++++++++ Tests/RulesTests.swift | 10 +++++----- Tests/XCTestManifests.swift | 1 + 6 files changed, 30 insertions(+), 22 deletions(-) diff --git a/Sources/CommandLine.swift b/Sources/CommandLine.swift index 572ed1b3..e4a8e216 100644 --- a/Sources/CommandLine.swift +++ b/Sources/CommandLine.swift @@ -782,8 +782,7 @@ func applyRules(_ source: String, options: Options, lineRange: ClosedRange? verbose: Bool, lint: Bool) throws -> String { // Parse source - let originalTokens = tokenize(source) - var tokens = originalTokens + var tokens = tokenize(source) // Get rules let rulesByName = FormatRules.byName @@ -802,14 +801,15 @@ func applyRules(_ source: String, options: Options, lineRange: ClosedRange? trackChanges: lint || verbose, range: range) // Display info - if lint, tokens != originalTokens { + let updatedSource = sourceCode(for: tokens) + if lint, updatedSource != source { changes.forEach { print($0.description, as: .warning) } } if verbose { let rulesApplied = changes.reduce(into: Set()) { $0.insert($1.rule.name) } - if rulesApplied.isEmpty || tokens == originalTokens { + if rulesApplied.isEmpty || updatedSource == source { print("-- no changes", as: .success) } else { let sortedNames = Array(rulesApplied).sorted().joined(separator: ", ") @@ -818,7 +818,7 @@ func applyRules(_ source: String, options: Options, lineRange: ClosedRange? } // Output - return sourceCode(for: tokens) + return updatedSource } func processInput(_ inputURLs: [URL], diff --git a/Sources/Formatter.swift b/Sources/Formatter.swift index d551c6e3..c5a8c9b0 100644 --- a/Sources/Formatter.swift +++ b/Sources/Formatter.swift @@ -235,10 +235,10 @@ public extension Formatter { /// Replaces the token at the specified index with a new token func replaceToken(at index: Int, with token: Token) { - if token != tokens[index] { + if trackChanges, token.string != tokens[index].string { trackChange(at: index) - tokens[index] = token } + tokens[index] = token } /// Replaces the tokens in the specified range with new tokens @@ -250,7 +250,7 @@ public extension Formatter { } if range.count > max { removeTokens(in: range.dropFirst(max)) - } else { + } else if tokens.count > max { insert(tokens.dropFirst(max), at: range.lowerBound + max) } return tokens.count - range.count diff --git a/Sources/Rules.swift b/Sources/Rules.swift index 59441c21..ef9d2b6a 100644 --- a/Sources/Rules.swift +++ b/Sources/Rules.swift @@ -752,7 +752,8 @@ public struct _FormatRules { /// Remove blank lines immediately after an opening brace, bracket, paren or chevron public let blankLinesAtStartOfScope = FormatRule( - help: "Remove leading blank line at the start of a scope." + help: "Remove leading blank line at the start of a scope.", + orderAfter: ["organizeDeclarations"] ) { formatter in formatter.forEach(.startOfScope) { i, token in guard ["{", "(", "[", "<"].contains(token.string), @@ -784,7 +785,8 @@ public struct _FormatRules { /// Remove blank lines immediately before a closing brace, bracket, paren or chevron /// unless it's followed by more code on the same line (e.g. } else { ) public let blankLinesAtEndOfScope = FormatRule( - help: "Remove trailing blank line at the end of a scope." + help: "Remove trailing blank line at the end of a scope.", + orderAfter: ["organizeDeclarations"] ) { formatter in formatter.forEach(.endOfScope) { i, token in guard ["}", ")", "]", ">"].contains(token.string), @@ -5322,12 +5324,6 @@ public struct _FormatRules { .map { organize($0) } let updatedTokens = organizedDeclarations.flatMap { $0.tokens } - - if sourceCode(for: formatter.tokens) != sourceCode(for: updatedTokens) { - formatter.replaceTokens( - in: 0 ..< formatter.tokens.count, - with: updatedTokens - ) - } + formatter.replaceTokens(in: 0 ..< formatter.tokens.count, with: updatedTokens) } } diff --git a/Tests/FormatterTests.swift b/Tests/FormatterTests.swift index d08fab1b..b26636ad 100644 --- a/Tests/FormatterTests.swift +++ b/Tests/FormatterTests.swift @@ -441,4 +441,15 @@ class FormatterTests: XCTestCase { """)) XCTAssertEqual(formatter.endOfScope(at: 4), 13) } + + // MARK: change tracking + + func testTrackChangesIgnoresLinebreakIndex() { + let formatter = Formatter(tokenize("\n\n"), trackChanges: true) + var tokens = formatter.tokens + tokens.insert(tokens.removeLast(), at: 0) + XCTAssertNotEqual(formatter.tokens, tokens) + formatter.replaceTokens(in: 0 ..< 2, with: tokens) + XCTAssert(formatter.changes.isEmpty) + } } diff --git a/Tests/RulesTests.swift b/Tests/RulesTests.swift index 15d9ccf6..cd6a09e3 100644 --- a/Tests/RulesTests.swift +++ b/Tests/RulesTests.swift @@ -13239,10 +13239,10 @@ class RulesTests: XCTestCase { let input = """ public class Foo { public class Bar { - fileprivate func baaz() + fileprivate func baaz() {} public var quux: Int init() {} - deinit() {} + deinit {} } } """ @@ -13255,7 +13255,7 @@ class RulesTests: XCTestCase { // MARK: Lifecycle init() {} - deinit() {} + deinit {} // MARK: Public @@ -13263,7 +13263,7 @@ class RulesTests: XCTestCase { // MARK: Fileprivate - fileprivate func baaz() + fileprivate func baaz() {} } @@ -13273,7 +13273,7 @@ class RulesTests: XCTestCase { testFormatting( for: input, output, rule: FormatRules.organizeDeclarations, - exclude: ["blankLinesAtStartOfScope", "blankLinesAtEndOfScope", "spaceAroundParens"] + exclude: ["blankLinesAtStartOfScope", "blankLinesAtEndOfScope"] ) } diff --git a/Tests/XCTestManifests.swift b/Tests/XCTestManifests.swift index 8bba086c..06f28d14 100644 --- a/Tests/XCTestManifests.swift +++ b/Tests/XCTestManifests.swift @@ -154,6 +154,7 @@ extension FormatterTests { ("testRemoveNextTokenWhileEnumerating", testRemoveNextTokenWhileEnumerating), ("testRemovePreviousTokenWhileEnumerating", testRemovePreviousTokenWhileEnumerating), ("testSwiftVersionNext", testSwiftVersionNext), + ("testTrackChangesIgnoresLinebreakIndex", testTrackChangesIgnoresLinebreakIndex), ] }