Introduce inspection for safe cast + return #KT-26230 Fixed
This commit is contained in:
Toshiaki Kameyama
committed by
Mikhail Glukhikh
Mikhail Glukhikh
parent
2c71b3873e
commit
53c10238d6
@@ -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) =
|
fun binaryExpressionRecursiveVisitor(block: (KtBinaryExpression) -> Unit) =
|
||||||
object : KtTreeVisitorVoid() {
|
object : KtTreeVisitorVoid() {
|
||||||
override fun visitBinaryExpression(binaryExpression: KtBinaryExpression) {
|
override fun visitBinaryExpression(binaryExpression: KtBinaryExpression) {
|
||||||
|
|||||||
@@ -0,0 +1,11 @@
|
|||||||
|
<html>
|
||||||
|
<body>
|
||||||
|
This inspection reports safe cast with 'return' should be replaced with 'if' type check. For example:
|
||||||
|
<br /><br />
|
||||||
|
<pre>
|
||||||
|
fun test(x: Any) {
|
||||||
|
<b>x as? String ?: return</b> // Should be replaced with '<b>if (x !is String) return</b>'
|
||||||
|
}
|
||||||
|
</pre>
|
||||||
|
</body>
|
||||||
|
</html>
|
||||||
@@ -3048,6 +3048,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio.
|
|||||||
language="kotlin"
|
language="kotlin"
|
||||||
/>
|
/>
|
||||||
|
|
||||||
|
<localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.SafeCastWithReturnInspection"
|
||||||
|
displayName="Safe cast with 'return' should be replaced with 'if' type check"
|
||||||
|
groupPath="Kotlin"
|
||||||
|
groupName="Style issues"
|
||||||
|
enabledByDefault="true"
|
||||||
|
level="WEAK WARNING"
|
||||||
|
language="kotlin"
|
||||||
|
/>
|
||||||
|
|
||||||
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
||||||
|
|
||||||
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
||||||
|
|||||||
@@ -3046,6 +3046,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio.
|
|||||||
language="kotlin"
|
language="kotlin"
|
||||||
/>
|
/>
|
||||||
|
|
||||||
|
<localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.SafeCastWithReturnInspection"
|
||||||
|
displayName="Safe cast with 'return' should be replaced with 'if' type check"
|
||||||
|
groupPath="Kotlin"
|
||||||
|
groupName="Style issues"
|
||||||
|
enabledByDefault="true"
|
||||||
|
level="WEAK WARNING"
|
||||||
|
language="kotlin"
|
||||||
|
/>
|
||||||
|
|
||||||
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
||||||
|
|
||||||
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
||||||
|
|||||||
@@ -3047,6 +3047,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio.
|
|||||||
language="kotlin"
|
language="kotlin"
|
||||||
/>
|
/>
|
||||||
|
|
||||||
|
<localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.SafeCastWithReturnInspection"
|
||||||
|
displayName="Safe cast with 'return' should be replaced with 'if' type check"
|
||||||
|
groupPath="Kotlin"
|
||||||
|
groupName="Style issues"
|
||||||
|
enabledByDefault="true"
|
||||||
|
level="WEAK WARNING"
|
||||||
|
language="kotlin"
|
||||||
|
/>
|
||||||
|
|
||||||
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
||||||
|
|
||||||
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
||||||
|
|||||||
@@ -3048,6 +3048,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio.
|
|||||||
language="kotlin"
|
language="kotlin"
|
||||||
/>
|
/>
|
||||||
|
|
||||||
|
<localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.SafeCastWithReturnInspection"
|
||||||
|
displayName="Safe cast with 'return' should be replaced with 'if' type check"
|
||||||
|
groupPath="Kotlin"
|
||||||
|
groupName="Style issues"
|
||||||
|
enabledByDefault="true"
|
||||||
|
level="WEAK WARNING"
|
||||||
|
language="kotlin"
|
||||||
|
/>
|
||||||
|
|
||||||
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
||||||
|
|
||||||
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
||||||
|
|||||||
@@ -3046,6 +3046,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio.
|
|||||||
language="kotlin"
|
language="kotlin"
|
||||||
/>
|
/>
|
||||||
|
|
||||||
|
<localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.SafeCastWithReturnInspection"
|
||||||
|
displayName="Safe cast with 'return' should be replaced with 'if' type check"
|
||||||
|
groupPath="Kotlin"
|
||||||
|
groupName="Style issues"
|
||||||
|
enabledByDefault="true"
|
||||||
|
level="WEAK WARNING"
|
||||||
|
language="kotlin"
|
||||||
|
/>
|
||||||
|
|
||||||
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
||||||
|
|
||||||
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
||||||
|
|||||||
@@ -3046,6 +3046,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio.
|
|||||||
language="kotlin"
|
language="kotlin"
|
||||||
/>
|
/>
|
||||||
|
|
||||||
|
<localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.SafeCastWithReturnInspection"
|
||||||
|
displayName="Safe cast with 'return' should be replaced with 'if' type check"
|
||||||
|
groupPath="Kotlin"
|
||||||
|
groupName="Style issues"
|
||||||
|
enabledByDefault="true"
|
||||||
|
level="WEAK WARNING"
|
||||||
|
language="kotlin"
|
||||||
|
/>
|
||||||
|
|
||||||
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
||||||
|
|
||||||
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
||||||
|
|||||||
@@ -3048,6 +3048,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio.
|
|||||||
language="kotlin"
|
language="kotlin"
|
||||||
/>
|
/>
|
||||||
|
|
||||||
|
<localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.SafeCastWithReturnInspection"
|
||||||
|
displayName="Safe cast with 'return' should be replaced with 'if' type check"
|
||||||
|
groupPath="Kotlin"
|
||||||
|
groupName="Style issues"
|
||||||
|
enabledByDefault="true"
|
||||||
|
level="WEAK WARNING"
|
||||||
|
language="kotlin"
|
||||||
|
/>
|
||||||
|
|
||||||
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
|
||||||
|
|
||||||
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
|
||||||
|
|||||||
@@ -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<KtBinaryExpression>() ?: 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
|
||||||
|
)
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
org.jetbrains.kotlin.idea.inspections.SafeCastWithReturnInspection
|
||||||
@@ -0,0 +1,4 @@
|
|||||||
|
// PROBLEM: none
|
||||||
|
fun test(x: Int) {
|
||||||
|
<caret>x as? String ?: return
|
||||||
|
}
|
||||||
@@ -0,0 +1,6 @@
|
|||||||
|
fun test(x: Any): Int? {
|
||||||
|
<caret>(x as? String) ?: (return null)
|
||||||
|
return foo(x)
|
||||||
|
}
|
||||||
|
|
||||||
|
fun foo(x: String) = 1
|
||||||
@@ -0,0 +1,6 @@
|
|||||||
|
fun test(x: Any): Int? {
|
||||||
|
if (x !is String) return null
|
||||||
|
return foo(x)
|
||||||
|
}
|
||||||
|
|
||||||
|
fun foo(x: String) = 1
|
||||||
@@ -0,0 +1,9 @@
|
|||||||
|
// WITH_RUNTIME
|
||||||
|
fun test(list: List<Any>) {
|
||||||
|
list.mapNotNull {
|
||||||
|
it as? String ?: return@mapNotNull null<caret>
|
||||||
|
foo(it)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fun foo(x: String) = 1
|
||||||
@@ -0,0 +1,9 @@
|
|||||||
|
// WITH_RUNTIME
|
||||||
|
fun test(list: List<Any>) {
|
||||||
|
list.mapNotNull {
|
||||||
|
if (it !is String) return@mapNotNull null
|
||||||
|
foo(it)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fun foo(x: String) = 1
|
||||||
@@ -0,0 +1,6 @@
|
|||||||
|
fun test(x: Any): Int? {
|
||||||
|
<caret>x as? String ?: return null
|
||||||
|
return foo(x)
|
||||||
|
}
|
||||||
|
|
||||||
|
fun foo(x: String) = 1
|
||||||
@@ -0,0 +1,6 @@
|
|||||||
|
fun test(x: Any): Int? {
|
||||||
|
if (x !is String) return null
|
||||||
|
return foo(x)
|
||||||
|
}
|
||||||
|
|
||||||
|
fun foo(x: String) = 1
|
||||||
@@ -0,0 +1,7 @@
|
|||||||
|
// PROBLEM: none
|
||||||
|
fun test(x: Any): Int? {
|
||||||
|
val s = <caret>x as? String ?: return null
|
||||||
|
return foo(s)
|
||||||
|
}
|
||||||
|
|
||||||
|
fun foo(x: String) = 1
|
||||||
+38
@@ -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")
|
@TestMetadata("idea/testData/inspectionsLocal/scopeFunctions")
|
||||||
@TestDataPath("$PROJECT_ROOT")
|
@TestDataPath("$PROJECT_ROOT")
|
||||||
@RunWith(JUnit3RunnerWithInners.class)
|
@RunWith(JUnit3RunnerWithInners.class)
|
||||||
|
|||||||
Reference in New Issue
Block a user