-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
multiline_arguments_brackets reports false positives with calls whose one parameter spans multiple lines #3167
Comments
I noticed that there's a similar problem with
|
I believe I've figured out a good way to fix this - we're currently assuming the call has newlines if any newlines exist in the body, when in fact the correct way is to check that the number of newlines in the body is more than the number of combined newlines in the actual argument bodies. Will try and get a pull request up for this soon. |
…nd multiline_arguments_brackets
…kets and multiline_arguments_brackets
Put up a PR over at #3171, happy to discuss. Thanks! |
…kets and multiline_arguments_brackets
…ltiline_arguments_brackets (#3171) * Issue #3167: Fixes false positives for multiline_parameters_brackets and multiline_arguments_brackets * Add changelog entry * Fix trailing comma in MultilineParametersBracketsRule nontriggering examples * Remove header comments from ExtendedStringTests.swift Co-authored-by: Paul Taykalo <ptaykalo@macpaw.com>
Fixed in #3171 |
New Issue Checklist
Describe the bug
The
multiline_arguments_brackets
rule produces what I would categorize as a false positive in calls which have a single argument with a multiline value. For example,multiline_arguments_brackets
is triggered on the following examples:I'm not 100% sure that this is not intentional, but it seems to me that these cases should pass the "smell test" of multiline_arguments_brackets, since the idea with this rule is to prevent argument brackets from being mis-aligned with the actual passed arguments. I think that if
return SignalProducer({ observer, _ in observer.sendCompleted() })
passes the rule, thenshould also pass the rule.
None of these cases are noted in the triggering or non-triggering examples at https://realm.github.io/SwiftLint/multiline_arguments_brackets.html, though a few non-triggering examples are similar:
I'd further argue that the currently triggering cases I listed above match the style of these non-triggering cases.
I'm looking for feedback here on whether this is intentional, and if it seems to the maintainers that the rule should cover these cases then I'm happy to take a crack at implementing it 😄
Environment
SwiftLint version (run
swiftlint version
to be sure)?0.39.1
Installation method used (Homebrew, CocoaPods, building from source, etc)? Cocoapods
Paste your configuration file: I can provide a full configuration file if necessary, but this should be fairly easy to reproduce
Are you using nested configurations? No
Which Xcode version are you using (check
xcodebuild -version
)?Xcode 11.2.1 Build version 11B500
Do you have a sample that shows the issue? Yes:
The above triggers multiline_arguments_brackets
The text was updated successfully, but these errors were encountered: