diff --git a/CHANGELOG.md b/CHANGELOG.md index 075a348207..d8ff7d805b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ [Daniel Beard](https://github.com/daniel-beard) [#764](https://github.com/realm/SwiftLint/issues/764) +* Added opt-In rule to makes closure expressions spacing consistent. + [J. Cheyo Jimenez](https://github.com/masters3d) + [#770](https://github.com/realm/SwiftLint/issues/770) + * Adds `allow_private_set` configuration for the `private_outlet` rule. [Rohan Dhaimade](https://github.com/HaloZero) diff --git a/Source/SwiftLintFramework/Extensions/File+Cache.swift b/Source/SwiftLintFramework/Extensions/File+Cache.swift index c91526c78d..6a8058b126 100644 --- a/Source/SwiftLintFramework/Extensions/File+Cache.swift +++ b/Source/SwiftLintFramework/Extensions/File+Cache.swift @@ -26,8 +26,9 @@ private var structureCache = Cache({file -> Structure? in } return nil }) -private var syntaxMapCache = Cache({file in responseCache.get(file).map(SyntaxMap.init)}) -private var syntaxKindsByLinesCache = Cache({file in file.syntaxKindsByLine()}) +private var syntaxMapCache = Cache({ file in responseCache.get(file).map(SyntaxMap.init) }) +private var syntaxKindsByLinesCache = Cache({ file in file.syntaxKindsByLine() }) +private var syntaxTokensByLinesCache = Cache({ file in file.syntaxTokensByLine() }) private typealias AssertHandler = () -> () private var assertHandlers = [String: AssertHandler?]() @@ -115,6 +116,17 @@ extension File { return syntaxMap } + internal var syntaxTokensByLines: [[SyntaxToken]] { + guard let syntaxTokensByLines = syntaxTokensByLinesCache.get(self) else { + if let handler = assertHandler { + handler() + return [] + } + fatalError("Never call this for file that sourcekitd fails.") + } + return syntaxTokensByLines + } + internal var syntaxKindsByLines: [[SyntaxKind]] { guard let syntaxKindsByLines = syntaxKindsByLinesCache.get(self) else { if let handler = assertHandler { @@ -131,6 +143,7 @@ extension File { assertHandlers.removeValueForKey(cacheKey) structureCache.invalidate(self) syntaxMapCache.invalidate(self) + syntaxTokensByLinesCache.invalidate(self) syntaxKindsByLinesCache.invalidate(self) } @@ -141,6 +154,7 @@ extension File { assertHandlers = [:] structureCache.clear() syntaxMapCache.clear() + syntaxTokensByLinesCache.clear() syntaxKindsByLinesCache.clear() } diff --git a/Source/SwiftLintFramework/Extensions/File+SwiftLint.swift b/Source/SwiftLintFramework/Extensions/File+SwiftLint.swift index 9d467a21a8..b932beacb8 100644 --- a/Source/SwiftLintFramework/Extensions/File+SwiftLint.swift +++ b/Source/SwiftLintFramework/Extensions/File+SwiftLint.swift @@ -103,11 +103,11 @@ extension File { } } - internal func syntaxKindsByLine() -> [[SyntaxKind]]? { + internal func syntaxTokensByLine() -> [[SyntaxToken]]? { if sourcekitdFailed { return nil } - var results = [[SyntaxKind]](count: lines.count + 1, repeatedValue: []) + var results = [[SyntaxToken]](count: lines.count + 1, repeatedValue: []) var tokenGenerator = syntaxMap.tokens.generate() var lineGenerator = lines.generate() var maybeLine = lineGenerator.next() @@ -116,7 +116,7 @@ extension File { let tokenRange = NSRange(location: token.offset, length: token.length) if NSLocationInRange(token.offset, line.byteRange) || NSLocationInRange(line.byteRange.location, tokenRange) { - results[line.index].append(SyntaxKind(rawValue: token.type)!) + results[line.index].append(token) } let tokenEnd = NSMaxRange(tokenRange) let lineEnd = NSMaxRange(line.byteRange) @@ -132,6 +132,19 @@ extension File { return results } + internal func syntaxKindsByLine() -> [[SyntaxKind]]? { + + if sourcekitdFailed { + return nil + } + guard let tokens = syntaxTokensByLine() else { + return nil + } + + return tokens.map { $0.flatMap { SyntaxKind.init(rawValue: $0.type) } } + + } + //Added by S2dent /** This function returns only matches that are not contained in a syntax kind diff --git a/Source/SwiftLintFramework/Extensions/Structure+SwiftLint.swift b/Source/SwiftLintFramework/Extensions/Structure+SwiftLint.swift index 018685ee0f..7a21a01c52 100644 --- a/Source/SwiftLintFramework/Extensions/Structure+SwiftLint.swift +++ b/Source/SwiftLintFramework/Extensions/Structure+SwiftLint.swift @@ -23,7 +23,7 @@ extension Structure { 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)}) + .map({ NSRange(location: offset, length: $0) }) where NSLocationInRange(byteOffset, byteRange) else { return } diff --git a/Source/SwiftLintFramework/Models/Configuration.swift b/Source/SwiftLintFramework/Models/Configuration.swift index cef283e3c5..e095ebc845 100644 --- a/Source/SwiftLintFramework/Models/Configuration.swift +++ b/Source/SwiftLintFramework/Models/Configuration.swift @@ -50,7 +50,7 @@ public struct Configuration: Equatable { $0.dynamicType.description.identifier } - let validDisabledRules = disabledRules.filter({ validRuleIdentifiers.contains($0)}) + let validDisabledRules = disabledRules.filter({ validRuleIdentifiers.contains($0) }) let invalidRules = disabledRules.filter({ !validRuleIdentifiers.contains($0) }) if !invalidRules.isEmpty { for invalidRule in invalidRules { diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index a6d3ebf7a5..1b00e7672e 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -42,6 +42,7 @@ public struct RuleList { public let masterRuleList = RuleList(rules: ClosingBraceRule.self, + ClosureSpacingRule.self, ColonRule.self, CommaRule.self, ConditionalReturnsOnNewline.self, diff --git a/Source/SwiftLintFramework/Rules/ClosureSpacingRule.swift b/Source/SwiftLintFramework/Rules/ClosureSpacingRule.swift new file mode 100644 index 0000000000..78a7ff65e5 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/ClosureSpacingRule.swift @@ -0,0 +1,109 @@ +// +// ClosureSpacingRule.swift +// SwiftLint +// +// Created by J. Cheyo Jimenez on 2016-08-26. +// Copyright © 2016 Realm. All rights reserved. +// + +import Foundation +import SourceKittenFramework + +public struct ClosureSpacingRule: Rule, ConfigurationProviderRule, OptInRule { + + public var configuration = SeverityConfiguration(.Warning) + + public init() {} + + public static let description = RuleDescription( + identifier: "closure_spacing", + name: "Closure Spacing", + description: "Closure expressions should have a single space inside each brace.", + nonTriggeringExamples: [ + "[].map ({ $0.description })", + "[].filter { $0.contains(location) }" + ], + triggeringExamples: [ + "[].filter({$0.contains(location)})", + "[].map({$0})" + ] + ) + + // this helps cut down the time to search true a file by + // skipping lines that do not have at least one { and one } brace + func lineContainsBracesIn(range: NSRange, content: NSString) -> NSRange? { + let start = content.rangeOfString("{", options: [.LiteralSearch], range: range) + guard start.length != 0 else { return nil } + let end = content.rangeOfString("}", + options: [.LiteralSearch, .BackwardsSearch], range: range) + guard end.length != 0 else { return nil } + guard start.location < end.location else { return nil } + return NSRange(location: start.location, + length: end.location - start.location + 1) + } + + // returns ranges of braces { or } in the same line + func validBraces(file: File) -> [NSRange] { + let nsstring = (file.contents as NSString) + let bracePattern = regex("\\{|\\}") + let linesTokens = file.syntaxTokensByLines + let kindsToExclude = SyntaxKind.commentAndStringKinds().map { $0.rawValue } + + // find all lines and accurences of open { and closed } braces + var linesWithBraces = [[NSRange]]() + for eachLine in file.lines { + guard let nsrange = lineContainsBracesIn(eachLine.range, content: nsstring) + else { continue } + + let braces = bracePattern.matchesInString(file.contents, options: [], + range: nsrange).map { $0.range } + // filter out braces in comments and strings + let tokens = linesTokens[eachLine.index].filter { kindsToExclude.contains($0.type) } + let tokenRanges = tokens.flatMap { + file.contents.byteRangeToNSRange(start: $0.offset, length: $0.length) } + linesWithBraces.append(braces.filter { !$0.intersectsRanges(tokenRanges) }) + } + return linesWithBraces.flatMap { $0 } + } + + public func validateFile(file: File) -> [StyleViolation] { + + // match open braces to corresponding closing braces + func matchBraces(validBraceLocations: [NSRange]) -> [NSRange] { + if validBraceLocations.isEmpty { return [] } + var validBraces = validBraceLocations + var ranges = [NSRange]() + var bracesAsString = validBraces.map { + file.contents.substring($0.location, length: $0.length) }.joinWithSeparator("") + while let foundRange = bracesAsString.rangeOfString("{}") { + let startIndex = bracesAsString.startIndex.distanceTo(foundRange.startIndex) + let location = validBraces[startIndex].location + let length = validBraces[startIndex + 1 ].location + 1 - location + ranges.append(NSRange(location:location, length: length)) + bracesAsString.replaceRange(foundRange, with: "") + validBraces.removeRange(startIndex...startIndex + 1) + } + return ranges + } + + // matching ranges of {} + let matchedUpBraces = matchBraces(validBraces(file)) + + var violationRanges = matchedUpBraces.filter { + //removes enclosing brances to just content + let content = file.contents.substring($0.location + 1, length: $0.length - 2) + if content.isEmpty { return false } // case when {} is not a closure + let cleaned = content.stringByTrimmingCharactersInSet(.whitespaceCharacterSet()) + return content != " " + cleaned + " " + } + + //filter out ranges where rule is disabled + violationRanges = file.ruleEnabledViolatingRanges(violationRanges, forRule: self) + + return violationRanges.flatMap { StyleViolation( + ruleDescription: self.dynamicType.description, + severity: configuration.severity, + location: Location(file: file, characterOffset: $0.location) + )} + } +} diff --git a/Source/SwiftLintFramework/Rules/ConditionalReturnsOnNewline.swift b/Source/SwiftLintFramework/Rules/ConditionalReturnsOnNewline.swift index e469e0197a..dea4d42ab4 100644 --- a/Source/SwiftLintFramework/Rules/ConditionalReturnsOnNewline.swift +++ b/Source/SwiftLintFramework/Rules/ConditionalReturnsOnNewline.swift @@ -13,7 +13,7 @@ public struct ConditionalReturnsOnNewline: ConfigurationProviderRule, Rule, OptI public let configurationDescription = "N/A" public var configuration = SeverityConfiguration(.Warning) - public init() { } + public init() {} public static let description = RuleDescription( identifier: "conditional_returns_on_newline", diff --git a/Source/SwiftLintFramework/Rules/LineLengthRule.swift b/Source/SwiftLintFramework/Rules/LineLengthRule.swift index d88960cf16..4b85096983 100644 --- a/Source/SwiftLintFramework/Rules/LineLengthRule.swift +++ b/Source/SwiftLintFramework/Rules/LineLengthRule.swift @@ -30,7 +30,7 @@ public struct LineLengthRule: ConfigurationProviderRule, SourceKitFreeRule { ) public func validateFile(file: File) -> [StyleViolation] { - let minValue = configuration.params.map({$0.value}).minElement(<) + let minValue = configuration.params.map({ $0.value }).minElement(<) return file.lines.flatMap { line in // `line.content.characters.count` <= `line.range.length` is true. // So, `check line.range.length` is larger than minimum parameter value. diff --git a/Source/SwiftLintFramework/Rules/OperatorFunctionWhitespaceRule.swift b/Source/SwiftLintFramework/Rules/OperatorFunctionWhitespaceRule.swift index f8dcc24822..8dfa05ccfb 100644 --- a/Source/SwiftLintFramework/Rules/OperatorFunctionWhitespaceRule.swift +++ b/Source/SwiftLintFramework/Rules/OperatorFunctionWhitespaceRule.swift @@ -34,7 +34,7 @@ public struct OperatorFunctionWhitespaceRule: ConfigurationProviderRule { ) public func validateFile(file: File) -> [StyleViolation] { - let operators = ["/", "=", "-", "+", "!", "*", "|", "^", "~", "?", "."].map({"\\\($0)"}) + + let operators = ["/", "=", "-", "+", "!", "*", "|", "^", "~", "?", "."].map({ "\\\($0)" }) + ["%", "<", ">", "&"] let zeroOrManySpaces = "(\\s{0}|\\s{2,})" let pattern1 = "func\\s+[" + operators.joinWithSeparator("") + diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index 78b121917d..bd40af46b5 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -10,6 +10,7 @@ 006ECFC41C44E99E00EF6364 /* LegacyConstantRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 006ECFC31C44E99E00EF6364 /* LegacyConstantRule.swift */; }; 02FD8AEF1BFC18D60014BFFB /* ExtendedNSStringTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02FD8AEE1BFC18D60014BFFB /* ExtendedNSStringTests.swift */; }; 094385041D5D4F7C009168CF /* PrivateOutletRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 094385021D5D4F78009168CF /* PrivateOutletRule.swift */; }; + 1E82D5591D7775C7009553D7 /* ClosureSpacingRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1E82D5581D7775C7009553D7 /* ClosureSpacingRule.swift */; }; 1EC163521D5992D900DD2928 /* VerticalWhitespaceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1EC163511D5992D900DD2928 /* VerticalWhitespaceRule.swift */; }; 1F11B3CF1C252F23002E8FA8 /* ClosingBraceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1F11B3CE1C252F23002E8FA8 /* ClosingBraceRule.swift */; }; 24B4DF0D1D6DFDE90097803B /* RedundantNilCoalesingRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 24B4DF0B1D6DFA370097803B /* RedundantNilCoalesingRule.swift */; }; @@ -185,6 +186,7 @@ 006ECFC31C44E99E00EF6364 /* LegacyConstantRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LegacyConstantRule.swift; sourceTree = ""; }; 02FD8AEE1BFC18D60014BFFB /* ExtendedNSStringTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ExtendedNSStringTests.swift; sourceTree = ""; }; 094385021D5D4F78009168CF /* PrivateOutletRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PrivateOutletRule.swift; sourceTree = ""; }; + 1E82D5581D7775C7009553D7 /* ClosureSpacingRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ClosureSpacingRule.swift; sourceTree = ""; }; 1EC163511D5992D900DD2928 /* VerticalWhitespaceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = VerticalWhitespaceRule.swift; sourceTree = ""; }; 1F11B3CE1C252F23002E8FA8 /* ClosingBraceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ClosingBraceRule.swift; sourceTree = ""; }; 24B4DF0B1D6DFA370097803B /* RedundantNilCoalesingRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RedundantNilCoalesingRule.swift; sourceTree = ""; }; @@ -604,6 +606,7 @@ isa = PBXGroup; children = ( 1F11B3CE1C252F23002E8FA8 /* ClosingBraceRule.swift */, + 1E82D5581D7775C7009553D7 /* ClosureSpacingRule.swift */, E88DEA831B0990F500A66CB0 /* ColonRule.swift */, 695BE9CE1BDFD92B0071E985 /* CommaRule.swift */, 93E0C3CD1D67BD7F007FA25D /* ConditionalReturnsOnNewline.swift */, @@ -975,6 +978,7 @@ 3BD9CD3D1C37175B009A5D25 /* YamlParser.swift in Sources */, F22314B01D4FA4D7009AD165 /* LegacyNSGeometryFunctionsRule.swift in Sources */, E88DEA8C1B0999A000A66CB0 /* ASTRule.swift in Sources */, + 1E82D5591D7775C7009553D7 /* ClosureSpacingRule.swift in Sources */, 094385041D5D4F7C009168CF /* PrivateOutletRule.swift in Sources */, E88DEA6B1B0983FE00A66CB0 /* StyleViolation.swift in Sources */, 3BB47D831C514E8100AE6A10 /* RegexConfiguration.swift in Sources */, diff --git a/Tests/SwiftLintFramework/IntegrationTests.swift b/Tests/SwiftLintFramework/IntegrationTests.swift index ee446ab2be..674b065fa2 100644 --- a/Tests/SwiftLintFramework/IntegrationTests.swift +++ b/Tests/SwiftLintFramework/IntegrationTests.swift @@ -25,7 +25,7 @@ class IntegrationTests: XCTestCase { func testSwiftLintLints() { // This is as close as we're ever going to get to a self-hosting linter. let swiftFiles = config.lintableFilesForPath("") - XCTAssert(swiftFiles.map({$0.path!}).contains(#file), "current file should be included") + XCTAssert(swiftFiles.map({ $0.path! }).contains(#file), "current file should be included") let violations = swiftFiles.flatMap { Linter(file: $0, configuration: config).styleViolations diff --git a/Tests/SwiftLintFramework/RulesTests.swift b/Tests/SwiftLintFramework/RulesTests.swift index f480d4ce7c..085a09b1a7 100644 --- a/Tests/SwiftLintFramework/RulesTests.swift +++ b/Tests/SwiftLintFramework/RulesTests.swift @@ -93,6 +93,10 @@ class RulesTests: XCTestCase { verifyRule(CommaRule.description) } + func testClosureSpacingRule() { + verifyRule(ClosureSpacingRule.description) + } + func testConditionalReturnsOnNewline() { verifyRule(ConditionalReturnsOnNewline.description) } diff --git a/Tests/SwiftLintFramework/SourceKitCrashTests.swift b/Tests/SwiftLintFramework/SourceKitCrashTests.swift index 6bc1d32541..f95d341554 100644 --- a/Tests/SwiftLintFramework/SourceKitCrashTests.swift +++ b/Tests/SwiftLintFramework/SourceKitCrashTests.swift @@ -33,6 +33,11 @@ class SourceKitCrashTests: XCTestCase { _ = file.syntaxKindsByLines XCTAssertFalse(assertHandlerCalled, "Expects assert handler was not called on accessing File.syntaxKindsByLines") + + assertHandlerCalled = false + _ = file.syntaxTokensByLines + XCTAssertFalse(assertHandlerCalled, + "Expects assert handler was not called on accessing File.syntaxTokensByLines") } func testAssertHandlerIsCalledOnFileThatCrashedSourceKitService() { @@ -55,6 +60,11 @@ class SourceKitCrashTests: XCTestCase { _ = file.syntaxKindsByLines XCTAssertTrue(assertHandlerCalled, "Expects assert handler was called on accessing File.syntaxKindsByLines") + + assertHandlerCalled = false + _ = file.syntaxTokensByLines + XCTAssertTrue(assertHandlerCalled, + "Expects assert handler was not called on accessing File.syntaxTokensByLines") } func testRulesWithFileThatCrashedSourceKitService() {