From 871bb30dbec7e302bc7fb170a04e2cb490feae0a Mon Sep 17 00:00:00 2001 From: Roman Golyshev Date: Tue, 30 Jun 2020 14:56:38 +0300 Subject: [PATCH] KT-39869 Refactor `ObsoleteExperimentalCoroutinesInspection.kt` - We need this to reuse the logic for migration inspections --- ...bsoleteExperimentalCoroutinesInspection.kt | 322 ++++++++++-------- .../quickfix/allowResolveInWriteAction.txt | 2 +- 2 files changed, 174 insertions(+), 150 deletions(-) diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteExperimentalCoroutinesInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteExperimentalCoroutinesInspection.kt index 089c7a0c27e..18c366c97c4 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteExperimentalCoroutinesInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteExperimentalCoroutinesInspection.kt @@ -37,126 +37,130 @@ import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.parents import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull -class ObsoleteExperimentalCoroutinesInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool, MigrationFix { - override fun isApplicable(migrationInfo: MigrationInfo): Boolean { - return migrationInfo.isLanguageVersionUpdate(LanguageVersion.KOTLIN_1_2, LanguageVersion.KOTLIN_1_3) +internal abstract class ObsoleteCodeMigrationInspection() : AbstractKotlinInspection(), CleanupLocalInspectionTool, MigrationFix { + protected abstract val fromVersion: LanguageVersion + protected abstract val toVersion: LanguageVersion + protected abstract val problems: List + + final override fun isApplicable(migrationInfo: MigrationInfo): Boolean { + return migrationInfo.isLanguageVersionUpdate(fromVersion, toVersion) } - override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): KtVisitorVoid { + final override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): KtVisitorVoid { return simpleNameExpressionVisitor(fun(simpleNameExpression) { run { - val versionAtLeast13 = simpleNameExpression.languageVersionSettings.languageVersion >= LanguageVersion.KOTLIN_1_3 - if (!versionAtLeast13 && !ApplicationManager.getApplication().isUnitTestMode) { + val versionIsSatisfied = simpleNameExpression.languageVersionSettings.languageVersion >= toVersion + if (!versionIsSatisfied && !ApplicationManager.getApplication().isUnitTestMode) { return } } - for (registeredProblem in PROBLEMS) { + for (registeredProblem in problems) { if (registeredProblem.report(holder, isOnTheFly, simpleNameExpression)) { return } } }) } - - companion object { - private val PROBLEMS = listOf( - ObsoleteTopLevelFunctionUsage( - "buildSequence", - "kotlin.coroutines.experimental.buildSequence", - "kotlin.sequences.sequence" - ), - ObsoleteTopLevelFunctionUsage( - "buildIterator", - "kotlin.coroutines.experimental.buildIterator", - "kotlin.sequences.iterator" - ), - ObsoleteExtensionFunctionUsage( - "resume", - "kotlin.coroutines.experimental.Continuation.resume", - "kotlin.coroutines.resume" - ), - ObsoleteExtensionFunctionUsage( - "resumeWithException", - "kotlin.coroutines.experimental.Continuation.resumeWithException", - "kotlin.coroutines.resumeWithException" - ), - ExperimentalImportUsage() - ) - } } -private interface CoroutineMigrationProblem { +internal interface ObsoleteCodeMigrationProblem { fun report(holder: ProblemsHolder, isOnTheFly: Boolean, simpleNameExpression: KtSimpleNameExpression): Boolean } +private interface ObsoleteCodeFix { + fun applyFix(project: Project, descriptor: ProblemDescriptor) +} + +internal class ObsoleteExperimentalCoroutinesInspection : ObsoleteCodeMigrationInspection() { + override val fromVersion: LanguageVersion = LanguageVersion.KOTLIN_1_2 + override val toVersion: LanguageVersion = LanguageVersion.KOTLIN_1_3 + + override val problems = listOf( + ObsoleteTopLevelFunctionUsage( + "buildSequence", + "kotlin.coroutines.experimental.buildSequence", + "kotlin.sequences.sequence" + ), + ObsoleteTopLevelFunctionUsage( + "buildIterator", + "kotlin.coroutines.experimental.buildIterator", + "kotlin.sequences.iterator" + ), + ObsoleteExtensionFunctionUsage( + "resume", + "kotlin.coroutines.experimental.Continuation.resume", + "kotlin.coroutines.resume" + ), + ObsoleteExtensionFunctionUsage( + "resumeWithException", + "kotlin.coroutines.experimental.Continuation.resumeWithException", + "kotlin.coroutines.resumeWithException" + ), + ObsoleteCoroutinesImportsUsage + ) +} + // Shortcut quick fix for running inspection in the project scope. // Should work like RunInspectionAction.runInspection. -private class ObsoleteCoroutineUsageInWholeFix : LocalQuickFix { - override fun getFamilyName(): String = KotlinBundle.message("obsolete.coroutine.usage.in.whole.fix.family.name") - - override fun applyFix(project: Project, descriptor: ProblemDescriptor) { - val toolWrapper = InspectionProjectProfileManager.getInstance(project).currentProfile.getInspectionTool( - ObsoleteExperimentalCoroutinesInspection().shortName, - project - )!! +private abstract class ObsoleteCodeInWholeProjectFix : LocalQuickFix { + protected abstract val inspectionName: String + final override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + val toolWrapper = InspectionProjectProfileManager.getInstance(project).currentProfile.getInspectionTool(inspectionName, project)!! runToolInProject(project, toolWrapper) } - override fun startInWriteAction(): Boolean = false + final override fun startInWriteAction(): Boolean = false - companion object { - val INSTANCE = ObsoleteCoroutineUsageInWholeFix() + private fun runToolInProject(project: Project, toolWrapper: InspectionToolWrapper<*, *>) { + val managerEx = InspectionManager.getInstance(project) as InspectionManagerEx + val kotlinSourcesScope: GlobalSearchScope = KotlinSourceFilterScope.projectSources(GlobalSearchScope.allScope(project), project) + val cleanupScope = AnalysisScope(kotlinSourcesScope, project) - private fun runToolInProject(project: Project, toolWrapper: InspectionToolWrapper<*, *>) { - val managerEx = InspectionManager.getInstance(project) as InspectionManagerEx - val kotlinSourcesScope: GlobalSearchScope = KotlinSourceFilterScope.projectSources(GlobalSearchScope.allScope(project), project) - val cleanupScope = AnalysisScope(kotlinSourcesScope, project) + val cleanupToolProfile = runInInspectionProfileInitMode { RunInspectionIntention.createProfile(toolWrapper, managerEx, null) } + managerEx.createNewGlobalContext(false) + .codeCleanup( + cleanupScope, + cleanupToolProfile, + KotlinBundle.message("apply.in.the.project.0", toolWrapper.displayName), + null, + false + ) + } - val cleanupToolProfile = runInInspectionProfileInitMode { RunInspectionIntention.createProfile(toolWrapper, managerEx, null) } - managerEx.createNewGlobalContext(false) - .codeCleanup( - cleanupScope, - cleanupToolProfile, - KotlinBundle.message("apply.in.the.project.0", toolWrapper.displayName), - null, - false - ) - } - - // Overcome failure during profile creating because of absent tools in tests - inline fun runInInspectionProfileInitMode(runnable: () -> T): T { - return if (!ApplicationManager.getApplication().isUnitTestMode) { + // Overcome failure during profile creating because of absent tools in tests + private inline fun runInInspectionProfileInitMode(runnable: () -> T): T { + return if (!ApplicationManager.getApplication().isUnitTestMode) { + runnable() + } else { + val old = InspectionProfileImpl.INIT_INSPECTIONS + try { + InspectionProfileImpl.INIT_INSPECTIONS = true runnable() - } else { - val old = InspectionProfileImpl.INIT_INSPECTIONS - try { - InspectionProfileImpl.INIT_INSPECTIONS = true - runnable() - } finally { - InspectionProfileImpl.INIT_INSPECTIONS = old - } + } finally { + InspectionProfileImpl.INIT_INSPECTIONS = old } } } } +private object ObsoleteCoroutineUsageInWholeProjectFix : ObsoleteCodeInWholeProjectFix() { + override val inspectionName = ObsoleteExperimentalCoroutinesInspection().shortName + override fun getFamilyName(): String = KotlinBundle.message("obsolete.coroutine.usage.in.whole.fix.family.name") +} + /** * There should be a single fix class with the same family name, this way it can be executed for all found coroutines problems from UI. */ -private class ObsoleteCoroutineUsageFix(val delegate: CoroutineFix) : LocalQuickFix { - override fun getFamilyName(): String = KotlinBundle.message("obsolete.coroutine.usage.fix.family.name") - - override fun applyFix(project: Project, descriptor: ProblemDescriptor) { +private abstract class ObsoleteCodeFixDelegateQuickFix(private val delegate: ObsoleteCodeFix) : LocalQuickFix { + final override fun applyFix(project: Project, descriptor: ProblemDescriptor) { delegate.applyFix(project, descriptor) } +} - companion object { - interface CoroutineFix { - fun applyFix(project: Project, descriptor: ProblemDescriptor) - } - } +private class ObsoleteCoroutinesDelegateQuickFix(delegate: ObsoleteCodeFix) : ObsoleteCodeFixDelegateQuickFix(delegate) { + override fun getFamilyName(): String = KotlinBundle.message("obsolete.coroutine.usage.fix.family.name") } private fun isTopLevelCallForReplace(simpleNameExpression: KtSimpleNameExpression, oldFqName: String, newFqName: String): Boolean { @@ -177,17 +181,17 @@ private fun isTopLevelCallForReplace(simpleNameExpression: KtSimpleNameExpressio return !isInIndex } -private fun fixesWithWholeProject(isOnTheFly: Boolean, fix: LocalQuickFix): Array { +private fun fixesWithWholeProject(isOnTheFly: Boolean, fix: LocalQuickFix, wholeProjectFix: LocalQuickFix): Array { if (!isOnTheFly) { return arrayOf(fix) } - return arrayOf(fix, ObsoleteCoroutineUsageInWholeFix.INSTANCE) + return arrayOf(fix, wholeProjectFix) } private class ObsoleteTopLevelFunctionUsage( val textMarker: String, val oldFqName: String, val newFqName: String -) : CoroutineMigrationProblem { +) : ObsoleteCodeMigrationProblem { override fun report(holder: ProblemsHolder, isOnTheFly: Boolean, simpleNameExpression: KtSimpleNameExpression): Boolean { if (simpleNameExpression.text != textMarker) return false @@ -199,7 +203,7 @@ private class ObsoleteTopLevelFunctionUsage( simpleNameExpression, KotlinBundle.message("0.is.expected.to.be.used.since.kotlin.1.3", newFqName), ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - *fixesWithWholeProject(isOnTheFly, ObsoleteCoroutineUsageFix(fix)) + *fixesWithWholeProject(isOnTheFly, ObsoleteCoroutinesDelegateQuickFix(fix), ObsoleteCoroutineUsageInWholeProjectFix) ) return true @@ -208,7 +212,7 @@ private class ObsoleteTopLevelFunctionUsage( private val fix = RebindReferenceFix(newFqName) companion object { - private class RebindReferenceFix(val fqName: String) : ObsoleteCoroutineUsageFix.Companion.CoroutineFix { + private class RebindReferenceFix(val fqName: String) : ObsoleteCodeFix { override fun applyFix(project: Project, descriptor: ProblemDescriptor) { val element = descriptor.psiElement if (element !is KtSimpleNameExpression) return @@ -223,7 +227,7 @@ private class ObsoleteTopLevelFunctionUsage( private class ObsoleteExtensionFunctionUsage( val textMarker: String, val oldFqName: String, val newFqName: String -) : CoroutineMigrationProblem { +) : ObsoleteCodeMigrationProblem { override fun report(holder: ProblemsHolder, isOnTheFly: Boolean, simpleNameExpression: KtSimpleNameExpression): Boolean { if (simpleNameExpression.text != textMarker) return false @@ -235,7 +239,7 @@ private class ObsoleteExtensionFunctionUsage( simpleNameExpression, KotlinBundle.message("methods.are.absent.in.coroutines.class.since.1.3"), ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - *fixesWithWholeProject(isOnTheFly, ObsoleteCoroutineUsageFix(fix)) + *fixesWithWholeProject(isOnTheFly, ObsoleteCoroutinesDelegateQuickFix(fix), ObsoleteCoroutineUsageInWholeProjectFix) ) return true @@ -244,7 +248,7 @@ private class ObsoleteExtensionFunctionUsage( private val fix = ImportExtensionFunctionFix(newFqName) companion object { - private class ImportExtensionFunctionFix(val fqName: String) : ObsoleteCoroutineUsageFix.Companion.CoroutineFix { + private class ImportExtensionFunctionFix(val fqName: String) : ObsoleteCodeFix { override fun applyFix(project: Project, descriptor: ProblemDescriptor) { val element = descriptor.psiElement if (element !is KtSimpleNameExpression) return @@ -256,16 +260,33 @@ private class ObsoleteExtensionFunctionUsage( .map { it.resolveToDescriptorIfAny() } .find { it != null && it.importableFqName?.asString() == fqName } ?: return - ImportInsertHelper.getInstance(element.project) - .importDescriptor(element.containingKtFile, importFun, forceAllUnderImport = false) + ImportInsertHelper.getInstance(element.project).importDescriptor( + element.containingKtFile, + importFun, + forceAllUnderImport = false + ) } } } } -private class ExperimentalImportUsage : CoroutineMigrationProblem { - override fun report(holder: ProblemsHolder, isOnTheFly: Boolean, simpleNameExpression: KtSimpleNameExpression): Boolean { - if (simpleNameExpression.text != EXPERIMENTAL_COROUTINES_MARKER) return false +private abstract class ObsoleteImportsUsage : ObsoleteCodeMigrationProblem { + /** + * Required to report the problem only on one psi element instead of the every single qualifier + * in the import statement. + */ + protected abstract val textMarker: String + + protected abstract val packageBindings: Map + protected open val importsToRemove: Set = emptySet() + + protected abstract val wholeProjectFix: LocalQuickFix + protected abstract fun problemMessage(): String + + protected abstract fun wrapFix(fix: ObsoleteCodeFix): LocalQuickFix + + final override fun report(holder: ProblemsHolder, isOnTheFly: Boolean, simpleNameExpression: KtSimpleNameExpression): Boolean { + if (simpleNameExpression.text != textMarker) return false val parent = simpleNameExpression.parent as? KtExpression ?: return false val reportExpression = parent as? KtDotQualifiedExpression ?: simpleNameExpression @@ -274,73 +295,76 @@ private class ExperimentalImportUsage : CoroutineMigrationProblem { holder.registerProblem( reportExpression, - KotlinBundle.message("experimental.coroutines.usages.are.obsolete.since.1.3"), + problemMessage(), ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - *fixesWithWholeProject(isOnTheFly, ObsoleteCoroutineUsageFix(ObsoleteCoroutineImportFix)) + *fixesWithWholeProject(isOnTheFly, wrapFix(ObsoleteCoroutineImportFix()), wholeProjectFix) ) return true } - companion object { - private class Binding( - val bindTo: FqName, - val shouldRemove: Boolean, - val importDirective: KtImportDirective + private fun findBinding(simpleNameExpression: KtSimpleNameExpression): Binding? { + if (simpleNameExpression.text != textMarker) return null + + val importDirective = simpleNameExpression.parents + .takeWhile { it is KtDotQualifiedExpression || it is KtImportDirective } + .lastOrNull() as? KtImportDirective ?: return null + + val fqNameStr = importDirective.importedFqName?.asString() ?: return null + + val bindEntry = packageBindings.entries.find { (affectedImportPrefix, _) -> + fqNameStr.startsWith(affectedImportPrefix) + } ?: return null + + return Binding( + FqName(bindEntry.value), + fqNameStr in importsToRemove, + importDirective ) + } - @Suppress("SpellCheckingInspection") - private val PACKAGE_BINDING = mapOf( - "kotlinx.coroutines.experimental" to "kotlinx.coroutines", - "kotlin.coroutines.experimental" to "kotlin.coroutines" - ) + private inner class ObsoleteCoroutineImportFix : ObsoleteCodeFix { + override fun applyFix(project: Project, descriptor: ProblemDescriptor) { + val element = descriptor.psiElement + val simpleNameExpression = when (element) { + is KtSimpleNameExpression -> element + is KtDotQualifiedExpression -> element.selectorExpression as? KtSimpleNameExpression + else -> null + } ?: return - private val IMPORTS_TO_REMOVE = setOf( - "kotlin.coroutines.experimental.buildSequence", - "kotlin.coroutines.experimental.buildIterator" - ) + val binding = findBinding(simpleNameExpression) ?: return - private const val EXPERIMENTAL_COROUTINES_MARKER = "experimental" - - private fun findBinding(simpleNameExpression: KtSimpleNameExpression): Binding? { - if (simpleNameExpression.text != EXPERIMENTAL_COROUTINES_MARKER) return null - - val importDirective = simpleNameExpression.parents - .takeWhile { it is KtDotQualifiedExpression || it is KtImportDirective } - .lastOrNull() as? KtImportDirective ?: return null - - val fqNameStr = importDirective.importedFqName?.asString() ?: return null - - val bindEntry = PACKAGE_BINDING.entries.find { (affectedImportPrefix, _) -> - fqNameStr.startsWith(affectedImportPrefix) - } ?: return null - - return Binding( - FqName(bindEntry.value), - fqNameStr in IMPORTS_TO_REMOVE, - importDirective - ) - } - - private object ObsoleteCoroutineImportFix : ObsoleteCoroutineUsageFix.Companion.CoroutineFix { - override fun applyFix(project: Project, descriptor: ProblemDescriptor) { - val element = descriptor.psiElement - val simpleNameExpression = when (element) { - is KtSimpleNameExpression -> element - is KtDotQualifiedExpression -> element.selectorExpression as? KtSimpleNameExpression - else -> null - } ?: return - - val binding = findBinding(simpleNameExpression) ?: return - - if (binding.shouldRemove) { - binding.importDirective.delete() - } else { - simpleNameExpression.mainReference.bindToFqName( - binding.bindTo, shorteningMode = KtSimpleNameReference.ShorteningMode.NO_SHORTENING - ) - } + if (binding.shouldRemove) { + binding.importDirective.delete() + } else { + simpleNameExpression.mainReference.bindToFqName( + binding.bindTo, shorteningMode = KtSimpleNameReference.ShorteningMode.NO_SHORTENING + ) } } } + + private class Binding( + val bindTo: FqName, + val shouldRemove: Boolean, + val importDirective: KtImportDirective + ) } + +private object ObsoleteCoroutinesImportsUsage : ObsoleteImportsUsage() { + override val textMarker: String = "experimental" + + override val packageBindings: Map = mapOf( + "kotlinx.coroutines.experimental" to "kotlinx.coroutines", + "kotlin.coroutines.experimental" to "kotlin.coroutines" + ) + override val importsToRemove: Set = setOf( + "kotlin.coroutines.experimental.buildSequence", + "kotlin.coroutines.experimental.buildIterator" + ) + + override val wholeProjectFix: LocalQuickFix = ObsoleteCoroutineUsageInWholeProjectFix + override fun problemMessage(): String = KotlinBundle.message("experimental.coroutines.usages.are.obsolete.since.1.3") + override fun wrapFix(fix: ObsoleteCodeFix): LocalQuickFix = ObsoleteCoroutinesDelegateQuickFix(fix) +} + diff --git a/idea/testData/quickfix/allowResolveInWriteAction.txt b/idea/testData/quickfix/allowResolveInWriteAction.txt index b45ac13dc33..84b22caac0f 100644 --- a/idea/testData/quickfix/allowResolveInWriteAction.txt +++ b/idea/testData/quickfix/allowResolveInWriteAction.txt @@ -6,7 +6,7 @@ org.jetbrains.kotlin.idea.core.overrideImplement.ImplementMembersHandler org.jetbrains.kotlin.idea.inspections.ConflictingExtensionPropertyInspection$DeleteRedundantExtensionAction org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection$HighPriorityIntentionBasedQuickFix org.jetbrains.kotlin.idea.inspections.KotlinUnusedImportInspection$OptimizeImportsQuickFix -org.jetbrains.kotlin.idea.inspections.migration.ObsoleteCoroutineUsageFix +org.jetbrains.kotlin.idea.inspections.migration.ObsoleteCoroutinesDelegateQuickFix org.jetbrains.kotlin.idea.inspections.SafeDeleteFix org.jetbrains.kotlin.idea.inspections.SuspiciousCollectionReassignmentInspection$ChangeTypeToMutableFix org.jetbrains.kotlin.idea.inspections.SuspiciousCollectionReassignmentInspection$JoinWithInitializerFix