From 5eb4dcd913f63a72d86e2b8f937cbe22bde21ba8 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Wed, 4 Mar 2015 19:02:09 +0300 Subject: [PATCH] Report conflicting overloads for constructors - It's worked almost fine but diagnostics were filtered out by positioning strategy. - Also a couple of "put" calls to Multimap are replaced by "putValues" within OverloadResolver, that is more semantically correct. - Note that constructors of top-level classes are handled when processing package, it helps to figure out if there are clashes with top-level functions that have the same name. - But constructors of different classes are not reported as overloads because containing classes has the same name and will be reported as redeclaration. --- .../diagnostics/PositioningStrategies.kt | 7 ++- .../kotlin/resolve/OverloadResolver.java | 21 ++++++--- .../secondaryConstructors/redeclarations.kt | 28 ++++++++++++ .../secondaryConstructors/redeclarations.txt | 45 +++++++++++++++++++ .../redeclarationsOfConstructorsIgnored.kt | 11 +++++ .../redeclarationsOfConstructorsIgnored.txt | 36 +++++++++++++++ .../checkers/JetDiagnosticsTestGenerated.java | 12 +++++ 7 files changed, 153 insertions(+), 7 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/secondaryConstructors/redeclarations.kt create mode 100644 compiler/testData/diagnostics/tests/secondaryConstructors/redeclarations.txt create mode 100644 compiler/testData/diagnostics/tests/secondaryConstructors/redeclarationsOfConstructorsIgnored.kt create mode 100644 compiler/testData/diagnostics/tests/secondaryConstructors/redeclarationsOfConstructorsIgnored.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt index 41c31a2312f..e785e3487be 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/PositioningStrategies.kt @@ -31,7 +31,9 @@ import org.jetbrains.kotlin.psi.psiUtil.* public object PositioningStrategies { private open class DeclarationHeader : PositioningStrategy() { override fun isValid(element: T): Boolean { - if (element is JetNamedDeclaration && element !is JetObjectDeclaration) { + if (element is JetNamedDeclaration && + (element !is JetObjectDeclaration && element !is JetSecondaryConstructor) + ) { if (element.getNameIdentifier() == null) { return false } @@ -137,6 +139,9 @@ public object PositioningStrategies { is JetObjectDeclaration -> { return DECLARATION_NAME.mark(element) } + is JetSecondaryConstructor -> { + return markRange(element.getConstructorKeyword(), element.getValueParameterList() ?: element.getConstructorKeyword()) + } } return super.mark(element) } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadResolver.java index 45e43b6423f..0164e441805 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadResolver.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadResolver.java @@ -71,10 +71,10 @@ public class OverloadResolver { DeclarationDescriptor containingDeclaration = klass.getContainingDeclaration(); if (containingDeclaration instanceof ClassDescriptor) { ClassDescriptor classDescriptor = (ClassDescriptor) containingDeclaration; - inClasses.put(classDescriptor, klass.getConstructors()); + inClasses.putValues(classDescriptor, klass.getConstructors()); } else if (containingDeclaration instanceof PackageFragmentDescriptor) { - inPackages.put(getFqName(klass), klass.getConstructors()); + inPackages.putValues(getFqName(klass), klass.getConstructors()); } else if (containingDeclaration instanceof ScriptDescriptor) { // TODO: check overload conflicts of functions with constructors in scripts @@ -146,9 +146,6 @@ public class OverloadResolver { for (Map.Entry> e : functionsByName.entrySet()) { checkOverloadsWithSameName(e.getValue(), nameForErrorMessage(classDescriptor, klass)); } - - // Kotlin has no secondary constructors at this time - } private void checkOverloadsWithSameName( @@ -167,7 +164,7 @@ public class OverloadResolver { Set> redeclarations = Sets.newLinkedHashSet(); for (CallableMemberDescriptor member : functions) { for (CallableMemberDescriptor member2 : functions) { - if (member == member2) { + if (member == member2 || isConstructorsOfDifferentRedeclaredClasses(member, member2)) { continue; } @@ -183,6 +180,18 @@ public class OverloadResolver { return redeclarations; } + private static boolean isConstructorsOfDifferentRedeclaredClasses( + @NotNull CallableMemberDescriptor member, @NotNull CallableMemberDescriptor member2 + ) { + if (!(member instanceof ConstructorDescriptor) || !(member2 instanceof ConstructorDescriptor)) return false; + // ignore conflicting overloads for constructors of different classes because their redeclarations will be reported + // but don't ignore if there's possibility that classes redeclarations will not be reported + // (e.g. they're declared in different packages) + assert member.getContainingDeclaration().getContainingDeclaration() != null : "Grandparent of constructor should not be null"; + return member.getContainingDeclaration() != member2.getContainingDeclaration() && + member.getContainingDeclaration().getContainingDeclaration().equals(member2.getContainingDeclaration().getContainingDeclaration()); + } + private void reportRedeclarations(@NotNull String functionContainer, @NotNull Set> redeclarations) { for (Pair redeclaration : redeclarations) { diff --git a/compiler/testData/diagnostics/tests/secondaryConstructors/redeclarations.kt b/compiler/testData/diagnostics/tests/secondaryConstructors/redeclarations.kt new file mode 100644 index 00000000000..eb542956b0c --- /dev/null +++ b/compiler/testData/diagnostics/tests/secondaryConstructors/redeclarations.kt @@ -0,0 +1,28 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER + +class A(x: String = "", y: String = "") { + constructor(x: String, y: String): this(x, y) {} + constructor(): this("", "") {} + constructor(): this("", "") {} +} + +class B { + constructor(x: Int) {} +} + +fun B(x: Int) {} + +class Outer { + class A(x: String = "", y: String = "") { + constructor(x: String, y: String): this(x, y) {} + constructor(): this("", "") {} + constructor(): this("", "") {} + } + + class B { + constructor(x: Int) { + } + } + + fun B(x: Int) {} +} diff --git a/compiler/testData/diagnostics/tests/secondaryConstructors/redeclarations.txt b/compiler/testData/diagnostics/tests/secondaryConstructors/redeclarations.txt new file mode 100644 index 00000000000..5e43f01aad3 --- /dev/null +++ b/compiler/testData/diagnostics/tests/secondaryConstructors/redeclarations.txt @@ -0,0 +1,45 @@ +package + +internal fun B(/*0*/ x: kotlin.Int): kotlin.Unit + +internal final class A { + public constructor A() + public constructor A() + public constructor A(/*0*/ x: kotlin.String = ..., /*1*/ y: kotlin.String = ...) + public constructor A(/*0*/ x: kotlin.String, /*1*/ y: kotlin.String) + 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 +} + +internal final class B { + public constructor B(/*0*/ x: kotlin.Int) + 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 +} + +internal final class Outer { + public constructor Outer() + internal final fun B(/*0*/ x: kotlin.Int): kotlin.Unit + 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 + + internal final class A { + public constructor A() + public constructor A() + public constructor A(/*0*/ x: kotlin.String = ..., /*1*/ y: kotlin.String = ...) + public constructor A(/*0*/ x: kotlin.String, /*1*/ y: kotlin.String) + 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 + } + + internal final class B { + public constructor B(/*0*/ x: kotlin.Int) + 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/secondaryConstructors/redeclarationsOfConstructorsIgnored.kt b/compiler/testData/diagnostics/tests/secondaryConstructors/redeclarationsOfConstructorsIgnored.kt new file mode 100644 index 00000000000..2eb5c2c13c2 --- /dev/null +++ b/compiler/testData/diagnostics/tests/secondaryConstructors/redeclarationsOfConstructorsIgnored.kt @@ -0,0 +1,11 @@ +class A +class A { + constructor() {} +} + +class B +class Outer { + class B { + constructor() {} + } +} diff --git a/compiler/testData/diagnostics/tests/secondaryConstructors/redeclarationsOfConstructorsIgnored.txt b/compiler/testData/diagnostics/tests/secondaryConstructors/redeclarationsOfConstructorsIgnored.txt new file mode 100644 index 00000000000..ad330dc605a --- /dev/null +++ b/compiler/testData/diagnostics/tests/secondaryConstructors/redeclarationsOfConstructorsIgnored.txt @@ -0,0 +1,36 @@ +package + +internal final class A { + public constructor 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 +} + +internal final class A { + public constructor 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 +} + +internal final class B { + public constructor B() + 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 +} + +internal final class Outer { + public constructor Outer() + 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 + + internal final class B { + public constructor B() + 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/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java index 88844c86967..50c21ce071a 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java @@ -10547,6 +10547,18 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { doTest(fileName); } + @TestMetadata("redeclarations.kt") + public void testRedeclarations() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/secondaryConstructors/redeclarations.kt"); + doTest(fileName); + } + + @TestMetadata("redeclarationsOfConstructorsIgnored.kt") + public void testRedeclarationsOfConstructorsIgnored() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/secondaryConstructors/redeclarationsOfConstructorsIgnored.kt"); + doTest(fileName); + } + @TestMetadata("resolvePropertyInitializerWithoutPrimary.kt") public void testResolvePropertyInitializerWithoutPrimary() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/secondaryConstructors/resolvePropertyInitializerWithoutPrimary.kt");