Merge pull request #854 from jaherhi/master

Add configuration for trailing_whitespace to ignore comments
realm · Nov 4, 2016 · 4ebe939 · 4ebe939
2 parents e8771d9 + c1df008
commit 4ebe939
Show file tree

Hide file tree

Showing 5 changed files with 115 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,10 @@
 
 ##### Enhancements
 
+* Add `ignores_comment` configuration for `trailing_whitespace` rule.  
+  [Javier Hernández](https://github.com/jaherhi)
+  [#576](https://github.com/realm/SwiftLint/issues/576)
+
 * Added HTML reporter, identifier is `html`.  
   [Johnykutty Mathew](https://github.com/Johnykutty)
 

diff --git a/Source/SwiftLintFramework/Rules/RuleConfigurations/TrailingWhitespaceConfiguration.swift b/Source/SwiftLintFramework/Rules/RuleConfigurations/TrailingWhitespaceConfiguration.swift
@@ -11,14 +11,17 @@ import Foundation
 public struct TrailingWhitespaceConfiguration: RuleConfiguration, Equatable {
     var severityConfiguration = SeverityConfiguration(.Warning)
     var ignoresEmptyLines = false
+    var ignoresComments = true
 
     public var consoleDescription: String {
         return severityConfiguration.consoleDescription +
-            ", ignores_empty_lines: \(ignoresEmptyLines)"
+            ", ignores_empty_lines: \(ignoresEmptyLines)" +
+            ", ignores_comments: \(ignoresComments)"
     }
 
-    public init(ignoresEmptyLines: Bool) {
+    public init(ignoresEmptyLines: Bool, ignoresComments: Bool) {
         self.ignoresEmptyLines = ignoresEmptyLines
+        self.ignoresComments = ignoresComments
     }
 
     public mutating func applyConfiguration(configuration: AnyObject) throws {
@@ -27,6 +30,7 @@ public struct TrailingWhitespaceConfiguration: RuleConfiguration, Equatable {
         }
 
         ignoresEmptyLines = (configuration["ignores_empty_lines"] as? Bool == true)
+        ignoresComments = (configuration["ignores_comments"] as? Bool == true)
 
         if let severityString = configuration["severity"] as? String {
             try severityConfiguration.applyConfiguration(severityString)
@@ -36,5 +40,6 @@ public struct TrailingWhitespaceConfiguration: RuleConfiguration, Equatable {
 
 public func == (lhs: TrailingWhitespaceConfiguration,
                 rhs: TrailingWhitespaceConfiguration) -> Bool {
-    return lhs.ignoresEmptyLines == rhs.ignoresEmptyLines
+    return lhs.ignoresEmptyLines == rhs.ignoresEmptyLines &&
+        lhs.ignoresComments == rhs.ignoresComments
 }
diff --git a/Source/SwiftLintFramework/Rules/TrailingWhitespaceRule.swift b/Source/SwiftLintFramework/Rules/TrailingWhitespaceRule.swift
@@ -9,28 +9,38 @@
 import Foundation
 import SourceKittenFramework
 
-public struct TrailingWhitespaceRule: CorrectableRule, ConfigurationProviderRule,
-                                      SourceKitFreeRule {
+public struct TrailingWhitespaceRule: CorrectableRule, ConfigurationProviderRule {
 
-    public var configuration = TrailingWhitespaceConfiguration(ignoresEmptyLines: false)
+    public var configuration = TrailingWhitespaceConfiguration(ignoresEmptyLines: false,
+                                                               ignoresComments: true)
 
     public init() {}
 
     public static let description = RuleDescription(
         identifier: "trailing_whitespace",
         name: "Trailing Whitespace",
         description: "Lines should not have trailing whitespace.",
-        nonTriggeringExamples: [ "//\n" ],
-        triggeringExamples: [ "// \n" ],
-        corrections: [ "// \n": "//\n" ]
+        nonTriggeringExamples: [ "let name: String\n", "//\n", "// \n",
+            "let name: String //\n", "let name: String // \n" ],
+        triggeringExamples: [ "let name: String \n", "/* */ let name: String \n" ],
+        corrections: [ "let name: String \n": "let name: String\n",
+            "/* */ let name: String \n": "/* */ let name: String\n"]
     )
 
     public func validateFile(file: File) -> [StyleViolation] {
         let filteredLines = file.lines.filter {
-            $0.content.hasTrailingWhitespace() &&
-                (!configuration.ignoresEmptyLines ||
-                    // If configured, ignore lines that contain nothing but whitespace (empty lines)
-                    !$0.content.stringByTrimmingCharactersInSet(.whitespaceCharacterSet()).isEmpty)
+            guard $0.content.hasTrailingWhitespace() else { return false }
+
+            let commentKinds = SyntaxKind.commentKinds()
+            if configuration.ignoresComments,
+                let lastSyntaxKind = file.syntaxKindsByLines[$0.index].last
+                where commentKinds.contains(lastSyntaxKind) {
+                return false
+            }
+
+            return !configuration.ignoresEmptyLines ||
+                // If configured, ignore lines that contain nothing but whitespace (empty lines)
+                !$0.content.stringByTrimmingCharactersInSet(.whitespaceCharacterSet()).isEmpty
         }
 
         return filteredLines.map {
@@ -45,6 +55,19 @@ public struct TrailingWhitespaceRule: CorrectableRule, ConfigurationProviderRule
         var correctedLines = [String]()
         var corrections = [Correction]()
         for line in file.lines {
+            guard line.content.hasTrailingWhitespace() else {
+                correctedLines.append(line.content)
+                continue
+            }
+
+            let commentKinds = SyntaxKind.commentKinds()
+            if configuration.ignoresComments,
+                let lastSyntaxKind = file.syntaxKindsByLines[line.index].last
+                where commentKinds.contains(lastSyntaxKind) {
+                correctedLines.append(line.content)
+                continue
+            }
+
             let correctedLine = (line.content as NSString)
                 .stringByTrimmingTrailingCharactersInSet(whitespaceCharacterSet)
 

diff --git a/Tests/SwiftLintFramework/RuleConfigurationTests.swift b/Tests/SwiftLintFramework/RuleConfigurationTests.swift
@@ -132,22 +132,36 @@ class RuleConfigurationsTests: XCTestCase {
 
     func testTrailingWhitespaceConfigurationThrowsOnBadConfig() {
         let config = "unknown"
-        var configuration = TrailingWhitespaceConfiguration(ignoresEmptyLines: false)
+        var configuration = TrailingWhitespaceConfiguration(ignoresEmptyLines: false,
+                                                            ignoresComments: true)
         checkError(ConfigurationError.UnknownConfiguration) {
             try configuration.applyConfiguration(config)
         }
     }
 
     func testTrailingWhitespaceConfigurationInitializerSetsIgnoresEmptyLines() {
-        let configuration1 = TrailingWhitespaceConfiguration(ignoresEmptyLines: false)
+        let configuration1 = TrailingWhitespaceConfiguration(ignoresEmptyLines: false,
+                                                             ignoresComments: true)
         XCTAssertFalse(configuration1.ignoresEmptyLines)
 
-        let configuration2 = TrailingWhitespaceConfiguration(ignoresEmptyLines: true)
+        let configuration2 = TrailingWhitespaceConfiguration(ignoresEmptyLines: true,
+                                                             ignoresComments: true)
         XCTAssertTrue(configuration2.ignoresEmptyLines)
     }
 
+    func testTrailingWhitespaceConfigurationInitializerSetsIgnoresComments() {
+        let configuration1 = TrailingWhitespaceConfiguration(ignoresEmptyLines: false,
+                                                             ignoresComments: true)
+        XCTAssertTrue(configuration1.ignoresComments)
+
+        let configuration2 = TrailingWhitespaceConfiguration(ignoresEmptyLines: false,
+                                                             ignoresComments: false)
+        XCTAssertFalse(configuration2.ignoresComments)
+    }
+
     func testTrailingWhitespaceConfigurationApplyConfigurationSetsIgnoresEmptyLines() {
-        var configuration = TrailingWhitespaceConfiguration(ignoresEmptyLines: false)
+        var configuration = TrailingWhitespaceConfiguration(ignoresEmptyLines: false,
+                                                            ignoresComments: true)
         do {
             let config1 = ["ignores_empty_lines": true]
             try configuration.applyConfiguration(config1)
@@ -161,17 +175,47 @@ class RuleConfigurationsTests: XCTestCase {
         }
     }
 
+    func testTrailingWhitespaceConfigurationApplyConfigurationSetsIgnoresComments() {
+        var configuration = TrailingWhitespaceConfiguration(ignoresEmptyLines: false,
+                                                            ignoresComments: true)
+        do {
+            let config1 = ["ignores_comments": true]
+            try configuration.applyConfiguration(config1)
+            XCTAssertTrue(configuration.ignoresComments)
+
+            let config2 = ["ignores_comments": false]
+            try configuration.applyConfiguration(config2)
+            XCTAssertFalse(configuration.ignoresComments)
+        } catch {
+            XCTFail()
+        }
+    }
+
     func testTrailingWhitespaceConfigurationCompares() {
-        let configuration1 = TrailingWhitespaceConfiguration(ignoresEmptyLines: false)
-        let configuration2 = TrailingWhitespaceConfiguration(ignoresEmptyLines: true)
+        let configuration1 = TrailingWhitespaceConfiguration(ignoresEmptyLines: false,
+                                                             ignoresComments: true)
+        let configuration2 = TrailingWhitespaceConfiguration(ignoresEmptyLines: true,
+                                                             ignoresComments: true)
         XCTAssertFalse(configuration1 == configuration2)
 
-        let configuration3 = TrailingWhitespaceConfiguration(ignoresEmptyLines: true)
+        let configuration3 = TrailingWhitespaceConfiguration(ignoresEmptyLines: true,
+                                                             ignoresComments: true)
         XCTAssertTrue(configuration2 == configuration3)
+
+        let configuration4 = TrailingWhitespaceConfiguration(ignoresEmptyLines: false,
+                                                             ignoresComments: false)
+
+        XCTAssertFalse(configuration1 == configuration4)
+
+        let configuration5 = TrailingWhitespaceConfiguration(ignoresEmptyLines: true,
+                                                             ignoresComments: false)
+
+        XCTAssertFalse(configuration1 == configuration5)
     }
 
     func testTrailingWhitespaceConfigurationApplyConfigurationUpdatesSeverityConfiguration() {
-        var configuration = TrailingWhitespaceConfiguration(ignoresEmptyLines: false)
+        var configuration = TrailingWhitespaceConfiguration(ignoresEmptyLines: false,
+                                                            ignoresComments: true)
         configuration.severityConfiguration.severity = .Warning
 
         do {

diff --git a/Tests/SwiftLintFramework/RulesTests.swift b/Tests/SwiftLintFramework/RulesTests.swift
@@ -247,7 +247,7 @@ class RulesTests: XCTestCase {
     }
 
     func testTrailingWhitespace() {
-        verifyRule(TrailingWhitespaceRule.description, commentDoesntViolate: false)
+        verifyRule(TrailingWhitespaceRule.description)
 
         // Perform additional tests with the ignores_empty_lines setting enabled.
         // The set of non-triggering examples is extended by a whitespace-indented empty line
@@ -259,7 +259,23 @@ class RulesTests: XCTestCase {
                                nonTriggeringExamples: nonTriggeringExamples,
                                   triggeringExamples: baseDescription.triggeringExamples,
                                          corrections: baseDescription.corrections)
-        verifyRule(description, ruleConfiguration: ["ignores_empty_lines": true],
+        verifyRule(description,
+                   ruleConfiguration: ["ignores_empty_lines": true, "ignores_comments": true])
+
+        // Perform additional tests with the ignores_comments settings disabled.
+        let triggeringComments = ["// \n", "let name: String // \n"]
+        let baseDescription2 = TrailingWhitespaceRule.description
+        let nonTriggeringExamples2 = baseDescription2.nonTriggeringExamples
+            .filter { !triggeringComments.contains($0) }
+        let triggeringExamples2 = baseDescription2.triggeringExamples + triggeringComments
+        let description2 = RuleDescription(identifier: baseDescription2.identifier,
+                                           name: baseDescription2.name,
+                                           description: baseDescription2.description,
+                                           nonTriggeringExamples: nonTriggeringExamples2,
+                                           triggeringExamples: triggeringExamples2,
+                                           corrections: baseDescription2.corrections)
+        verifyRule(description2,
+                   ruleConfiguration: ["ignores_empty_lines": false, "ignores_comments": false],
                    commentDoesntViolate: false)
     }