Skip to content

Commit

Permalink
Issue realm#3167: Fixes false positives for multiline_parameters_brac…
Browse files Browse the repository at this point in the history
…kets and multiline_arguments_brackets
  • Loading branch information
noahsark769 authored and PaulTaykalo committed May 28, 2020
1 parent 8ed7c31 commit e0be20f
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
9 changes: 9 additions & 0 deletions Source/SwiftLintFramework/Extensions/String+SwiftLint.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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("""
Expand All @@ -71,6 +94,12 @@ public struct MultilineArgumentsBracketsRule: ASTRule, OptInRule, ConfigurationP
x: 5,
y: 7
)↓)
"""),
Example("""
SomeOtherType(↓a: [
1, 2, 3
],
b: "two"↓)
""")
]
)
Expand All @@ -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 []
}
Expand All @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ public struct MultilineParametersBracketsRule: OptInRule, ConfigurationProviderR
"""),
Example("""
func foo<T>(param1: T, param2: String, param3: String) -> T { /* some code */ }
"""),
Example("""
func foo(a: [Int] = [
1
])
""")
],
triggeringExamples: [
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 16 additions & 0 deletions Tests/SwiftLintFrameworkTests/ExtendedStringTests.swift
Original file line number Diff line number Diff line change
@@ -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)
}
}

0 comments on commit e0be20f

Please sign in to comment.