From e0be20f08ca5c1ddd16cad670fa885f0fcb40321 Mon Sep 17 00:00:00 2001 From: Noah Gilmore Date: Sun, 5 Apr 2020 13:17:19 -0700 Subject: [PATCH] Issue #3167: Fixes false positives for multiline_parameters_brackets and multiline_arguments_brackets --- .../SourceKittenDictionary+Swiftlint.swift | 15 ++++++ .../Extensions/String+SwiftLint.swift | 9 ++++ .../MultilineArgumentsBracketsRule.swift | 50 +++++++++++++++++-- .../MultilineParametersBracketsRule.swift | 14 +++++- .../ExtendedStringTests.swift | 16 ++++++ 5 files changed, 97 insertions(+), 7 deletions(-) create mode 100644 Tests/SwiftLintFrameworkTests/ExtendedStringTests.swift diff --git a/Source/SwiftLintFramework/Extensions/SourceKittenDictionary+Swiftlint.swift b/Source/SwiftLintFramework/Extensions/SourceKittenDictionary+Swiftlint.swift index 2fd028b65e..f417d49809 100644 --- a/Source/SwiftLintFramework/Extensions/SourceKittenDictionary+Swiftlint.swift +++ b/Source/SwiftLintFramework/Extensions/SourceKittenDictionary+Swiftlint.swift @@ -41,4 +41,19 @@ extension SourceKittenDictionary { parse(self) return results } + + /// Return the string content of this structure in the given file. + /// - Parameter file: File this structure occurs in + /// - Returns: The content of the file which this `SourceKittenDictionary` structure represents + func content(in file: SwiftLintFile) -> String? { + guard + let byteRange = self.byteRange, + let range = file.stringView.byteRangeToNSRange(byteRange) + else { + return nil + } + + let body = file.stringView.nsString.substring(with: range) + return String(body) + } } diff --git a/Source/SwiftLintFramework/Extensions/String+SwiftLint.swift b/Source/SwiftLintFramework/Extensions/String+SwiftLint.swift index bf932612e3..920c8d80f5 100644 --- a/Source/SwiftLintFramework/Extensions/String+SwiftLint.swift +++ b/Source/SwiftLintFramework/Extensions/String+SwiftLint.swift @@ -91,4 +91,13 @@ extension String { } return false } + + /// Count the number of occurrences of the given character in `self` + /// - Parameter character: Character to count + /// - Returns: Number of times `character` occurs in `self` + public func countOccurrences(of character: Character) -> Int { + return self.reduce(0, { + $1 == character ? $0 + 1 : $0 + }) + } } diff --git a/Source/SwiftLintFramework/Rules/Style/MultilineArgumentsBracketsRule.swift b/Source/SwiftLintFramework/Rules/Style/MultilineArgumentsBracketsRule.swift index 084566af55..5bdc4f1ad9 100644 --- a/Source/SwiftLintFramework/Rules/Style/MultilineArgumentsBracketsRule.swift +++ b/Source/SwiftLintFramework/Rules/Style/MultilineArgumentsBracketsRule.swift @@ -44,7 +44,30 @@ public struct MultilineArgumentsBracketsRule: ASTRule, OptInRule, ConfigurationP AlertViewModel.AlertAction(title: "some title", style: .default) { AlertManager.shared.presentNextDebugAlert() } - """) + """), + Example(""" + public final class Logger { + public static let shared = Logger(outputs: [ + OSLoggerOutput(), + ErrorLoggerOutput() + ]) + } + """), + Example(""" + let errors = try self.download([ + (description: description, priority: priority), + ]) + """), + Example(""" + return SignalProducer({ observer, _ in + observer.sendCompleted() + }).onMainQueue() + """), + Example(""" + SomeType(a: [ + 1, 2, 3 + ], b: [1, 2]) + """), ], triggeringExamples: [ Example(""" @@ -71,6 +94,12 @@ public struct MultilineArgumentsBracketsRule: ASTRule, OptInRule, ConfigurationP x: 5, y: 7 )↓) + """), + Example(""" + SomeOtherType(↓a: [ + 1, 2, 3 + ], + b: "two"↓) """) ] ) @@ -86,8 +115,19 @@ public struct MultilineArgumentsBracketsRule: ASTRule, OptInRule, ConfigurationP return [] } - let body = file.contents.substring(from: range.location, length: range.length) - let isMultiline = body.contains("\n") + let callBody = file.contents.substring(from: range.location, length: range.length) + + let parameters = dictionary.substructure.filter { + // Argument expression types that can contain newlines + [.argument, .array, .dictionary, .closure].contains($0.expressionKind) + } + let parameterBodies = parameters.compactMap { $0.content(in: file) } + let parametersNewlineCount = parameterBodies.map { body in + return body.countOccurrences(of: "\n") + }.reduce(0, +) + let callNewlineCount = callBody.countOccurrences(of: "\n") + let isMultiline = callNewlineCount > parametersNewlineCount + guard isMultiline else { return [] } @@ -96,11 +136,11 @@ public struct MultilineArgumentsBracketsRule: ASTRule, OptInRule, ConfigurationP let expectedBodyEndRegex = regex("\\n[ \\t]*\\z") var violatingByteOffsets = [ByteCount]() - if expectedBodyBeginRegex.firstMatch(in: body, options: [], range: body.fullNSRange) == nil { + if expectedBodyBeginRegex.firstMatch(in: callBody, options: [], range: callBody.fullNSRange) == nil { violatingByteOffsets.append(bodyRange.location) } - if expectedBodyEndRegex.firstMatch(in: body, options: [], range: body.fullNSRange) == nil { + if expectedBodyEndRegex.firstMatch(in: callBody, options: [], range: callBody.fullNSRange) == nil { violatingByteOffsets.append(bodyRange.upperBound) } diff --git a/Source/SwiftLintFramework/Rules/Style/MultilineParametersBracketsRule.swift b/Source/SwiftLintFramework/Rules/Style/MultilineParametersBracketsRule.swift index a2f2f57fa6..0e74241334 100644 --- a/Source/SwiftLintFramework/Rules/Style/MultilineParametersBracketsRule.swift +++ b/Source/SwiftLintFramework/Rules/Style/MultilineParametersBracketsRule.swift @@ -50,6 +50,11 @@ public struct MultilineParametersBracketsRule: OptInRule, ConfigurationProviderR """), Example(""" func foo(param1: T, param2: String, param3: String) -> T { /* some code */ } + """), + Example(""" + func foo(a: [Int] = [ + 1 + ]) """) ], triggeringExamples: [ @@ -106,9 +111,14 @@ public struct MultilineParametersBracketsRule: OptInRule, ConfigurationProviderR return [] } - let isMultiline = functionName.contains("\n") - let parameters = substructure.substructure.filter { $0.declarationKind == .varParameter } + let parameterBodies = parameters.compactMap { $0.content(in: file) } + let parametersNewlineCount = parameterBodies.map { body in + return body.countOccurrences(of: "\n") + }.reduce(0, +) + let declarationNewlineCount = functionName.countOccurrences(of: "\n") + let isMultiline = declarationNewlineCount > parametersNewlineCount + if isMultiline && !parameters.isEmpty { if let openingBracketViolation = openingBracketViolation(parameters: parameters, file: file) { violations.append(openingBracketViolation) diff --git a/Tests/SwiftLintFrameworkTests/ExtendedStringTests.swift b/Tests/SwiftLintFrameworkTests/ExtendedStringTests.swift new file mode 100644 index 0000000000..9fa2a931f5 --- /dev/null +++ b/Tests/SwiftLintFrameworkTests/ExtendedStringTests.swift @@ -0,0 +1,16 @@ +// +// ExtendedStringTests.swift +// SwiftLintFrameworkTests +// +// Created by Noah Gilmore on 4/5/20. +// + +import XCTest + +class ExtendedStringTests: XCTestCase { + func testCountOccurrences() { + XCTAssertEqual("aabbabaaba".countOccurrences(of: "a"), 6) + XCTAssertEqual("".countOccurrences(of: "a"), 0) + XCTAssertEqual("\n\n".countOccurrences(of: "\n"), 2) + } +}