Edge cases for multiline_arguments_brackets when there are nested multiline calls
New Issue Checklist
- [x] Updated SwiftLint to the latest version
- [x] I searched for existing GitHub issues
Describe the bug
There are several cases we noticed in our codebase where Swiftlint seems to report warnings unexpectedly for the multiline_arguments_brackets rule. Though around 500 (amazing!) were resolved by #3167, we realized there are a few others as well.
Mainly, the code which triggers these warnings are of the form:
someFunction(someOtherFunction(
parameters
))
It seems to me that this type of code should still be valid in the "spirit" of this rule - there aren't any specific cases like this called out in the triggering examples, and all calls still have either a newline surrounding the parameters or no newlines surrounding the parameters, which seems to be what this rule is going for (it's also in line with my personal opinion). If anyone objects, I'd be happy to hear other thoughts on this.
Note: There may be some use in a rule which forces all parameters to be on newlines if the parameter itself contains a newline, i.e. that would emit a violation in the above code which would be fixed as:
someFunction(
someOtherFunction(
parameters
)
)
I think that should be an adjacent rule to multiline_arguments_brackets, though.
I've created a gist here of several other false positives, but I'll paste the content as well:
expect(issue.createdAt).to(equal(
Helper.date(["$date": NSNumber(value: value1)])
))
expect(followers).to(containExactUnorderedElements([
Follower(
followerUid: "a",
followerType: .group
),
Follower(
followerUid: "b",
followerType: .user
),
]))
expect(
Table.statement(forSpecialFTSCommand: "rebuild").prepare().0
).to(equal(
"INSERT INTO table(fts_index) VALUES('rebuild')"
))
expect(changes.filter {
guard case let .delete(_, type) = $0 else { return false }
return modelType is SetModel.Type
}).to(haveCount(1))
expect(a?.uids).to(equal(
[
"07262910-DF29-465C-9DE8-2FB81BA50938",
"B4297AB8-6766-4EAA-BF95-F9CF4B95C7AA",
"0665EF65-3DA8-464E-B49C-B88F9FD1AED3",
]
))
expect(uidsAndTags).to(containExactUnorderedElements([
("0c40e99f-4391-41f9-b91a-3ce389694264", "asdfasdf"),
("bae31c74-e647-4a33-9eb9-9980c81aaf8f", "fdsaadfs"),
("c40c7d92-18fa-4fb9-b73b-d2435859f31f", "dfssdfad"),
("aaeba695-9246-4cd6-9920-1c8616a05cbc", "dssdfadd"),
("238cd085-7c89-4704-b171-67a0b32dbdd5", "adsfafsd"),
].map(TwoTuple.init)))
try context.context.update(Update(
Table.self,
[(Table.Columns.nextUid, nil)],
whereClause: Table.Columns.next != nil
))
guard let maxResult = (try self.database.context.queryOne(
Statement(text: "SELECT MAX(ROWID) FROM \(Table.name)")
) { $0.currentRow[0] as Int64? }), let maxRowId = maxResult else {
// no data
return
}
guard let model = try self.queryOne(Select(
Table.self,
whereClause: (
Table.Columns.uid == Where.param(uid) &&
Table.Columns.project == Where.param(self.context.project)
)
)) else {
// Example
throw Errors.addingToNonexistent
}
models.append(Model(
modelUid: self.modelUid,
otherUid: key.stringValue,
tertiaryUid: metadata.uid,
quaternaryUid: metadata.uid,
status: Model.Status(rawValue: metadata.status ?? "") ?? .success
))
completion(.failure(.requestPreparationError(
request: URLRequest(url: self._requestUrl(for: project, service: service)),
error: error,
type: .unknown
)))
public func toDictionary<K, V>(_ transform: (Iterator.Element) -> (K, V)) -> [K: V] {
return [K: V](
self.map(transform)
) { _, last in last }
}
@objc public dynamic var customServices: [Service] {
return Array(self.records.values.sorted(by: { first, second -> Bool in
first.name < second.name
}))
}
Currently 24 multiline_arguments_brackets are emitted when linting this code, but I think in each case, the brackets and the arguments are actually aligned "correctly" (or at least, in a way that I think should not trigger the violation).
Complete output when running SwiftLint, including the stack trace and command used
Done linting! Found 33 violations, 0 serious in 1 file.
<nopath>:14:3: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:16:1: warning: Nimble Operator Violation: Prefer Nimble operator overloads over free matcher functions. (nimble_operator)
<nopath>:18:6: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:1:1: warning: Nimble Operator Violation: Prefer Nimble operator overloads over free matcher functions. (nimble_operator)
<nopath>:1:28: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:20:2: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:23:5: warning: Conditional Returns on Newline Violation: Conditional statements should always return on the next line (conditional_returns_on_newline)
<nopath>:25:2: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:27:1: warning: Nimble Operator Violation: Prefer Nimble operator overloads over free matcher functions. (nimble_operator)
<nopath>:27:20: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:33:2: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:35:24: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:35:54: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:3:2: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:41:21: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:41:22: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:43:28: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:47:2: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:51:32: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:56:37: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:5:22: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:62:2: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:67:15: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:73:2: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:75:12: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:79:3: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:81:8: warning: Missing Docs Violation: public declarations should be documented. (missing_docs)
<nopath>:82:5: warning: Implicit Return Violation: Prefer implicit returns in closures, functions and getters. (implicit_return)
<nopath>:84:25: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
<nopath>:87:22: warning: Missing Docs Violation: public declarations should be documented. (missing_docs)
<nopath>:88:18: warning: Trailing Closure Violation: Trailing closure syntax should be used whenever possible. (trailing_closure)
<nopath>:88:5: warning: Implicit Return Violation: Prefer implicit returns in closures, functions and getters. (implicit_return)
<nopath>:90:7: warning: Multiline Arguments Brackets Violation: Multiline arguments should have their surrounding brackets in a new line. (multiline_arguments_brackets)
Environment
-
SwiftLint version (run
swiftlint versionto be sure)?0.40.0 -
Installation method used (Homebrew, CocoaPods, building from source, etc)? Cocoapods
-
Are you using nested configurations? No
-
Which Xcode version are you using (check
xcodebuild -version)?Xcode 11.5 Build version 11E608c -
Do you have a sample that shows the issue?
swiftlint lint --path swiftlint-example.swift --no-cache --enable-all-rules.
For what it's worth, I think the current behavior of the rule is my personal preference, but I see how some might prefer allowing for functions with a single parameter could be opted out of the behavior as you're describing.
I don't consider these to be false positives, but would welcome any contributions you'd like to make to the rule that would support the pattern you've described.
I agree, since there's discussion about what's preferred here I don't think we can characterize them as false positives. I'll update the issue title to reflect that - most likely the way to go here is to add a configuration to this rule to opt into the described behavior. I'll try to see if I can look into that soon!
This issue has been automatically marked as stale because it has not had any recent activity. Please comment to prevent this issue from being closed. Thank you for your contributions!
I believe this is still something which I'd like to add a configuration parameter to this rule for. Hopefully should have time to do that soon
Unsure if it's related, but we're having an issue where by the same version of SwiftLint on two machines is reporting different error for multiline_arguments_brackets.
We've tested with 0.46.2 0.47.0 0.47.1, on one machine no reports are mentioned:
Done linting! Found 0 violations, 0 serious in 233 files.
on the other:
Done linting! Found 4 violations, 4 serious in 233 files.
Exact same codebase. The machine which suggests violations does not output the errors to console.
Could it be due to different xcode/swift versions?
swift-driver version: 1.45.2 Apple Swift version 5.6 (swiftlang-5.6.0.323.62 clang-1316.0.20.8)
Target: arm64-apple-macosx12.0
swift-driver version: 1.26.21 Apple Swift version 5.5.2 (swiftlang-1300.0.47.5 clang-1300.0.29.30)
Target: arm64-apple-macosx12.0
Apparently it was due to toolchain version. I suppose #3934 is relevant