Add "Redundant nullable return type" inspection

^KT-19321 Fixed
2020-08-16 01:06:20 +02:00
parent d703284d01
commit 8420c0b7c7
23 changed files with 297 additions and 0 deletions
@@ -0,0 +1,5 @@
+<html>
+<body>
+This inspections reports functions/variables with nullable return type always returns non-null.
+</body>
+</html>
@@ -2261,6 +2261,9 @@ redundant.qualifier.unnecessary.non.direct.parent.class.qualifier=Unnecessary no
 fix.add.exception.to.throws=Add ''{0}''
 fix.add.eq.eq.true=Add '== true'
 inspection.replace.with.ignore.case.equals.display.name=Replace with 'equals(..., ignoreCase = true)'
+inspection.redundant.nullable.return.type.display.name=Redundant nullable return type
+0.always.returns.non.null.type=''{0}'' always returns non-null type
+0.is.always.non.null.type=''{0}'' is always non-null type
 inspection.simplifiable.scope.function.display.name=Scope function with nested forEach can be simplified
 nested.1.call.in.0.could.be.simplified.to.2=Nested ''{1}'' call in ''{0}'' could be simplified to {2}

@@ -2550,6 +2550,15 @@
                     level="WEAK WARNING"
                     language="kotlin"
                     key="inspection.replace.with.ignore.case.equals.display.name" bundle="messages.KotlinBundle"/>
+
+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.RedundantNullableReturnTypeInspection"
+                     groupPath="Kotlin"
+                     groupName="Redundant constructs"
+                     enabledByDefault="true"
+                     level="WARNING"
+                     language="kotlin"
+                     key="inspection.redundant.nullable.return.type.display.name" bundle="messages.KotlinBundle"/>
+
    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.SimplifyNestedEachInScopeFunctionInspection"
                     groupPath="Kotlin"
                     groupName="Style issues"
@@ -2557,6 +2566,7 @@
                     level="WEAK WARNING"
                     language="kotlin"
                     key="inspection.simplifiable.scope.function.display.name" bundle="messages.KotlinBundle"/>
+
  </extensions>

 </idea-plugin>
@@ -0,0 +1,112 @@
+/*
+ * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * 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.ProblemsHolder
+import com.intellij.openapi.project.Project
+import com.intellij.psi.impl.source.tree.LeafPsiElement
+import org.jetbrains.kotlin.config.LanguageVersionSettings
+import org.jetbrains.kotlin.descriptors.ModuleDescriptor
+import org.jetbrains.kotlin.idea.KotlinBundle
+import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.idea.caches.resolve.findModuleDescriptor
+import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
+import org.jetbrains.kotlin.idea.core.isOverridable
+import org.jetbrains.kotlin.idea.project.languageVersionSettings
+import org.jetbrains.kotlin.idea.resolve.getDataFlowValueFactory
+import org.jetbrains.kotlin.idea.util.textRangeIn
+import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValueFactory
+import org.jetbrains.kotlin.types.KotlinType
+import org.jetbrains.kotlin.types.isNullable
+
+class RedundantNullableReturnTypeInspection : AbstractKotlinInspection() {
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) = object : KtVisitorVoid() {
+        override fun visitNamedFunction(function: KtNamedFunction) {
+            check(function)
+        }
+
+        override fun visitProperty(property: KtProperty) {
+            if (property.isVar) return
+            check(property)
+        }
+
+        private fun check(declaration: KtCallableDeclaration) {
+            val typeReference = declaration.typeReference ?: return
+            val typeElement = typeReference.typeElement as? KtNullableType ?: return
+            if (typeElement.innerType == null) return
+            val questionMark = typeElement.questionMarkNode as? LeafPsiElement ?: return
+
+            if (declaration.isOverridable()) return
+
+            val body = when (declaration) {
+                is KtNamedFunction -> declaration.bodyExpression
+                is KtProperty -> declaration.initializer ?: declaration.accessors.singleOrNull { it.isGetter }?.bodyExpression
+                else -> null
+            } ?: return
+            val actualReturnTypes = body.actualReturnTypes()
+            if (actualReturnTypes.isEmpty() || actualReturnTypes.any { it.isNullable() }) return
+
+            val declarationName = declaration.nameAsSafeName.asString()
+            val description = if (declaration is KtProperty) {
+                KotlinBundle.message("0.is.always.non.null.type", declarationName)
+            } else {
+                KotlinBundle.message("0.always.returns.non.null.type", declarationName)
+            }
+            holder.registerProblem(
+                typeReference,
+                questionMark.textRangeIn(typeReference),
+                description,
+                MakeNotNullableFix()
+            )
+        }
+    }
+
+    private fun KtExpression.actualReturnTypes(): List<KotlinType> {
+        val context = analyze()
+        val dataFlowValueFactory = getResolutionFacade().getDataFlowValueFactory()
+        val moduleDescriptor = findModuleDescriptor()
+        val languageVersionSettings = languageVersionSettings
+        return if (this is KtBlockExpression) {
+            collectDescendantsOfType<KtReturnExpression>().flatMap {
+                it.returnedExpression.types(context, dataFlowValueFactory, moduleDescriptor, languageVersionSettings)
+            }
+        } else {
+            types(context, dataFlowValueFactory, moduleDescriptor, languageVersionSettings)
+        }
+    }
+
+    private fun KtExpression?.types(
+        context: BindingContext,
+        dataFlowValueFactory: DataFlowValueFactory,
+        moduleDescriptor: ModuleDescriptor,
+        languageVersionSettings: LanguageVersionSettings
+    ): List<KotlinType> {
+        if (this == null) return emptyList()
+        val type = context.getType(this) ?: return emptyList()
+        val dataFlowInfo = context[BindingContext.EXPRESSION_TYPE_INFO, this]?.dataFlowInfo ?: return emptyList()
+        val dataFlowValue = dataFlowValueFactory.createDataFlowValue(this, type, context, moduleDescriptor)
+        val stableTypes = dataFlowInfo.getStableTypes(dataFlowValue, languageVersionSettings)
+        return if (stableTypes.isNotEmpty()) stableTypes.toList() else listOf(type)
+    }
+
+    private class MakeNotNullableFix : LocalQuickFix {
+        override fun getName() = KotlinBundle.message("make.not.nullable")
+
+        override fun getFamilyName() = name
+
+        override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+            val typeReference = descriptor.psiElement as? KtTypeReference ?: return
+            val typeElement = typeReference.typeElement as? KtNullableType ?: return
+            val innerType = typeElement.innerType ?: return
+            typeElement.replace(innerType)
+        }
+    }
+}
@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.inspections.RedundantNullableReturnTypeInspection
@@ -0,0 +1,5 @@
+// PROBLEM: 'foo' always returns non-null type
+// WITH_RUNTIME
+fun foo(xs: List<Int>): Int?<caret> {
+    return xs.first()
+}
@@ -0,0 +1,5 @@
+// PROBLEM: 'foo' always returns non-null type
+// WITH_RUNTIME
+fun foo(xs: List<Int>): Int {
+    return xs.first()
+}
@@ -0,0 +1,9 @@
+// PROBLEM: none
+// WITH_RUNTIME
+fun foo(xs: List<Int>, b: Boolean): Int?<caret> {
+    if (b) {
+        return xs.first()
+    } else {
+        return xs.lastOrNull()
+    }
+}
@@ -0,0 +1,8 @@
+// PROBLEM: 'foo' always returns non-null type
+fun foo(i: Int?): Int?<caret> {
+    if (i != null) {
+        return i
+    } else {
+        return 0
+    }
+}
@@ -0,0 +1,8 @@
+// PROBLEM: 'foo' always returns non-null type
+fun foo(i: Int?): Int {
+    if (i != null) {
+        return i
+    } else {
+        return 0
+    }
+}
@@ -0,0 +1,2 @@
+// PROBLEM: none
+fun foo(): String<caret> = ""
@@ -0,0 +1,4 @@
+// PROBLEM: none
+abstract class Foo {
+    open fun foo(): String?<caret> = ""
+}
@@ -0,0 +1,3 @@
+// PROBLEM: 'foo' always returns non-null type
+// WITH_RUNTIME
+fun foo(xs: List<Int>): Int?<caret> = xs.first()
@@ -0,0 +1,3 @@
+// PROBLEM: 'foo' always returns non-null type
+// WITH_RUNTIME
+fun foo(xs: List<Int>): Int = xs.first()
@@ -0,0 +1,3 @@
+// PROBLEM: none
+// WITH_RUNTIME
+fun foo(xs: List<Int>, b: Boolean): Int?<caret> = if (b) xs.first() else xs.lastOrNull()
@@ -0,0 +1,3 @@
+// PROBLEM: 'foo' is always non-null type
+val foo: Int?<caret>
+    get() = 1
@@ -0,0 +1,3 @@
+// PROBLEM: 'foo' is always non-null type
+val foo: Int
+    get() = 1
@@ -0,0 +1,5 @@
+// PROBLEM: 'foo' is always non-null type
+val foo: Int?<caret>
+    get() {
+        return 1
+    }
@@ -0,0 +1,5 @@
+// PROBLEM: 'foo' is always non-null type
+val foo: Int
+    get() {
+        return 1
+    }
@@ -0,0 +1,2 @@
+// PROBLEM: 'foo' is always non-null type
+val foo: String?<caret> = ""
@@ -0,0 +1,2 @@
+// PROBLEM: 'foo' is always non-null type
+val foo: String = ""
@@ -0,0 +1,2 @@
+// PROBLEM: none
+var foo: String?<caret> = ""
@@ -8218,6 +8218,100 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
        }
    }

+    @TestMetadata("idea/testData/inspectionsLocal/redundantNullableReturnType")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class RedundantNullableReturnType extends AbstractLocalInspectionTest {
+        private void runTest(String testDataFilePath) throws Exception {
+            KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
+        }
+
+        public void testAllFilesPresentInRedundantNullableReturnType() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("idea/testData/inspectionsLocal/redundantNullableReturnType"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), null, true);
+        }
+
+        @TestMetadata("idea/testData/inspectionsLocal/redundantNullableReturnType/function")
+        @TestDataPath("$PROJECT_ROOT")
+        @RunWith(JUnit3RunnerWithInners.class)
+        public static class Function extends AbstractLocalInspectionTest {
+            private void runTest(String testDataFilePath) throws Exception {
+                KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
+            }
+
+            public void testAllFilesPresentInFunction() throws Exception {
+                KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("idea/testData/inspectionsLocal/redundantNullableReturnType/function"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), null, true);
+            }
+
+            @TestMetadata("blockBody.kt")
+            public void testBlockBody() throws Exception {
+                runTest("idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBody.kt");
+            }
+
+            @TestMetadata("blockBodyReturnsNullable.kt")
+            public void testBlockBodyReturnsNullable() throws Exception {
+                runTest("idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBodyReturnsNullable.kt");
+            }
+
+            @TestMetadata("blockBodyWithSmartCast.kt")
+            public void testBlockBodyWithSmartCast() throws Exception {
+                runTest("idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBodyWithSmartCast.kt");
+            }
+
+            @TestMetadata("notNullableReturnType.kt")
+            public void testNotNullableReturnType() throws Exception {
+                runTest("idea/testData/inspectionsLocal/redundantNullableReturnType/function/notNullableReturnType.kt");
+            }
+
+            @TestMetadata("overridable.kt")
+            public void testOverridable() throws Exception {
+                runTest("idea/testData/inspectionsLocal/redundantNullableReturnType/function/overridable.kt");
+            }
+
+            @TestMetadata("singleExpressionBody.kt")
+            public void testSingleExpressionBody() throws Exception {
+                runTest("idea/testData/inspectionsLocal/redundantNullableReturnType/function/singleExpressionBody.kt");
+            }
+
+            @TestMetadata("singleExpressionBodyReturnsNullable.kt")
+            public void testSingleExpressionBodyReturnsNullable() throws Exception {
+                runTest("idea/testData/inspectionsLocal/redundantNullableReturnType/function/singleExpressionBodyReturnsNullable.kt");
+            }
+        }
+
+        @TestMetadata("idea/testData/inspectionsLocal/redundantNullableReturnType/property")
+        @TestDataPath("$PROJECT_ROOT")
+        @RunWith(JUnit3RunnerWithInners.class)
+        public static class Property extends AbstractLocalInspectionTest {
+            private void runTest(String testDataFilePath) throws Exception {
+                KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
+            }
+
+            public void testAllFilesPresentInProperty() throws Exception {
+                KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("idea/testData/inspectionsLocal/redundantNullableReturnType/property"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), null, true);
+            }
+
+            @TestMetadata("getter.kt")
+            public void testGetter() throws Exception {
+                runTest("idea/testData/inspectionsLocal/redundantNullableReturnType/property/getter.kt");
+            }
+
+            @TestMetadata("getterWithBody.kt")
+            public void testGetterWithBody() throws Exception {
+                runTest("idea/testData/inspectionsLocal/redundantNullableReturnType/property/getterWithBody.kt");
+            }
+
+            @TestMetadata("initializer.kt")
+            public void testInitializer() throws Exception {
+                runTest("idea/testData/inspectionsLocal/redundantNullableReturnType/property/initializer.kt");
+            }
+
+            @TestMetadata("var.kt")
+            public void testVar() throws Exception {
+                runTest("idea/testData/inspectionsLocal/redundantNullableReturnType/property/var.kt");
+            }
+        }
+    }
+
    @TestMetadata("idea/testData/inspectionsLocal/redundantObjectTypeCheck")
    @TestDataPath("$PROJECT_ROOT")
    @RunWith(JUnit3RunnerWithInners.class)
				`@@ -0,0 +1 @@`
				`org.jetbrains.kotlin.idea.inspections.RedundantNullableReturnTypeInspection`