From 91fa5d38b34ed6a1c05d7bcbe4be0ba53699e70f Mon Sep 17 00:00:00 2001 From: Roman Golyshev Date: Thu, 2 Jul 2020 23:55:39 +0300 Subject: [PATCH] KT-39869 Fix review suggestions - Review: https://jetbrains.team/p/kt/review/1349 --- .../messages/KotlinBundle.properties | 6 ++--- .../ObsoleteCodeMigrationInspection.kt | 27 +++++++++---------- ...bsoleteExperimentalCoroutinesInspection.kt | 22 +++++++-------- .../ObsoleteKotlinJsPackagesInspection.kt | 25 +++++++++-------- 4 files changed, 39 insertions(+), 41 deletions(-) diff --git a/idea/resources-en/messages/KotlinBundle.properties b/idea/resources-en/messages/KotlinBundle.properties index 70671e0293b..980c4f8a34d 100644 --- a/idea/resources-en/messages/KotlinBundle.properties +++ b/idea/resources-en/messages/KotlinBundle.properties @@ -1317,13 +1317,11 @@ obsolete.coroutine.usage.in.whole.fix.family.name=Fix experimental coroutines us obsolete.kotlin.js.packages.usage.in.whole.fix.family.name=Fix 'kotlin.dom' and 'kotlin.browser' packages usages in the project apply.in.the.project.0=Apply in the project: {0} obsolete.coroutine.usage.fix.family.name=Fix experimental coroutines usage -obsolete.kotlin.browser.usage.fix.family.name=Fix 'kotlin.browser' package usage -obsolete.kotlin.dom.usage.fix.family.name=Fix 'kotlin.dom' package usage +obsolete.package.usage.fix.family.name=Fix ''{0}'' package usage 0.is.expected.to.be.used.since.kotlin.1.3=`{0}` is expected to be used since Kotlin 1.3 methods.are.absent.in.coroutines.class.since.1.3=Methods are absent in coroutines class since 1.3 experimental.coroutines.usages.are.obsolete.since.1.3=Experimental coroutines usages are obsolete since 1.3 -kotlin.browser.usages.are.obsolete.since.1.3='kotlin.browser' package usages are obsolete since 1.4 -kotlin.dom.usages.are.obsolete.since.1.3='kotlin.dom' package usages are obsolete since 1.4 +package.usages.are.obsolete.since.1.4=''{0}'' package usages are obsolete since 1.4 replace.substring.call.with.droplast.call=Replace 'substring' call with 'dropLast' call replace.substring.call.with.indexing.operation.call=Replace 'substring' call with indexing operation call replace.substring.call.with.substringbefore.call=Replace 'substring' call with 'substringBefore' call diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteCodeMigrationInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteCodeMigrationInspection.kt index 1f97edfd41f..a39f8a4074d 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteCodeMigrationInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteCodeMigrationInspection.kt @@ -32,7 +32,7 @@ import org.jetbrains.kotlin.psi.psiUtil.parents internal abstract class ObsoleteCodeMigrationInspection : AbstractKotlinInspection(), CleanupLocalInspectionTool, MigrationFix { protected abstract val fromVersion: LanguageVersion protected abstract val toVersion: LanguageVersion - protected abstract val problems: List + protected abstract val problemReporters: List final override fun isApplicable(migrationInfo: MigrationInfo): Boolean { return migrationInfo.isLanguageVersionUpdate(fromVersion, toVersion) @@ -40,15 +40,13 @@ internal abstract class ObsoleteCodeMigrationInspection : AbstractKotlinInspecti final override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): KtVisitorVoid { return simpleNameExpressionVisitor(fun(simpleNameExpression) { - run { - val versionIsSatisfied = simpleNameExpression.languageVersionSettings.languageVersion >= toVersion - if (!versionIsSatisfied && !ApplicationManager.getApplication().isUnitTestMode) { - return - } + val versionIsSatisfied = simpleNameExpression.languageVersionSettings.languageVersion >= toVersion + if (!versionIsSatisfied && !ApplicationManager.getApplication().isUnitTestMode) { + return } - for (registeredProblem in problems) { - if (registeredProblem.report(holder, isOnTheFly, simpleNameExpression)) { + for (reporter in problemReporters) { + if (reporter.report(holder, isOnTheFly, simpleNameExpression)) { return } } @@ -56,7 +54,7 @@ internal abstract class ObsoleteCodeMigrationInspection : AbstractKotlinInspecti } } -internal interface ObsoleteCodeMigrationProblem { +internal interface ObsoleteCodeProblemReporter { fun report(holder: ProblemsHolder, isOnTheFly: Boolean, simpleNameExpression: KtSimpleNameExpression): Boolean } @@ -109,7 +107,7 @@ internal abstract class ObsoleteCodeInWholeProjectFix : LocalQuickFix { } /** - * 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. + * There should be a single fix class with the same family name, this way it can be executed for all found problems from UI. */ internal abstract class ObsoleteCodeFixDelegateQuickFix(private val delegate: ObsoleteCodeFix) : LocalQuickFix { final override fun applyFix(project: Project, descriptor: ProblemDescriptor) { @@ -117,7 +115,7 @@ internal abstract class ObsoleteCodeFixDelegateQuickFix(private val delegate: Ob } } -internal abstract class ObsoleteImportsUsage : ObsoleteCodeMigrationProblem { +internal abstract class ObsoleteImportsUsageReporter : ObsoleteCodeProblemReporter { /** * Required to report the problem only on one psi element instead of the every single qualifier * in the import statement. @@ -144,7 +142,7 @@ internal abstract class ObsoleteImportsUsage : ObsoleteCodeMigrationProblem { reportExpression, problemMessage(), ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - *fixesWithWholeProject(isOnTheFly, wrapFix(ObsoleteCoroutineImportFix()), wholeProjectFix) + *fixesWithWholeProject(isOnTheFly, wrapFix(ObsoleteImportFix()), wholeProjectFix) ) return true @@ -170,10 +168,9 @@ internal abstract class ObsoleteImportsUsage : ObsoleteCodeMigrationProblem { ) } - private inner class ObsoleteCoroutineImportFix : ObsoleteCodeFix { + private inner class ObsoleteImportFix : ObsoleteCodeFix { override fun applyFix(project: Project, descriptor: ProblemDescriptor) { - val element = descriptor.psiElement - val simpleNameExpression = when (element) { + val simpleNameExpression = when (val element = descriptor.psiElement) { is KtSimpleNameExpression -> element is KtDotQualifiedExpression -> element.selectorExpression as? KtSimpleNameExpression else -> null 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 f8f5436467e..f78ba707e3f 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteExperimentalCoroutinesInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteExperimentalCoroutinesInspection.kt @@ -31,28 +31,28 @@ internal class ObsoleteExperimentalCoroutinesInspection : ObsoleteCodeMigrationI override val fromVersion: LanguageVersion = LanguageVersion.KOTLIN_1_2 override val toVersion: LanguageVersion = LanguageVersion.KOTLIN_1_3 - override val problems = listOf( - ObsoleteTopLevelFunctionUsage( + override val problemReporters = listOf( + ObsoleteTopLevelFunctionUsageReporter( "buildSequence", "kotlin.coroutines.experimental.buildSequence", "kotlin.sequences.sequence" ), - ObsoleteTopLevelFunctionUsage( + ObsoleteTopLevelFunctionUsageReporter( "buildIterator", "kotlin.coroutines.experimental.buildIterator", "kotlin.sequences.iterator" ), - ObsoleteExtensionFunctionUsage( + ObsoleteExtensionFunctionUsageReporter( "resume", "kotlin.coroutines.experimental.Continuation.resume", "kotlin.coroutines.resume" ), - ObsoleteExtensionFunctionUsage( + ObsoleteExtensionFunctionUsageReporter( "resumeWithException", "kotlin.coroutines.experimental.Continuation.resumeWithException", "kotlin.coroutines.resumeWithException" ), - ObsoleteCoroutinesImportsUsage + ObsoleteCoroutinesImportsUsageReporter ) } @@ -91,9 +91,9 @@ internal fun fixesWithWholeProject(isOnTheFly: Boolean, fix: LocalQuickFix, whol return arrayOf(fix, wholeProjectFix) } -private class ObsoleteTopLevelFunctionUsage( +private class ObsoleteTopLevelFunctionUsageReporter( val textMarker: String, val oldFqName: String, val newFqName: String -) : ObsoleteCodeMigrationProblem { +) : ObsoleteCodeProblemReporter { override fun report(holder: ProblemsHolder, isOnTheFly: Boolean, simpleNameExpression: KtSimpleNameExpression): Boolean { if (simpleNameExpression.text != textMarker) return false @@ -127,9 +127,9 @@ private class ObsoleteTopLevelFunctionUsage( } } -private class ObsoleteExtensionFunctionUsage( +private class ObsoleteExtensionFunctionUsageReporter( val textMarker: String, val oldFqName: String, val newFqName: String -) : ObsoleteCodeMigrationProblem { +) : ObsoleteCodeProblemReporter { override fun report(holder: ProblemsHolder, isOnTheFly: Boolean, simpleNameExpression: KtSimpleNameExpression): Boolean { if (simpleNameExpression.text != textMarker) return false @@ -172,7 +172,7 @@ private class ObsoleteExtensionFunctionUsage( } } -private object ObsoleteCoroutinesImportsUsage : ObsoleteImportsUsage() { +private object ObsoleteCoroutinesImportsUsageReporter : ObsoleteImportsUsageReporter() { override val textMarker: String = "experimental" override val packageBindings: Map = mapOf( diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteKotlinJsPackagesInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteKotlinJsPackagesInspection.kt index d9804d38cd0..998d76c2e83 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteKotlinJsPackagesInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteKotlinJsPackagesInspection.kt @@ -13,9 +13,9 @@ internal class ObsoleteKotlinJsPackagesInspection : ObsoleteCodeMigrationInspect override val fromVersion: LanguageVersion = LanguageVersion.KOTLIN_1_3 override val toVersion: LanguageVersion = LanguageVersion.KOTLIN_1_4 - override val problems: List = listOf( - KotlinBrowserImportUsageProblem, - KotlinDomImportUsageProblem + override val problemReporters: List = listOf( + KotlinBrowserImportUsageReporter, + KotlinDomImportUsageReporter ) } @@ -24,30 +24,33 @@ private object ObsoleteKotlinJsPackagesUsagesInWholeProjectFix : ObsoleteCodeInW override fun getFamilyName(): String = KotlinBundle.message("obsolete.kotlin.js.packages.usage.in.whole.fix.family.name") } +private const val KOTLIN_BROWSER_PACKAGE = "kotlin.browser" +private const val KOTLIN_DOM_PACKAGE = "kotlin.dom" + private class ObsoleteKotlinBrowserUsageFix(delegate: ObsoleteCodeFix) : ObsoleteCodeFixDelegateQuickFix(delegate) { - override fun getFamilyName(): String = KotlinBundle.message("obsolete.kotlin.browser.usage.fix.family.name") + override fun getFamilyName(): String = KotlinBundle.message("obsolete.package.usage.fix.family.name", KOTLIN_BROWSER_PACKAGE) } private class ObsoleteKotlinDomUsageFix(delegate: ObsoleteCodeFix) : ObsoleteCodeFixDelegateQuickFix(delegate) { - override fun getFamilyName(): String = KotlinBundle.message("obsolete.kotlin.dom.usage.fix.family.name") + override fun getFamilyName(): String = KotlinBundle.message("obsolete.package.usage.fix.family.name", KOTLIN_DOM_PACKAGE) } -private object KotlinBrowserImportUsageProblem : ObsoleteImportsUsage() { +private object KotlinBrowserImportUsageReporter : ObsoleteImportsUsageReporter() { override val textMarker: String = "browser" - override val packageBindings: Map = mapOf("kotlin.browser" to "kotlinx.browser") + override val packageBindings: Map = mapOf(KOTLIN_BROWSER_PACKAGE to "kotlinx.browser") override val wholeProjectFix: LocalQuickFix = ObsoleteKotlinJsPackagesUsagesInWholeProjectFix - override fun problemMessage(): String = KotlinBundle.message("kotlin.browser.usages.are.obsolete.since.1.3") + override fun problemMessage(): String = KotlinBundle.message("package.usages.are.obsolete.since.1.4", KOTLIN_BROWSER_PACKAGE) override fun wrapFix(fix: ObsoleteCodeFix): LocalQuickFix = ObsoleteKotlinBrowserUsageFix(fix) } -private object KotlinDomImportUsageProblem : ObsoleteImportsUsage() { +private object KotlinDomImportUsageReporter : ObsoleteImportsUsageReporter() { override val textMarker: String = "dom" - override val packageBindings: Map = mapOf("kotlin.dom" to "kotlinx.dom") + override val packageBindings: Map = mapOf(KOTLIN_DOM_PACKAGE to "kotlinx.dom") override val wholeProjectFix: LocalQuickFix = ObsoleteKotlinJsPackagesUsagesInWholeProjectFix - override fun problemMessage(): String = KotlinBundle.message("kotlin.dom.usages.are.obsolete.since.1.3") + override fun problemMessage(): String = KotlinBundle.message("package.usages.are.obsolete.since.1.4", KOTLIN_DOM_PACKAGE) override fun wrapFix(fix: ObsoleteCodeFix): LocalQuickFix = ObsoleteKotlinDomUsageFix(fix) }