diff --git a/compiler/psi/src/org/jetbrains/kotlin/psi/VisitorWrappers.kt b/compiler/psi/src/org/jetbrains/kotlin/psi/VisitorWrappers.kt index 0e5b8f4f04d..4039a4a1394 100644 --- a/compiler/psi/src/org/jetbrains/kotlin/psi/VisitorWrappers.kt +++ b/compiler/psi/src/org/jetbrains/kotlin/psi/VisitorWrappers.kt @@ -260,6 +260,13 @@ fun binaryExpressionVisitor(block: (KtBinaryExpression) -> Unit) = } } +fun binaryWithTypeRHSExpressionVisitor(block: (KtBinaryExpressionWithTypeRHS) -> Unit) = + object : KtVisitorVoid() { + override fun visitBinaryWithTypeRHSExpression(expression: KtBinaryExpressionWithTypeRHS) { + block(expression) + } + } + fun binaryExpressionRecursiveVisitor(block: (KtBinaryExpression) -> Unit) = object : KtTreeVisitorVoid() { override fun visitBinaryExpression(binaryExpression: KtBinaryExpression) { diff --git a/idea/resources/inspectionDescriptions/SafeCastWithReturn.html b/idea/resources/inspectionDescriptions/SafeCastWithReturn.html new file mode 100644 index 00000000000..ac77487c3eb --- /dev/null +++ b/idea/resources/inspectionDescriptions/SafeCastWithReturn.html @@ -0,0 +1,11 @@ + + +This inspection reports safe cast with 'return' should be replaced with 'if' type check. For example: +

+
+fun test(x: Any) {
+    x as? String ?: return // Should be replaced with 'if (x !is String) return'
+}
+
+ + \ No newline at end of file diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index aa31e7b761b..87b7e622def 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -3048,6 +3048,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio. language="kotlin" /> + + diff --git a/idea/src/META-INF/plugin.xml.173 b/idea/src/META-INF/plugin.xml.173 index 27d6c8f455d..047fd3ab3d8 100644 --- a/idea/src/META-INF/plugin.xml.173 +++ b/idea/src/META-INF/plugin.xml.173 @@ -3046,6 +3046,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio. language="kotlin" /> + + diff --git a/idea/src/META-INF/plugin.xml.181 b/idea/src/META-INF/plugin.xml.181 index dfdd3b736b1..419849cbf49 100644 --- a/idea/src/META-INF/plugin.xml.181 +++ b/idea/src/META-INF/plugin.xml.181 @@ -3047,6 +3047,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio. language="kotlin" /> + + diff --git a/idea/src/META-INF/plugin.xml.183 b/idea/src/META-INF/plugin.xml.183 index 3af9fba0e56..aacafb0eeb9 100644 --- a/idea/src/META-INF/plugin.xml.183 +++ b/idea/src/META-INF/plugin.xml.183 @@ -3048,6 +3048,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio. language="kotlin" /> + + diff --git a/idea/src/META-INF/plugin.xml.as31 b/idea/src/META-INF/plugin.xml.as31 index 5c3c95f81b0..ee1da50bb6b 100644 --- a/idea/src/META-INF/plugin.xml.as31 +++ b/idea/src/META-INF/plugin.xml.as31 @@ -3046,6 +3046,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio. language="kotlin" /> + + diff --git a/idea/src/META-INF/plugin.xml.as32 b/idea/src/META-INF/plugin.xml.as32 index a987366d62e..c0b87ae8d18 100644 --- a/idea/src/META-INF/plugin.xml.as32 +++ b/idea/src/META-INF/plugin.xml.as32 @@ -3046,6 +3046,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio. language="kotlin" /> + + diff --git a/idea/src/META-INF/plugin.xml.as33 b/idea/src/META-INF/plugin.xml.as33 index b988fa76aa8..2d429c733cb 100644 --- a/idea/src/META-INF/plugin.xml.as33 +++ b/idea/src/META-INF/plugin.xml.as33 @@ -3048,6 +3048,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio. language="kotlin" /> + + diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/SafeCastWithReturnInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/SafeCastWithReturnInspection.kt new file mode 100644 index 00000000000..db6ce95fff4 --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/SafeCastWithReturnInspection.kt @@ -0,0 +1,64 @@ +/* + * Copyright 2010-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license + * that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.idea.inspections + +import com.intellij.codeInspection.LocalQuickFix +import com.intellij.codeInspection.ProblemDescriptor +import com.intellij.codeInspection.ProblemHighlightType +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.openapi.project.Project +import com.intellij.psi.PsiElementVisitor +import org.jetbrains.kotlin.diagnostics.Errors +import org.jetbrains.kotlin.idea.caches.resolve.analyze +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode + +class SafeCastWithReturnInspection : AbstractKotlinInspection() { + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor = + binaryWithTypeRHSExpressionVisitor(fun(expression) { + if (expression.right == null) return + if (expression.operationReference.getReferencedName() != "as?") return + + val parent = expression.getStrictParentOfType() ?: return + if (KtPsiUtil.deparenthesize(parent.left) != expression) return + if (parent.operationReference.getReferencedName() != "?:") return + if (KtPsiUtil.deparenthesize(parent.right) !is KtReturnExpression) return + + val context = expression.analyze(BodyResolveMode.PARTIAL_WITH_DIAGNOSTICS) + if (context[BindingContext.USED_AS_EXPRESSION, parent] == true) return + if (context.diagnostics.forElement(expression.operationReference).any { it.factory == Errors.CAST_NEVER_SUCCEEDS }) return + + holder.registerProblem( + parent, + "Should be replaced with 'if' type check", + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + ReplaceWithIfFix() + ) + }) +} + +private class ReplaceWithIfFix : LocalQuickFix { + override fun getName() = "Replace with 'if' type check" + + override fun getFamilyName() = name + + override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + val elvisExpression = descriptor.psiElement as? KtBinaryExpression ?: return + val returnExpression = KtPsiUtil.deparenthesize(elvisExpression.right) ?: return + val safeCastExpression = KtPsiUtil.deparenthesize(elvisExpression.left) as? KtBinaryExpressionWithTypeRHS ?: return + val typeReference = safeCastExpression.right ?: return + elvisExpression.replace( + KtPsiFactory(elvisExpression).createExpressionByPattern( + "if ($0 !is $1) $2", + safeCastExpression.left, + typeReference, + returnExpression + ) + ) + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/safeCastWithReturn/.inspection b/idea/testData/inspectionsLocal/safeCastWithReturn/.inspection new file mode 100644 index 00000000000..ae66589e64c --- /dev/null +++ b/idea/testData/inspectionsLocal/safeCastWithReturn/.inspection @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.inspections.SafeCastWithReturnInspection diff --git a/idea/testData/inspectionsLocal/safeCastWithReturn/castNeverSucceeds.kt b/idea/testData/inspectionsLocal/safeCastWithReturn/castNeverSucceeds.kt new file mode 100644 index 00000000000..66fcaf42de7 --- /dev/null +++ b/idea/testData/inspectionsLocal/safeCastWithReturn/castNeverSucceeds.kt @@ -0,0 +1,4 @@ +// PROBLEM: none +fun test(x: Int) { + x as? String ?: return +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/safeCastWithReturn/hasParenthesize.kt b/idea/testData/inspectionsLocal/safeCastWithReturn/hasParenthesize.kt new file mode 100644 index 00000000000..61aacd6b868 --- /dev/null +++ b/idea/testData/inspectionsLocal/safeCastWithReturn/hasParenthesize.kt @@ -0,0 +1,6 @@ +fun test(x: Any): Int? { + (x as? String) ?: (return null) + return foo(x) +} + +fun foo(x: String) = 1 \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/safeCastWithReturn/hasParenthesize.kt.after b/idea/testData/inspectionsLocal/safeCastWithReturn/hasParenthesize.kt.after new file mode 100644 index 00000000000..525f2a2497c --- /dev/null +++ b/idea/testData/inspectionsLocal/safeCastWithReturn/hasParenthesize.kt.after @@ -0,0 +1,6 @@ +fun test(x: Any): Int? { + if (x !is String) return null + return foo(x) +} + +fun foo(x: String) = 1 \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/safeCastWithReturn/labeledReturn.kt b/idea/testData/inspectionsLocal/safeCastWithReturn/labeledReturn.kt new file mode 100644 index 00000000000..a039fc38005 --- /dev/null +++ b/idea/testData/inspectionsLocal/safeCastWithReturn/labeledReturn.kt @@ -0,0 +1,9 @@ +// WITH_RUNTIME +fun test(list: List) { + list.mapNotNull { + it as? String ?: return@mapNotNull null + foo(it) + } +} + +fun foo(x: String) = 1 \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/safeCastWithReturn/labeledReturn.kt.after b/idea/testData/inspectionsLocal/safeCastWithReturn/labeledReturn.kt.after new file mode 100644 index 00000000000..d8195ad2ea9 --- /dev/null +++ b/idea/testData/inspectionsLocal/safeCastWithReturn/labeledReturn.kt.after @@ -0,0 +1,9 @@ +// WITH_RUNTIME +fun test(list: List) { + list.mapNotNull { + if (it !is String) return@mapNotNull null + foo(it) + } +} + +fun foo(x: String) = 1 \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/safeCastWithReturn/simple.kt b/idea/testData/inspectionsLocal/safeCastWithReturn/simple.kt new file mode 100644 index 00000000000..a7cbdbc1fe9 --- /dev/null +++ b/idea/testData/inspectionsLocal/safeCastWithReturn/simple.kt @@ -0,0 +1,6 @@ +fun test(x: Any): Int? { + x as? String ?: return null + return foo(x) +} + +fun foo(x: String) = 1 \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/safeCastWithReturn/simple.kt.after b/idea/testData/inspectionsLocal/safeCastWithReturn/simple.kt.after new file mode 100644 index 00000000000..525f2a2497c --- /dev/null +++ b/idea/testData/inspectionsLocal/safeCastWithReturn/simple.kt.after @@ -0,0 +1,6 @@ +fun test(x: Any): Int? { + if (x !is String) return null + return foo(x) +} + +fun foo(x: String) = 1 \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/safeCastWithReturn/usedAsExpression.kt b/idea/testData/inspectionsLocal/safeCastWithReturn/usedAsExpression.kt new file mode 100644 index 00000000000..156918fe5d7 --- /dev/null +++ b/idea/testData/inspectionsLocal/safeCastWithReturn/usedAsExpression.kt @@ -0,0 +1,7 @@ +// PROBLEM: none +fun test(x: Any): Int? { + val s = x as? String ?: return null + return foo(s) +} + +fun foo(x: String) = 1 \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index 7f4926cbcb6..2baa377b849 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -5295,6 +5295,44 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { } } + @TestMetadata("idea/testData/inspectionsLocal/safeCastWithReturn") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class SafeCastWithReturn extends AbstractLocalInspectionTest { + private void runTest(String testDataFilePath) throws Exception { + KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath); + } + + public void testAllFilesPresentInSafeCastWithReturn() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/safeCastWithReturn"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true); + } + + @TestMetadata("castNeverSucceeds.kt") + public void testCastNeverSucceeds() throws Exception { + runTest("idea/testData/inspectionsLocal/safeCastWithReturn/castNeverSucceeds.kt"); + } + + @TestMetadata("hasParenthesize.kt") + public void testHasParenthesize() throws Exception { + runTest("idea/testData/inspectionsLocal/safeCastWithReturn/hasParenthesize.kt"); + } + + @TestMetadata("labeledReturn.kt") + public void testLabeledReturn() throws Exception { + runTest("idea/testData/inspectionsLocal/safeCastWithReturn/labeledReturn.kt"); + } + + @TestMetadata("simple.kt") + public void testSimple() throws Exception { + runTest("idea/testData/inspectionsLocal/safeCastWithReturn/simple.kt"); + } + + @TestMetadata("usedAsExpression.kt") + public void testUsedAsExpression() throws Exception { + runTest("idea/testData/inspectionsLocal/safeCastWithReturn/usedAsExpression.kt"); + } + } + @TestMetadata("idea/testData/inspectionsLocal/scopeFunctions") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)