Merge pull request #552 from realm/nn-rewrite-force-unwrapping-rule

Rewrite `ForceUnwrappingRule`

CHANGELOG.md

@@ -18,6 +18,11 @@
   [Norio Nomura](https://github.com/norio-nomura)
   [#466](https://github.com/realm/SwiftLint/issues/466)
 
+* Fix more false positives in `ForceUnwrappingRule`.  
+  [Norio Nomura](https://github.com/norio-nomura)
+  [#546](https://github.com/realm/SwiftLint/issues/546)
+  [#547](https://github.com/realm/SwiftLint/issues/547)
+
 ## 0.9.1: Air Duct Cleaning
 
 ##### Breaking

Source/SwiftLintFramework/Extensions/Structure+SwiftLint.swift

@@ -0,0 +1,42 @@
+//
+//  Structure+SwiftLint.swift
+//  SwiftLint
+//
+//  Created by Norio Nomura on 2/18/16.
+//  Copyright © 2016 Realm. All rights reserved.
+//
+
+import Foundation
+import SourceKittenFramework
+
+extension Structure {
+
+    /// Returns array of tuples containing "key.kind" and "byteRange" from Structure
+    /// that contains the byte offset.
+    ///
+    /// - Parameter byteOffset: Int
+    // swiftlint:disable:next valid_docs
+    internal func kindsFor(byteOffset: Int) -> [(kind: String, byteRange: NSRange)] {
+        var results = [(kind: String, byteRange: NSRange)]()
+
+        func parse(dictionary: [String : SourceKitRepresentable]) {
+            guard let
+                offset = (dictionary["key.offset"] as? Int64).map({ Int($0) }),
+                byteRange = (dictionary["key.length"] as? Int64).map({ Int($0) })
+                    .map({NSRange(location: offset, length: $0)})
+                where NSLocationInRange(byteOffset, byteRange) else {
+                    return
+            }
+            if let kind = dictionary["key.kind"] as? String {
+                results.append((kind: kind, byteRange: byteRange))
+            }
+            if let subStructure = dictionary["key.substructure"] as? [SourceKitRepresentable] {
+                for case let dictionary as [String : SourceKitRepresentable] in subStructure {
+                    parse(dictionary)
+                }
+            }
+        }
+        parse(dictionary)
+        return results
+    }
+}

Source/SwiftLintFramework/Rules/ForceUnwrappingRule.swift

@@ -25,6 +25,12 @@ public struct ForceUnwrappingRule: OptInRule, ConfigurationProviderRule {
             "let s as! Test",
             "try! canThrowErrors()",
             "let object: AnyObject!",
+            "@IBOutlet var constraints: [NSLayoutConstraint]!",
+            "setEditing(!editing, animated: true)",
+            "navigationController.setNavigationBarHidden(!navigationController." +
+                "navigationBarHidden, animated: true)",
+            "if addedToPlaylist && (!self.selectedFilters.isEmpty || " +
+                "self.searchBar?.text?.isEmpty == false) {}",
         ],
         triggeringExamples: [
             "let url = NSURL(string: query)↓!",
@@ -36,11 +42,116 @@ public struct ForceUnwrappingRule: OptInRule, ConfigurationProviderRule {
     )
 
     public func validateFile(file: File) -> [StyleViolation] {
-        return file.matchPattern("\\S!",
-            excludingSyntaxKinds: SyntaxKind.commentKeywordStringAndTypeidentifierKinds()).map {
-            StyleViolation(ruleDescription: self.dynamicType.description,
+        return violationRangesInFile(file).map {
+            return StyleViolation(ruleDescription: self.dynamicType.description,
                 severity: configuration.severity,
-                location: Location(file: file, characterOffset: NSMaxRange($0) - 1))
+                location: Location(file: file, characterOffset: $0.location))
         }
     }
+
+    // capture previous and next of "!"
+    // http://userguide.icu-project.org/strings/regexp
+    private static let pattern = "(\\S)!(.?)"
+
+    // swiftlint:disable:next force_try
+    private static let regularExpression = try! NSRegularExpression(pattern: pattern,
+        options: [.DotMatchesLineSeparators])
+    private static let excludingSyntaxKindsForFirstCapture = SyntaxKind
+        .commentKeywordStringAndTypeidentifierKinds().map { $0.rawValue }
+    private static let excludingSyntaxKindsForSecondCapture = [SyntaxKind.Identifier.rawValue]
+
+    private func violationRangesInFile(file: File) -> [NSRange] {
+        let contents = file.contents
+        let nsstring = contents as NSString
+        let range = NSRange(location: 0, length: contents.utf16.count)
+        let syntaxMap = file.syntaxMap
+        return ForceUnwrappingRule.regularExpression
+            .matchesInString(contents, options: [], range: range)
+            .flatMap { match -> NSRange? in
+                if match.numberOfRanges < 2 { return nil }
+
+                // check first captured range
+                let firstRange = match.rangeAtIndex(1)
+                let violationRange = NSRange(location: NSMaxRange(firstRange), length: 0)
+
+                guard let matchByteFirstRange = contents
+                    .NSRangeToByteRange(start: firstRange.location, length: firstRange.length)
+                    else { return nil }
+
+                let tokensInFirstRange = syntaxMap.tokensIn(matchByteFirstRange)
+
+                // If not empty, first captured range is comment, string, keyword or typeidentifier.
+                // We checks "not empty" because tokens may empty without filtering.
+                let tokensInFirstRangeExcludingSyntaxKindsOnly = tokensInFirstRange.filter({
+                    ForceUnwrappingRule.excludingSyntaxKindsForFirstCapture.contains($0.type)
+                })
+                if !tokensInFirstRangeExcludingSyntaxKindsOnly.isEmpty { return nil }
+
+                // if first captured range is identifier, generate violation
+                if tokensInFirstRange.map({ $0.type }).contains(SyntaxKind.Identifier.rawValue) {
+                    return violationRange
+                }
+
+                // check firstCapturedString is ")"
+                let firstCapturedString = nsstring.substringWithRange(firstRange)
+                if firstCapturedString == ")" { return violationRange }
+
+                // check second capture
+                if match.numberOfRanges == 3 {
+
+                    // check second captured range
+                    let secondRange = match.rangeAtIndex(2)
+                    guard let matchByteSecondRange = contents
+                        .NSRangeToByteRange(start: secondRange.location, length: secondRange.length)
+                        else { return nil }
+
+                    let tokensInSecondRange = syntaxMap.tokensIn(matchByteSecondRange).filter {
+                        ForceUnwrappingRule.excludingSyntaxKindsForSecondCapture.contains($0.type)
+                    }
+                    // If not empty, second captured range is identifier.
+                    // "!" is "operator prefix !".
+                    if !tokensInSecondRange.isEmpty { return nil }
+                }
+
+                // check structure
+                if checkStructure(file, byteRange: matchByteFirstRange) {
+                    return violationRange
+                } else {
+                    return nil
+                }
+        }
+    }
+
+    // Returns if range should generate violation
+    // check deepest kind matching range in structure
+    private func checkStructure(file: File, byteRange: NSRange) -> Bool {
+        let nsstring = file.contents as NSString
+        let kinds = file.structure.kindsFor(byteRange.location)
+        if let lastKind = kinds.last {
+            switch lastKind.kind {
+            // range is in some "source.lang.swift.decl.var.*"
+            case SwiftDeclarationKind.VarClass.rawValue: fallthrough
+            case SwiftDeclarationKind.VarGlobal.rawValue: fallthrough
+            case SwiftDeclarationKind.VarInstance.rawValue: fallthrough
+            case SwiftDeclarationKind.VarStatic.rawValue:
+                let byteOffset = lastKind.byteRange.location
+                let byteLength = byteRange.location - byteOffset
+                if let varDeclarationString = nsstring
+                    .substringWithByteRange(start: byteOffset, length: byteLength)
+                    where varDeclarationString.containsString("=") {
+                        // if declarations contains "=", range is not type annotation
+                        return true
+                } else {
+                    // range is type annotation of declaration
+                    return false
+                }
+            // followings have invalid "key.length" returned from SourceKitService w/ Xcode 7.2.1
+//            case SwiftDeclarationKind.VarParameter.rawValue: fallthrough
+//            case SwiftDeclarationKind.VarLocal.rawValue: fallthrough
+            default:
+                break
+            }
+        }
+        return false
+    }
 }

Source/SwiftLintFrameworkTests/TestHelpers.swift

@@ -65,7 +65,7 @@ extension XCTestCase {
         let nonTriggers = ruleDescription.nonTriggeringExamples
 
         // Non-triggering examples don't violate
-        XCTAssert(nonTriggers.flatMap({ violations($0, config: config) }).isEmpty)
+        XCTAssertEqual(nonTriggers.flatMap({ violations($0, config: config) }), [])
 
         var violationsCount = 0
         for trigger in triggers {

SwiftLint.xcodeproj/project.pbxproj

@@ -37,6 +37,7 @@
 		3BD9CD3D1C37175B009A5D25 /* YamlParser.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3BD9CD3C1C37175B009A5D25 /* YamlParser.swift */; };
 		3BDB224B1C345B4900473680 /* ProjectMock in Resources */ = {isa = PBXBuildFile; fileRef = 3BDB224A1C345B4900473680 /* ProjectMock */; };
 		69F88BF71BDA38A6005E7CAE /* OpeningBraceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 692B1EB11BD7E00F00EAABFF /* OpeningBraceRule.swift */; };
+		6CB514E91C760C6900FA02C4 /* Structure+SwiftLint.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6CB514E81C760C6900FA02C4 /* Structure+SwiftLint.swift */; };
 		6CC4259B1C77046200AEA885 /* SyntaxMap+SwiftLint.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6CC4259A1C77046200AEA885 /* SyntaxMap+SwiftLint.swift */; };
 		83894F221B0C928A006214E1 /* RulesCommand.swift in Sources */ = {isa = PBXBuildFile; fileRef = 83894F211B0C928A006214E1 /* RulesCommand.swift */; };
 		83D71E281B131ECE000395DE /* RuleDescription.swift in Sources */ = {isa = PBXBuildFile; fileRef = 83D71E261B131EB5000395DE /* RuleDescription.swift */; };
@@ -199,6 +200,7 @@
 		692B1EB11BD7E00F00EAABFF /* OpeningBraceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OpeningBraceRule.swift; sourceTree = "<group>"; };
 		692B60AB1BD8F2E700C7AA22 /* StatementPositionRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = StatementPositionRule.swift; sourceTree = "<group>"; };
 		695BE9CE1BDFD92B0071E985 /* CommaRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CommaRule.swift; sourceTree = "<group>"; };
+		6CB514E81C760C6900FA02C4 /* Structure+SwiftLint.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "Structure+SwiftLint.swift"; sourceTree = "<group>"; };
 		6CC4259A1C77046200AEA885 /* SyntaxMap+SwiftLint.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "SyntaxMap+SwiftLint.swift"; sourceTree = "<group>"; };
 		6CDD62CE1C6193300094A198 /* main.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = main.swift; sourceTree = "<group>"; };
 		83894F211B0C928A006214E1 /* RulesCommand.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RulesCommand.swift; sourceTree = "<group>"; };
@@ -628,6 +630,7 @@
 				E832F10A1B17E2F5003F265F /* NSFileManager+SwiftLint.swift */,
 				E81619521BFC162C00946723 /* QueuedPrint.swift */,
 				E88DEA721B0984C400A66CB0 /* String+SwiftLint.swift */,
+				6CB514E81C760C6900FA02C4 /* Structure+SwiftLint.swift */,
 				E816194B1BFBF35D00946723 /* SwiftDeclarationKind+SwiftLint.swift */,
 				E87E4A081BFB9CAE00FCFE46 /* SyntaxKind+SwiftLint.swift */,
 				6CC4259A1C77046200AEA885 /* SyntaxMap+SwiftLint.swift */,
@@ -862,6 +865,7 @@
 				E88198581BEA956C00333A11 /* FunctionBodyLengthRule.swift in Sources */,
 				E88DEA751B09852000A66CB0 /* File+SwiftLint.swift in Sources */,
 				3BCC04D11C4F56D3006073C3 /* SeverityLevelsConfiguration.swift in Sources */,
+				6CB514E91C760C6900FA02C4 /* Structure+SwiftLint.swift in Sources */,
 				E86396C51BADAC15002C9E88 /* XcodeReporter.swift in Sources */,
 				3B1DF0121C5148140011BCED /* CustomRules.swift in Sources */,
 				2E5761AA1C573B83003271AF /* FunctionParameterCountRule.swift in Sources */,