KT-9547: private package member can conflict only with the members

declared in the same file.
Cleanup OverloadUtil stuff.
Update testData.

compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationResolver.kt

@@ -109,18 +109,8 @@ public class DeclarationResolver(
     }
 
     private fun getTopLevelDescriptorsByFqName(topLevelDescriptorProvider: TopLevelDescriptorProvider, fqName: FqName, location: LookupLocation): Set<DeclarationDescriptor> {
-        val parentFqName = fqName.parent()
-
         val descriptors = HashSet<DeclarationDescriptor>()
 
-        val parentFragment = topLevelDescriptorProvider.getPackageFragment(parentFqName)
-        if (parentFragment != null) {
-            // Filter out extension properties
-            descriptors.addAll(parentFragment.getMemberScope().getProperties(fqName.shortName(), location).filter {
-                it.getExtensionReceiverParameter() == null
-            })
-        }
-
         descriptors.addIfNotNull(topLevelDescriptorProvider.getPackageFragment(fqName))
         descriptors.addAll(topLevelDescriptorProvider.getTopLevelClassDescriptors(fqName, location))
         return descriptors

compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadResolver.java

@@ -91,12 +91,11 @@ public class OverloadResolver {
             @NotNull BodiesResolveContext c,
             @NotNull MultiMap<FqNameUnsafe, ConstructorDescriptor> inPackages
     ) {
-        MultiMap<FqNameUnsafe, CallableMemberDescriptor> functionsByName = OverloadUtil.groupModulePackageMembersByFqName(c, inPackages);
+        MultiMap<FqNameUnsafe, CallableMemberDescriptor> membersByName = OverloadUtil.groupModulePackageMembersByFqName(c, inPackages);
 
-        for (Map.Entry<FqNameUnsafe, Collection<CallableMemberDescriptor>> e : functionsByName.entrySet()) {
-            // TODO: don't render FQ name here, extract this logic to somewhere
+        for (Map.Entry<FqNameUnsafe, Collection<CallableMemberDescriptor>> e : membersByName.entrySet()) {
             FqNameUnsafe fqName = e.getKey().parent();
-            checkOverloadsWithSameName(e.getValue(), fqName.isRoot() ? "root package" : fqName.asString());
+            checkOverloadsInPackage(e.getValue(), fqName);
         }
     }
 
@@ -129,32 +128,43 @@ public class OverloadResolver {
         }
         
         for (Map.Entry<Name, Collection<CallableMemberDescriptor>> e : functionsByName.entrySet()) {
-            checkOverloadsWithSameName(e.getValue(), nameForErrorMessage(classDescriptor, klass));
+            checkOverloadsInClass(e.getValue(), classDescriptor, klass);
         }
     }
     
-    private void checkOverloadsWithSameName(
-            Collection<CallableMemberDescriptor> functions,
-            @NotNull String functionContainer
+    private void checkOverloadsInPackage(
+            @NotNull Collection<CallableMemberDescriptor> members,
+            @NotNull FqNameUnsafe packageFQN
     ) {
-        if (functions.size() == 1) {
-            // micro-optimization
-            return;
+        if (members.size() == 1) return;
+
+        for (Collection<? extends CallableMemberDescriptor> redeclarationGroup : OverloadUtil.getPossibleRedeclarationGroups(members)) {
+            Set<Pair<KtDeclaration, CallableMemberDescriptor>> redeclarations = findRedeclarations(redeclarationGroup);
+            // TODO: don't render FQ name here, extract this logic to somewhere
+            reportRedeclarations(packageFQN.isRoot() ? "root package" : packageFQN.asString(), redeclarations);
         }
-        reportRedeclarations(functionContainer, findRedeclarations(functions));
+    }
+
+    private void checkOverloadsInClass(
+            @NotNull Collection<CallableMemberDescriptor> members,
+            @NotNull ClassDescriptor classDescriptor,
+            @NotNull KtClassOrObject ktClass
+    ) {
+        if (members.size() == 1) return;
+
+        reportRedeclarations(nameForErrorMessage(classDescriptor, ktClass), findRedeclarations(members));
     }
 
     @NotNull
-    private Set<Pair<KtDeclaration, CallableMemberDescriptor>> findRedeclarations(@NotNull Collection<CallableMemberDescriptor> functions) {
+    private static Set<Pair<KtDeclaration, CallableMemberDescriptor>> findRedeclarations(@NotNull Collection<? extends CallableMemberDescriptor> members) {
         Set<Pair<KtDeclaration, CallableMemberDescriptor>> redeclarations = Sets.newLinkedHashSet();
-        for (CallableMemberDescriptor member : functions) {
-            for (CallableMemberDescriptor member2 : functions) {
+        for (CallableMemberDescriptor member : members) {
+            for (CallableMemberDescriptor member2 : members) {
                 if (member == member2 || isConstructorsOfDifferentRedeclaredClasses(member, member2)) {
                     continue;
                 }
 
-                OverloadUtil.OverloadCompatibilityInfo overloadable = OverloadUtil.isOverloadable(member, member2);
-                if (!overloadable.isSuccess() && member.getKind() != CallableMemberDescriptor.Kind.SYNTHESIZED) {
+                if (!OverloadUtil.isOverloadable(member, member2) && member.getKind() != CallableMemberDescriptor.Kind.SYNTHESIZED) {
                     KtDeclaration ktDeclaration = (KtDeclaration) DescriptorToSourceUtils.descriptorToDeclaration(member);
                     if (ktDeclaration != null) {
                         redeclarations.add(Pair.create(ktDeclaration, member));

compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadUtil.kt

@@ -31,42 +31,32 @@ object OverloadUtil {
     /**
      * Does not check names.
      */
-    public @JvmStatic fun isOverloadable(a: CallableDescriptor, b: CallableDescriptor): OverloadCompatibilityInfo {
+    public @JvmStatic fun isOverloadable(a: CallableDescriptor, b: CallableDescriptor): Boolean {
         val abc = braceCount(a)
         val bbc = braceCount(b)
 
         if (abc != bbc) {
-            return OverloadCompatibilityInfo.success()
+            return true
         }
 
-        val overrideCompatibilityInfo = isOverloadableBy(a, b)
-        when (overrideCompatibilityInfo.result) {
-            OVERRIDABLE, CONFLICT -> return OverloadCompatibilityInfo.someError()
-            INCOMPATIBLE -> return OverloadCompatibilityInfo.success()
-            else -> throw IllegalStateException()
-        }
-    }
-
-    private fun isOverloadableBy(
-            superDescriptor: CallableDescriptor,
-            subDescriptor: CallableDescriptor): OverridingUtil.OverrideCompatibilityInfo {
-        val receiverAndParameterResult = OverridingUtil.checkReceiverAndParameterCount(superDescriptor, subDescriptor)
+        val receiverAndParameterResult = OverridingUtil.checkReceiverAndParameterCount(a, b)
         if (receiverAndParameterResult != null) {
-            return receiverAndParameterResult
+            return receiverAndParameterResult.result == INCOMPATIBLE
         }
 
-        val superValueParameters = OverridingUtil.compiledValueParameters(superDescriptor)
-        val subValueParameters = OverridingUtil.compiledValueParameters(subDescriptor)
+        val aValueParameters = OverridingUtil.compiledValueParameters(a)
+        val bValueParameters = OverridingUtil.compiledValueParameters(b)
 
-        for (i in superValueParameters.indices) {
-            val superValueParameterType = OverridingUtil.getUpperBound(superValueParameters[i])
-            val subValueParameterType = OverridingUtil.getUpperBound(subValueParameters[i])
-            if (!KotlinTypeChecker.DEFAULT.equalTypes(superValueParameterType, subValueParameterType) || oneMoreSpecificThanAnother(subValueParameterType, superValueParameterType)) {
-                return OverridingUtil.OverrideCompatibilityInfo.valueParameterTypeMismatch(superValueParameterType, subValueParameterType, INCOMPATIBLE)
+        for (i in aValueParameters.indices) {
+            val superValueParameterType = OverridingUtil.getUpperBound(aValueParameters[i])
+            val subValueParameterType = OverridingUtil.getUpperBound(bValueParameters[i])
+            if (!KotlinTypeChecker.DEFAULT.equalTypes(superValueParameterType, subValueParameterType) ||
+                    oneMoreSpecificThanAnother(subValueParameterType, superValueParameterType)) {
+                return true
             }
         }
 
-        return OverridingUtil.OverrideCompatibilityInfo.success()
+        return false
     }
 
     private fun braceCount(a: CallableDescriptor): Int =
@@ -77,17 +67,6 @@ object OverloadUtil {
                 else -> throw IllegalStateException()
             }
 
-    class OverloadCompatibilityInfo(val isSuccess: Boolean, val message: String) {
-        companion object {
-
-            private val SUCCESS = OverloadCompatibilityInfo(true, "SUCCESS")
-
-            fun success() = SUCCESS
-
-            fun someError() = OverloadCompatibilityInfo(false, "XXX")
-        }
-    }
-
     public @JvmStatic fun groupModulePackageMembersByFqName(
             c: BodiesResolveContext,
             constructorsInPackages: MultiMap<FqNameUnsafe, ConstructorDescriptor>
@@ -104,7 +83,7 @@ object OverloadUtil {
             scope.getProperties(name, NoLookupLocation.WHEN_CHECK_REDECLARATIONS).filterIsInstance<CallableMemberDescriptor>()
         }
 
-        // TODO handle constructor redeclarations in modules. See also https://youtrack.jetbrains.com/issue/KT-3632
+        // TODO handle constructor redeclarations in modules. See also: https://youtrack.jetbrains.com/issue/KT-3632
         packageMembersByName.putAllValues(constructorsInPackages)
 
         return packageMembersByName
@@ -146,4 +125,31 @@ object OverloadUtil {
         return possibleOverloads.filter { DescriptorUtils.getContainingModule(it) == containingModule }
     }
 
+    private fun MemberDescriptor.isPrivate() = Visibilities.isPrivate(this.visibility)
+
+    public @JvmStatic fun getPossibleRedeclarationGroups(members: Collection<CallableMemberDescriptor>): Collection<Collection<CallableMemberDescriptor>> {
+        val result = arrayListOf<Collection<CallableMemberDescriptor>>()
+
+        val nonPrivates = members.filter { !it.isPrivate() }
+        if (nonPrivates.size > 1) {
+            result.add(nonPrivates)
+        }
+
+        val bySourceFile = MultiMap.createSmart<SourceFile, CallableMemberDescriptor>()
+        for (member in members) {
+            val sourceFile = DescriptorUtils.getContainingSourceFile(member)
+            if (sourceFile != SourceFile.NO_SOURCE_FILE) {
+                bySourceFile.putValue(sourceFile, member)
+            }
+        }
+
+        for ((sourceFile, membersInFile) in bySourceFile.entrySet()) {
+            // File member groups are interesting in redeclaration check if at least one file member is private.
+            if (membersInFile.size > 1 && membersInFile.any { it.isPrivate() }) {
+                result.add(membersInFile)
+            }
+        }
+
+        return result
+    }
 }

compiler/testData/diagnostics/tests/redeclarations/RedeclarationInMultiFile.kt

@@ -1,9 +1,9 @@
 // FILE: a.kt
-val <!REDECLARATION, REDECLARATION!>a<!> : Int = 1
+val <!REDECLARATION!>a<!> : Int = 1
 <!CONFLICTING_OVERLOADS!>fun f()<!> {
 }
 
 // FILE: b.kt
-val <!REDECLARATION, REDECLARATION!>a<!> : Int = 1
+val <!REDECLARATION!>a<!> : Int = 1
 <!CONFLICTING_OVERLOADS!>fun f()<!> {
 }

compiler/testData/diagnostics/tests/redeclarations/RedeclaringPrivateToFile.kt

@@ -0,0 +1,40 @@
+// FILE: a.kt
+package a
+
+interface A
+interface B : A
+
+private fun validFun() {}
+
+private val validProp = 1
+
+<!CONFLICTING_OVERLOADS!>private fun invalidFun1()<!> {}
+<!CONFLICTING_OVERLOADS!>private fun invalidFun1()<!> {}
+
+<!CONFLICTING_OVERLOADS!>private fun invalidFun2()<!> {}
+<!CONFLICTING_OVERLOADS!>public fun invalidFun2()<!> {}
+
+<!CONFLICTING_OVERLOADS!>public fun invalidFun3()<!> {}
+
+<!CONFLICTING_OVERLOADS!>private fun invalidFun4()<!> {}
+<!CONFLICTING_OVERLOADS, CONFLICTING_OVERLOADS!>public fun invalidFun4()<!> {}
+
+public fun validFun2(a: A) = a
+public fun validFun2(b: B) = b
+
+// FILE: b.kt
+package a
+
+private fun validFun() {}
+
+private val validProp = 1
+
+<!CONFLICTING_OVERLOADS!>internal fun invalidFun3()<!> {}
+<!CONFLICTING_OVERLOADS!>internal fun invalidFun4()<!> {}
+
+// FILE: c.kt
+package a
+
+public fun validFun() {}
+
+public val validProp = 1

compiler/testData/diagnostics/tests/redeclarations/RedeclaringPrivateToFile.txt

@@ -0,0 +1,33 @@
+package
+
+package a {
+    private val validProp: kotlin.Int = 1
+    private val validProp: kotlin.Int = 1
+    public val validProp: kotlin.Int = 1
+    private fun invalidFun1(): kotlin.Unit
+    private fun invalidFun1(): kotlin.Unit
+    private fun invalidFun2(): kotlin.Unit
+    public fun invalidFun2(): kotlin.Unit
+    internal fun invalidFun3(): kotlin.Unit
+    public fun invalidFun3(): kotlin.Unit
+    internal fun invalidFun4(): kotlin.Unit
+    private fun invalidFun4(): kotlin.Unit
+    public fun invalidFun4(): kotlin.Unit
+    private fun validFun(): kotlin.Unit
+    private fun validFun(): kotlin.Unit
+    public fun validFun(): kotlin.Unit
+    public fun validFun2(/*0*/ a: a.A): a.A
+    public fun validFun2(/*0*/ b: a.B): a.B
+
+    public interface A {
+        public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+        public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+        public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+    }
+
+    public interface B : a.A {
+        public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+        public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+        public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+    }
+}

compiler/testData/diagnostics/tests/redeclarations/kt470.kt

@@ -1,11 +1,11 @@
-val <!REDECLARATION, REDECLARATION!>a<!> : Int = 1
-val <!REDECLARATION, REDECLARATION!>a<!> : Int = 1
-val <!REDECLARATION, REDECLARATION!>a<!> : Int = 1
+val <!REDECLARATION!>a<!> : Int = 1
+val <!REDECLARATION!>a<!> : Int = 1
+val <!REDECLARATION!>a<!> : Int = 1
 
-val <!REDECLARATION, REDECLARATION!>b<!> : Int = 1
-val <!REDECLARATION, REDECLARATION!>b<!> : Int = 1
-val <!REDECLARATION, REDECLARATION!>b<!> : Int = 1
-val <!REDECLARATION, REDECLARATION!>b<!> : Int = 1
+val <!REDECLARATION!>b<!> : Int = 1
+val <!REDECLARATION!>b<!> : Int = 1
+val <!REDECLARATION!>b<!> : Int = 1
+val <!REDECLARATION!>b<!> : Int = 1
 
 <!CONFLICTING_OVERLOADS!>fun foo()<!> {} // and here too
 <!CONFLICTING_OVERLOADS!>fun foo()<!> {} // and here

compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java

@@ -11931,6 +11931,12 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest {
                 doTest(fileName);
             }
 
+            @TestMetadata("RedeclaringPrivateToFile.kt")
+            public void testRedeclaringPrivateToFile() throws Exception {
+                String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/redeclarations/RedeclaringPrivateToFile.kt");
+                doTest(fileName);
+            }
+
             @TestMetadata("SingletonAndFunctionSameName.kt")
             public void testSingletonAndFunctionSameName() throws Exception {
                 String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/redeclarations/SingletonAndFunctionSameName.kt");

compiler/tests/org/jetbrains/kotlin/types/JetOverloadTest.java

@@ -161,14 +161,12 @@ public class JetOverloadTest extends JetLiteFixture {
     private void assertOverloadabilityRelation(String funA, String funB, boolean expectedIsError) {
         FunctionDescriptor a = makeFunction(funA);
         FunctionDescriptor b = makeFunction(funB);
-        {
-            OverloadUtil.OverloadCompatibilityInfo overloadableWith = OverloadUtil.isOverloadable(a, b);
-            assertEquals(overloadableWith.getMessage(), expectedIsError, !overloadableWith.isSuccess());
-        }
-        {
-            OverloadUtil.OverloadCompatibilityInfo overloadableWith = OverloadUtil.isOverloadable(b, a);
-            assertEquals(overloadableWith.getMessage(), expectedIsError, !overloadableWith.isSuccess());
-        }
+
+        boolean aOverloadableWithB = OverloadUtil.isOverloadable(a, b);
+        assertEquals(expectedIsError, !aOverloadableWithB);
+
+        boolean bOverloadableWithA = OverloadUtil.isOverloadable(b, a);
+        assertEquals(expectedIsError, !bOverloadableWithA);
     }
 
     private FunctionDescriptor makeFunction(String funDecl) {

idea/src/org/jetbrains/kotlin/idea/refactoring/memberInfo/KotlinMemberInfoStorage.kt

@@ -43,7 +43,7 @@ public class KotlinMemberInfoStorage(
 
         return when {
             descriptor1 is FunctionDescriptor && descriptor is FunctionDescriptor -> {
-                !OverloadUtil.isOverloadable(descriptor1, descriptor).isSuccess
+                !OverloadUtil.isOverloadable(descriptor1, descriptor)
             }
             descriptor1 is PropertyDescriptor && descriptor is PropertyDescriptor,
             descriptor1 is ClassDescriptor && descriptor is ClassDescriptor -> true

idea/testData/copyPaste/imports/NoImportForSamePackage.expected.kt

@@ -1,13 +1,13 @@
 // ERROR: No value passed for parameter i
 package a
 
-fun f(p: A, t: T) {
+private fun f(p: A, t: T) {
     g(A(c).ext())
     O1.f()
     O2
     E.ENTRY
 }
 
-fun f2(i: Outer.Inner, n: Outer.Nested, e: Outer.NestedEnum, o: Outer.NestedObj, t: Outer.NestedTrait, a: Outer.NestedAnnotation) {
+private fun f2(i: Outer.Inner, n: Outer.Nested, e: Outer.NestedEnum, o: Outer.NestedObj, t: Outer.NestedTrait, a: Outer.NestedAnnotation) {
     ClassObject
 }

idea/testData/copyPaste/imports/NoImportForSamePackage.kt

@@ -44,13 +44,13 @@ class ClassObject {
     }
 }
 
-<selection>fun f(p: A, t: T) {
+<selection>private fun f(p: A, t: T) {
     g(A(c).ext())
     O1.f()
     O2
     ENTRY
 }
 
-fun f2(i: Inner, n: Nested, e: NestedEnum, o: NestedObj, t: NestedTrait, a: NestedAnnotation) {
+private fun f2(i: Inner, n: Nested, e: NestedEnum, o: NestedObj, t: NestedTrait, a: NestedAnnotation) {
     ClassObject
 }</selection>