Ignore SwiftLint commands in file_header rule

CHANGELOG.md

@@ -65,6 +65,11 @@
   [JP Simard](https://github.com/jpsim)
   [#1002](https://github.com/realm/SwiftLint/issues/1002)
 
+* Ignore SwiftLint commands (`swiftlint:(disable|enable)`) in `file_header`
+  rule, making it work better with `superfluous_disable_command` rule.  
+  [Marcelo Fabri](https://github.com/marcelofabri)
+  [#1810](https://github.com/realm/SwiftLint/issues/1810)  
+
 ## 0.22.0: Wrinkle-free
 
 ##### Breaking

Rules.md

@@ -6343,8 +6343,9 @@ var x = 0
 ```
 
 ```swift
-@objc func f() {}
-var x = 0
+@objc func f() {
+}
+↓var x = 0
 
 ```
 

Source/SwiftLintFramework/Extensions/File+SwiftLint.swift

@@ -53,13 +53,14 @@ extension File {
         return regions
     }
 
-    internal func commands() -> [Command] {
+    internal func commands(in range: NSRange? = nil) -> [Command] {
         if sourcekitdFailed {
             return []
         }
         let contents = self.contents.bridge()
+        let range = range ?? NSRange(location: 0, length: contents.length)
         let pattern = "swiftlint:(enable|disable)(:previous|:this|:next)?\\ [^\\n]+"
-        return match(pattern: pattern, with: [.comment]).flatMap { range in
+        return match(pattern: pattern, with: [.comment], range: range).flatMap { range in
             return Command(string: contents, range: range)
         }.flatMap { command in
             return command.expand()

Source/SwiftLintFramework/Rules/FileHeaderRule.swift

@@ -40,76 +40,75 @@ public struct FileHeaderRule: ConfigurationProviderRule, OptInRule {
     public func validate(file: File) -> [StyleViolation] {
         var firstToken: SyntaxToken?
         var lastToken: SyntaxToken?
+        var firstNonCommentToken: SyntaxToken?
 
-        for token in file.syntaxTokensByLines.joined() {
+        for token in file.syntaxTokensByLines.lazy.joined() {
             guard let kind = SyntaxKind(rawValue: token.type), kind.isFileHeaderKind else {
                 // found a token that is not a comment, which means it's not the top of the file
                 // so we can just skip the remaining tokens
+                firstNonCommentToken = token
                 break
             }
 
+            // skip SwiftLint commands
+            guard !isSwiftLintCommand(token: token, file: file) else {
+                continue
+            }
+
             if firstToken == nil {
                 firstToken = token
             }
             lastToken = token
         }
 
-        // first location will be used for region purposes, second one will be the one reported
-        var violationsOffsets = [(Int, Int)]()
+        var violationsOffsets = [Int]()
         if let firstToken = firstToken, let lastToken = lastToken {
             let start = firstToken.offset
             let length = lastToken.offset + lastToken.length - firstToken.offset
-            guard let range = file.contents.bridge()
-                .byteRangeToNSRange(start: start, length: length) else {
+            guard let range = file.contents.bridge().byteRangeToNSRange(start: start, length: length) else {
                 return []
             }
 
-            if let regex = configuration.forbiddenRegex {
-                let matches = regex.matches(in: file.contents, options: [], range: range)
-                if let firstMatch = matches.first {
-                    let location = firstMatch.range.location + firstMatch.range.length - 1
-                    violationsOffsets.append((location, firstMatch.range.location))
-                }
+            if let regex = configuration.forbiddenRegex,
+                let firstMatch = regex.matches(in: file.contents, options: [], range: range).first {
+                violationsOffsets.append(firstMatch.range.location)
             }
 
-            if let regex = configuration.requiredRegex {
-                let matches = regex.matches(in: file.contents, options: [], range: range)
-                if matches.isEmpty {
-                    let location = range.location + range.length - 1
-                    violationsOffsets.append((location, start))
-                }
+            if let regex = configuration.requiredRegex,
+                case let matches = regex.matches(in: file.contents, options: [], range: range),
+                matches.isEmpty {
+                violationsOffsets.append(start)
             }
         } else if configuration.requiredRegex != nil {
+            let location = firstNonCommentToken.map {
+                Location(file: file, byteOffset: $0.offset)
+            } ?? Location(file: file.path, line: 1)
             return [
                 StyleViolation(
                     ruleDescription: type(of: self).description,
                     severity: configuration.severityConfiguration.severity,
-                    location: Location(file: file.path, line: 1)
+                    location: location
                 )
             ]
         }
 
-        return violations(fromOffsets: violationsOffsets, file: file)
-    }
-
-    private func violations(fromOffsets violationsOffsets: [(Int, Int)], file: File) -> [StyleViolation] {
-        let locations: [Int] = violationsOffsets.flatMap {
-            let ranges = [NSRange(location: $0.0, length: 0)]
-            guard !file.ruleEnabled(violatingRanges: ranges, for: self).isEmpty else {
-                return nil
-            }
-
-            return $0.1
-        }
-
-        return locations.map {
+        return violationsOffsets.map {
             StyleViolation(
                 ruleDescription: type(of: self).description,
                 severity: configuration.severityConfiguration.severity,
                 location: Location(file: file, characterOffset: $0)
             )
         }
     }
+
+    private func isSwiftLintCommand(token: SyntaxToken, file: File) -> Bool {
+        guard let range = file.contents.bridge().byteRangeToNSRange(start: token.offset,
+                                                                    length: token.length) else {
+            return false
+        }
+
+        return !file.commands(in: range).isEmpty
+    }
 }
 
 private extension SyntaxKind {

Source/SwiftLintFramework/Rules/LetVarWhitespaceRule.swift

@@ -43,7 +43,7 @@ public struct LetVarWhitespaceRule: ConfigurationProviderRule, OptInRule {
             "struct X {\n\tlet a\n\t↓func x() {}\n}\n",
             "var x = 0\n↓@objc func f() {}\n",
             "var x = 0\n↓@objc\n\tfunc f() {}\n",
-            "@objc func f() {}\nvar x = 0\n"
+            "@objc func f() {\n}\n↓var x = 0\n"
         ]
     )
 

Tests/SwiftLintFrameworkTests/FileHeaderRuleTests.swift

@@ -12,7 +12,7 @@ import XCTest
 class FileHeaderRuleTests: XCTestCase {
 
     func testFileHeaderWithDefaultConfiguration() {
-        verifyRule(FileHeaderRule.description, skipCommentTests: true, skipDisableCommandTests: true)
+        verifyRule(FileHeaderRule.description, skipCommentTests: true)
     }
 
     func testFileHeaderWithRequiredString() {
@@ -33,7 +33,7 @@ class FileHeaderRuleTests: XCTestCase {
 
         verifyRule(description, ruleConfiguration: ["required_string": "**Header"],
                    stringDoesntViolate: false, skipCommentTests: true,
-                   skipDisableCommandTests: true, testMultiByteOffsets: false, testShebang: false)
+                   testMultiByteOffsets: false, testShebang: false)
     }
 
     func testFileHeaderWithRequiredPattern() {
@@ -51,7 +51,7 @@ class FileHeaderRuleTests: XCTestCase {
             .with(triggeringExamples: triggeringExamples)
 
         verifyRule(description, ruleConfiguration: ["required_pattern": "\\d{4} Realm"],
-                   stringDoesntViolate: false, skipCommentTests: true, skipDisableCommandTests: true,
+                   stringDoesntViolate: false, skipCommentTests: true,
                    testMultiByteOffsets: false)
     }
 
@@ -68,7 +68,7 @@ class FileHeaderRuleTests: XCTestCase {
 
         let config = ["required_string": "/* Check this url: https://github.com/realm/SwiftLint */"]
         verifyRule(description, ruleConfiguration: config,
-                   stringDoesntViolate: false, skipCommentTests: true, skipDisableCommandTests: true,
+                   stringDoesntViolate: false, skipCommentTests: true,
                    testMultiByteOffsets: false)
     }
 
@@ -89,7 +89,7 @@ class FileHeaderRuleTests: XCTestCase {
             .with(triggeringExamples: triggeringExamples)
 
         verifyRule(description, ruleConfiguration: ["forbidden_string": "**All rights reserved."],
-                   skipCommentTests: true, skipDisableCommandTests: true)
+                   skipCommentTests: true)
     }
 
     func testFileHeaderWithForbiddenPattern() {
@@ -109,23 +109,23 @@ class FileHeaderRuleTests: XCTestCase {
             .with(triggeringExamples: triggeringExamples)
 
         verifyRule(description, ruleConfiguration: ["forbidden_pattern": "\\s\\w+\\.swift"],
-                   skipCommentTests: true, skipDisableCommandTests: true)
+                   skipCommentTests: true)
     }
 
     func testFileHeaderWithForbiddenPatternAndDocComment() {
         let nonTriggeringExamples = [
-            "/// This is great tool.\nclass GreatTool {}",
+            "/// This is great tool with tests.\nclass GreatTool {}",
             "class GreatTool {}"
         ]
         let triggeringExamples = [
-            "↓// FileHeaderRuleTests.swift",
-            "↓//\n// FileHeaderRuleTests.swift"
+            "// FileHeaderRule↓Tests.swift",
+            "//\n// FileHeaderRule↓Tests.swift"
         ]
         let description = FileHeaderRule.description
             .with(nonTriggeringExamples: nonTriggeringExamples)
             .with(triggeringExamples: triggeringExamples)
 
-        verifyRule(description, ruleConfiguration: ["forbidden_pattern": ".?"],
-                   skipCommentTests: true, skipDisableCommandTests: true, testMultiByteOffsets: false)
+        verifyRule(description, ruleConfiguration: ["forbidden_pattern": "[tT]ests"],
+                   skipCommentTests: true, testMultiByteOffsets: false)
     }
 }

Tests/SwiftLintFrameworkTests/TestHelpers.swift

@@ -118,14 +118,19 @@ private extension String {
     }
 }
 
-internal func makeConfig(_ ruleConfiguration: Any?, _ identifier: String) -> Configuration? {
+internal func makeConfig(_ ruleConfiguration: Any?, _ identifier: String,
+                         skipDisableCommandTests: Bool = false) -> Configuration? {
+    let superfluousDisableCommandRuleIdentifier = SuperfluousDisableCommandRule.description.identifier
+    let identifiers = skipDisableCommandTests ? [identifier] : [identifier, superfluousDisableCommandRuleIdentifier]
+
     if let ruleConfiguration = ruleConfiguration, let ruleType = masterRuleList.list[identifier] {
         // The caller has provided a custom configuration for the rule under test
         return (try? ruleType.init(configuration: ruleConfiguration)).flatMap { configuredRule in
-            return Configuration(rulesMode: .whitelisted([identifier]), configuredRules: [configuredRule])
+            let rules = skipDisableCommandTests ? [configuredRule] : [configuredRule, SuperfluousDisableCommandRule()]
+            return Configuration(rulesMode: .whitelisted(identifiers), configuredRules: rules)
         }
     }
-    return Configuration(rulesMode: .whitelisted([identifier]))
+    return Configuration(rulesMode: .whitelisted(identifiers))
 }
 
 private func testCorrection(_ correction: (String, String),
@@ -155,7 +160,9 @@ extension XCTestCase {
                     skipDisableCommandTests: Bool = false,
                     testMultiByteOffsets: Bool = true,
                     testShebang: Bool = true) {
-        guard let config = makeConfig(ruleConfiguration, ruleDescription.identifier) else {
+        guard let config = makeConfig(ruleConfiguration,
+                                      ruleDescription.identifier,
+                                      skipDisableCommandTests: skipDisableCommandTests) else {
             XCTFail("Failed to create configuration")
             return
         }