Add inspection to detect functions with SuccessOrFailure return type

Partial implementation of KT-25621 (no quick-fixes yet)
2018-08-21 20:56:45 +03:00
parent 48784fd95e
commit 87d750aa1c
13 changed files with 296 additions and 0 deletions
@@ -0,0 +1,8 @@
+<html>
+<body>
+This inspection reports functions with <b>SuccessOrFailure</b> result.
+
+<b>SuccessOrFailure</b> should never be used as return type.
+Throw exception, or use nullable type, or use domain-specific result class to indicate failure.
+</body>
+</html>
@@ -3021,6 +3021,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio.
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.coroutines.ResultIsSuccessOrFailureInspection"
+                     displayName="Function returning SuccessOrFailure"
+                     groupPath="Kotlin"
+                     groupName="Style issues"
+                     enabledByDefault="true"
+                     level="WEAK WARNING"
+                     language="kotlin"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -3019,6 +3019,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio.
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.coroutines.ResultIsSuccessOrFailureInspection"
+                     displayName="Function returning SuccessOrFailure"
+                     groupPath="Kotlin"
+                     groupName="Style issues"
+                     enabledByDefault="true"
+                     level="WEAK WARNING"
+                     language="kotlin"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -3020,6 +3020,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio.
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.coroutines.ResultIsSuccessOrFailureInspection"
+                     displayName="Function returning SuccessOrFailure"
+                     groupPath="Kotlin"
+                     groupName="Style issues"
+                     enabledByDefault="true"
+                     level="WEAK WARNING"
+                     language="kotlin"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -3021,6 +3021,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio.
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.coroutines.ResultIsSuccessOrFailureInspection"
+                     displayName="Function returning SuccessOrFailure"
+                     groupPath="Kotlin"
+                     groupName="Style issues"
+                     enabledByDefault="true"
+                     level="WEAK WARNING"
+                     language="kotlin"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -3019,6 +3019,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio.
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.coroutines.ResultIsSuccessOrFailureInspection"
+                     displayName="Function returning SuccessOrFailure"
+                     groupPath="Kotlin"
+                     groupName="Style issues"
+                     enabledByDefault="true"
+                     level="WEAK WARNING"
+                     language="kotlin"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -3019,6 +3019,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio.
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.coroutines.ResultIsSuccessOrFailureInspection"
+                     displayName="Function returning SuccessOrFailure"
+                     groupPath="Kotlin"
+                     groupName="Style issues"
+                     enabledByDefault="true"
+                     level="WEAK WARNING"
+                     language="kotlin"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -3021,6 +3021,15 @@ The Kotlin plugin provides language support in IntelliJ IDEA and Android Studio.
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.coroutines.ResultIsSuccessOrFailureInspection"
+                     displayName="Function returning SuccessOrFailure"
+                     groupPath="Kotlin"
+                     groupName="Style issues"
+                     enabledByDefault="true"
+                     level="WEAK WARNING"
+                     language="kotlin"
+    />
+
    <referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>

    <fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
@@ -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"
+    }
+}
@@ -0,0 +1,92 @@
+<problems>
+  <problem>
+    <file>test.kt</file>
+    <line>7</line>
+    <module>light_idea_test_case</module>
+    <package>&lt;default&gt;</package>
+    <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+    <problem_class severity="WEAK WARNING" attribute_key="INFO_ATTRIBUTES">Function returning SuccessOrFailure</problem_class>
+    <description>Function returning SuccessOrFailure with a name that does not end with Catching</description>
+  </problem>
+  <problem>
+    <file>test.kt</file>
+    <line>9</line>
+    <module>light_idea_test_case</module>
+    <package>&lt;default&gt;</package>
+    <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+    <problem_class severity="WEAK WARNING" attribute_key="INFO_ATTRIBUTES">Function returning SuccessOrFailure</problem_class>
+    <description>Function returning SuccessOrFailure with a name that does not end with Catching</description>
+  </problem>
+  <problem>
+    <file>test.kt</file>
+    <line>15</line>
+    <module>light_idea_test_case</module>
+    <package>&lt;default&gt;</package>
+    <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+    <problem_class severity="WEAK WARNING" attribute_key="INFO_ATTRIBUTES">Function returning SuccessOrFailure</problem_class>
+    <description>Function 'incorrectCatching' returning 'SuccessOrFailure&lt;Double&gt;' without the corresponding function 'incorrect' returning 'Double'</description>
+  </problem>
+  <problem>
+    <file>test.kt</file>
+    <line>17</line>
+    <module>light_idea_test_case</module>
+    <package>&lt;default&gt;</package>
+    <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+    <problem_class severity="WEAK WARNING" attribute_key="INFO_ATTRIBUTES">Function returning SuccessOrFailure</problem_class>
+    <description>Function 'strangeCatching' returning 'SuccessOrFailure&lt;Boolean&gt;' without the corresponding function 'strange' returning 'Boolean'</description>
+  </problem>
+  <problem>
+    <file>test.kt</file>
+    <line>23</line>
+    <module>light_idea_test_case</module>
+    <package>&lt;default&gt;</package>
+    <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+    <problem_class severity="WEAK WARNING" attribute_key="INFO_ATTRIBUTES">Function returning SuccessOrFailure</problem_class>
+    <description>Function returning SuccessOrFailure with a name that does not end with Catching</description>
+  </problem>
+  <problem>
+    <file>test.kt</file>
+    <line>25</line>
+    <module>light_idea_test_case</module>
+    <package>&lt;default&gt;</package>
+    <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+    <problem_class severity="WEAK WARNING" attribute_key="INFO_ATTRIBUTES">Function returning SuccessOrFailure</problem_class>
+    <description>Function returning SuccessOrFailure with a name that does not end with Catching</description>
+  </problem>
+  <problem>
+    <file>test.kt</file>
+    <line>31</line>
+    <module>light_idea_test_case</module>
+    <package>&lt;default&gt;</package>
+    <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+    <problem_class severity="WEAK WARNING" attribute_key="INFO_ATTRIBUTES">Function returning SuccessOrFailure</problem_class>
+    <description>Function 'classIncorrectCatching' returning 'SuccessOrFailure&lt;Double&gt;' without the corresponding function 'classIncorrect' returning 'Double'</description>
+  </problem>
+  <problem>
+    <file>test.kt</file>
+    <line>36</line>
+    <module>light_idea_test_case</module>
+    <package>&lt;default&gt;</package>
+    <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+    <problem_class severity="WEAK WARNING" attribute_key="INFO_ATTRIBUTES">Function returning SuccessOrFailure</problem_class>
+    <description>Function returning SuccessOrFailure with a name that does not end with Catching</description>
+  </problem>
+  <problem>
+    <file>test.kt</file>
+    <line>38</line>
+    <module>light_idea_test_case</module>
+    <package>&lt;default&gt;</package>
+    <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+    <problem_class severity="WEAK WARNING" attribute_key="INFO_ATTRIBUTES">Function returning SuccessOrFailure</problem_class>
+    <description>Function returning SuccessOrFailure with a name that does not end with Catching</description>
+  </problem>
+  <problem>
+    <file>test.kt</file>
+    <line>40</line>
+    <module>light_idea_test_case</module>
+    <package>&lt;default&gt;</package>
+    <entry_point TYPE="file" FQNAME="temp:///src/test.kt" />
+    <problem_class severity="WEAK WARNING" attribute_key="INFO_ATTRIBUTES">Function returning SuccessOrFailure</problem_class>
+    <description>Function returning SuccessOrFailure with a name that does not end with Catching</description>
+  </problem>
+</problems>
@@ -0,0 +1 @@
+// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.coroutines.ResultIsSuccessOrFailureInspection
@@ -0,0 +1,43 @@
+package kotlin
+// NO (constructor)
+class SuccessOrFailure<T>(val value: T?) {
+    fun getOrThrow(): T = value ?: throw AssertionError("")
+}
+// YES
+fun getSuccess() = SuccessOrFailure("123")
+// YES
+fun getSuccessExplicit(): SuccessOrFailure<Int> = 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<Int> = 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)
+}
@@ -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");
				`@@ -0,0 +1 @@`
				`// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.coroutines.ResultIsSuccessOrFailureInspection`