Assert new quick fixes don't use resolve

2019-04-19 17:49:53 +03:00
parent 682183a150
commit b83d07ae9a
4 changed files with 199 additions and 8 deletions
@@ -8,6 +8,7 @@ package org.jetbrains.kotlin.idea.caches.resolve
 import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.diagnostic.Logger
 import com.intellij.openapi.util.registry.Registry
+import org.jetbrains.annotations.TestOnly

 /**
 * Temporary allow resolve in write action.
@@ -19,10 +20,24 @@ fun <T> allowResolveInWriteAction(runnable: () -> T): T {
    return ResolveInWriteActionManager.runWithResolveAllowedInWriteAction(runnable)
 }

+/**
+ * Force resolve check in tests.
+ */
+@TestOnly
+fun <T> forceResolveInWriteActionCheckInTests(errorHandler: (() -> Unit)? = null, runnable: () -> T): T {
+    return ResolveInWriteActionManager.runWithForceResolveAllowedCheckInTests(errorHandler, runnable)
+}
+
+private const val RESOLVE_IN_WRITE_ACTION_ERROR_MESSAGE = "Resolve is not allowed under the write action!"
+
+class ResolveInWriteActionException(message: String? = null) : IllegalThreadStateException(message ?: RESOLVE_IN_WRITE_ACTION_ERROR_MESSAGE)
+
 internal object ResolveInWriteActionManager {
    private val LOG = Logger.getInstance(ResolveInWriteActionManager::class.java)

    private val isResolveInWriteActionAllowed: ThreadLocal<Boolean> = ThreadLocal.withInitial { false }
+    private val isForceCheckInTests: ThreadLocal<Boolean> = ThreadLocal.withInitial { false }
+    private val errorHandler: ThreadLocal<(() -> Unit)?> = ThreadLocal.withInitial { null }

    fun assertNoResolveUnderWriteAction() {
        if (isResolveInWriteActionAllowed.get()) return
@@ -31,15 +46,25 @@ internal object ResolveInWriteActionManager {

        if (!application.isWriteAccessAllowed) return

-        if (application.isUnitTestMode) return
+        if (application.isUnitTestMode) {
+            if (!isForceCheckInTests.get()) return

-        @Suppress("InvalidBundleOrProperty")
-        if (!Registry.`is`("kotlin.write.resolve.check", false)) return
+            val handler = errorHandler.get()
+            if (handler != null) {
+                handler()
+                return
+            }

-        LOG.error("Resolve is not allowed under the write action!")
+            throw ResolveInWriteActionException()
+        } else {
+            @Suppress("InvalidBundleOrProperty")
+            if (!Registry.`is`("kotlin.write.resolve.check", false)) return
+
+            LOG.error(RESOLVE_IN_WRITE_ACTION_ERROR_MESSAGE)
+        }
    }

-    inline fun <T> runWithResolveAllowedInWriteAction(runnable: () -> T): T {
+    internal inline fun <T> runWithResolveAllowedInWriteAction(runnable: () -> T): T {
        val wasSet =
            if (ApplicationManager.getApplication()?.isWriteAccessAllowed == true && isResolveInWriteActionAllowed.get() == false) {
                isResolveInWriteActionAllowed.set(true)
@@ -56,4 +81,23 @@ internal object ResolveInWriteActionManager {
            }
        }
    }
+
+    internal fun <T> runWithForceResolveAllowedCheckInTests(errorHandler: (() -> Unit)?, runnable: () -> T): T {
+        val wasSet = if (isForceCheckInTests.get() == false) {
+            isForceCheckInTests.set(true)
+            this.errorHandler.set(errorHandler)
+            true
+        } else {
+            false
+        }
+
+        try {
+            return runnable()
+        } finally {
+            if (wasSet) {
+                isForceCheckInTests.set(false)
+                this.errorHandler.set(null)
+            }
+        }
+    }
 }
@@ -0,0 +1,68 @@
+# Actions that are allowed to resolve in write action. Normally this set shouldn't be extended and eventually should be dropped.
+# Please consider rewriting a quick-fix and remove resolve from it before adding a new entry to this list.
+
+org.jetbrains.kotlin.idea.core.overrideImplement.ImplementMembersHandler
+org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection$HighPriorityIntentionBasedQuickFix
+org.jetbrains.kotlin.idea.inspections.KotlinUnusedImportInspection$OptimizeImportsQuickFix
+org.jetbrains.kotlin.idea.inspections.SafeDeleteFix
+org.jetbrains.kotlin.idea.inspections.SuspiciousCollectionReassignmentInspection$ChangeTypeToMutableFix
+org.jetbrains.kotlin.idea.inspections.SuspiciousCollectionReassignmentInspection$JoinWithInitializerFix
+org.jetbrains.kotlin.idea.inspections.UnusedReceiverParameterInspection$RemoveReceiverFix
+org.jetbrains.kotlin.idea.inspections.migration.ObsoleteCoroutineUsageFix
+org.jetbrains.kotlin.idea.intentions.AddPropertyAccessorsIntention
+org.jetbrains.kotlin.idea.intentions.AddPropertyGetterIntention
+org.jetbrains.kotlin.idea.intentions.AddPropertySetterIntention
+org.jetbrains.kotlin.idea.intentions.ConvertPropertyInitializerToGetterIntention
+org.jetbrains.kotlin.idea.intentions.CreateKotlinSubClassIntention
+org.jetbrains.kotlin.idea.intentions.MovePropertyToConstructorIntention
+org.jetbrains.kotlin.idea.intentions.SpecifyTypeExplicitlyIntention
+org.jetbrains.kotlin.idea.quickfix.AddAnnotationTargetFix
+org.jetbrains.kotlin.idea.quickfix.AddExclExclCallFix
+org.jetbrains.kotlin.idea.quickfix.AddFunctionToSupertypeFix
+org.jetbrains.kotlin.idea.quickfix.AddModifierFix
+org.jetbrains.kotlin.idea.quickfix.AddNameToArgumentFix
+org.jetbrains.kotlin.idea.quickfix.AddPropertyToSupertypeFix
+org.jetbrains.kotlin.idea.quickfix.AddStartProjectionsForInnerClass
+org.jetbrains.kotlin.idea.quickfix.AddWhenRemainingBranchesFix
+org.jetbrains.kotlin.idea.quickfix.AssignToPropertyFix
+org.jetbrains.kotlin.idea.quickfix.ChangeAccessorTypeFix
+org.jetbrains.kotlin.idea.quickfix.ChangeCallableReturnTypeFix$ForEnclosing
+org.jetbrains.kotlin.idea.quickfix.ChangeParameterTypeFix
+org.jetbrains.kotlin.idea.quickfix.ChangeToMutableCollectionFix
+org.jetbrains.kotlin.idea.quickfix.ChangeVisibilityFix$ChangeToInternalFix
+org.jetbrains.kotlin.idea.quickfix.ChangeVisibilityFix$ChangeToPrivateFix
+org.jetbrains.kotlin.idea.quickfix.ChangeVisibilityFix$ChangeToProtectedFix
+org.jetbrains.kotlin.idea.quickfix.ChangeVisibilityFix$ChangeToPublicFix
+org.jetbrains.kotlin.idea.quickfix.ConvertExtensionPropertyInitializerToGetterFix
+org.jetbrains.kotlin.idea.quickfix.ConvertMemberToExtensionFix
+org.jetbrains.kotlin.idea.quickfix.ConvertToAnonymousObjectFix
+org.jetbrains.kotlin.idea.quickfix.ImportFix
+org.jetbrains.kotlin.idea.quickfix.InitializePropertyQuickFixFactory$AddInitializerFix
+org.jetbrains.kotlin.idea.quickfix.InitializePropertyQuickFixFactory$InitializeWithConstructorParameter
+org.jetbrains.kotlin.idea.quickfix.InitializePropertyQuickFixFactory$MoveToConstructorParameters
+org.jetbrains.kotlin.idea.quickfix.InlineTypeParameterFix
+org.jetbrains.kotlin.idea.quickfix.InsertDelegationCallQuickfix
+org.jetbrains.kotlin.idea.quickfix.KotlinSuppressIntentionAction
+org.jetbrains.kotlin.idea.quickfix.LetImplementInterfaceFix
+org.jetbrains.kotlin.idea.quickfix.LowPriorityQuickFixWithDelegateFactory
+org.jetbrains.kotlin.idea.quickfix.QuickFixWithDelegateFactory
+org.jetbrains.kotlin.idea.quickfix.RemoveNameFromFunctionExpressionFix
+org.jetbrains.kotlin.idea.quickfix.RemovePartsFromPropertyFix
+org.jetbrains.kotlin.idea.quickfix.RemoveRedundantInitializerFix
+org.jetbrains.kotlin.idea.quickfix.RemoveUnusedFunctionParameterFix
+org.jetbrains.kotlin.idea.quickfix.RemoveUnusedValueFix
+org.jetbrains.kotlin.idea.quickfix.RenameModToRemFix
+org.jetbrains.kotlin.idea.quickfix.RenameParameterToMatchOverriddenMethodFix
+org.jetbrains.kotlin.idea.quickfix.RenameUnresolvedReferenceFix
+org.jetbrains.kotlin.idea.quickfix.ReplaceInfixOrOperatorCallFix
+org.jetbrains.kotlin.idea.quickfix.RestrictedRetentionForExpressionAnnotationFactory$AddSourceRetentionFix
+org.jetbrains.kotlin.idea.quickfix.SimplifyComparisonFix
+org.jetbrains.kotlin.idea.quickfix.SmartCastImpossibleInIfThenFactory$createQuickFix$1
+org.jetbrains.kotlin.idea.quickfix.SpecifyOverrideExplicitlyFix
+org.jetbrains.kotlin.idea.quickfix.SpecifyTypeExplicitlyFix
+org.jetbrains.kotlin.idea.quickfix.SuperClassNotInitialized$AddParenthesisFix
+org.jetbrains.kotlin.idea.quickfix.WrapWithSafeLetCallFix
+org.jetbrains.kotlin.idea.quickfix.createFromUsage.createVariable.CreateLocalVariableActionFactory$createAction$2
+org.jetbrains.kotlin.idea.quickfix.migration.MigrateExternalExtensionFix
+org.jetbrains.kotlin.idea.quickfix.replaceWith.DeprecatedSymbolUsageFix
+org.jetbrains.kotlin.idea.quickfix.replaceWith.DeprecatedSymbolUsageInWholeProjectFix
@@ -7,7 +7,10 @@ package org.jetbrains.kotlin.idea.quickfix

 import com.intellij.codeInsight.daemon.quickFix.ActionHint
 import com.intellij.codeInsight.intention.IntentionAction
+import com.intellij.codeInsight.intention.IntentionActionDelegate
+import com.intellij.codeInsight.intention.impl.config.IntentionActionWrapper
 import com.intellij.codeInspection.SuppressableProblemGroup
+import com.intellij.codeInspection.ex.QuickFixWrapper
 import com.intellij.openapi.command.CommandProcessor
 import com.intellij.openapi.util.io.FileUtil
 import com.intellij.openapi.util.text.StringUtil
@@ -18,6 +21,8 @@ import com.intellij.testFramework.LightProjectDescriptor
 import com.intellij.testFramework.UsefulTestCase
 import com.intellij.util.ui.UIUtil
 import junit.framework.TestCase
+import org.jetbrains.kotlin.idea.caches.resolve.ResolveInWriteActionException
+import org.jetbrains.kotlin.idea.caches.resolve.forceResolveInWriteActionCheckInTests
 import org.jetbrains.kotlin.idea.facet.KotlinFacet
 import org.jetbrains.kotlin.idea.test.*
 import org.jetbrains.kotlin.psi.KtFile
@@ -26,8 +31,28 @@ import org.junit.Assert
 import org.junit.ComparisonFailure
 import java.io.File
 import java.io.IOException
+import kotlin.collections.HashSet

 abstract class AbstractQuickFixTest : KotlinLightCodeInsightFixtureTestCase(), QuickFixTest {
+    companion object {
+        private val quickFixesAllowedToResolveInWriteAction = AllowedToResolveUnderWriteActionData(
+            "idea/testData/quickfix/allowResolveInWriteAction.txt",
+            """
+                # Actions that are allowed to resolve in write action. Normally this list shouldn't be extended and eventually should 
+                # be dropped. Please consider rewriting a quick-fix and remove resolve from it before adding a new entry to this list.
+            """.trimIndent()
+        )
+
+        private fun unwrapIntention(action: Any): Any {
+            return when (action) {
+                is IntentionActionDelegate -> unwrapIntention(action.delegate)
+                is IntentionActionWrapper -> unwrapIntention(action.delegate)
+                is QuickFixWrapper -> unwrapIntention(action.fix)
+                else -> action
+            }
+        }
+    }
+
    @Throws(Exception::class)
    protected fun doTest(beforeFileName: String) {
        val beforeFileText = FileUtil.loadFile(File(beforeFileName))
@@ -144,12 +169,25 @@ abstract class AbstractQuickFixTest : KotlinLightCodeInsightFixtureTestCase(), Q
        val intention = findActionWithText(actionHint.expectedText)
        if (actionHint.shouldPresent()) {
            if (intention == null) {
-                fail("Action with text '" + actionHint.expectedText + "' not found\nAvailable actions: " +
-                     myFixture.availableIntentions.joinToString(prefix = "[", postfix = "]") { it.text })
+                fail(
+                    "Action with text '" + actionHint.expectedText + "' not found\nAvailable actions: " +
+                            myFixture.availableIntentions.joinToString(prefix = "[", postfix = "]") { it.text })
+                return
+            }
+
+            val writeActionResolveHandler: () -> Unit = {
+                val unwrappedIntention = unwrapIntention(intention)
+
+                val intentionClassName = unwrappedIntention.javaClass.name
+                if (!quickFixesAllowedToResolveInWriteAction.isWriteActionAllowed(intentionClassName)) {
+                    throw ResolveInWriteActionException()
+                }
            }

            val stubComparisonFailure: ComparisonFailure? = try {
-                myFixture.launchAction(intention!!)
+                forceResolveInWriteActionCheckInTests(writeActionResolveHandler) {
+                    myFixture.launchAction(intention)
+                }
                null
            } catch (comparisonFailure: ComparisonFailure) {
                comparisonFailure
@@ -0,0 +1,41 @@
+/*
+ * Copyright 2010-2019 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.quickfix
+
+import java.io.File
+import java.util.*
+
+class AllowedToResolveUnderWriteActionData(path: String, private val comment: String) {
+    private val file = File(path)
+
+    private val allowedActions = if (file.exists()) {
+        file.readLines().filter {
+            val trimmed = it.trim()
+            !(trimmed.isEmpty() || trimmed.startsWith("#"))
+        }.toSet()
+    } else {
+        setOf()
+    }
+
+    // Set to true to update test data.
+    private val updateTestData = false
+    private val updateTestDataSet: MutableSet<String> = TreeSet(allowedActions)
+
+    fun isWriteActionAllowed(action: String): Boolean {
+        if (action in allowedActions) {
+            return true
+        }
+
+        @Suppress("ConstantConditionIf")
+        if (updateTestData && updateTestDataSet.add(action)) {
+            file.writeText(
+                "$comment\n\n${updateTestDataSet.joinToString("\n")}"
+            )
+        }
+
+        return false
+    }
+}