From 4afe98a0f6c06270cab16b24e5f4f18cf3d4a77d Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Tue, 2 Feb 2016 18:24:34 +0300 Subject: [PATCH] Better diagnostics for conflicting overloads. Skip declarations without sources in reporting, not when determining redeclaration groups: this allows emitting informative diagnostics for incremental compilation. Provide containing declaration with "kind", e.g., "package ''", "class A", and so on. --- .../jetbrains/kotlin/diagnostics/Errors.java | 2 +- .../rendering/DefaultErrorMessages.java | 3 +- .../kotlin/diagnostics/rendering/Renderers.kt | 13 +++++ .../kotlin/resolve/OverloadResolver.java | 52 +++++-------------- .../lazy/descriptors/LazyClassMemberScope.kt | 2 +- .../testData/cli/jvm/conflictingOverloads.out | 4 +- .../idea/highlighter/IdeErrorMessages.java | 5 +- .../overrideImplement/ambiguousSuper.kt.after | 4 +- .../conflictingOverloadsClass1.html | 2 +- .../conflictingOverloadsClass2.html | 2 +- .../conflictingOverloadsDefaultPackage1.txt | 2 +- .../conflictingOverloadsDefaultPackage2.txt | 2 +- .../constructorsRedeclaration1.txt | 2 +- .../constructorsRedeclaration2.txt | 2 +- .../constructorsRedeclarationTopLevel1.txt | 2 +- .../constructorsRedeclarationTopLevel2.txt | 2 +- .../secondaryConstructorAdded/build.log | 2 +- .../pureKotlin/funRedeclaration/build.log | 2 +- .../build.log | 2 +- 19 files changed, 49 insertions(+), 58 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java index 2a055cbaa8c..ca5f322bffb 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/Errors.java @@ -296,7 +296,7 @@ public interface Errors { // Members - DiagnosticFactory2 CONFLICTING_OVERLOADS = + DiagnosticFactory2 CONFLICTING_OVERLOADS = DiagnosticFactory2.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT); DiagnosticFactory0 NON_FINAL_MEMBER_IN_FINAL_CLASS = DiagnosticFactory0.create(WARNING, modifierSetPosition( diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java index e0af1354d0c..2999e293565 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/DefaultErrorMessages.java @@ -611,7 +611,8 @@ public class DefaultErrorMessages { MAP.put(MANY_INTERFACES_MEMBER_NOT_IMPLEMENTED, "{0} must override {1} because it inherits multiple interface methods of it", RENDER_CLASS_OR_OBJECT, FQ_NAMES_IN_TYPES); - MAP.put(CONFLICTING_OVERLOADS, "''{0}'' conflicts with another declaration: {1}", COMPACT_WITH_MODIFIERS, COMPACT_WITH_MODIFIERS); + MAP.put(CONFLICTING_OVERLOADS, "''{0}'' conflicts with another declaration in {1}", COMPACT_WITH_MODIFIERS, + DECLARATION_NAME_WITH_KIND); MAP.put(FUNCTION_EXPECTED, "Expression ''{0}''{1} cannot be invoked as a function. " + "The function '" + OperatorNameConventions.INVOKE.asString() + "()' is not found", diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/Renderers.kt b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/Renderers.kt index ba2f874e7d7..88a73f25d79 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/Renderers.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/rendering/Renderers.kt @@ -76,6 +76,19 @@ object Renderers { @JvmField val NAME: Renderer = Renderer { it.name.asString() } + @JvmField val DECLARATION_NAME_WITH_KIND: Renderer = Renderer { + val declarationKindWithSpace = when (it) { + is PackageFragmentDescriptor -> "package " + is ClassDescriptor -> "${it.renderKind()} " + is ConstructorDescriptor -> "constructor " + is PropertyGetterDescriptor -> "property getter " + is PropertySetterDescriptor -> "property setter " + is FunctionDescriptor -> "function " + else -> throw AssertionError("Unexpected declaration kind: $it") + } + "$declarationKindWithSpace'${it.name.asString()}'" + } + @JvmField val NAME_OF_PARENT_OR_FILE: Renderer = Renderer { if (DescriptorUtils.isTopLevelDeclaration(it) && it is DeclarationDescriptorWithVisibility && it.visibility == Visibilities.PRIVATE) { "file" diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadResolver.java index 0feb8924694..0dfb6c937d6 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadResolver.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/OverloadResolver.java @@ -52,7 +52,7 @@ public class OverloadResolver { MultiMap inClasses = findConstructorsInNestedClasses(c); for (Map.Entry entry : c.getDeclaredClasses().entrySet()) { - checkOverloadsInAClass(entry.getValue(), entry.getKey(), inClasses.get(entry.getValue())); + checkOverloadsInAClass(entry.getValue(), inClasses.get(entry.getValue())); } checkOverloadsInPackages(c); } @@ -88,27 +88,12 @@ public class OverloadResolver { OverloadUtil.groupModulePackageMembersByFqName(c, overloadFilter); for (Map.Entry> e : membersByName.entrySet()) { - FqNameUnsafe fqName = e.getKey().parent(); - checkOverloadsInPackage(e.getValue(), fqName); + checkOverloadsInPackage(e.getValue()); } } - private static String nameForErrorMessage(ClassDescriptor classDescriptor, KtClassOrObject jetClass) { - String name = jetClass.getName(); - if (name != null) { - return name; - } - if (jetClass instanceof KtObjectDeclaration) { - // must be companion object - name = classDescriptor.getContainingDeclaration().getName().asString(); - return "companion object " + name; - } - // safe - return ""; - } - private void checkOverloadsInAClass( - ClassDescriptorWithResolutionScopes classDescriptor, KtClassOrObject klass, + ClassDescriptorWithResolutionScopes classDescriptor, Collection nestedClassConstructors ) { MultiMap functionsByName = MultiMap.create(); @@ -122,31 +107,24 @@ public class OverloadResolver { } for (Map.Entry> e : functionsByName.entrySet()) { - checkOverloadsInClass(e.getValue(), classDescriptor, klass); + checkOverloadsInClass(e.getValue()); } } - private void checkOverloadsInPackage( - @NotNull Collection members, - @NotNull FqNameUnsafe packageFQN - ) { + private void checkOverloadsInPackage(@NotNull Collection members) { 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(redeclarations); } } - private void checkOverloadsInClass( - @NotNull Collection members, - @NotNull ClassDescriptor classDescriptor, - @NotNull KtClassOrObject ktClass - ) { + private void checkOverloadsInClass(@NotNull Collection members) { if (members.size() == 1) return; - reportRedeclarations(nameForErrorMessage(classDescriptor, ktClass), findRedeclarations(members)); + reportRedeclarations(findRedeclarations(members)); } @NotNull @@ -164,9 +142,7 @@ public class OverloadResolver { } KtDeclaration ktDeclaration = (KtDeclaration) DescriptorToSourceUtils.descriptorToDeclaration(member); - if (ktDeclaration != null) { - redeclarations.add(Pair.create(ktDeclaration, member)); - } + redeclarations.add(Pair.create(ktDeclaration, member)); } } } @@ -196,10 +172,7 @@ public class OverloadResolver { return file == null || file2 == null || file != file2; } - private void reportRedeclarations( - @NotNull String functionContainer, - @NotNull Set> redeclarations - ) { + private void reportRedeclarations(@NotNull Set> redeclarations) { if (redeclarations.isEmpty()) return; Iterator> redeclarationsIterator = redeclarations.iterator(); @@ -210,6 +183,8 @@ public class OverloadResolver { for (Pair redeclaration : redeclarations) { KtDeclaration ktDeclaration = redeclaration.getFirst(); + if (ktDeclaration == null) continue; + CallableMemberDescriptor memberDescriptor = redeclaration.getSecond(); CallableMemberDescriptor redeclarationDescriptor; @@ -227,7 +202,8 @@ public class OverloadResolver { trace.report(Errors.REDECLARATION.on(ktDeclaration, memberDescriptor.getName().asString())); } else { - trace.report(Errors.CONFLICTING_OVERLOADS.on(ktDeclaration, memberDescriptor, redeclarationDescriptor)); + trace.report(Errors.CONFLICTING_OVERLOADS.on(ktDeclaration, memberDescriptor, + redeclarationDescriptor.getContainingDeclaration())); } } } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyClassMemberScope.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyClassMemberScope.kt index c0af18a12d5..1837e8cbb6b 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyClassMemberScope.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyClassMemberScope.kt @@ -108,7 +108,7 @@ open class LazyClassMemberScope( override fun conflict(fromSuper: CallableMemberDescriptor, fromCurrent: CallableMemberDescriptor) { val declaration = DescriptorToSourceUtils.descriptorToDeclaration(fromCurrent) as? KtDeclaration ?: error("fromCurrent can not be a fake override") - trace.report(Errors.CONFLICTING_OVERLOADS.on(declaration, fromCurrent, fromSuper)) + trace.report(Errors.CONFLICTING_OVERLOADS.on(declaration, fromCurrent, fromSuper.containingDeclaration)) } }) OverrideResolver.resolveUnknownVisibilities(result, trace) diff --git a/compiler/testData/cli/jvm/conflictingOverloads.out b/compiler/testData/cli/jvm/conflictingOverloads.out index 51b79f53071..ba95ccd2a0f 100644 --- a/compiler/testData/cli/jvm/conflictingOverloads.out +++ b/compiler/testData/cli/jvm/conflictingOverloads.out @@ -1,7 +1,7 @@ -compiler/testData/cli/jvm/conflictingOverloads.kt:1:1: error: 'public fun a(): kotlin.collections.List' conflicts with another declaration: public fun a(): kotlin.collections.List +compiler/testData/cli/jvm/conflictingOverloads.kt:1:1: error: 'public fun a(): kotlin.collections.List' conflicts with another declaration in package '' fun a(): List = null!! ^ -compiler/testData/cli/jvm/conflictingOverloads.kt:2:1: error: 'public fun a(): kotlin.collections.List' conflicts with another declaration: public fun a(): kotlin.collections.List +compiler/testData/cli/jvm/conflictingOverloads.kt:2:1: error: 'public fun a(): kotlin.collections.List' conflicts with another declaration in package '' fun a(): List = null!! ^ COMPILATION_ERROR \ No newline at end of file diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/IdeErrorMessages.java b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/IdeErrorMessages.java index 02b602daaaa..174675ffec4 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/IdeErrorMessages.java +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/IdeErrorMessages.java @@ -24,6 +24,7 @@ import org.jetbrains.kotlin.diagnostics.TypeMismatchDueToTypeProjectionsData; import org.jetbrains.kotlin.diagnostics.rendering.DefaultErrorMessages; import org.jetbrains.kotlin.diagnostics.rendering.DiagnosticFactoryToRendererMap; import org.jetbrains.kotlin.diagnostics.rendering.DiagnosticRenderer; +import org.jetbrains.kotlin.diagnostics.rendering.Renderers; import org.jetbrains.kotlin.js.resolve.diagnostics.ErrorsJs; import org.jetbrains.kotlin.js.resolve.diagnostics.JsCallDataHtmlRenderer; import org.jetbrains.kotlin.renderer.DescriptorRenderer; @@ -137,8 +138,8 @@ public class IdeErrorMessages { MAP.put(MANY_IMPL_MEMBER_NOT_IMPLEMENTED, "{0} must override {1}
because it inherits many implementations of it", RENDER_CLASS_OR_OBJECT, DescriptorRenderer.HTML); - MAP.put(CONFLICTING_OVERLOADS, "''{0}''
conflicts with another declaration: {1}", - IdeRenderers.HTML_COMPACT_WITH_MODIFIERS, IdeRenderers.HTML_COMPACT_WITH_MODIFIERS); + MAP.put(CONFLICTING_OVERLOADS, "''{0}''
conflicts with another declaration in {1}", + IdeRenderers.HTML_COMPACT_WITH_MODIFIERS, Renderers.DECLARATION_NAME_WITH_KIND); MAP.put(RESULT_TYPE_MISMATCH, "Function return type mismatch." + "" + diff --git a/idea/testData/codeInsight/overrideImplement/ambiguousSuper.kt.after b/idea/testData/codeInsight/overrideImplement/ambiguousSuper.kt.after index f6647ea4237..278b8738ab4 100644 --- a/idea/testData/codeInsight/overrideImplement/ambiguousSuper.kt.after +++ b/idea/testData/codeInsight/overrideImplement/ambiguousSuper.kt.after @@ -1,5 +1,5 @@ -// ERROR: 'public open fun foo(): kotlin.Unit' conflicts with another declaration: public open fun foo(): kotlin.Unit -// ERROR: 'public open fun foo(): kotlin.Unit' conflicts with another declaration: public open fun foo(): kotlin.Unit +// ERROR: 'public open fun foo(): kotlin.Unit' conflicts with another declaration in class 'C' +// ERROR: 'public open fun foo(): kotlin.Unit' conflicts with another declaration in class 'C' interface I { open fun foo(){} } diff --git a/idea/testData/diagnosticMessage/conflictingOverloadsClass1.html b/idea/testData/diagnosticMessage/conflictingOverloadsClass1.html index 53268043a68..da51b028e10 100644 --- a/idea/testData/diagnosticMessage/conflictingOverloadsClass1.html +++ b/idea/testData/diagnosticMessage/conflictingOverloadsClass1.html @@ -1,3 +1,3 @@ -'publicfinalfun lol(x: kotlin.Int): kotlin.Int'
conflicts with another declaration: publicfinalfun lol(y: kotlin.Int): kotlin.Int \ No newline at end of file +'publicfinalfun lol(x: kotlin.Int): kotlin.Int'
conflicts with another declaration in class 'conflictingOverloads' \ No newline at end of file diff --git a/idea/testData/diagnosticMessage/conflictingOverloadsClass2.html b/idea/testData/diagnosticMessage/conflictingOverloadsClass2.html index c1d67ac84fe..7bb533b2ab5 100644 --- a/idea/testData/diagnosticMessage/conflictingOverloadsClass2.html +++ b/idea/testData/diagnosticMessage/conflictingOverloadsClass2.html @@ -1,3 +1,3 @@ -'publicfinalfun lol(y: kotlin.Int): kotlin.Int'
conflicts with another declaration: publicfinalfun lol(x: kotlin.Int): kotlin.Int \ No newline at end of file +'publicfinalfun lol(y: kotlin.Int): kotlin.Int'
conflicts with another declaration in class 'conflictingOverloads' \ No newline at end of file diff --git a/idea/testData/diagnosticMessage/conflictingOverloadsDefaultPackage1.txt b/idea/testData/diagnosticMessage/conflictingOverloadsDefaultPackage1.txt index 42a2a4d5103..b41855c1694 100644 --- a/idea/testData/diagnosticMessage/conflictingOverloadsDefaultPackage1.txt +++ b/idea/testData/diagnosticMessage/conflictingOverloadsDefaultPackage1.txt @@ -1,2 +1,2 @@ -'public fun foo(x: kotlin.Int): kotlin.Int' conflicts with another declaration: public fun foo(y: kotlin.Int): kotlin.Int \ No newline at end of file +'public fun foo(x: kotlin.Int): kotlin.Int' conflicts with another declaration in package '' \ No newline at end of file diff --git a/idea/testData/diagnosticMessage/conflictingOverloadsDefaultPackage2.txt b/idea/testData/diagnosticMessage/conflictingOverloadsDefaultPackage2.txt index e3652a85914..4f85235fbe7 100644 --- a/idea/testData/diagnosticMessage/conflictingOverloadsDefaultPackage2.txt +++ b/idea/testData/diagnosticMessage/conflictingOverloadsDefaultPackage2.txt @@ -1,2 +1,2 @@ -'public fun foo(y: kotlin.Int): kotlin.Int' conflicts with another declaration: public fun foo(x: kotlin.Int): kotlin.Int \ No newline at end of file +'public fun foo(y: kotlin.Int): kotlin.Int' conflicts with another declaration in package '' \ No newline at end of file diff --git a/idea/testData/diagnosticMessage/constructorsRedeclaration1.txt b/idea/testData/diagnosticMessage/constructorsRedeclaration1.txt index dd7082a4cf0..461c8bfe26e 100644 --- a/idea/testData/diagnosticMessage/constructorsRedeclaration1.txt +++ b/idea/testData/diagnosticMessage/constructorsRedeclaration1.txt @@ -1,2 +1,2 @@ -'public constructor Element(x: kotlin.String)' conflicts with another declaration: public constructor Element(x: kotlin.String) \ No newline at end of file +'public constructor Element(x: kotlin.String)' conflicts with another declaration in class 'Element' \ No newline at end of file diff --git a/idea/testData/diagnosticMessage/constructorsRedeclaration2.txt b/idea/testData/diagnosticMessage/constructorsRedeclaration2.txt index cd8b41f5b85..ddbfe553c9d 100644 --- a/idea/testData/diagnosticMessage/constructorsRedeclaration2.txt +++ b/idea/testData/diagnosticMessage/constructorsRedeclaration2.txt @@ -1,2 +1,2 @@ -'public constructor Element(x: kotlin.String)' conflicts with another declaration: public constructor Element(x: kotlin.String) \ No newline at end of file +'public constructor Element(x: kotlin.String)' conflicts with another declaration in class 'Element' \ No newline at end of file diff --git a/idea/testData/diagnosticMessage/constructorsRedeclarationTopLevel1.txt b/idea/testData/diagnosticMessage/constructorsRedeclarationTopLevel1.txt index 0ca4910816e..88eb437c3f4 100644 --- a/idea/testData/diagnosticMessage/constructorsRedeclarationTopLevel1.txt +++ b/idea/testData/diagnosticMessage/constructorsRedeclarationTopLevel1.txt @@ -1,2 +1,2 @@ -'public fun Element(x: kotlin.String): kotlin.Unit' conflicts with another declaration: public constructor Element(x: kotlin.String) \ No newline at end of file +'public fun Element(x: kotlin.String): kotlin.Unit' conflicts with another declaration in class 'Element' \ No newline at end of file diff --git a/idea/testData/diagnosticMessage/constructorsRedeclarationTopLevel2.txt b/idea/testData/diagnosticMessage/constructorsRedeclarationTopLevel2.txt index a27264502e9..53c625fdb97 100644 --- a/idea/testData/diagnosticMessage/constructorsRedeclarationTopLevel2.txt +++ b/idea/testData/diagnosticMessage/constructorsRedeclarationTopLevel2.txt @@ -1,2 +1,2 @@ -'public constructor Element(x: kotlin.String)' conflicts with another declaration: public fun Element(x: kotlin.String): kotlin.Unit \ No newline at end of file +'public constructor Element(x: kotlin.String)' conflicts with another declaration in package '' \ No newline at end of file diff --git a/jps-plugin/testData/incremental/classHierarchyAffected/secondaryConstructorAdded/build.log b/jps-plugin/testData/incremental/classHierarchyAffected/secondaryConstructorAdded/build.log index 7122f36b6d3..977648d6f3f 100644 --- a/jps-plugin/testData/incremental/classHierarchyAffected/secondaryConstructorAdded/build.log +++ b/jps-plugin/testData/incremental/classHierarchyAffected/secondaryConstructorAdded/build.log @@ -5,7 +5,7 @@ Compiling files: src/A.kt End of files COMPILATION FAILED -'public constructor A(x: kotlin.String)' conflicts with another declaration: public constructor A(x: kotlin.String) +'public constructor A(x: kotlin.String)' conflicts with another declaration in package '' Cleaning output files: diff --git a/jps-plugin/testData/incremental/pureKotlin/funRedeclaration/build.log b/jps-plugin/testData/incremental/pureKotlin/funRedeclaration/build.log index 1224d634ee4..39ae0ba5aea 100644 --- a/jps-plugin/testData/incremental/pureKotlin/funRedeclaration/build.log +++ b/jps-plugin/testData/incremental/pureKotlin/funRedeclaration/build.log @@ -6,4 +6,4 @@ Compiling files: src/fun2.kt End of files COMPILATION FAILED -'public fun function(): kotlin.Unit' conflicts with another declaration: public fun function(): kotlin.Unit \ No newline at end of file +'public fun function(): kotlin.Unit' conflicts with another declaration in package 'test' \ No newline at end of file diff --git a/jps-plugin/testData/incremental/pureKotlin/funVsConstructorOverloadConflict/build.log b/jps-plugin/testData/incremental/pureKotlin/funVsConstructorOverloadConflict/build.log index e3d39a6b779..66735e9dcd1 100644 --- a/jps-plugin/testData/incremental/pureKotlin/funVsConstructorOverloadConflict/build.log +++ b/jps-plugin/testData/incremental/pureKotlin/funVsConstructorOverloadConflict/build.log @@ -6,4 +6,4 @@ Compiling files: src/fun2.kt End of files COMPILATION FAILED -'public constructor function()' conflicts with another declaration: public constructor function() \ No newline at end of file +'public constructor function()' conflicts with another declaration in package 'test' \ No newline at end of file
Expected:{1}