KT-18538 Fix inspection to detect unnecessary grand-base class qualifier

- Add separate option to enable/disable this inspection, as it is not obvious if it should be always enabled or not - This option can be used to detect all unnecessary qualifiers in tests - Add possibility to configure inspections via `settings.xml` in the `AbstractMultiFileLocalInspectionTest.kt` - ^KT-18538 Fixed
2020-05-29 19:53:26 +03:00
parent 2c12d26d28
commit 7fb5acc718
31 changed files with 212 additions and 15 deletions
@@ -52,8 +52,12 @@ class BasicLookupElementFactory(
        includeClassTypeArguments: Boolean = true,
        parametersAndTypeGrayed: Boolean = false
    ): LookupElement {
-        val _descriptor = descriptor.unwrapIfFakeOverride()
-        return createLookupElementUnwrappedDescriptor(_descriptor, qualifyNestedClasses, includeClassTypeArguments, parametersAndTypeGrayed)
+        return createLookupElementUnwrappedDescriptor(
+            descriptor.unwrapIfFakeOverride(),
+            qualifyNestedClasses,
+            includeClassTypeArguments,
+            parametersAndTypeGrayed
+        )
    }

    fun createLookupElementForJavaClass(
@@ -2220,4 +2220,5 @@ button.add.package=Add Package
 listbox.import.package=Package
 listbox.import.with.subpackages=With Subpackages
 title.import.layout=Import Layout
-title.packages.to.use.import.with=Packages to Use Import with '*'
+title.packages.to.use.import.with=Packages to Use Import with '*'
+redundant.qualifier.unnecessary.non.direct.parent.class.qualifier=Unnecessary non-direct parent classes qualifiers
@@ -6,6 +6,7 @@
 package org.jetbrains.kotlin.idea.inspections

 import com.intellij.codeInspection.*
+import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.util.TextRange
 import com.intellij.psi.PsiElementVisitor
@@ -16,6 +17,7 @@ import org.jetbrains.kotlin.idea.analysis.analyzeAsReplacement
 import org.jetbrains.kotlin.idea.caches.resolve.analyze
 import org.jetbrains.kotlin.idea.core.ShortenReferences
 import org.jetbrains.kotlin.idea.core.compareDescriptors
+import org.jetbrains.kotlin.idea.core.unwrapIfFakeOverride
 import org.jetbrains.kotlin.idea.imports.importableFqName
 import org.jetbrains.kotlin.idea.intentions.callExpression
 import org.jetbrains.kotlin.idea.references.mainReference
@@ -30,12 +32,28 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
 import org.jetbrains.kotlin.resolve.descriptorUtil.isCompanionObject
 import org.jetbrains.kotlin.resolve.scopes.utils.findFirstClassifierWithDeprecationStatus
 import org.jetbrains.kotlin.utils.addToStdlib.safeAs
+import javax.swing.JComponent

 class RemoveRedundantQualifierNameInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool {
    companion object {
        private val ENUM_STATIC_METHODS = listOf("values", "valueOf")
    }

+    /**
+     * In order to detect that `foo()` and `GrandBase.foo()` point to the same method,
+     * we need to unwrap fake overrides from descriptors. If we don't do that, they will
+     * have different `fqName`s, and the inspection will not detect `GrandBase` as a
+     * redundant qualifier.
+     */
+    var unwrapFakeOverrides: Boolean = false
+
+    override fun createOptionsPanel(): JComponent =
+        SingleCheckboxOptionsPanel(
+            KotlinBundle.message("redundant.qualifier.unnecessary.non.direct.parent.class.qualifier"),
+            this,
+            ::unwrapFakeOverrides.name
+        )
+
    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor =
        object : KtVisitorVoid() {
            override fun visitDotQualifiedExpression(expression: KtDotQualifiedExpression) {
@@ -67,7 +85,7 @@ class RemoveRedundantQualifierNameInspection : AbstractKotlinInspection(), Clean
                    ?.firstOrNull() ?: return

                val applicableExpression = expressionForAnalyze.firstApplicableExpression(
-                    validator = { applicableExpression(originalExpression, context, originalDescriptor) },
+                    validator = { applicableExpression(originalExpression, context, originalDescriptor, unwrapFakeOverrides) },
                    generator = { firstChild as? KtDotQualifiedExpression }
                ) ?: return

@@ -106,7 +124,8 @@ private tailrec fun <T : KtElement> T.firstApplicableExpression(validator: T.()
 private fun KtDotQualifiedExpression.applicableExpression(
    originalExpression: KtExpression,
    oldContext: BindingContext,
-    originalDescriptor: DeclarationDescriptor
+    originalDescriptor: DeclarationDescriptor,
+    unwrapFakeOverrides: Boolean
 ): KtDotQualifiedExpression? {
    if (!receiverExpression.isApplicableReceiver(oldContext) || !ShortenReferences.canBePossibleToDropReceiver(this, oldContext)) {
        return null
@@ -119,12 +138,19 @@ private fun KtDotQualifiedExpression.applicableExpression(
        ?.mainReference?.resolveToDescriptors(newContext)
        ?.firstOrNull() ?: return null

-    return takeIf {
-        originalDescriptor.fqNameSafe == newDescriptor.fqNameSafe &&
-                if (newDescriptor is ImportedFromObjectCallableDescriptor<*>)
-                    compareDescriptors(project, newDescriptor.callableFromObject, originalDescriptor)
-                else
-                    compareDescriptors(project, newDescriptor, originalDescriptor)
+    fun DeclarationDescriptor.unwrapFakeOverrideIfNecessary(): DeclarationDescriptor {
+        return if (unwrapFakeOverrides) this.unwrapIfFakeOverride() else this
+    }
+
+    val originalDescriptorFqName = originalDescriptor.unwrapFakeOverrideIfNecessary().fqNameSafe
+    val newDescriptorFqName = newDescriptor.unwrapFakeOverrideIfNecessary().fqNameSafe
+    if (originalDescriptorFqName != newDescriptorFqName) return null
+
+    return this.takeIf {
+        if (newDescriptor is ImportedFromObjectCallableDescriptor<*>)
+            compareDescriptors(project, newDescriptor.callableFromObject, originalDescriptor)
+        else
+            compareDescriptors(project, newDescriptor, originalDescriptor)
    }
 }

@@ -0,0 +1,4 @@
+package my;
+
+public class Reproducer extends ReproducerBase {
+}
@@ -0,0 +1,5 @@
+package my;
+
+public class ReproducerBase {
+    public static void test() {}
+}
@@ -0,0 +1,10 @@
+package my.simple.name
+
+import my.Reproducer
+import my.ReproducerBase
+
+class Test : Reproducer() {
+    init {
+        test()
+    }
+}
@@ -0,0 +1,4 @@
+package my;
+
+public class Reproducer extends ReproducerBase {
+}
@@ -0,0 +1,5 @@
+package my;
+
+public class ReproducerBase {
+    public static void test() {}
+}
@@ -0,0 +1,10 @@
+package my.simple.name
+
+import my.Reproducer
+import my.ReproducerBase
+
+class Test : Reproducer() {
+    init {
+        ReproducerBase<caret>.test()
+    }
+}
@@ -0,0 +1,4 @@
+{
+    "inspectionClass": "org.jetbrains.kotlin.idea.inspections.RemoveRedundantQualifierNameInspection",
+    "mainFile": "test.kt"
+}
@@ -0,0 +1,3 @@
+<inspection_tool class="RemoveRedundantQualifierNameInspection" enabled="true" enabled_by_default="true">
+  <option name="unwrapFakeOverrides" value="true"/>
+</inspection_tool>
@@ -0,0 +1,5 @@
+package my;
+
+public class Reproducer extends ReproducerBase {
+    public static void test() {}
+}
@@ -0,0 +1,5 @@
+package my;
+
+public class ReproducerBase {
+    public static void test() {}
+}
@@ -0,0 +1,10 @@
+package my.simple.name
+
+import my.Reproducer
+import my.ReproducerBase
+
+class Test : Reproducer() {
+    init {
+        ReproducerBase.test()
+    }
+}
@@ -0,0 +1,5 @@
+package my;
+
+public class Reproducer extends ReproducerBase {
+    public static void test() {}
+}
@@ -0,0 +1,5 @@
+package my;
+
+public class ReproducerBase {
+    public static void test() {}
+}
@@ -0,0 +1,10 @@
+package my.simple.name
+
+import my.Reproducer
+import my.ReproducerBase
+
+class Test : Reproducer() {
+    init {
+        ReproducerBase<caret>.test()
+    }
+}
@@ -0,0 +1,5 @@
+{
+    "inspectionClass": "org.jetbrains.kotlin.idea.inspections.RemoveRedundantQualifierNameInspection",
+    "problem": "none",
+    "mainFile": "test.kt"
+}
@@ -0,0 +1,3 @@
+<inspection_tool class="RemoveRedundantQualifierNameInspection" enabled="true" enabled_by_default="true">
+  <option name="unwrapFakeOverrides" value="true"/>
+</inspection_tool>
@@ -0,0 +1,4 @@
+package my;
+
+public class Reproducer extends ReproducerBase {
+}
@@ -0,0 +1,5 @@
+package my;
+
+public class ReproducerBase {
+    public static void test() {}
+}
@@ -0,0 +1,10 @@
+package my.simple.name
+
+import my.Reproducer
+import my.ReproducerBase
+
+class Test : Reproducer() {
+    init {
+        ReproducerBase.test()
+    }
+}
@@ -0,0 +1,4 @@
+package my;
+
+public class Reproducer extends ReproducerBase {
+}
@@ -0,0 +1,5 @@
+package my;
+
+public class ReproducerBase {
+    public static void test() {}
+}
@@ -0,0 +1,10 @@
+package my.simple.name
+
+import my.Reproducer
+import my.ReproducerBase
+
+class Test : Reproducer() {
+    init {
+        ReproducerBase<caret>.test()
+    }
+}
@@ -0,0 +1,5 @@
+{
+    "inspectionClass": "org.jetbrains.kotlin.idea.inspections.RemoveRedundantQualifierNameInspection",
+    "problem": "none",
+    "mainFile": "test.kt"
+}
@@ -0,0 +1,3 @@
+<inspection_tool class="RemoveRedundantQualifierNameInspection" enabled="true" enabled_by_default="true">
+  <option name="unwrapFakeOverrides" value="false"/>
+</inspection_tool>
@@ -17,6 +17,7 @@ import com.intellij.profile.codeInspection.ProjectInspectionProfileManager
 import com.intellij.testFramework.fixtures.impl.CodeInsightTestFixtureImpl
 import junit.framework.ComparisonFailure
 import junit.framework.TestCase
+import org.jdom.Element
 import org.jetbrains.kotlin.idea.core.script.ScriptConfigurationManager
 import org.jetbrains.kotlin.idea.test.DirectiveBasedActionUtils
 import org.jetbrains.kotlin.idea.test.KotlinLightCodeInsightFixtureTestCase
@@ -128,7 +129,8 @@ abstract class AbstractLocalInspectionTest : KotlinLightCodeInsightFixtureTestCa
        inspection: AbstractKotlinInspection,
        expectedProblemString: String?,
        expectedHighlightString: String?,
-        localFixTextString: String?
+        localFixTextString: String?,
+        inspectionSettings: Element? = null
    ): Boolean {
        val problemExpected = expectedProblemString == null || expectedProblemString != "none"
        myFixture.enableInspections(inspection::class.java)
@@ -139,6 +141,10 @@ abstract class AbstractLocalInspectionTest : KotlinLightCodeInsightFixtureTestCa
        val state = inspectionProfile.getToolDefaultState(inspection.shortName, project)
        state.level = HighlightDisplayLevel.WARNING

+        if (inspectionSettings != null) {
+            state.tool.tool.readSettings(inspectionSettings)
+        }
+
        val caretOffset = myFixture.caretOffset
        val highlightInfos = CodeInsightTestFixtureImpl.instantiateAndRun(
            file, editor, intArrayOf(
@@ -16,6 +16,8 @@ import com.intellij.testFramework.LightProjectDescriptor
 import com.intellij.testFramework.PlatformTestUtil
 import com.intellij.testFramework.UsefulTestCase
 import junit.framework.TestCase
+import org.jdom.Document
+import org.jdom.input.SAXBuilder
 import org.jetbrains.kotlin.idea.jsonUtils.getString
 import org.jetbrains.kotlin.idea.test.KotlinLightProjectDescriptor
 import org.jetbrains.kotlin.idea.test.KotlinWithJdkAndRuntimeLightProjectDescriptor
@@ -47,10 +49,15 @@ abstract class AbstractMultiFileLocalInspectionTest : AbstractLocalInspectionTes
        val problemExpectedString = config["problem"]?.asString // null means "some problem", "none" means no problem
        val localFixTextString = config["fix"]?.asString // null means "some single fix" or "none" if no problem expected

+        val inspectionSettings = File(testFile.parentFile, "settings.xml")
+            .takeIf { it.exists() }
+            ?.let { (SAXBuilder().build(it) as Document).rootElement }
+
+
        doTest(path) test@{ _ ->
            myFixture.configureFromTempProjectFile(mainFilePath)

-            runInspectionWithFixesAndCheck(inspection, problemExpectedString, null, localFixTextString)
+            runInspectionWithFixesAndCheck(inspection, problemExpectedString, null, localFixTextString, inspectionSettings)
        }
    }

@@ -98,6 +98,21 @@ public class MultiFileLocalInspectionTestGenerated extends AbstractMultiFileLoca
        runTest("idea/testData/multiFileLocalInspections/redundantQualifierName/javaStatic/fromKotlinTest.test");
    }

+    @TestMetadata("redundantQualifierName/unnecessaryNonDirectParentClassQualifier/fromKotlinTest.test")
+    public void testRedundantQualifierName_unnecessaryNonDirectParentClassQualifier_FromKotlinTest() throws Exception {
+        runTest("idea/testData/multiFileLocalInspections/redundantQualifierName/unnecessaryNonDirectParentClassQualifier/fromKotlinTest.test");
+    }
+
+    @TestMetadata("redundantQualifierName/unnecessaryNonDirectParentClassQualifierAmbiguous/fromKotlinTest.test")
+    public void testRedundantQualifierName_unnecessaryNonDirectParentClassQualifierAmbiguous_FromKotlinTest() throws Exception {
+        runTest("idea/testData/multiFileLocalInspections/redundantQualifierName/unnecessaryNonDirectParentClassQualifierAmbiguous/fromKotlinTest.test");
+    }
+
+    @TestMetadata("redundantQualifierName/unnecessaryNonDirectParentClassQualifierDisabled/fromKotlinTest.test")
+    public void testRedundantQualifierName_unnecessaryNonDirectParentClassQualifierDisabled_FromKotlinTest() throws Exception {
+        runTest("idea/testData/multiFileLocalInspections/redundantQualifierName/unnecessaryNonDirectParentClassQualifierDisabled/fromKotlinTest.test");
+    }
+
    @TestMetadata("unusedSymbol/fromKotlinTest/fromKotlinTest.test")
    public void testUnusedSymbol_fromKotlinTest_FromKotlinTest() throws Exception {
        runTest("idea/testData/multiFileLocalInspections/unusedSymbol/fromKotlinTest/fromKotlinTest.test");
@@ -1,8 +1,7 @@
 import java.awt.image.AreaAveragingScaleFilter
-import java.awt.image.ImageConsumer

 class TestInterfaceStaticFieldReference(width: Int, height: Int) : AreaAveragingScaleFilter(width, height) {
    fun test() {
-        println(ImageConsumer.TOPDOWNLEFTRIGHT)
+        println(TOPDOWNLEFTRIGHT)
    }
 }