diff --git a/idea/resources/inspectionDescriptions/ResultIsSuccessOrFailure.html b/idea/resources/inspectionDescriptions/ResultIsSuccessOrFailure.html new file mode 100644 index 00000000000..a5707e1053f --- /dev/null +++ b/idea/resources/inspectionDescriptions/ResultIsSuccessOrFailure.html @@ -0,0 +1,8 @@ + + +This inspection reports functions with SuccessOrFailure result. + +SuccessOrFailure should never be used as return type. +Throw exception, or use nullable type, or use domain-specific result class to indicate failure. + + \ No newline at end of file diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index a1e1dc8b841..07d3721ecba 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -3021,6 +3021,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 ff73b7e1f8f..ef3653a0822 100644 --- a/idea/src/META-INF/plugin.xml.173 +++ b/idea/src/META-INF/plugin.xml.173 @@ -3019,6 +3019,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 7f6d7e91e66..c9ec7c48914 100644 --- a/idea/src/META-INF/plugin.xml.181 +++ b/idea/src/META-INF/plugin.xml.181 @@ -3020,6 +3020,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 3ed6b1fdc91..69b9706687d 100644 --- a/idea/src/META-INF/plugin.xml.183 +++ b/idea/src/META-INF/plugin.xml.183 @@ -3021,6 +3021,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 9e569610a1c..28bd6f9d9c5 100644 --- a/idea/src/META-INF/plugin.xml.as31 +++ b/idea/src/META-INF/plugin.xml.as31 @@ -3019,6 +3019,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 c6d5d2c516f..a11e151bdb4 100644 --- a/idea/src/META-INF/plugin.xml.as32 +++ b/idea/src/META-INF/plugin.xml.as32 @@ -3019,6 +3019,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 d9e77a8cfbf..508155b8aec 100644 --- a/idea/src/META-INF/plugin.xml.as33 +++ b/idea/src/META-INF/plugin.xml.as33 @@ -3021,6 +3021,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/coroutines/ResultIsSuccessOrFailureInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/coroutines/ResultIsSuccessOrFailureInspection.kt new file mode 100644 index 00000000000..808572689df --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/coroutines/ResultIsSuccessOrFailureInspection.kt @@ -0,0 +1,84 @@ +/* + * 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.coroutines + +import com.intellij.codeInspection.ProblemHighlightType +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.psi.PsiElement +import com.intellij.psi.PsiElementVisitor +import org.jetbrains.kotlin.descriptors.ClassDescriptor +import org.jetbrains.kotlin.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.descriptors.PackageFragmentDescriptor +import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny +import org.jetbrains.kotlin.idea.inspections.AbstractKotlinInspection +import org.jetbrains.kotlin.idea.quickfix.createFromUsage.callableBuilder.getReturnTypeReference +import org.jetbrains.kotlin.incremental.components.NoLookupLocation +import org.jetbrains.kotlin.name.Name +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe + +class ResultIsSuccessOrFailureInspection : AbstractKotlinInspection() { + private fun analyzeFunction(function: KtFunction, toReport: PsiElement, holder: ProblemsHolder) { + if (function is KtConstructor<*>) return + val returnTypeText = function.getReturnTypeReference()?.text + if (returnTypeText != null && SHORT_NAME !in returnTypeText) return + + val descriptor = function.resolveToDescriptorIfAny() as? FunctionDescriptor ?: return + val returnType = descriptor.returnType ?: return + val returnTypeClass = returnType.constructor.declarationDescriptor as? ClassDescriptor ?: return + if (returnTypeClass.fqNameSafe.asString() != FULL_NAME) return + + val name = (function as? KtNamedFunction)?.nameAsName?.asString() + if (name != null && name.endsWith(CATCHING)) { + val nameWithoutCatching = name.substringBeforeLast(CATCHING) + val containingDescriptor = descriptor.containingDeclaration + val scope = when (containingDescriptor) { + is ClassDescriptor -> containingDescriptor.unsubstitutedMemberScope + is PackageFragmentDescriptor -> containingDescriptor.getMemberScope() + else -> return + } + val returnTypeArgument = returnType.arguments.firstOrNull()?.type + val nonCatchingFunctions = scope.getContributedFunctions(Name.identifier(nameWithoutCatching), NoLookupLocation.FROM_IDE) + if (nonCatchingFunctions.none { nonCatchingFun -> + nonCatchingFun.returnType == returnTypeArgument + }) { + val typeName = returnTypeArgument?.constructor?.declarationDescriptor?.name?.asString() ?: "T" + holder.registerProblem( + toReport, + "Function '$name' returning '$SHORT_NAME<$typeName>' without the corresponding " + + "function '$nameWithoutCatching' returning '$typeName'", + ProblemHighlightType.GENERIC_ERROR_OR_WARNING + ) + } + } else { + holder.registerProblem( + toReport, + "Function returning $SHORT_NAME with a name that does not end with $CATCHING", + ProblemHighlightType.GENERIC_ERROR_OR_WARNING + ) + } + } + + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { + return object : KtVisitorVoid() { + override fun visitNamedFunction(function: KtNamedFunction) { + analyzeFunction(function, function.nameIdentifier ?: function.funKeyword ?: function, holder) + } + + override fun visitLambdaExpression(lambdaExpression: KtLambdaExpression) { + analyzeFunction(lambdaExpression.functionLiteral, lambdaExpression.functionLiteral.lBrace, holder) + } + } + } + + companion object { + private const val SHORT_NAME = "SuccessOrFailure" + + private const val FULL_NAME = "kotlin.$SHORT_NAME" + + private const val CATCHING = "Catching" + } +} \ No newline at end of file diff --git a/idea/testData/inspections/coroutines/resultIsSuccessOrFailure/inspectionData/expected.xml b/idea/testData/inspections/coroutines/resultIsSuccessOrFailure/inspectionData/expected.xml new file mode 100644 index 00000000000..a2823c81296 --- /dev/null +++ b/idea/testData/inspections/coroutines/resultIsSuccessOrFailure/inspectionData/expected.xml @@ -0,0 +1,92 @@ + + + test.kt + 7 + light_idea_test_case + <default> + + Function returning SuccessOrFailure + Function returning SuccessOrFailure with a name that does not end with Catching + + + test.kt + 9 + light_idea_test_case + <default> + + Function returning SuccessOrFailure + Function returning SuccessOrFailure with a name that does not end with Catching + + + test.kt + 15 + light_idea_test_case + <default> + + Function returning SuccessOrFailure + Function 'incorrectCatching' returning 'SuccessOrFailure<Double>' without the corresponding function 'incorrect' returning 'Double' + + + test.kt + 17 + light_idea_test_case + <default> + + Function returning SuccessOrFailure + Function 'strangeCatching' returning 'SuccessOrFailure<Boolean>' without the corresponding function 'strange' returning 'Boolean' + + + test.kt + 23 + light_idea_test_case + <default> + + Function returning SuccessOrFailure + Function returning SuccessOrFailure with a name that does not end with Catching + + + test.kt + 25 + light_idea_test_case + <default> + + Function returning SuccessOrFailure + Function returning SuccessOrFailure with a name that does not end with Catching + + + test.kt + 31 + light_idea_test_case + <default> + + Function returning SuccessOrFailure + Function 'classIncorrectCatching' returning 'SuccessOrFailure<Double>' without the corresponding function 'classIncorrect' returning 'Double' + + + test.kt + 36 + light_idea_test_case + <default> + + Function returning SuccessOrFailure + Function returning SuccessOrFailure with a name that does not end with Catching + + + test.kt + 38 + light_idea_test_case + <default> + + Function returning SuccessOrFailure + Function returning SuccessOrFailure with a name that does not end with Catching + + + test.kt + 40 + light_idea_test_case + <default> + + Function returning SuccessOrFailure + Function returning SuccessOrFailure with a name that does not end with Catching + + \ No newline at end of file diff --git a/idea/testData/inspections/coroutines/resultIsSuccessOrFailure/inspectionData/inspections.test b/idea/testData/inspections/coroutines/resultIsSuccessOrFailure/inspectionData/inspections.test new file mode 100644 index 00000000000..df31c1167b7 --- /dev/null +++ b/idea/testData/inspections/coroutines/resultIsSuccessOrFailure/inspectionData/inspections.test @@ -0,0 +1 @@ +// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.coroutines.ResultIsSuccessOrFailureInspection diff --git a/idea/testData/inspections/coroutines/resultIsSuccessOrFailure/test.kt b/idea/testData/inspections/coroutines/resultIsSuccessOrFailure/test.kt new file mode 100644 index 00000000000..d74649a3fde --- /dev/null +++ b/idea/testData/inspections/coroutines/resultIsSuccessOrFailure/test.kt @@ -0,0 +1,43 @@ +package kotlin +// NO (constructor) +class SuccessOrFailure(val value: T?) { + fun getOrThrow(): T = value ?: throw AssertionError("") +} +// YES +fun getSuccess() = SuccessOrFailure("123") +// YES +fun getSuccessExplicit(): SuccessOrFailure = SuccessOrFailure(456) +// NO (noCatching available) +fun correctCatching() = SuccessOrFailure(true) +// NO (not SuccessOrFailure) +fun correct() = true +// YES +fun incorrectCatching() = SuccessOrFailure(3.14) +// YES +fun strangeCatching() = SuccessOrFailure(false) +// YES +fun strange() = 1 + +class Container { + // YES + fun classGetSuccess() = SuccessOrFailure("123") + // YES + fun classGetSuccessExplicit(): SuccessOrFailure = SuccessOrFailure(456) + // NO (noCatching available) + fun classCorrectCatching() = SuccessOrFailure(true) + // NO (not SuccessOrFailure) + fun classCorrect() = true + // YES + fun classIncorrectCatching() = SuccessOrFailure(3.14) +} + +fun test() { + // YES + fun localGetSuccess() = SuccessOrFailure("123") + // YES + val anonymous = fun() = SuccessOrFailure(45) + // YES + val lambda = { SuccessOrFailure(true) } + // NO yet + fun localCatching() = SuccessOrFailure(2.72) +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java index 669d0ea338f..bf1c3be5f7b 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java @@ -144,6 +144,11 @@ public class InspectionTestGenerated extends AbstractInspectionTest { runTest("idea/testData/inspections/coroutines/asyncResultUnused/inspectionData/inspections.test"); } + @TestMetadata("coroutines/resultIsSuccessOrFailure/inspectionData/inspections.test") + public void testCoroutines_resultIsSuccessOrFailure_inspectionData_Inspections_test() throws Exception { + runTest("idea/testData/inspections/coroutines/resultIsSuccessOrFailure/inspectionData/inspections.test"); + } + @TestMetadata("dataClassPrivateConstructor/inspectionData/inspections.test") public void testDataClassPrivateConstructor_inspectionData_Inspections_test() throws Exception { runTest("idea/testData/inspections/dataClassPrivateConstructor/inspectionData/inspections.test");