KT-10631 Consider creating a synthetic property even when the setter returns 'this'

#KT-10631 Fixed

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()
     }
 

compiler/testData/diagnostics/tests/syntheticExtensions/javaProperties/FalseSetters.kt

@@ -4,8 +4,7 @@ fun foo(javaClass: JavaClass) {
     <!VAL_REASSIGNMENT!>javaClass.something2<!>++
     <!VAL_REASSIGNMENT!>javaClass.something3<!>++
     <!VAL_REASSIGNMENT!>javaClass.something4<!>++
-    <!VAL_REASSIGNMENT!>javaClass.something5<!>++
+    <!VAL_REASSIGNMENT!>javaClass.something5<!> = null
-    <!VAL_REASSIGNMENT!>javaClass.something6<!> = 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 <T> void setSomething3(int value) { return value; }
 
     public int getSomething4() { return 1; }
-    public <T> void setSomething4(int value) { return value; }
+    public static void setSomething4(int value) { }
 
-    public int getSomething5() { return 1; }
+    public int[] getSomething5() { return null; }
-    public static void setSomething5(int value) { }
+    public void setSomething5(int... value) { }
-
-    public int[] getSomething6() { return null; }
-    public void setSomething6(int... value) { }
 }

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 getSomething5(): kotlin.IntArray!
-    public open fun getSomething6(): 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 </*0*/ T : kotlin.Any!> setSomething3(/*0*/ value: kotlin.Int): kotlin.Unit
-    public open fun </*0*/ T : kotlin.Any!> setSomething4(/*0*/ value: kotlin.Int): kotlin.Unit
+    public open fun setSomething5(/*0*/ vararg value: kotlin.Int /*kotlin.IntArray!*/): kotlin.Unit
-    public open fun setSomething6(/*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
 }

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 int getSomething1() { return 1; }
-    public void setSomething(int value) { }
+    public void setSomething1(int value) { }
+
+    public int getSomething2() { return 1; }
+    public JavaClass setSomething2(int value) { return this; }
 }

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
 }

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)
+                }
             }
         }
 

idea/idea-completion/testData/basic/multifile/SyntheticExtensionNonVoidSetter/SyntheticExtensionNonVoidSetter.dependency.java

@@ -0,0 +1,5 @@
+interface JavaClass{
+    public int getSomething();
+
+    public JavaClass setSomething(int value);
+}

idea/idea-completion/testData/basic/multifile/SyntheticExtensionNonVoidSetter/SyntheticExtensionNonVoidSetter.kt

@@ -0,0 +1,7 @@
+fun foo(javaClass: JavaClass) {
+    javaClass.<caret>
+}
+
+// EXIST: { lookupString: "something", attributes: "bold" }
+// EXIST: { lookupString: "setSomething", attributes: "bold" }
+// ABSENT: getSomething

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/");

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<KtCallExpression>(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

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);
+}

idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter1.kt

@@ -0,0 +1,7 @@
+// IS_APPLICABLE: false
+// WITH_RUNTIME
+
+fun foo(j: JavaInterface) {
+    j.<caret>setSomething1("").setSomething2(1)
+}
+

idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter2.1.java

@@ -0,0 +1,4 @@
+public interface JavaInterface {
+    Object getSomething();
+    JavaInterface setSomething(Object value);
+}

idea/testData/intentions/usePropertyAccessSyntax/nonVoidSetter2.kt

@@ -0,0 +1,5 @@
+// IS_APPLICABLE: false
+// WITH_RUNTIME
+
+fun foo(j: JavaInterface): JavaInterface = j.<caret>setSomething("")
+

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");