diff --git a/idea/resources-en/inspectionDescriptions/RedundantNullableReturnType.html b/idea/resources-en/inspectionDescriptions/RedundantNullableReturnType.html new file mode 100644 index 00000000000..0b7ab521b11 --- /dev/null +++ b/idea/resources-en/inspectionDescriptions/RedundantNullableReturnType.html @@ -0,0 +1,5 @@ + + +This inspections reports functions/variables with nullable return type always returns non-null. + + diff --git a/idea/resources-en/messages/KotlinBundle.properties b/idea/resources-en/messages/KotlinBundle.properties index b97b28e5fdb..c678bc4b760 100644 --- a/idea/resources-en/messages/KotlinBundle.properties +++ b/idea/resources-en/messages/KotlinBundle.properties @@ -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} diff --git a/idea/resources/META-INF/inspections.xml b/idea/resources/META-INF/inspections.xml index 228ddb90394..3bfe0cd0a0c 100644 --- a/idea/resources/META-INF/inspections.xml +++ b/idea/resources/META-INF/inspections.xml @@ -2550,6 +2550,15 @@ level="WEAK WARNING" language="kotlin" key="inspection.replace.with.ignore.case.equals.display.name" bundle="messages.KotlinBundle"/> + + + + \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantNullableReturnTypeInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantNullableReturnTypeInspection.kt new file mode 100644 index 00000000000..3506e801664 --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/RedundantNullableReturnTypeInspection.kt @@ -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 { + val context = analyze() + val dataFlowValueFactory = getResolutionFacade().getDataFlowValueFactory() + val moduleDescriptor = findModuleDescriptor() + val languageVersionSettings = languageVersionSettings + return if (this is KtBlockExpression) { + collectDescendantsOfType().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 { + 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) + } + } +} diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/.inspection b/idea/testData/inspectionsLocal/redundantNullableReturnType/.inspection new file mode 100644 index 00000000000..d3580750c4d --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/.inspection @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.inspections.RedundantNullableReturnTypeInspection \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBody.kt b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBody.kt new file mode 100644 index 00000000000..6573d1014db --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBody.kt @@ -0,0 +1,5 @@ +// PROBLEM: 'foo' always returns non-null type +// WITH_RUNTIME +fun foo(xs: List): Int? { + return xs.first() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBody.kt.after b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBody.kt.after new file mode 100644 index 00000000000..7a82c3628f3 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBody.kt.after @@ -0,0 +1,5 @@ +// PROBLEM: 'foo' always returns non-null type +// WITH_RUNTIME +fun foo(xs: List): Int { + return xs.first() +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBodyReturnsNullable.kt b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBodyReturnsNullable.kt new file mode 100644 index 00000000000..bdd89583b42 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBodyReturnsNullable.kt @@ -0,0 +1,9 @@ +// PROBLEM: none +// WITH_RUNTIME +fun foo(xs: List, b: Boolean): Int? { + if (b) { + return xs.first() + } else { + return xs.lastOrNull() + } +} \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBodyWithSmartCast.kt b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBodyWithSmartCast.kt new file mode 100644 index 00000000000..4581b1d22ad --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBodyWithSmartCast.kt @@ -0,0 +1,8 @@ +// PROBLEM: 'foo' always returns non-null type +fun foo(i: Int?): Int? { + if (i != null) { + return i + } else { + return 0 + } +} diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBodyWithSmartCast.kt.after b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBodyWithSmartCast.kt.after new file mode 100644 index 00000000000..c2350020014 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/blockBodyWithSmartCast.kt.after @@ -0,0 +1,8 @@ +// PROBLEM: 'foo' always returns non-null type +fun foo(i: Int?): Int { + if (i != null) { + return i + } else { + return 0 + } +} diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/function/notNullableReturnType.kt b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/notNullableReturnType.kt new file mode 100644 index 00000000000..c8425ebbd24 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/notNullableReturnType.kt @@ -0,0 +1,2 @@ +// PROBLEM: none +fun foo(): String = "" \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/function/overridable.kt b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/overridable.kt new file mode 100644 index 00000000000..276ccb87fe7 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/overridable.kt @@ -0,0 +1,4 @@ +// PROBLEM: none +abstract class Foo { + open fun foo(): String? = "" +} diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/function/singleExpressionBody.kt b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/singleExpressionBody.kt new file mode 100644 index 00000000000..6b3eb57560d --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/singleExpressionBody.kt @@ -0,0 +1,3 @@ +// PROBLEM: 'foo' always returns non-null type +// WITH_RUNTIME +fun foo(xs: List): Int? = xs.first() \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/function/singleExpressionBody.kt.after b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/singleExpressionBody.kt.after new file mode 100644 index 00000000000..9e5198ba85a --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/singleExpressionBody.kt.after @@ -0,0 +1,3 @@ +// PROBLEM: 'foo' always returns non-null type +// WITH_RUNTIME +fun foo(xs: List): Int = xs.first() \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/function/singleExpressionBodyReturnsNullable.kt b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/singleExpressionBodyReturnsNullable.kt new file mode 100644 index 00000000000..2149423fe02 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/function/singleExpressionBodyReturnsNullable.kt @@ -0,0 +1,3 @@ +// PROBLEM: none +// WITH_RUNTIME +fun foo(xs: List, b: Boolean): Int? = if (b) xs.first() else xs.lastOrNull() \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/property/getter.kt b/idea/testData/inspectionsLocal/redundantNullableReturnType/property/getter.kt new file mode 100644 index 00000000000..13412b882ac --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/property/getter.kt @@ -0,0 +1,3 @@ +// PROBLEM: 'foo' is always non-null type +val foo: Int? + get() = 1 \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/property/getter.kt.after b/idea/testData/inspectionsLocal/redundantNullableReturnType/property/getter.kt.after new file mode 100644 index 00000000000..9d1578bae5f --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/property/getter.kt.after @@ -0,0 +1,3 @@ +// PROBLEM: 'foo' is always non-null type +val foo: Int + get() = 1 \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/property/getterWithBody.kt b/idea/testData/inspectionsLocal/redundantNullableReturnType/property/getterWithBody.kt new file mode 100644 index 00000000000..818e9d361ec --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/property/getterWithBody.kt @@ -0,0 +1,5 @@ +// PROBLEM: 'foo' is always non-null type +val foo: Int? + get() { + return 1 + } \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/property/getterWithBody.kt.after b/idea/testData/inspectionsLocal/redundantNullableReturnType/property/getterWithBody.kt.after new file mode 100644 index 00000000000..118cea51a66 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/property/getterWithBody.kt.after @@ -0,0 +1,5 @@ +// PROBLEM: 'foo' is always non-null type +val foo: Int + get() { + return 1 + } \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/property/initializer.kt b/idea/testData/inspectionsLocal/redundantNullableReturnType/property/initializer.kt new file mode 100644 index 00000000000..db7a637344e --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/property/initializer.kt @@ -0,0 +1,2 @@ +// PROBLEM: 'foo' is always non-null type +val foo: String? = "" \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/property/initializer.kt.after b/idea/testData/inspectionsLocal/redundantNullableReturnType/property/initializer.kt.after new file mode 100644 index 00000000000..d71c2c93640 --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/property/initializer.kt.after @@ -0,0 +1,2 @@ +// PROBLEM: 'foo' is always non-null type +val foo: String = "" \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/redundantNullableReturnType/property/var.kt b/idea/testData/inspectionsLocal/redundantNullableReturnType/property/var.kt new file mode 100644 index 00000000000..c995cfd44ac --- /dev/null +++ b/idea/testData/inspectionsLocal/redundantNullableReturnType/property/var.kt @@ -0,0 +1,2 @@ +// PROBLEM: none +var foo: String? = "" \ 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 ecb25058a82..afc2a535ed3 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -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)