diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationResolver.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationResolver.kt index c3d70f6821a..091ee0ccf59 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationResolver.kt +++ b/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 { - val parentFqName = fqName.parent() - val descriptors = HashSet() - 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 diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadResolver.java index 060732db4f9..df6d933f377 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadResolver.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadResolver.java @@ -91,12 +91,11 @@ public class OverloadResolver { @NotNull BodiesResolveContext c, @NotNull MultiMap inPackages ) { - MultiMap functionsByName = OverloadUtil.groupModulePackageMembersByFqName(c, inPackages); + MultiMap membersByName = OverloadUtil.groupModulePackageMembersByFqName(c, inPackages); - for (Map.Entry> e : functionsByName.entrySet()) { - // TODO: don't render FQ name here, extract this logic to somewhere + for (Map.Entry> 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> e : functionsByName.entrySet()) { - checkOverloadsWithSameName(e.getValue(), nameForErrorMessage(classDescriptor, klass)); + checkOverloadsInClass(e.getValue(), classDescriptor, klass); } } - private void checkOverloadsWithSameName( - Collection functions, - @NotNull String functionContainer + private void checkOverloadsInPackage( + @NotNull Collection members, + @NotNull FqNameUnsafe packageFQN ) { - if (functions.size() == 1) { - // micro-optimization - return; + if (members.size() == 1) return; + + for (Collection redeclarationGroup : OverloadUtil.getPossibleRedeclarationGroups(members)) { + Set> 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 members, + @NotNull ClassDescriptor classDescriptor, + @NotNull KtClassOrObject ktClass + ) { + if (members.size() == 1) return; + + reportRedeclarations(nameForErrorMessage(classDescriptor, ktClass), findRedeclarations(members)); } @NotNull - private Set> findRedeclarations(@NotNull Collection functions) { + private static Set> findRedeclarations(@NotNull Collection members) { Set> 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)); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadUtil.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadUtil.kt index e64a3739bcb..58183a6de37 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadUtil.kt +++ b/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 @@ -104,7 +83,7 @@ object OverloadUtil { scope.getProperties(name, NoLookupLocation.WHEN_CHECK_REDECLARATIONS).filterIsInstance() } - // 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): Collection> { + val result = arrayListOf>() + + val nonPrivates = members.filter { !it.isPrivate() } + if (nonPrivates.size > 1) { + result.add(nonPrivates) + } + + val bySourceFile = MultiMap.createSmart() + 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 + } } diff --git a/compiler/testData/diagnostics/tests/redeclarations/RedeclarationInMultiFile.kt b/compiler/testData/diagnostics/tests/redeclarations/RedeclarationInMultiFile.kt index e75d37a5cf0..fafba886a28 100644 --- a/compiler/testData/diagnostics/tests/redeclarations/RedeclarationInMultiFile.kt +++ b/compiler/testData/diagnostics/tests/redeclarations/RedeclarationInMultiFile.kt @@ -1,9 +1,9 @@ // FILE: a.kt -val a : Int = 1 +val a : Int = 1 fun f() { } // FILE: b.kt -val a : Int = 1 +val a : Int = 1 fun f() { } diff --git a/compiler/testData/diagnostics/tests/redeclarations/RedeclaringPrivateToFile.kt b/compiler/testData/diagnostics/tests/redeclarations/RedeclaringPrivateToFile.kt new file mode 100644 index 00000000000..ca6468232f5 --- /dev/null +++ b/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 + +private fun invalidFun1() {} +private fun invalidFun1() {} + +private fun invalidFun2() {} +public fun invalidFun2() {} + +public fun invalidFun3() {} + +private fun invalidFun4() {} +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 + +internal fun invalidFun3() {} +internal fun invalidFun4() {} + +// FILE: c.kt +package a + +public fun validFun() {} + +public val validProp = 1 \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/redeclarations/RedeclaringPrivateToFile.txt b/compiler/testData/diagnostics/tests/redeclarations/RedeclaringPrivateToFile.txt new file mode 100644 index 00000000000..06394a5cc97 --- /dev/null +++ b/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 + } +} diff --git a/compiler/testData/diagnostics/tests/redeclarations/kt470.kt b/compiler/testData/diagnostics/tests/redeclarations/kt470.kt index 6e0d1c45b1e..f8c5fff6abe 100644 --- a/compiler/testData/diagnostics/tests/redeclarations/kt470.kt +++ b/compiler/testData/diagnostics/tests/redeclarations/kt470.kt @@ -1,11 +1,11 @@ -val a : Int = 1 -val a : Int = 1 -val a : Int = 1 +val a : Int = 1 +val a : Int = 1 +val a : Int = 1 -val b : Int = 1 -val b : Int = 1 -val b : Int = 1 -val b : Int = 1 +val b : Int = 1 +val b : Int = 1 +val b : Int = 1 +val b : Int = 1 fun foo() {} // and here too fun foo() {} // and here diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java index 5e3eb477c04..73efae6e7e7 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java +++ b/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"); diff --git a/compiler/tests/org/jetbrains/kotlin/types/JetOverloadTest.java b/compiler/tests/org/jetbrains/kotlin/types/JetOverloadTest.java index 89f5930c1f2..3878eb619b5 100644 --- a/compiler/tests/org/jetbrains/kotlin/types/JetOverloadTest.java +++ b/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) { diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/memberInfo/KotlinMemberInfoStorage.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/memberInfo/KotlinMemberInfoStorage.kt index 8a476e9696f..7b321a31b9d 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/memberInfo/KotlinMemberInfoStorage.kt +++ b/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 diff --git a/idea/testData/copyPaste/imports/NoImportForSamePackage.expected.kt b/idea/testData/copyPaste/imports/NoImportForSamePackage.expected.kt index 9a0e5ade2be..5d4be0511c8 100644 --- a/idea/testData/copyPaste/imports/NoImportForSamePackage.expected.kt +++ b/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 } \ No newline at end of file diff --git a/idea/testData/copyPaste/imports/NoImportForSamePackage.kt b/idea/testData/copyPaste/imports/NoImportForSamePackage.kt index 25d3c09b8b1..2aabd1ed355 100644 --- a/idea/testData/copyPaste/imports/NoImportForSamePackage.kt +++ b/idea/testData/copyPaste/imports/NoImportForSamePackage.kt @@ -44,13 +44,13 @@ class ClassObject { } } -fun f(p: A, t: T) { +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 } \ No newline at end of file