Lift return out: do not report for zero returns
Also, do not highlight (but suggest fix) for exactly one return Related to KT-14900
This commit is contained in:
@@ -31,15 +31,18 @@ class LiftReturnOrAssignmentInspection : AbstractKotlinInspection() {
|
|||||||
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession) =
|
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession) =
|
||||||
object : KtVisitorVoid() {
|
object : KtVisitorVoid() {
|
||||||
private fun visitIfOrWhen(expression: KtExpression, keyword: PsiElement) {
|
private fun visitIfOrWhen(expression: KtExpression, keyword: PsiElement) {
|
||||||
if (BranchedFoldingUtils.canFoldToReturn(expression)) {
|
val returnNumber = BranchedFoldingUtils.getFoldableReturnNumber(expression)
|
||||||
|
if (returnNumber > 0) {
|
||||||
holder.registerProblem(
|
holder.registerProblem(
|
||||||
keyword,
|
keyword,
|
||||||
"Return can be lifted out of '${keyword.text}'",
|
"Return can be lifted out of '${keyword.text}'",
|
||||||
ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
|
if (returnNumber > 1) ProblemHighlightType.GENERIC_ERROR_OR_WARNING
|
||||||
|
else ProblemHighlightType.INFORMATION,
|
||||||
LiftReturnOutFix(keyword.text)
|
LiftReturnOutFix(keyword.text)
|
||||||
)
|
)
|
||||||
|
return
|
||||||
}
|
}
|
||||||
else if (BranchedFoldingUtils.canFoldToAssignment(expression)) {
|
if (BranchedFoldingUtils.canFoldToAssignment(expression)) {
|
||||||
holder.registerProblem(
|
holder.registerProblem(
|
||||||
keyword,
|
keyword,
|
||||||
"Assignment can be lifted out of '${keyword.text}'",
|
"Assignment can be lifted out of '${keyword.text}'",
|
||||||
|
|||||||
+27
-12
@@ -82,30 +82,45 @@ object BranchedFoldingUtils {
|
|||||||
return assignments.all { BranchedFoldingUtils.checkAssignmentsMatch(it, firstAssignment) }
|
return assignments.all { BranchedFoldingUtils.checkAssignmentsMatch(it, firstAssignment) }
|
||||||
}
|
}
|
||||||
|
|
||||||
fun canFoldToReturn(expression: KtExpression?): Boolean = when (expression) {
|
private fun getFoldableReturnNumber(branches: List<KtExpression?>) =
|
||||||
|
branches.fold(0) { prevNumber, branch ->
|
||||||
|
when {
|
||||||
|
prevNumber == -1 -> -1
|
||||||
|
getFoldableBranchedReturn(branch) != null -> prevNumber + 1
|
||||||
|
else -> {
|
||||||
|
val currNumber = getFoldableReturnNumber(branch?.lastBlockStatementOrThis())
|
||||||
|
if (currNumber == -1) -1 else prevNumber + currNumber
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
internal fun getFoldableReturnNumber(expression: KtExpression?): Int = when (expression) {
|
||||||
is KtWhenExpression -> {
|
is KtWhenExpression -> {
|
||||||
val entries = expression.entries
|
val entries = expression.entries
|
||||||
KtPsiUtil.checkWhenExpressionHasSingleElse(expression) &&
|
when {
|
||||||
entries.isNotEmpty() &&
|
!KtPsiUtil.checkWhenExpressionHasSingleElse(expression) -> -1
|
||||||
entries.all { entry ->
|
entries.isEmpty() -> -1
|
||||||
getFoldableBranchedReturn(entry.expression) != null || canFoldToReturn(entry.expression?.lastBlockStatementOrThis())
|
else -> getFoldableReturnNumber(entries.map { it.expression })
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
is KtIfExpression -> {
|
is KtIfExpression -> {
|
||||||
val branches = expression.branches
|
val branches = expression.branches
|
||||||
branches.isNotEmpty() &&
|
when {
|
||||||
(branches.lastOrNull()?.getStrictParentOfType<KtIfExpression>()?.`else` != null) &&
|
branches.isEmpty() -> -1
|
||||||
branches.all { branch ->
|
branches.lastOrNull()?.getStrictParentOfType<KtIfExpression>()?.`else` == null -> -1
|
||||||
getFoldableBranchedReturn(branch) != null || canFoldToReturn(branch?.lastBlockStatementOrThis())
|
else -> getFoldableReturnNumber(branches)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
is KtCallExpression -> {
|
is KtCallExpression -> {
|
||||||
expression.analyze().getType(expression)?.isNothing() ?: false
|
if (expression.analyze().getType(expression)?.isNothing() == true) 0 else -1
|
||||||
}
|
}
|
||||||
is KtBreakExpression, is KtContinueExpression, is KtThrowExpression -> true
|
is KtBreakExpression, is KtContinueExpression, is KtThrowExpression -> 0
|
||||||
else -> false
|
else -> -1
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fun canFoldToReturn(expression: KtExpression?): Boolean =
|
||||||
|
getFoldableReturnNumber(expression) > 0
|
||||||
|
|
||||||
fun foldToAssignment(expression: KtExpression) {
|
fun foldToAssignment(expression: KtExpression) {
|
||||||
var lhs: KtExpression? = null
|
var lhs: KtExpression? = null
|
||||||
var op: String? = null
|
var op: String? = null
|
||||||
|
|||||||
@@ -1,3 +1,5 @@
|
|||||||
|
// HIGHLIGHT: GENERIC_ERROR_OR_WARNING
|
||||||
|
|
||||||
fun foo(): Int {
|
fun foo(): Int {
|
||||||
loop@ while (true) {
|
loop@ while (true) {
|
||||||
<caret>when (1) {
|
<caret>when (1) {
|
||||||
|
|||||||
@@ -1,3 +1,5 @@
|
|||||||
|
// HIGHLIGHT: GENERIC_ERROR_OR_WARNING
|
||||||
|
|
||||||
fun foo(): Int {
|
fun foo(): Int {
|
||||||
loop@ while (true) {
|
loop@ while (true) {
|
||||||
return when (1) {
|
return when (1) {
|
||||||
|
|||||||
@@ -0,0 +1,8 @@
|
|||||||
|
// HIGHLIGHT: INFORMATION
|
||||||
|
|
||||||
|
fun foo(arg: Int): Int {
|
||||||
|
<caret>when (arg) {
|
||||||
|
0 -> return 0
|
||||||
|
else -> throw Exception()
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,8 @@
|
|||||||
|
// HIGHLIGHT: INFORMATION
|
||||||
|
|
||||||
|
fun foo(arg: Int): Int {
|
||||||
|
return when (arg) {
|
||||||
|
0 -> 0
|
||||||
|
else -> throw Exception()
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,7 @@
|
|||||||
|
// PROBLEM: none
|
||||||
|
|
||||||
|
fun foo(): Int {
|
||||||
|
<caret>when {
|
||||||
|
else -> throw Exception()
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -636,6 +636,18 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
|
|||||||
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/liftOut/whenToReturn/simpleWhenWithBlocks.kt");
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/liftOut/whenToReturn/simpleWhenWithBlocks.kt");
|
||||||
doTest(fileName);
|
doTest(fileName);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("whenOneReturn.kt")
|
||||||
|
public void testWhenOneReturn() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/liftOut/whenToReturn/whenOneReturn.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
|
|
||||||
|
@TestMetadata("whenThrowOnly.kt")
|
||||||
|
public void testWhenThrowOnly() throws Exception {
|
||||||
|
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspectionsLocal/liftOut/whenToReturn/whenThrowOnly.kt");
|
||||||
|
doTest(fileName);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user