From a1d760fc36e2dff835c52d6a147b32902f3eda4d Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Tue, 19 Jan 2016 15:27:41 +0300 Subject: [PATCH] KT-10631 Consider creating a synthetic property even when the setter returns 'this' #KT-10631 Fixed --- .../synthetic/JavaSyntheticPropertiesScope.kt | 1 - .../javaProperties/FalseSetters.kt | 14 +++++--------- .../javaProperties/FalseSetters.txt | 10 ++++------ .../javaProperties/GetterAndSetter.kt | 10 +++++++--- .../javaProperties/GetterAndSetter.txt | 6 ++++-- .../idea/codeInsight/ReferenceVariantsHelper.kt | 8 ++++++-- ...SyntheticExtensionNonVoidSetter.dependency.java | 5 +++++ .../SyntheticExtensionNonVoidSetter.kt | 7 +++++++ .../MultiFileJvmBasicCompletionTestGenerated.java | 6 ++++++ .../intentions/UsePropertyAccessSyntaxIntention.kt | 10 +++++++++- .../usePropertyAccessSyntax/nonVoidSetter1.1.java | 7 +++++++ .../usePropertyAccessSyntax/nonVoidSetter1.kt | 7 +++++++ .../usePropertyAccessSyntax/nonVoidSetter2.1.java | 4 ++++ .../usePropertyAccessSyntax/nonVoidSetter2.kt | 5 +++++ .../idea/intentions/IntentionTestGenerated.java | 12 ++++++++++++ 15 files changed, 88 insertions(+), 24 deletions(-) create mode 100644 idea/idea-completion/testData/basic/multifile/SyntheticExtensionNonVoidSetter/SyntheticExtensionNonVoidSetter.dependency.java create mode 100644 idea/idea-completion/testData/basic/multifile/SyntheticExtensionNonVoidSetter/SyntheticExtensionNonVoidSetter.kt create mode 100644 idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter1.1.java create mode 100644 idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter1.kt create mode 100644 idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter2.1.java create mode 100644 idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter2.kt diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/JavaSyntheticPropertiesScope.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/JavaSyntheticPropertiesScope.kt index b4f3d263746..cf7b4bee7fe 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/JavaSyntheticPropertiesScope.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/JavaSyntheticPropertiesScope.kt @@ -155,7 +155,6 @@ class JavaSyntheticPropertiesScope(storageManager: StorageManager, private val l return parameter.varargElementType == null && descriptor.typeParameters.isEmpty() - && descriptor.returnType?.let { it.isUnit() } ?: false && descriptor.visibility.isVisibleOutside() } diff --git a/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/FalseSetters.kt b/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/FalseSetters.kt index 1c46a2c1073..3361643967c 100644 --- a/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/FalseSetters.kt +++ b/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/FalseSetters.kt @@ -4,8 +4,7 @@ fun foo(javaClass: JavaClass) { javaClass.something2++ javaClass.something3++ javaClass.something4++ - javaClass.something5++ - javaClass.something6 = null + javaClass.something5 = null } // FILE: JavaClass.java @@ -17,14 +16,11 @@ public class JavaClass { public void setSomething2(String value) { } public int getSomething3() { return 1; } - public int setSomething3(int value) { return value; } + public void setSomething3(int value) { return value; } public int getSomething4() { return 1; } - public void setSomething4(int value) { return value; } + public static void setSomething4(int value) { } - public int getSomething5() { return 1; } - public static void setSomething5(int value) { } - - public int[] getSomething6() { return null; } - public void setSomething6(int... value) { } + public int[] getSomething5() { return null; } + public void setSomething5(int... value) { } } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/FalseSetters.txt b/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/FalseSetters.txt index d0fa9f8288a..57d13fe4541 100644 --- a/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/FalseSetters.txt +++ b/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/FalseSetters.txt @@ -9,16 +9,14 @@ public open class JavaClass { public open fun getSomething2(): kotlin.Int public open fun getSomething3(): kotlin.Int public open fun getSomething4(): kotlin.Int - public open fun getSomething5(): kotlin.Int - public open fun getSomething6(): kotlin.IntArray! + public open fun getSomething5(): kotlin.IntArray! public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open fun setSomething1(/*0*/ value: kotlin.Int, /*1*/ c: kotlin.Char): kotlin.Unit public open fun setSomething2(/*0*/ value: kotlin.String!): kotlin.Unit - public open fun setSomething3(/*0*/ value: kotlin.Int): kotlin.Int - public open fun setSomething4(/*0*/ value: kotlin.Int): kotlin.Unit - public open fun setSomething6(/*0*/ vararg value: kotlin.Int /*kotlin.IntArray!*/): kotlin.Unit + public open fun setSomething3(/*0*/ value: kotlin.Int): kotlin.Unit + public open fun setSomething5(/*0*/ vararg value: kotlin.Int /*kotlin.IntArray!*/): kotlin.Unit public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String // Static members - public open fun setSomething5(/*0*/ value: kotlin.Int): kotlin.Unit + public open fun setSomething4(/*0*/ value: kotlin.Int): kotlin.Unit } diff --git a/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/GetterAndSetter.kt b/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/GetterAndSetter.kt index c26c6947cd1..3bc634c4e43 100644 --- a/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/GetterAndSetter.kt +++ b/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/GetterAndSetter.kt @@ -1,10 +1,14 @@ // FILE: KotlinFile.kt fun foo(javaClass: JavaClass) { - javaClass.something++ + javaClass.something1++ + javaClass.something2++ } // FILE: JavaClass.java public class JavaClass { - public int getSomething() { return 1; } - public void setSomething(int value) { } + public int getSomething1() { return 1; } + public void setSomething1(int value) { } + + public int getSomething2() { return 1; } + public JavaClass setSomething2(int value) { return this; } } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/GetterAndSetter.txt b/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/GetterAndSetter.txt index 1958b9255dd..c7e7027cbd8 100644 --- a/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/GetterAndSetter.txt +++ b/compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/GetterAndSetter.txt @@ -5,8 +5,10 @@ public fun foo(/*0*/ javaClass: JavaClass): kotlin.Unit public open class JavaClass { public constructor JavaClass() public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean - public open fun getSomething(): kotlin.Int + public open fun getSomething1(): kotlin.Int + public open fun getSomething2(): kotlin.Int public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int - public open fun setSomething(/*0*/ value: kotlin.Int): kotlin.Unit + public open fun setSomething1(/*0*/ value: kotlin.Int): kotlin.Unit + public open fun setSomething2(/*0*/ value: kotlin.Int): JavaClass! public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } diff --git a/idea/ide-common/src/org/jetbrains/kotlin/idea/codeInsight/ReferenceVariantsHelper.kt b/idea/ide-common/src/org/jetbrains/kotlin/idea/codeInsight/ReferenceVariantsHelper.kt index 0c465246d58..dec74dd8e1d 100644 --- a/idea/ide-common/src/org/jetbrains/kotlin/idea/codeInsight/ReferenceVariantsHelper.kt +++ b/idea/ide-common/src/org/jetbrains/kotlin/idea/codeInsight/ReferenceVariantsHelper.kt @@ -35,7 +35,7 @@ import org.jetbrains.kotlin.resolve.scopes.utils.collectDescriptorsFiltered import org.jetbrains.kotlin.resolve.scopes.utils.memberScopeAsImportingScope import org.jetbrains.kotlin.synthetic.SyntheticJavaPropertyDescriptor import org.jetbrains.kotlin.types.KotlinType -import org.jetbrains.kotlin.utils.addIfNotNull +import org.jetbrains.kotlin.types.typeUtil.isUnit import java.util.* class ReferenceVariantsHelper( @@ -92,7 +92,11 @@ class ReferenceVariantsHelper( for (variant in variants) { if (variant is SyntheticJavaPropertyDescriptor) { accessorMethodsToRemove.add(variant.getMethod.original) - accessorMethodsToRemove.addIfNotNull(variant.setMethod?.original) + + val setter = variant.setMethod + if (setter != null && setter.returnType?.isUnit() == true) { // we do not filter out non-Unit setters + accessorMethodsToRemove.add(setter.original) + } } } diff --git a/idea/idea-completion/testData/basic/multifile/SyntheticExtensionNonVoidSetter/SyntheticExtensionNonVoidSetter.dependency.java b/idea/idea-completion/testData/basic/multifile/SyntheticExtensionNonVoidSetter/SyntheticExtensionNonVoidSetter.dependency.java new file mode 100644 index 00000000000..45790e072a3 --- /dev/null +++ b/idea/idea-completion/testData/basic/multifile/SyntheticExtensionNonVoidSetter/SyntheticExtensionNonVoidSetter.dependency.java @@ -0,0 +1,5 @@ +interface JavaClass{ + public int getSomething(); + + public JavaClass setSomething(int value); +} \ No newline at end of file diff --git a/idea/idea-completion/testData/basic/multifile/SyntheticExtensionNonVoidSetter/SyntheticExtensionNonVoidSetter.kt b/idea/idea-completion/testData/basic/multifile/SyntheticExtensionNonVoidSetter/SyntheticExtensionNonVoidSetter.kt new file mode 100644 index 00000000000..2d2ac40bced --- /dev/null +++ b/idea/idea-completion/testData/basic/multifile/SyntheticExtensionNonVoidSetter/SyntheticExtensionNonVoidSetter.kt @@ -0,0 +1,7 @@ +fun foo(javaClass: JavaClass) { + javaClass. +} + +// EXIST: { lookupString: "something", attributes: "bold" } +// EXIST: { lookupString: "setSomething", attributes: "bold" } +// ABSENT: getSomething \ No newline at end of file diff --git a/idea/idea-completion/tests/org/jetbrains/kotlin/idea/completion/test/MultiFileJvmBasicCompletionTestGenerated.java b/idea/idea-completion/tests/org/jetbrains/kotlin/idea/completion/test/MultiFileJvmBasicCompletionTestGenerated.java index 2f33152a18c..e5569aebca6 100644 --- a/idea/idea-completion/tests/org/jetbrains/kotlin/idea/completion/test/MultiFileJvmBasicCompletionTestGenerated.java +++ b/idea/idea-completion/tests/org/jetbrains/kotlin/idea/completion/test/MultiFileJvmBasicCompletionTestGenerated.java @@ -353,6 +353,12 @@ public class MultiFileJvmBasicCompletionTestGenerated extends AbstractMultiFileJ doTest(fileName); } + @TestMetadata("SyntheticExtensionNonVoidSetter") + public void testSyntheticExtensionNonVoidSetter() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/idea-completion/testData/basic/multifile/SyntheticExtensionNonVoidSetter/"); + doTest(fileName); + } + @TestMetadata("TopLevelFunction") public void testTopLevelFunction() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/idea-completion/testData/basic/multifile/TopLevelFunction/"); diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/UsePropertyAccessSyntaxIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/UsePropertyAccessSyntaxIntention.kt index 2c7519f6d0a..2e728436a8a 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/UsePropertyAccessSyntaxIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/UsePropertyAccessSyntaxIntention.kt @@ -53,6 +53,7 @@ import org.jetbrains.kotlin.resolve.scopes.SyntheticScopes import org.jetbrains.kotlin.synthetic.SyntheticJavaPropertyDescriptor import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.TypeUtils +import org.jetbrains.kotlin.types.typeUtil.isUnit class UsePropertyAccessSyntaxInspection : IntentionBasedInspection(UsePropertyAccessSyntaxIntention()), CleanupLocalInspectionTool @@ -95,6 +96,14 @@ class UsePropertyAccessSyntaxIntention : SelfTargetingOffsetIndependentIntention if (!checkWillResolveToProperty(resolvedCall, property, bindingContext, resolutionScope, dataFlowInfo, expectedType, resolutionFacade)) return null val isSetUsage = callExpression.valueArguments.size == 1 + + if (isSetUsage && bindingContext[BindingContext.USED_AS_EXPRESSION, qualifiedExpression] == true) { + // call to the setter used as expression can be converted in the only case when it's used as body expression for some declaration and its type is Unit + val parent = qualifiedExpression.parent + if (parent !is KtDeclarationWithBody || qualifiedExpression != parent.bodyExpression) return null + if (function.returnType?.isUnit() != true) return null + } + if (isSetUsage && property.type != function.valueParameters.single().type) { val qualifiedExpressionCopy = qualifiedExpression.copied() val callExpressionCopy = ((qualifiedExpressionCopy as? KtQualifiedExpression)?.selectorExpression ?: qualifiedExpressionCopy) as KtCallExpression @@ -157,7 +166,6 @@ class UsePropertyAccessSyntaxIntention : SelfTargetingOffsetIndependentIntention return callExpression.replaced(newExpression) } - //TODO: what if it was used as expression (of type Unit)? private fun replaceWithPropertySet(callExpression: KtCallExpression, propertyName: Name): KtExpression { val call = callExpression.getQualifiedExpressionForSelector() ?: callExpression val callParent = call.parent diff --git a/idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter1.1.java b/idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter1.1.java new file mode 100644 index 00000000000..658fb69de2b --- /dev/null +++ b/idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter1.1.java @@ -0,0 +1,7 @@ +public interface JavaInterface { + Object getSomething1(); + JavaInterface setSomething1(Object value); + + int getSomething2(); + JavaInterface setSomething2(int value); +} diff --git a/idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter1.kt b/idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter1.kt new file mode 100644 index 00000000000..b0d7da5d8ad --- /dev/null +++ b/idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter1.kt @@ -0,0 +1,7 @@ +// IS_APPLICABLE: false +// WITH_RUNTIME + +fun foo(j: JavaInterface) { + j.setSomething1("").setSomething2(1) +} + diff --git a/idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter2.1.java b/idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter2.1.java new file mode 100644 index 00000000000..fc5f32e1761 --- /dev/null +++ b/idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter2.1.java @@ -0,0 +1,4 @@ +public interface JavaInterface { + Object getSomething(); + JavaInterface setSomething(Object value); +} diff --git a/idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter2.kt b/idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter2.kt new file mode 100644 index 00000000000..84088444064 --- /dev/null +++ b/idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter2.kt @@ -0,0 +1,5 @@ +// IS_APPLICABLE: false +// WITH_RUNTIME + +fun foo(j: JavaInterface): JavaInterface = j.setSomething("") + diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index 4ca24dfb6e6..1c657e55102 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -9059,6 +9059,18 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("nonVoidSetter1.kt") + public void testNonVoidSetter1() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter1.kt"); + doTest(fileName); + } + + @TestMetadata("nonVoidSetter2.kt") + public void testNonVoidSetter2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter2.kt"); + doTest(fileName); + } + @TestMetadata("propertyTypeIsMoreSpecific1.kt") public void testPropertyTypeIsMoreSpecific1() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/usePropertyAccessSyntax/propertyTypeIsMoreSpecific1.kt");