From 53373b66b9bdc386fc2eeeb0e78c462df7da509b Mon Sep 17 00:00:00 2001 From: Nikolay Krasko Date: Mon, 24 Jul 2017 16:04:22 +0300 Subject: [PATCH] Do not replace in alias when constructor has special pattern (KT-19202) #KT-19202 Fixed --- .../DeprecatedSymbolUsageFixBase.kt | 68 +++++++++++++++++-- .../classUsages/inAliasSimple.kt | 10 +++ .../classUsages/inAliasSimple.kt.after | 10 +++ ...nReplaceForConstructorForAliasArguments.kt | 13 ++++ ...ceForConstructorForAliasArguments.kt.after | 13 ++++ ...AliasWithOwnReplaceForConstructorUnused.kt | 10 +++ ...ithOwnReplaceForConstructorUnused.kt.after | 10 +++ ...inAliasWithOwnReplaceForConstructorUsed.kt | 14 ++++ ...sWithOwnReplaceForConstructorUsed.kt.after | 14 ++++ ...OwnReplaceForSecondaryConstructorUnused.kt | 12 ++++ ...laceForSecondaryConstructorUnused.kt.after | 12 ++++ ...thOwnReplaceForSecondaryConstructorUsed.kt | 14 ++++ ...eplaceForSecondaryConstructorUsed.kt.after | 14 ++++ .../idea/quickfix/QuickFixTestGenerated.java | 36 ++++++++++ 14 files changed, 245 insertions(+), 5 deletions(-) create mode 100644 idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasSimple.kt create mode 100644 idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasSimple.kt.after create mode 100644 idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorForAliasArguments.kt create mode 100644 idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorForAliasArguments.kt.after create mode 100644 idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUnused.kt create mode 100644 idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUnused.kt.after create mode 100644 idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUsed.kt create mode 100644 idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUsed.kt.after create mode 100644 idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUnused.kt create mode 100644 idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUnused.kt.after create mode 100644 idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUsed.kt create mode 100644 idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUsed.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/replaceWith/DeprecatedSymbolUsageFixBase.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/replaceWith/DeprecatedSymbolUsageFixBase.kt index 2a7cf6a6ea1..8ff07b6186c 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/replaceWith/DeprecatedSymbolUsageFixBase.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/replaceWith/DeprecatedSymbolUsageFixBase.kt @@ -16,12 +16,17 @@ package org.jetbrains.kotlin.idea.quickfix.replaceWith +import com.intellij.codeInsight.hint.HintManager import com.intellij.openapi.editor.Editor import com.intellij.openapi.project.Project +import com.intellij.psi.PsiElement import com.intellij.psi.PsiFile +import com.intellij.psi.search.GlobalSearchScope +import com.intellij.psi.search.searches.ReferencesSearch import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor +import org.jetbrains.kotlin.descriptors.impl.TypeAliasConstructorDescriptor import org.jetbrains.kotlin.diagnostics.Diagnostic import org.jetbrains.kotlin.diagnostics.DiagnosticFactory import org.jetbrains.kotlin.diagnostics.Errors @@ -32,15 +37,19 @@ import org.jetbrains.kotlin.idea.codeInliner.UsageReplacementStrategy import org.jetbrains.kotlin.idea.core.OptionalParametersHelper import org.jetbrains.kotlin.idea.quickfix.KotlinQuickFixAction import org.jetbrains.kotlin.idea.references.mainReference -import org.jetbrains.kotlin.psi.KtConstructorCalleeExpression -import org.jetbrains.kotlin.psi.KtFile -import org.jetbrains.kotlin.psi.KtSimpleNameExpression +import org.jetbrains.kotlin.idea.references.resolveMainReferenceToDescriptors +import org.jetbrains.kotlin.idea.search.restrictToKotlinSources +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType +import org.jetbrains.kotlin.psi.psiUtil.isCallee +import org.jetbrains.kotlin.renderer.DescriptorRenderer import org.jetbrains.kotlin.resolve.annotations.argumentValue import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall import org.jetbrains.kotlin.resolve.calls.model.isReallySuccess import org.jetbrains.kotlin.resolve.constants.StringValue import org.jetbrains.kotlin.resolve.descriptorUtil.hasDefaultValue import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode +import org.jetbrains.kotlin.util.constructors //TODO: different replacements for property accessors @@ -58,7 +67,7 @@ abstract class DeprecatedSymbolUsageFixBase( final override fun invoke(project: Project, editor: Editor?, file: KtFile) { val element = element ?: return - val strategy = buildUsageReplacementStrategy(element, replaceWith, recheckAnnotation = false)!! + val strategy = buildUsageReplacementStrategy(element, replaceWith, recheckAnnotation = false, editor = editor) ?: return invoke(strategy, project, editor) } @@ -112,7 +121,7 @@ abstract class DeprecatedSymbolUsageFixBase( return Data(nameExpression, replacement, descriptor) } - private fun buildUsageReplacementStrategy(element: KtSimpleNameExpression, replaceWith: ReplaceWith, recheckAnnotation: Boolean): UsageReplacementStrategy? { + private fun buildUsageReplacementStrategy(element: KtSimpleNameExpression, replaceWith: ReplaceWith, recheckAnnotation: Boolean, editor: Editor? = null): UsageReplacementStrategy? { val resolutionFacade = element.getResolutionFacade() val bindingContext = resolutionFacade.analyze(element, BodyResolveMode.PARTIAL) var target = element.mainReference.resolveToDescriptors(bindingContext).singleOrNull() ?: return null @@ -138,6 +147,26 @@ abstract class DeprecatedSymbolUsageFixBase( val replacementType = ReplaceWithAnnotationAnalyzer.analyzeClassifierReplacement(replaceWith, target, resolutionFacade) return when { replacementType != null -> { + if (editor != null) { + val typeAlias = element + .getStrictParentOfType() + ?.getStrictParentOfType() + ?.getStrictParentOfType() + if (typeAlias != null) { + val usedConstructorWithOwnReplaceWith = usedConstructorsWithOwnReplaceWith( + element.project, target, typeAlias) + + if (usedConstructorWithOwnReplaceWith != null) { + val constructorStr = DescriptorRenderer.ONLY_NAMES_WITH_SHORT_TYPES.render(usedConstructorWithOwnReplaceWith) + HintManager.getInstance().showErrorHint( + editor, + "There is own 'ReplaceWith' on '$constructorStr' that is used through this alias. " + + "Please replace usages first.") + return null + } + } + } + //TODO: check that it's really resolved and is not an object otherwise it can be expression as well ClassUsageReplacementStrategy(replacementType, null, element.project) } @@ -153,5 +182,34 @@ abstract class DeprecatedSymbolUsageFixBase( else -> return null } } + + private fun usedConstructorsWithOwnReplaceWith( + project: Project, classifier: ClassifierDescriptorWithTypeParameters, typeAlias: PsiElement): ConstructorDescriptor? { + val specialReplaceWithForConstructor = classifier.constructors.filter { + DeprecatedSymbolUsageFixBase.fetchReplaceWithPattern(it, project) != null + }.toSet() + + if (specialReplaceWithForConstructor.isEmpty()) { + return null + } + + val searchAliasConstructorUsagesScope = GlobalSearchScope.allScope(project).restrictToKotlinSources() + ReferencesSearch.search(typeAlias, searchAliasConstructorUsagesScope).find { reference -> + val element = reference.element + + if (element is KtSimpleNameExpression && element.isCallee()) { + val aliasConstructors = element.resolveMainReferenceToDescriptors().filterIsInstance() + for (referenceConstructor in aliasConstructors) { + if (referenceConstructor.underlyingConstructorDescriptor in specialReplaceWithForConstructor) { + return referenceConstructor.underlyingConstructorDescriptor + } + } + } + + false + } + + return null + } } } diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasSimple.kt b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasSimple.kt new file mode 100644 index 00000000000..ad577091a0b --- /dev/null +++ b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasSimple.kt @@ -0,0 +1,10 @@ +// "Replace with 'NewClass'" "true" + +@Deprecated("", ReplaceWith("NewClass")) +class OldClass + +class NewClass + +typealias Old = OldClass + +val a: Old = Old() \ No newline at end of file diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasSimple.kt.after b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasSimple.kt.after new file mode 100644 index 00000000000..8512e752604 --- /dev/null +++ b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasSimple.kt.after @@ -0,0 +1,10 @@ +// "Replace with 'NewClass'" "true" + +@Deprecated("", ReplaceWith("NewClass")) +class OldClass + +class NewClass + +typealias Old = NewClass + +val a: Old = Old() \ No newline at end of file diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorForAliasArguments.kt b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorForAliasArguments.kt new file mode 100644 index 00000000000..9741254a40f --- /dev/null +++ b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorForAliasArguments.kt @@ -0,0 +1,13 @@ +// "Replace with 'B'" "true" + +class OldClass + +@Deprecated("Bad!", ReplaceWith("B")) +class A @Deprecated("Bad!", ReplaceWith("B()")) constructor() + +class B + +typealias Old = OldClass<A> + +val o: Old = Old() +val a = A() // Usage of A() \ No newline at end of file diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorForAliasArguments.kt.after b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorForAliasArguments.kt.after new file mode 100644 index 00000000000..adf782a5f0e --- /dev/null +++ b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorForAliasArguments.kt.after @@ -0,0 +1,13 @@ +// "Replace with 'B'" "true" + +class OldClass + +@Deprecated("Bad!", ReplaceWith("B")) +class A @Deprecated("Bad!", ReplaceWith("B()")) constructor() + +class B + +typealias Old = OldClass<B> + +val o: Old = Old() +val a = A() // Usage of A() \ No newline at end of file diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUnused.kt b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUnused.kt new file mode 100644 index 00000000000..e516853c853 --- /dev/null +++ b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUnused.kt @@ -0,0 +1,10 @@ +// "Replace with 'NewClass'" "true" + +@Deprecated("", ReplaceWith("NewClass")) +class OldClass @Deprecated("", ReplaceWith("NewClass(12)")) constructor() + +class NewClass(p: Int) + +typealias Old = OldClass + +val a: Old = null!! \ No newline at end of file diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUnused.kt.after b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUnused.kt.after new file mode 100644 index 00000000000..6767bfb85ee --- /dev/null +++ b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUnused.kt.after @@ -0,0 +1,10 @@ +// "Replace with 'NewClass'" "true" + +@Deprecated("", ReplaceWith("NewClass")) +class OldClass @Deprecated("", ReplaceWith("NewClass(12)")) constructor() + +class NewClass(p: Int) + +typealias Old = NewClass + +val a: Old = null!! \ No newline at end of file diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUsed.kt b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUsed.kt new file mode 100644 index 00000000000..60d282488b8 --- /dev/null +++ b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUsed.kt @@ -0,0 +1,14 @@ +// "Replace with 'NewClass'" "true" +// SHOULD_BE_AVAILABLE_AFTER_EXECUTION + +package aa + +@Deprecated("", ReplaceWith("NewClass")) +class OldClass @Deprecated("", ReplaceWith("NewClass(12)")) constructor() + +class NewClass(p: Int) + +// No apply, error instead +typealias Old = OldClass + +val a: Old = aa.Old() \ No newline at end of file diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUsed.kt.after b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUsed.kt.after new file mode 100644 index 00000000000..60d282488b8 --- /dev/null +++ b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUsed.kt.after @@ -0,0 +1,14 @@ +// "Replace with 'NewClass'" "true" +// SHOULD_BE_AVAILABLE_AFTER_EXECUTION + +package aa + +@Deprecated("", ReplaceWith("NewClass")) +class OldClass @Deprecated("", ReplaceWith("NewClass(12)")) constructor() + +class NewClass(p: Int) + +// No apply, error instead +typealias Old = OldClass + +val a: Old = aa.Old() \ No newline at end of file diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUnused.kt b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUnused.kt new file mode 100644 index 00000000000..99344dadbc8 --- /dev/null +++ b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUnused.kt @@ -0,0 +1,12 @@ +// "Replace with 'NewClass'" "true" + +@Deprecated("", ReplaceWith("NewClass")) +class OldClass constructor() { + @Deprecated("", ReplaceWith("NewClass(12)")) constructor(i: Int): this() +} + +class NewClass(p: Int = 0) + +typealias Old = OldClass + +val a: Old = Old() \ No newline at end of file diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUnused.kt.after b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUnused.kt.after new file mode 100644 index 00000000000..c4f3df7b822 --- /dev/null +++ b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUnused.kt.after @@ -0,0 +1,12 @@ +// "Replace with 'NewClass'" "true" + +@Deprecated("", ReplaceWith("NewClass")) +class OldClass constructor() { + @Deprecated("", ReplaceWith("NewClass(12)")) constructor(i: Int): this() +} + +class NewClass(p: Int = 0) + +typealias Old = NewClass + +val a: Old = Old() \ No newline at end of file diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUsed.kt b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUsed.kt new file mode 100644 index 00000000000..24833c09b7d --- /dev/null +++ b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUsed.kt @@ -0,0 +1,14 @@ +// "Replace with 'NewClass'" "true" +// SHOULD_BE_AVAILABLE_AFTER_EXECUTION + +@Deprecated("", ReplaceWith("NewClass")) +class OldClass constructor() { + @Deprecated("", ReplaceWith("NewClass(12)")) constructor(i: Int): this() +} + +class NewClass(p: Int) + +// No apply, error instead +typealias Old = OldClass + +val a: Old = Old(1) \ No newline at end of file diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUsed.kt.after b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUsed.kt.after new file mode 100644 index 00000000000..24833c09b7d --- /dev/null +++ b/idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUsed.kt.after @@ -0,0 +1,14 @@ +// "Replace with 'NewClass'" "true" +// SHOULD_BE_AVAILABLE_AFTER_EXECUTION + +@Deprecated("", ReplaceWith("NewClass")) +class OldClass constructor() { + @Deprecated("", ReplaceWith("NewClass(12)")) constructor(i: Int): this() +} + +class NewClass(p: Int) + +// No apply, error instead +typealias Old = OldClass + +val a: Old = Old(1) \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java index cdfbfda3cd3..f8427b87bd2 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java @@ -5026,6 +5026,42 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest { doTest(fileName); } + @TestMetadata("inAliasSimple.kt") + public void testInAliasSimple() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasSimple.kt"); + doTest(fileName); + } + + @TestMetadata("inAliasWithOwnReplaceForConstructorForAliasArguments.kt") + public void testInAliasWithOwnReplaceForConstructorForAliasArguments() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorForAliasArguments.kt"); + doTest(fileName); + } + + @TestMetadata("inAliasWithOwnReplaceForConstructorUnused.kt") + public void testInAliasWithOwnReplaceForConstructorUnused() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUnused.kt"); + doTest(fileName); + } + + @TestMetadata("inAliasWithOwnReplaceForConstructorUsed.kt") + public void testInAliasWithOwnReplaceForConstructorUsed() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForConstructorUsed.kt"); + doTest(fileName); + } + + @TestMetadata("inAliasWithOwnReplaceForSecondaryConstructorUnused.kt") + public void testInAliasWithOwnReplaceForSecondaryConstructorUnused() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUnused.kt"); + doTest(fileName); + } + + @TestMetadata("inAliasWithOwnReplaceForSecondaryConstructorUsed.kt") + public void testInAliasWithOwnReplaceForSecondaryConstructorUsed() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inAliasWithOwnReplaceForSecondaryConstructorUsed.kt"); + doTest(fileName); + } + @TestMetadata("inTypeArgument.kt") public void testInTypeArgument() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/deprecatedSymbolUsage/classUsages/inTypeArgument.kt");