SwiftLint
SwiftLint copied to clipboard
realm
opening_braces allows some multiline if statements, others get flagged
New Issue Checklist
- [x] I've Updated SwiftLint to the latest version.
- [x] I've searched for existing GitHub issues.
Bug Description
Related issues: #5521, #5602
Some multiline if statements with a starting brace don't trigger an error, but more complicated lines do. For instance no warning:
// Instance variables
var foo = 5
var goo = 7
if
self.foo == 9,
self.goo == 10
{
print("SUCCESS")
} else {
fatalError()
}
More usual code triggers an opening_brace error:
if
let rootVC = AppDelegate.myDelegate.window?.rootViewController as? XYZ,
let abcVC = rootVC.myArray.first as? ABC
{
...
} else {
fatalError()
}
Environment
- SwiftLint version 0.57.1
- Xcode version 16.2
- Installation method used Swift Package
- Configuration file:
# Case-sensitive paths to include during linting. Directory paths supplied on the
# command line will be ignored.
included:
- abc
- def
excluded: # case-sensitive paths to ignore during linting. Takes precedence over `included`
- 1111
# By default, SwiftLint uses a set of sensible default rules you can adjust:
disabled_rules: # rule identifiers turned on by default to exclude from running
- trailing_whitespace
- line_length
- force_cast
- cyclomatic_complexity
- function_body_length
- identifier_name
- file_length
- function_parameter_count
- large_tuple
- type_name
- type_body_length
- todo
- no_fallthrough_only
- static_over_final_class
- inclusive_language
- legacy_random
- nesting
- nsobject_prefer_isequal
- force_try
- closure_parameter_position
opt_in_rules: # some rules are turned off by default, so you need to opt-in
- empty_count # find all the available rules by running: `swiftlint rules`
analyzer_rules: # rules run by `swiftlint analyze`
- explicit_self
# If true, SwiftLint will not fail if no lintable files are found.
allow_zero_lintable_files: false
# If true, SwiftLint will treat all warnings as errors.
strict: false
# If true, SwiftLint will treat all errors as warnings.
lenient: false
# If true, SwiftLint will check for updates after linting or analyzing.
check_for_updates: true
reporter: "xcode" # reporter type (xcode, json, csv, checkstyle, codeclimate, junit, html, emoji, sonarqube, markdown, github-actions-logging, summary)
colon:
severity: error
comma:
severity: error
control_statement:
severity: error
trailing_comma:
severity: error
mandatory_comma: true
Are you using nested configurations? If so, paste their relative paths and respective contents. N/A
I'm unable to reproduce this issue. Both examples trigger with version 0.57.1 on my side. With the option ignore_multiline_statement_conditions set to true there's no violation. Please double check.
I'm unable to reproduce this issue. Both examples trigger with version 0.57.1 on my side. With the option
ignore_multiline_statement_conditionsset totruethere's no violation. Please double check.
You are correct. I tried to create a simple test file but in that case every attempt generates the error. While I had read of that new rule you mentioned, it's not in the documentation file, so I was unsure it was actually implemented (of course today on a deep search I see it added to the announcement of 0.57.0).
Can you re-use this ticket so that it gets added to https://realm.github.io/SwiftLint/rule-directory.html? If so I'll edit the title.
It's not a rule but an option of
opening_brace. See its documentation.
Documentation for those options would be super helpful, but again understand this is a volunteer effort.
Thank you for responses here and sorry that I wasted your time with the initial issue. I thought I had verified the one non-complain but obviously I had not.
Adding documentation for the various options of all rules is a long term effort, but definitely desired.
At the moment, though, there's no infrastructure prepared to do that.
Adding documentation for the various options of all rules is a long term effort, but definitely desired.
At the moment, though, there's no infrastructure prepared to do that.
It could get added to the page on open_braces itself. I looked but there is no way to contribute to the project. I'd gladly fund someone to add additional documentation on all those opens to open_brace, as this issue I reported has generated a lot of discussions in my company.
What we would really like is a way to only allow the brace to appear after the predictates on its own line. Turning on ignore_multiline_statement_conditions opens the door for much more.
Would it be unreasonable to request "allows_single_brace_on_its_own_line_for_multiline_predicates"? Could we fund that effort?
Adding documentation for the various options of all rules is a long term effort, but definitely desired. At the moment, though, there's no infrastructure prepared to do that.
It could get added to the page on open_braces itself. I looked but there is no way to contribute to the project. I'd gladly fund someone to add additional documentation on all those opens to open_brace, as this issue I reported has generated a lot of discussions in my company.
The documentation is fully auto-generated. One cannot just add manual comments afterwards. That's why I mentioned the need for the right infrastructure first.
What we would really like is a way to only allow the brace to appear after the predictates on its own line. Turning on ignore_multiline_statement_conditions opens the door for much more.
Would it be unreasonable to request "allows_single_brace_on_its_own_line_for_multiline_predicates"? Could we fund that effort?
What would be the difference between ignore_multiline_statement_conditions and allows_single_brace_on_its_own_line_for_multiline_predicates? Is contrasted_opening_brace of any help?
Again, thank you so much for engaging with me in this ticket!
The contrasted_opening_brace is absolutely not what we want! Our coding style should allow: if a { do something }
and disallow
if a { do something }
The sole reason for ignore_multiline_statement_conditions is that we want he brace on a new line ONLY if there are more than one conditions in the for loop:
if a, b { do something }
and probably
if a, b { do something }
Our issue with ignore_multiline_statement_conditions is we don't understand what other side effects setting this does. Will it let developers do crazy things if there multiline conditionals and a brace?
I didn't try but guess I could, is this now allowed
if a, b, { do something }
Even the google style guide requires that open and close braces be at the same tab stop if the are preceded by a multiline conditional.
So your point is that you don't just want to ignore braces after multiline conditions but enforce them to be on their own line. Correct?
So your point is that you don't just want to ignore braces after multiline conditions but enforce them to be on their own line. Correct?
Yes, but more - they must vertically align with the "if" statement.
So your point is that you don't just want to ignore braces after multiline conditions but enforce them to be on their own line. Correct?
Yes, but more - they must vertically align with the "if" statement.
That's clear, I think. What I'm not quite sure about is how to support the enforcement of a new line for multiline constructs.
For functions, types and statements, there're those ignore_multiline_* options, which could be made 3-fold to support on, off and enforce for instance. Not very intuitive due to the "ignore" which implies a switch, but not be too hard to implement, actually.
An integration into contrasted_opening_brace doesn't seem reasonable. nor does a totally new rule.