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

Fix false positives for 'multiple_closures_with_trailing_closure' #3353

Merged
merged 1 commit into from
Sep 22, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@
[hank121314](https://github.com/hank121314)
[#2367](https://github.com/realm/SwiftLint/issues/3267)

* Rule `multiple_closures_with_trailing_closure` no longer triggers when Swift
5.3's 'multiple trailing closures' feature is used.
[Jumhyn](https://github.com/jumhyn)
[#3295](https://github.com/realm/SwiftLint/issues/3295)

## 0.40.2: Demo Unit

#### Breaking
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ public struct MultipleClosuresWithTrailingClosureRule: ASTRule, ConfigurationPro
UIView.animate(withDuration: 1.0) {
someView.alpha = 0.0
}
""")
"""),
Example("foo.method { print(0) } arg2: { print(1) }"),
Example("foo.methodWithParenArgs((0, 1), arg2: (0, 1, 2)) { $0 } arg4: { $0 }")
],
triggeringExamples: [
Example("foo.something(param1: { $0 }) ↓{ $0 + 1 }"),
Expand All @@ -30,40 +32,98 @@ public struct MultipleClosuresWithTrailingClosureRule: ASTRule, ConfigurationPro
}) ↓{ _ in
someView.removeFromSuperview()
}
""")
"""),
Example("foo.multipleTrailing(arg1: { $0 }) { $0 } arg3: { $0 }"),
Example("foo.methodWithParenArgs(param1: { $0 }, param2: (0, 1), (0, 1)) { $0 }")
]
)

public func validate(file: SwiftLintFile, kind: SwiftExpressionKind,
dictionary: SourceKittenDictionary) -> [StyleViolation] {
guard kind == .call,
case let arguments = dictionary.enclosedArguments,
arguments.count > 1,
let lastArgument = arguments.last,
isTrailingClosure(argument: lastArgument, call: dictionary),
case let closureArguments = arguments.filterClosures(file: file),
closureArguments.count > 1,
let trailingClosureOffset = lastArgument.offset else {
// Any violations must have at least one closure argument.
closureArguments.isEmpty == false,
// If there is no closing paren (e.g. `foo { ... }`), there is no violation.
let closingParenOffset = dictionary.closingParenLocation(file: file),
// Find all trailing closures.
case let trailingClosureArguments = closureArguments.filter({
isTrailingClosure(argument: $0, closingParenOffset: closingParenOffset)
}),
// If there are no trailing closures, there is no violation.
trailingClosureArguments.isEmpty == false,
// If all closure arguments are trailing closures, there is no violation
trailingClosureArguments.count != closureArguments.count,
let firstTrailingClosureOffset = trailingClosureArguments.first?.offset else {
return []
}

return [
StyleViolation(ruleDescription: Self.description,
severity: configuration.severity,
location: Location(file: file, byteOffset: trailingClosureOffset))
location: Location(file: file, byteOffset: firstTrailingClosureOffset))
]
}

// A closure is 'trailing' if it appears outside the closing paren.
private func isTrailingClosure(argument: SourceKittenDictionary,
call: SourceKittenDictionary) -> Bool {
guard let callOffset = call.offset,
let callLength = call.length,
let argumentOffset = argument.offset,
let argumentLength = argument.length else {
closingParenOffset: ByteCount) -> Bool {
guard let argOffset = argument.offset else {
return false
}

return callOffset + callLength == argumentOffset + argumentLength
return argOffset > closingParenOffset
}
}

private extension SourceKittenDictionary {
func closingParenLocation(file: SwiftLintFile) -> ByteCount? {
guard self.expressionKind == .call,
case let arguments = self.enclosedArguments,
arguments.isEmpty == false else {
return nil
}

func rangeBetween(_ expr1: SourceKittenDictionary, and expr2: SourceKittenDictionary) -> ByteRange? {
guard let offset1 = expr1.offset,
let length1 = expr1.length,
let offset2 = expr2.offset,
case let end1 = offset1 + length1,
end1 <= offset2 else {
return nil
}

return ByteRange(location: end1, length: offset2 - end1)
}

var searchRanges: [ByteRange] = []
for index in arguments.indices.dropLast() {
let currentArg = arguments[index]
let nextArg = arguments[index + 1]
if let range = rangeBetween(currentArg, and: nextArg) {
searchRanges.append(range)
}
}

if let lastOffset = arguments.last?.offset,
let lastLength = arguments.last?.length,
let callOffset = self.offset,
let callLength = self.length,
case let lastEnd = lastOffset + lastLength,
case let callEnd = callOffset + callLength,
lastEnd <= callEnd {
searchRanges.append(ByteRange(location: lastEnd, length: callEnd - lastEnd))
}

for byteRange in searchRanges {
if let range = file.stringView.byteRangeToNSRange(byteRange),
let match = regex("^\\s*\\)").firstMatch(in: file.contents, options: [], range: range)?.range {
return file.stringView.byteOffset(fromLocation: match.location)
}
}

return nil
}
}

Expand Down