From 615a2fbbc111ca1222331296bcd296fab302d82f Mon Sep 17 00:00:00 2001 From: Nikita Bobko Date: Tue, 13 Dec 2022 18:41:58 +0100 Subject: [PATCH] Make UsePropertyAccessSyntax to report references to Java getters ^KTIJ-23892 Fixed Review: https://jetbrains.team/p/ij/reviews/100206 I can afford to use `FrontendInternals` API because this API is already used in this intention GitOrigin-RevId: 5a24ea37951f679a01bc134a88b69313ca868b3a --- .../UsePropertyAccessSyntaxIntention.kt | 111 +++++++++++++----- .../intentions/K1IntentionTestGenerated.java | 25 ++++ .../referenceGetter0.1.java | 3 + .../referenceGetter0.1.java.after | 3 + .../referenceGetter0.kt | 6 + .../referenceGetter0.kt.after | 6 + .../referenceGetter1.1.java | 3 + .../referenceGetter1.1.java.after | 3 + .../referenceGetter1.kt | 6 + .../referenceGetter1.kt.after | 6 + .../referenceGetterOldLv.1.java | 3 + .../referenceGetterOldLv.kt | 7 ++ .../referenceIsGetter.1.java | 3 + .../referenceIsGetter.kt | 7 ++ .../referenceSetter.1.java | 4 + .../referenceSetter.kt | 7 ++ .../J2KPostProcessingRegistrarImpl.kt | 2 +- 17 files changed, 175 insertions(+), 30 deletions(-) create mode 100644 plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.1.java create mode 100644 plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.1.java.after create mode 100644 plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.kt create mode 100644 plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.kt.after create mode 100644 plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.1.java create mode 100644 plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.1.java.after create mode 100644 plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.kt create mode 100644 plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.kt.after create mode 100644 plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetterOldLv.1.java create mode 100644 plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetterOldLv.kt create mode 100644 plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceIsGetter.1.java create mode 100644 plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceIsGetter.kt create mode 100644 plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceSetter.1.java create mode 100644 plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceSetter.kt diff --git a/plugins/kotlin/idea/src/org/jetbrains/kotlin/idea/intentions/UsePropertyAccessSyntaxIntention.kt b/plugins/kotlin/idea/src/org/jetbrains/kotlin/idea/intentions/UsePropertyAccessSyntaxIntention.kt index 458c1476afac..874f47aae37c 100644 --- a/plugins/kotlin/idea/src/org/jetbrains/kotlin/idea/intentions/UsePropertyAccessSyntaxIntention.kt +++ b/plugins/kotlin/idea/src/org/jetbrains/kotlin/idea/intentions/UsePropertyAccessSyntaxIntention.kt @@ -10,20 +10,23 @@ import com.intellij.openapi.util.Key import com.intellij.profile.codeInspection.InspectionProjectProfileManager import com.intellij.psi.PsiElement import org.jdom.Element +import org.jetbrains.kotlin.config.LanguageFeature import org.jetbrains.kotlin.descriptors.CallableDescriptor import org.jetbrains.kotlin.descriptors.FunctionDescriptor import org.jetbrains.kotlin.diagnostics.Severity import org.jetbrains.kotlin.idea.FrontendInternals +import org.jetbrains.kotlin.idea.base.projectStructure.languageVersionSettings +import org.jetbrains.kotlin.idea.base.psi.copied +import org.jetbrains.kotlin.idea.base.psi.replaced import org.jetbrains.kotlin.idea.base.resources.KotlinBundle import org.jetbrains.kotlin.idea.caches.resolve.analyzeInContext import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade +import org.jetbrains.kotlin.idea.caches.resolve.resolveToCall import org.jetbrains.kotlin.idea.caches.resolve.safeAnalyzeNonSourceRootCode +import org.jetbrains.kotlin.idea.codeinsight.api.classic.inspections.IntentionBasedInspection +import org.jetbrains.kotlin.idea.codeinsight.api.classic.intentions.SelfTargetingOffsetIndependentIntention import org.jetbrains.kotlin.idea.configuration.ui.NotPropertyListPanel import org.jetbrains.kotlin.idea.core.NotPropertiesService -import org.jetbrains.kotlin.idea.base.psi.copied -import org.jetbrains.kotlin.idea.base.psi.replaced -import org.jetbrains.kotlin.idea.codeinsight.api.classic.intentions.SelfTargetingOffsetIndependentIntention -import org.jetbrains.kotlin.idea.codeinsight.api.classic.inspections.IntentionBasedInspection import org.jetbrains.kotlin.idea.resolve.ResolutionFacade import org.jetbrains.kotlin.idea.resolve.dataFlowValueFactory import org.jetbrains.kotlin.idea.resolve.frontendService @@ -63,7 +66,7 @@ import org.jetbrains.kotlin.utils.KotlinExceptionWithAttachments import javax.swing.JComponent @Suppress("DEPRECATION") -class UsePropertyAccessSyntaxInspection : IntentionBasedInspection(UsePropertyAccessSyntaxIntention::class), +class UsePropertyAccessSyntaxInspection : IntentionBasedInspection(UsePropertyAccessSyntaxIntention::class), CleanupLocalInspectionTool { val fqNameList = NotPropertiesService.DEFAULT.map(::FqNameUnsafe).toMutableList() @@ -89,17 +92,22 @@ class UsePropertyAccessSyntaxInspection : IntentionBasedInspection KotlinBundle.message("use.of.getter.method.instead.of.property.access.syntax") - 1 -> KotlinBundle.message("use.of.setter.method.instead.of.property.access.syntax") - else -> error("getter or setter arg length can't be !in 0..1") - } - } + override fun inspectionProblemText(element: KtExpression): String? = + element.callOrReferenceOrNull( + { + when (it.valueArguments.size) { + 0 -> KotlinBundle.message("use.of.getter.method.instead.of.property.access.syntax") + 1 -> KotlinBundle.message("use.of.setter.method.instead.of.property.access.syntax") + else -> error("getter or setter arg length can't be !in 0..1") + } + }, + { + KotlinBundle.message("use.of.getter.method.instead.of.property.access.syntax") + } + ) } class NotPropertiesServiceImpl(private val project: Project) : NotPropertiesService { @@ -114,26 +122,61 @@ class NotPropertiesServiceImpl(private val project: Project) : NotPropertiesServ } } -class UsePropertyAccessSyntaxIntention : SelfTargetingOffsetIndependentIntention( - KtCallExpression::class.java, +/** + * Affected tests: + * [org.jetbrains.kotlin.idea.intentions.K1IntentionTestGenerated.UsePropertyAccessSyntax] + * [org.jetbrains.kotlin.idea.inspections.LocalInspectionTestGenerated.UsePropertyAccessSyntax] + * [org.jetbrains.kotlin.idea.inspections.MultiFileLocalInspectionTestGenerated] + */ +class UsePropertyAccessSyntaxIntention : SelfTargetingOffsetIndependentIntention( + KtExpression::class.java, KotlinBundle.lazyMessage("use.property.access.syntax") ) { - override fun isApplicableTo(element: KtCallExpression): Boolean = detectPropertyNameToUse(element) != null + override fun isApplicableTo(element: KtExpression): Boolean = + element.callOrReferenceOrNull(::detectPropertyNameToUseForCall, ::detectPropertyNameToUseForReference) != null - override fun applyTo(element: KtCallExpression, editor: Editor?) { - val propertyName = detectPropertyNameToUse(element) ?: return + override fun applyTo(element: KtExpression, editor: Editor?) { + val propertyName = element.callOrReferenceOrNull(::detectPropertyNameToUseForCall, ::detectPropertyNameToUseForReference) ?: return runWriteActionIfPhysical(element) { applyTo(element, propertyName, reformat = true) } } - fun applyTo(element: KtCallExpression, propertyName: Name, reformat: Boolean): KtExpression = when (element.valueArguments.size) { - 0 -> replaceWithPropertyGet(element, propertyName) - 1 -> replaceWithPropertySet(element, propertyName, reformat) - else -> error("More than one argument in call to accessor") + fun applyTo(element: KtExpression, propertyName: Name, reformat: Boolean): KtExpression = + element.callOrReferenceOrNull( + { + when (it.valueArguments.size) { + 0 -> replaceWithPropertyGet(it, propertyName) + 1 -> replaceWithPropertySet(it, propertyName, reformat) + else -> error("More than one argument in call to accessor") + } + }, + { + replaceWithPropertyGet(it.callableReference, propertyName) + } + ) ?: error("Can't parse $element (${element::class})") + + private fun detectPropertyNameToUseForReference(referenceExpression: KtCallableReferenceExpression): Name? { + if (!referenceExpression.languageVersionSettings.supportsFeature(LanguageFeature.ReferencesToSyntheticJavaProperties)) { + return null + } + if (!referenceExpression.callableReference.getReferencedName().startsWith("get")) { + // Suggest to convert only getters. Keep setters and is-getters untouched + // Don't suggest replacing setters because setters and property references have different types + // Don't suggest replacing is-getters because is-getter method reference and is-getter property references have the same syntax + return null + } + + return referenceExpression.callableReference.resolveToCall()?.resultingDescriptor + ?.let { it as? FunctionDescriptor } + ?.let { + @OptIn(FrontendInternals::class) + findSyntheticProperty(it, referenceExpression.getResolutionFacade().getFrontendService(SyntheticScopes::class.java)) + } + ?.name } - fun detectPropertyNameToUse(callExpression: KtCallExpression): Name? { + fun detectPropertyNameToUseForCall(callExpression: KtCallExpression): Name? { if (callExpression.getQualifiedExpressionForSelector() ?.receiverExpression is KtSuperExpression ) return null // cannot call extensions on "super" @@ -259,9 +302,9 @@ class UsePropertyAccessSyntaxIntention : SelfTargetingOffsetIndependentIntention return null } - private fun replaceWithPropertyGet(callExpression: KtCallExpression, propertyName: Name): KtExpression { - val newExpression = KtPsiFactory(callExpression.project).createExpression(propertyName.render()) - return callExpression.replaced(newExpression) + private fun replaceWithPropertyGet(oldElement: KtElement, propertyName: Name): KtExpression { + val newExpression = KtPsiFactory(oldElement.project).createExpression(propertyName.render()) + return oldElement.replaced(newExpression) } private fun KtCallExpression.convertExpressionBodyToBlockBodyIfPossible(): KtCallExpression { @@ -313,4 +356,14 @@ private val commonGetterLikePrefixes: Set = setOf( "^getOr[A-Z]".toRegex(), "^getAnd[A-Z]".toRegex(), "^getIf[A-Z]".toRegex(), -) \ No newline at end of file +) + +private inline fun KtExpression.callOrReferenceOrNull( + call: (KtCallExpression) -> T, + reference: (KtCallableReferenceExpression) -> T +): T? = + when { + this is KtCallExpression -> call(this) + this is KtSimpleNameExpression && parent is KtCallableReferenceExpression -> reference(parent as KtCallableReferenceExpression) + else -> null + } diff --git a/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/intentions/K1IntentionTestGenerated.java b/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/intentions/K1IntentionTestGenerated.java index 02cc855b4ef8..228336e45952 100644 --- a/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/intentions/K1IntentionTestGenerated.java +++ b/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/intentions/K1IntentionTestGenerated.java @@ -17707,6 +17707,31 @@ public abstract class K1IntentionTestGenerated extends AbstractK1IntentionTest { runTest("testData/intentions/usePropertyAccessSyntax/propertyTypeIsMoreSpecific2.kt"); } + @TestMetadata("referenceGetter0.kt") + public void testReferenceGetter0() throws Exception { + runTest("testData/intentions/usePropertyAccessSyntax/referenceGetter0.kt"); + } + + @TestMetadata("referenceGetter1.kt") + public void testReferenceGetter1() throws Exception { + runTest("testData/intentions/usePropertyAccessSyntax/referenceGetter1.kt"); + } + + @TestMetadata("referenceGetterOldLv.kt") + public void testReferenceGetterOldLv() throws Exception { + runTest("testData/intentions/usePropertyAccessSyntax/referenceGetterOldLv.kt"); + } + + @TestMetadata("referenceIsGetter.kt") + public void testReferenceIsGetter() throws Exception { + runTest("testData/intentions/usePropertyAccessSyntax/referenceIsGetter.kt"); + } + + @TestMetadata("referenceSetter.kt") + public void testReferenceSetter() throws Exception { + runTest("testData/intentions/usePropertyAccessSyntax/referenceSetter.kt"); + } + @TestMetadata("set.kt") public void testSet() throws Exception { runTest("testData/intentions/usePropertyAccessSyntax/set.kt"); diff --git a/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.1.java b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.1.java new file mode 100644 index 000000000000..907ac1c9a532 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.1.java @@ -0,0 +1,3 @@ +public class Foo { + public int getFoo() {return 1;} +} diff --git a/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.1.java.after b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.1.java.after new file mode 100644 index 000000000000..907ac1c9a532 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.1.java.after @@ -0,0 +1,3 @@ +public class Foo { + public int getFoo() {return 1;} +} diff --git a/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.kt b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.kt new file mode 100644 index 000000000000..ab39063480d6 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.kt @@ -0,0 +1,6 @@ +// COMPILER_ARGUMENTS: -XXLanguage:+ReferencesToSyntheticJavaProperties +fun main() { + suppressUnused(Foo()::getFoo) +} + +fun suppressUnused(foo: () -> Int): Any = foo diff --git a/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.kt.after b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.kt.after new file mode 100644 index 000000000000..e215941e7f00 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter0.kt.after @@ -0,0 +1,6 @@ +// COMPILER_ARGUMENTS: -XXLanguage:+ReferencesToSyntheticJavaProperties +fun main() { + suppressUnused(Foo()::foo) +} + +fun suppressUnused(foo: () -> Int): Any = foo diff --git a/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.1.java b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.1.java new file mode 100644 index 000000000000..907ac1c9a532 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.1.java @@ -0,0 +1,3 @@ +public class Foo { + public int getFoo() {return 1;} +} diff --git a/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.1.java.after b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.1.java.after new file mode 100644 index 000000000000..907ac1c9a532 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.1.java.after @@ -0,0 +1,3 @@ +public class Foo { + public int getFoo() {return 1;} +} diff --git a/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.kt b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.kt new file mode 100644 index 000000000000..d060272a4c69 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.kt @@ -0,0 +1,6 @@ +// COMPILER_ARGUMENTS: -XXLanguage:+ReferencesToSyntheticJavaProperties +fun main() { + suppressUnused(Foo::getFoo) +} + +fun suppressUnused(foo: (Foo) -> Int): Any = foo diff --git a/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.kt.after b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.kt.after new file mode 100644 index 000000000000..f5d09669bba7 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetter1.kt.after @@ -0,0 +1,6 @@ +// COMPILER_ARGUMENTS: -XXLanguage:+ReferencesToSyntheticJavaProperties +fun main() { + suppressUnused(Foo::foo) +} + +fun suppressUnused(foo: (Foo) -> Int): Any = foo diff --git a/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetterOldLv.1.java b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetterOldLv.1.java new file mode 100644 index 000000000000..907ac1c9a532 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetterOldLv.1.java @@ -0,0 +1,3 @@ +public class Foo { + public int getFoo() {return 1;} +} diff --git a/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetterOldLv.kt b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetterOldLv.kt new file mode 100644 index 000000000000..4d2cdc5957b5 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceGetterOldLv.kt @@ -0,0 +1,7 @@ +// COMPILER_ARGUMENTS: -XXLanguage:-ReferencesToSyntheticJavaProperties +// IS_APPLICABLE: false +fun main() { + suppressUnused(Foo::getFoo) +} + +fun suppressUnused(foo: (Foo) -> Int): Any = foo diff --git a/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceIsGetter.1.java b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceIsGetter.1.java new file mode 100644 index 000000000000..b01da087e41a --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceIsGetter.1.java @@ -0,0 +1,3 @@ +public class Foo { + public boolean isFoo() {return true;} +} diff --git a/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceIsGetter.kt b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceIsGetter.kt new file mode 100644 index 000000000000..a04da6bc4eb1 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceIsGetter.kt @@ -0,0 +1,7 @@ +// COMPILER_ARGUMENTS: -XXLanguage:+ReferencesToSyntheticJavaProperties +// IS_APPLICABLE: false +fun main() { + suppressUnused(Foo::isFoo) +} + +fun suppressUnused(foo: (Foo) -> Boolean): Any = foo diff --git a/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceSetter.1.java b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceSetter.1.java new file mode 100644 index 000000000000..0a544635ca3e --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceSetter.1.java @@ -0,0 +1,4 @@ +public class Foo { + public int getFoo() {return 1;} + public void setFoo(int foo) {} +} diff --git a/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceSetter.kt b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceSetter.kt new file mode 100644 index 000000000000..62430ab36634 --- /dev/null +++ b/plugins/kotlin/idea/tests/testData/intentions/usePropertyAccessSyntax/referenceSetter.kt @@ -0,0 +1,7 @@ +// COMPILER_ARGUMENTS: -XXLanguage:+ReferencesToSyntheticJavaProperties +// IS_APPLICABLE: false +fun main() { + suppressUnused(Foo::setFoo) +} + +fun suppressUnused(foo: (Foo, Int) -> Unit): Any = foo diff --git a/plugins/kotlin/j2k/old-post-processing/src/org/jetbrains/kotlin/idea/j2k/old/post/processing/J2KPostProcessingRegistrarImpl.kt b/plugins/kotlin/j2k/old-post-processing/src/org/jetbrains/kotlin/idea/j2k/old/post/processing/J2KPostProcessingRegistrarImpl.kt index edf3317c0cb2..37ddf52f2576 100644 --- a/plugins/kotlin/j2k/old-post-processing/src/org/jetbrains/kotlin/idea/j2k/old/post/processing/J2KPostProcessingRegistrarImpl.kt +++ b/plugins/kotlin/j2k/old-post-processing/src/org/jetbrains/kotlin/idea/j2k/old/post/processing/J2KPostProcessingRegistrarImpl.kt @@ -274,7 +274,7 @@ internal class J2KPostProcessingRegistrarImpl : J2KPostProcessingRegistrar { override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? { if (element !is KtCallExpression) return null - val propertyName = intention.detectPropertyNameToUse(element) ?: return null + val propertyName = intention.detectPropertyNameToUseForCall(element) ?: return null return { intention.applyTo(element, propertyName, reformat = true) } } }