Fix void_function_in_ternary incorrectly triggered with if statement#5674
Fix void_function_in_ternary incorrectly triggered with if statement#5674yuya-okuse wants to merge 19 commits intorealm:mainfrom
Conversation
Here's an example of your CHANGELOG entry: * Fix void_function_in_ternary incorrectly triggered with if statement.
[u-abyss](https://github.com/u-abyss)
[#issue_number](https://github.com/realm/SwiftLint/issues/issue_number)note: There are two invisible spaces after the entry's text. Generated by 🚫 Danger |
529c862 to
337358f
Compare
SimplyDanny
left a comment
There was a problem hiding this comment.
Thanks taking up the issue!
The current implementation is very specific to implicit returns. However, if and switch expression can also appear as variable initializers and they can be nested into each other. I don't think the current solution addresses these cases, does it?
Source/SwiftLintBuiltInRules/Rules/Idiomatic/VoidFunctionInTernaryConditionRule.swift
Show resolved
Hide resolved
fcae9a6 to
af9a0ca
Compare
|
@SimplyDanny |
Can you provide an example? The code func f(i: Int) -> Int {
if i > 1 {
let j = i - 1
j
} else {
0
}
}wouldn't compile. There you need the |
| if true { | ||
| if true { | ||
| isTrue ↓? defaultValue() : defaultValue() | ||
| retun "True" |
There was a problem hiding this comment.
| retun "True" | |
| return "True" |
| return targetSyntax | ||
| } | ||
| if let ifExprSyntax = targetSyntax?.as(IfExprSyntax.self) { | ||
| if ifExprSyntax.body.statements.last != codeBlockItem { |
There was a problem hiding this comment.
What about else blocks?
| // For codeBlockItem, recursively traverse it to determine if it is within its own FunctionDeclSyntax. | ||
| func getFunctionDeclSyntax(codeBlockItem: CodeBlockItemSyntax) -> FunctionDeclSyntax? { | ||
| let targetSyntax = codeBlockItem.parent?.parent?.parent?.parent | ||
| if let targetSyntax = targetSyntax?.as(FunctionDeclSyntax.self) { |
There was a problem hiding this comment.
This covers only functions, but there are also the other cases which are checked above in the is...Return properties.
| func exampleNestedIfExprAndSwitchExpr(index: Int) -> String { | ||
| if index <= 3 { | ||
| switch index { | ||
| case 1: | ||
| if isTrue { | ||
| return isTrue ? "1" : "2" | ||
| } else { | ||
| return "3" | ||
| } | ||
| case 2: | ||
| if isTrue { | ||
| return "4" | ||
| } else { | ||
| return "5" | ||
| } | ||
| default: | ||
| return "6" | ||
| } | ||
| } else { | ||
| return "7" | ||
| } | ||
| } |
There was a problem hiding this comment.
This code can be refactored and nested if-else can be removed.
| func exampleNestedIfExprAndSwitchExpr(index: Int) -> String { | |
| if index <= 3 { | |
| switch index { | |
| case 1: | |
| if isTrue { | |
| return isTrue ? "1" : "2" | |
| } else { | |
| return "3" | |
| } | |
| case 2: | |
| if isTrue { | |
| return "4" | |
| } else { | |
| return "5" | |
| } | |
| default: | |
| return "6" | |
| } | |
| } else { | |
| return "7" | |
| } | |
| } | |
| func exampleNestedIfExprAndSwitchExpr(index: Int, isTrue: Bool) -> String { | |
| guard index <= 3 else { | |
| return "7" | |
| } | |
| switch index { | |
| case 1: | |
| return isTrue ? "1" : "3" | |
| case 2: | |
| return isTrue ? "4" : "5" | |
| default: | |
| return "6" | |
| } | |
| } |
Fixes #5611
This PR corrects that VoidFunctionInTernaryConditionRule incorrectly triggered with if statement.
This new rule is triggered or not depending on the return type of the function.