From 04bd5139d53f9cf2a38145e3e7a80299c26c4d5c Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Fri, 18 Jan 2019 10:00:44 +0300 Subject: [PATCH] Don't suggest "return type is..." inspections for anonymous functions #KT-29290 Fixed --- .../coroutines/AbstractIsResultInspection.kt | 28 ++++++++----------- .../inspectionData/expected.xml | 18 ------------ .../coroutines/directUseOfResultType/test.kt | 4 +-- .../coroutines/deferredIsResult/lambda.kt | 12 ++++++++ .../directUseOfResultType/anonymous.kt | 1 + .../directUseOfResultType/anonymous.kt.after | 5 ---- .../directUseOfResultType/lambdaSimple.kt | 1 + .../lambdaSimple.kt.after | 5 ---- .../lambdaWithPartialReturn.kt | 1 + .../lambdaWithPartialReturn.kt.after | 11 -------- .../directUseOfResultType/lambdaWithReturn.kt | 1 + .../lambdaWithReturn.kt.after | 5 ---- .../LocalInspectionTestGenerated.java | 5 ++++ 13 files changed, 34 insertions(+), 63 deletions(-) create mode 100644 idea/testData/inspectionsLocal/coroutines/deferredIsResult/lambda.kt delete mode 100644 idea/testData/inspectionsLocal/coroutines/directUseOfResultType/anonymous.kt.after delete mode 100644 idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaSimple.kt.after delete mode 100644 idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithPartialReturn.kt.after delete mode 100644 idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithReturn.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/coroutines/AbstractIsResultInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/coroutines/AbstractIsResultInspection.kt index 1961fe5adac..14511f1ea47 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/coroutines/AbstractIsResultInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/coroutines/AbstractIsResultInspection.kt @@ -27,37 +27,35 @@ abstract class AbstractIsResultInspection( private val simplify: (KtExpression) -> Unit = {} ) : AbstractKotlinInspection() { - protected fun analyzeFunction(function: KtFunction, toReport: PsiElement, holder: ProblemsHolder) { + protected fun analyzeFunction(function: KtNamedFunction, toReport: PsiElement, holder: ProblemsHolder) { if (function is KtConstructor<*>) return val returnTypeText = function.getReturnTypeReference()?.text if (returnTypeText != null && typeShortName !in returnTypeText) return - val name = (function as? KtNamedFunction)?.nameAsName?.asString() - if (name in allowedNames) return - if (function is KtNamedFunction) { - val receiverTypeReference = function.receiverTypeReference - // Filter given type extensions - if (receiverTypeReference != null && typeShortName in receiverTypeReference.text) return - } - if (function is KtFunctionLiteral || returnTypeText == null) { + val name = function.nameAsName?.asString() + if (name == null || name in allowedNames) return + val receiverTypeReference = function.receiverTypeReference + // Filter given type extensions + if (receiverTypeReference != null && typeShortName in receiverTypeReference.text) return + if (returnTypeText == null) { // Heuristics to save performance: check if something creates given type in function text val text = function.bodyExpression?.text if (text != null && allowedNames.none { it in text } && typeShortName !in text && allowedSuffix !in text) return } - val descriptor = function.resolveToDescriptorIfAny() as? FunctionDescriptor ?: return + val descriptor = function.resolveToDescriptorIfAny() ?: return val returnType = descriptor.returnType ?: return val returnTypeClass = returnType.constructor.declarationDescriptor as? ClassDescriptor ?: return if (returnTypeClass.fqNameSafe.asString() != typeFullName) return - if (name != null && name.endsWith(allowedSuffix)) { + if (name.endsWith(allowedSuffix)) { analyzeFunctionWithAllowedSuffix(name, descriptor, toReport, holder) } else { holder.registerProblem( toReport, "Function returning $typeShortName with a name that does not end with $allowedSuffix", ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - *listOfNotNull( - name?.let { RenameToFix("$it$allowedSuffix") }, + *listOf( + RenameToFix("$name$allowedSuffix"), AddCallOrUnwrapTypeFix( withBody = function.hasBody(), functionName = suggestedFunctionNameToCall, @@ -75,10 +73,6 @@ abstract class AbstractIsResultInspection( 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) - } } } diff --git a/idea/testData/inspections/coroutines/directUseOfResultType/inspectionData/expected.xml b/idea/testData/inspections/coroutines/directUseOfResultType/inspectionData/expected.xml index 51e262e6440..57732e4b919 100644 --- a/idea/testData/inspections/coroutines/directUseOfResultType/inspectionData/expected.xml +++ b/idea/testData/inspections/coroutines/directUseOfResultType/inspectionData/expected.xml @@ -71,24 +71,6 @@ Function returning Result directly Function returning Result with a name that does not end with Catching - - test.kt - 38 - light_idea_test_case - <default> - - Function returning Result directly - Function returning Result with a name that does not end with Catching - - - test.kt - 40 - light_idea_test_case - <default> - - Function returning Result directly - Function returning Result with a name that does not end with Catching - test.kt 60 diff --git a/idea/testData/inspections/coroutines/directUseOfResultType/test.kt b/idea/testData/inspections/coroutines/directUseOfResultType/test.kt index c3e3107aae8..94d31b0d6ac 100644 --- a/idea/testData/inspections/coroutines/directUseOfResultType/test.kt +++ b/idea/testData/inspections/coroutines/directUseOfResultType/test.kt @@ -34,9 +34,9 @@ class Container { fun test() { // YES fun localGetSuccess() = Result("123") - // YES + // NO (no name) val anonymous = fun() = Result(45) - // YES + // NO (no name) val lambda = { Result(true) } // NO yet (we do not report local *catching functions) fun localCatching() = Result(2.72) diff --git a/idea/testData/inspectionsLocal/coroutines/deferredIsResult/lambda.kt b/idea/testData/inspectionsLocal/coroutines/deferredIsResult/lambda.kt new file mode 100644 index 00000000000..aa8f6ef6024 --- /dev/null +++ b/idea/testData/inspectionsLocal/coroutines/deferredIsResult/lambda.kt @@ -0,0 +1,12 @@ +// WITH_RUNTIME +// PROBLEM: none +package kotlinx.coroutines + +suspend fun barAsync(): Deferred? { + return null +} + +suspend fun foo() { + " ".let { barAsync() } +} + diff --git a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/anonymous.kt b/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/anonymous.kt index d520b4d27d8..a767862ea6b 100644 --- a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/anonymous.kt +++ b/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/anonymous.kt @@ -1,3 +1,4 @@ +// PROBLEM: none package kotlin fun test() { diff --git a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/anonymous.kt.after b/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/anonymous.kt.after deleted file mode 100644 index 66c3da061f4..00000000000 --- a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/anonymous.kt.after +++ /dev/null @@ -1,5 +0,0 @@ -package kotlin - -fun test() { - val x = fun() = Result("123").getOrThrow() -} diff --git a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaSimple.kt b/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaSimple.kt index af81384e572..87b9ea58b97 100644 --- a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaSimple.kt +++ b/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaSimple.kt @@ -1,3 +1,4 @@ +// PROBLEM: none package kotlin fun test() { diff --git a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaSimple.kt.after b/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaSimple.kt.after deleted file mode 100644 index 9c1d6c2d0e1..00000000000 --- a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaSimple.kt.after +++ /dev/null @@ -1,5 +0,0 @@ -package kotlin - -fun test() { - val x = { Result(true).getOrThrow() } -} diff --git a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithPartialReturn.kt b/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithPartialReturn.kt index c9a9490aaa7..bf42a46a4ff 100644 --- a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithPartialReturn.kt +++ b/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithPartialReturn.kt @@ -1,3 +1,4 @@ +// PROBLEM: none package kotlin fun test(arg: Boolean) { diff --git a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithPartialReturn.kt.after b/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithPartialReturn.kt.after deleted file mode 100644 index 8d979634680..00000000000 --- a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithPartialReturn.kt.after +++ /dev/null @@ -1,11 +0,0 @@ -package kotlin - -fun test(arg: Boolean) { - val x = foo@{ - (if (!arg) { - return@foo Result(true).getOrThrow() - } else { - Result(false) - }).getOrThrow() - } -} diff --git a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithReturn.kt b/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithReturn.kt index e210be0474f..b9f62a13b21 100644 --- a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithReturn.kt +++ b/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithReturn.kt @@ -1,3 +1,4 @@ +// PROBLEM: none package kotlin fun test() { diff --git a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithReturn.kt.after b/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithReturn.kt.after deleted file mode 100644 index 2ce344b37b8..00000000000 --- a/idea/testData/inspectionsLocal/coroutines/directUseOfResultType/lambdaWithReturn.kt.after +++ /dev/null @@ -1,5 +0,0 @@ -package kotlin - -fun test() { - val x = foo@{ return@foo Result(true).getOrThrow() } -} diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index 2c14f6e1559..601e8560c09 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -2160,6 +2160,11 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest { runTest("idea/testData/inspectionsLocal/coroutines/deferredIsResult/complex.kt"); } + @TestMetadata("lambda.kt") + public void testLambda() throws Exception { + runTest("idea/testData/inspectionsLocal/coroutines/deferredIsResult/lambda.kt"); + } + @TestMetadata("rename.kt") public void testRename() throws Exception { runTest("idea/testData/inspectionsLocal/coroutines/deferredIsResult/rename.kt");