Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite ForceUnwrappingRule #552

Merged
merged 6 commits into from
Mar 10, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,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
Expand Down
42 changes: 42 additions & 0 deletions Source/SwiftLintFramework/Extensions/Structure+SwiftLint.swift
Original file line number Diff line number Diff line change
@@ -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
}
}
119 changes: 115 additions & 4 deletions Source/SwiftLintFramework/Rules/ForceUnwrappingRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)↓!",
Expand All @@ -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
}
}
2 changes: 1 addition & 1 deletion Source/SwiftLintFrameworkTests/TestHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -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>"; };
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down