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 '<root>'", "class A", and so on.
This commit is contained in:
Dmitry Petrov
2016-02-02 18:24:34 +03:00
parent 130301aa27
commit 4afe98a0f6
19 changed files with 49 additions and 58 deletions
@@ -296,7 +296,7 @@ public interface Errors {
// Members // Members
DiagnosticFactory2<KtDeclaration, CallableMemberDescriptor, CallableMemberDescriptor> CONFLICTING_OVERLOADS = DiagnosticFactory2<KtDeclaration, CallableMemberDescriptor, DeclarationDescriptor> CONFLICTING_OVERLOADS =
DiagnosticFactory2.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT); DiagnosticFactory2.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT);
DiagnosticFactory0<KtNamedDeclaration> NON_FINAL_MEMBER_IN_FINAL_CLASS = DiagnosticFactory0.create(WARNING, modifierSetPosition( DiagnosticFactory0<KtNamedDeclaration> NON_FINAL_MEMBER_IN_FINAL_CLASS = DiagnosticFactory0.create(WARNING, modifierSetPosition(
@@ -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", 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); 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. " + MAP.put(FUNCTION_EXPECTED, "Expression ''{0}''{1} cannot be invoked as a function. " +
"The function '" + OperatorNameConventions.INVOKE.asString() + "()' is not found", "The function '" + OperatorNameConventions.INVOKE.asString() + "()' is not found",
@@ -76,6 +76,19 @@ object Renderers {
@JvmField val NAME: Renderer<Named> = Renderer { it.name.asString() } @JvmField val NAME: Renderer<Named> = Renderer { it.name.asString() }
@JvmField val DECLARATION_NAME_WITH_KIND: Renderer<DeclarationDescriptor> = 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<DeclarationDescriptor> = Renderer { @JvmField val NAME_OF_PARENT_OR_FILE: Renderer<DeclarationDescriptor> = Renderer {
if (DescriptorUtils.isTopLevelDeclaration(it) && it is DeclarationDescriptorWithVisibility && it.visibility == Visibilities.PRIVATE) { if (DescriptorUtils.isTopLevelDeclaration(it) && it is DeclarationDescriptorWithVisibility && it.visibility == Visibilities.PRIVATE) {
"file" "file"
@@ -52,7 +52,7 @@ public class OverloadResolver {
MultiMap<ClassDescriptor, ConstructorDescriptor> inClasses = findConstructorsInNestedClasses(c); MultiMap<ClassDescriptor, ConstructorDescriptor> inClasses = findConstructorsInNestedClasses(c);
for (Map.Entry<KtClassOrObject, ClassDescriptorWithResolutionScopes> entry : c.getDeclaredClasses().entrySet()) { for (Map.Entry<KtClassOrObject, ClassDescriptorWithResolutionScopes> entry : c.getDeclaredClasses().entrySet()) {
checkOverloadsInAClass(entry.getValue(), entry.getKey(), inClasses.get(entry.getValue())); checkOverloadsInAClass(entry.getValue(), inClasses.get(entry.getValue()));
} }
checkOverloadsInPackages(c); checkOverloadsInPackages(c);
} }
@@ -88,27 +88,12 @@ public class OverloadResolver {
OverloadUtil.groupModulePackageMembersByFqName(c, overloadFilter); OverloadUtil.groupModulePackageMembersByFqName(c, overloadFilter);
for (Map.Entry<FqNameUnsafe, Collection<CallableMemberDescriptor>> e : membersByName.entrySet()) { for (Map.Entry<FqNameUnsafe, Collection<CallableMemberDescriptor>> e : membersByName.entrySet()) {
FqNameUnsafe fqName = e.getKey().parent(); checkOverloadsInPackage(e.getValue());
checkOverloadsInPackage(e.getValue(), fqName);
} }
} }
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 "<unknown>";
}
private void checkOverloadsInAClass( private void checkOverloadsInAClass(
ClassDescriptorWithResolutionScopes classDescriptor, KtClassOrObject klass, ClassDescriptorWithResolutionScopes classDescriptor,
Collection<ConstructorDescriptor> nestedClassConstructors Collection<ConstructorDescriptor> nestedClassConstructors
) { ) {
MultiMap<Name, CallableMemberDescriptor> functionsByName = MultiMap.create(); MultiMap<Name, CallableMemberDescriptor> functionsByName = MultiMap.create();
@@ -122,31 +107,24 @@ public class OverloadResolver {
} }
for (Map.Entry<Name, Collection<CallableMemberDescriptor>> e : functionsByName.entrySet()) { for (Map.Entry<Name, Collection<CallableMemberDescriptor>> e : functionsByName.entrySet()) {
checkOverloadsInClass(e.getValue(), classDescriptor, klass); checkOverloadsInClass(e.getValue());
} }
} }
private void checkOverloadsInPackage( private void checkOverloadsInPackage(@NotNull Collection<CallableMemberDescriptor> members) {
@NotNull Collection<CallableMemberDescriptor> members,
@NotNull FqNameUnsafe packageFQN
) {
if (members.size() == 1) return; if (members.size() == 1) return;
for (Collection<? extends CallableMemberDescriptor> redeclarationGroup : OverloadUtil.getPossibleRedeclarationGroups(members)) { for (Collection<? extends CallableMemberDescriptor> redeclarationGroup : OverloadUtil.getPossibleRedeclarationGroups(members)) {
Set<Pair<KtDeclaration, CallableMemberDescriptor>> redeclarations = findRedeclarations(redeclarationGroup); Set<Pair<KtDeclaration, CallableMemberDescriptor>> redeclarations = findRedeclarations(redeclarationGroup);
// TODO: don't render FQ name here, extract this logic to somewhere // 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( private void checkOverloadsInClass(@NotNull Collection<CallableMemberDescriptor> members) {
@NotNull Collection<CallableMemberDescriptor> members,
@NotNull ClassDescriptor classDescriptor,
@NotNull KtClassOrObject ktClass
) {
if (members.size() == 1) return; if (members.size() == 1) return;
reportRedeclarations(nameForErrorMessage(classDescriptor, ktClass), findRedeclarations(members)); reportRedeclarations(findRedeclarations(members));
} }
@NotNull @NotNull
@@ -164,9 +142,7 @@ public class OverloadResolver {
} }
KtDeclaration ktDeclaration = (KtDeclaration) DescriptorToSourceUtils.descriptorToDeclaration(member); 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; return file == null || file2 == null || file != file2;
} }
private void reportRedeclarations( private void reportRedeclarations(@NotNull Set<Pair<KtDeclaration, CallableMemberDescriptor>> redeclarations) {
@NotNull String functionContainer,
@NotNull Set<Pair<KtDeclaration, CallableMemberDescriptor>> redeclarations
) {
if (redeclarations.isEmpty()) return; if (redeclarations.isEmpty()) return;
Iterator<Pair<KtDeclaration, CallableMemberDescriptor>> redeclarationsIterator = redeclarations.iterator(); Iterator<Pair<KtDeclaration, CallableMemberDescriptor>> redeclarationsIterator = redeclarations.iterator();
@@ -210,6 +183,8 @@ public class OverloadResolver {
for (Pair<KtDeclaration, CallableMemberDescriptor> redeclaration : redeclarations) { for (Pair<KtDeclaration, CallableMemberDescriptor> redeclaration : redeclarations) {
KtDeclaration ktDeclaration = redeclaration.getFirst(); KtDeclaration ktDeclaration = redeclaration.getFirst();
if (ktDeclaration == null) continue;
CallableMemberDescriptor memberDescriptor = redeclaration.getSecond(); CallableMemberDescriptor memberDescriptor = redeclaration.getSecond();
CallableMemberDescriptor redeclarationDescriptor; CallableMemberDescriptor redeclarationDescriptor;
@@ -227,7 +202,8 @@ public class OverloadResolver {
trace.report(Errors.REDECLARATION.on(ktDeclaration, memberDescriptor.getName().asString())); trace.report(Errors.REDECLARATION.on(ktDeclaration, memberDescriptor.getName().asString()));
} }
else { else {
trace.report(Errors.CONFLICTING_OVERLOADS.on(ktDeclaration, memberDescriptor, redeclarationDescriptor)); trace.report(Errors.CONFLICTING_OVERLOADS.on(ktDeclaration, memberDescriptor,
redeclarationDescriptor.getContainingDeclaration()));
} }
} }
} }
@@ -108,7 +108,7 @@ open class LazyClassMemberScope(
override fun conflict(fromSuper: CallableMemberDescriptor, fromCurrent: CallableMemberDescriptor) { override fun conflict(fromSuper: CallableMemberDescriptor, fromCurrent: CallableMemberDescriptor) {
val declaration = DescriptorToSourceUtils.descriptorToDeclaration(fromCurrent) as? KtDeclaration ?: error("fromCurrent can not be a fake override") 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) OverrideResolver.resolveUnknownVisibilities(result, trace)
+2 -2
View File
@@ -1,7 +1,7 @@
compiler/testData/cli/jvm/conflictingOverloads.kt:1:1: error: 'public fun a(): kotlin.collections.List<kotlin.Int>' conflicts with another declaration: public fun a(): kotlin.collections.List<kotlin.String> compiler/testData/cli/jvm/conflictingOverloads.kt:1:1: error: 'public fun a(): kotlin.collections.List<kotlin.Int>' conflicts with another declaration in package '<root>'
fun a(): List<Int> = null!! fun a(): List<Int> = null!!
^ ^
compiler/testData/cli/jvm/conflictingOverloads.kt:2:1: error: 'public fun a(): kotlin.collections.List<kotlin.String>' conflicts with another declaration: public fun a(): kotlin.collections.List<kotlin.Int> compiler/testData/cli/jvm/conflictingOverloads.kt:2:1: error: 'public fun a(): kotlin.collections.List<kotlin.String>' conflicts with another declaration in package '<root>'
fun a(): List<String> = null!! fun a(): List<String> = null!!
^ ^
COMPILATION_ERROR COMPILATION_ERROR
@@ -24,6 +24,7 @@ import org.jetbrains.kotlin.diagnostics.TypeMismatchDueToTypeProjectionsData;
import org.jetbrains.kotlin.diagnostics.rendering.DefaultErrorMessages; import org.jetbrains.kotlin.diagnostics.rendering.DefaultErrorMessages;
import org.jetbrains.kotlin.diagnostics.rendering.DiagnosticFactoryToRendererMap; import org.jetbrains.kotlin.diagnostics.rendering.DiagnosticFactoryToRendererMap;
import org.jetbrains.kotlin.diagnostics.rendering.DiagnosticRenderer; 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.ErrorsJs;
import org.jetbrains.kotlin.js.resolve.diagnostics.JsCallDataHtmlRenderer; import org.jetbrains.kotlin.js.resolve.diagnostics.JsCallDataHtmlRenderer;
import org.jetbrains.kotlin.renderer.DescriptorRenderer; import org.jetbrains.kotlin.renderer.DescriptorRenderer;
@@ -137,8 +138,8 @@ public class IdeErrorMessages {
MAP.put(MANY_IMPL_MEMBER_NOT_IMPLEMENTED, "<html>{0} must override {1}<br />because it inherits many implementations of it</html>", MAP.put(MANY_IMPL_MEMBER_NOT_IMPLEMENTED, "<html>{0} must override {1}<br />because it inherits many implementations of it</html>",
RENDER_CLASS_OR_OBJECT, DescriptorRenderer.HTML); RENDER_CLASS_OR_OBJECT, DescriptorRenderer.HTML);
MAP.put(CONFLICTING_OVERLOADS, "<html>''{0}''<br />conflicts with another declaration: {1}</html>", MAP.put(CONFLICTING_OVERLOADS, "<html>''{0}''<br />conflicts with another declaration in {1}</html>",
IdeRenderers.HTML_COMPACT_WITH_MODIFIERS, IdeRenderers.HTML_COMPACT_WITH_MODIFIERS); IdeRenderers.HTML_COMPACT_WITH_MODIFIERS, Renderers.DECLARATION_NAME_WITH_KIND);
MAP.put(RESULT_TYPE_MISMATCH, "<html>Function return type mismatch." + MAP.put(RESULT_TYPE_MISMATCH, "<html>Function return type mismatch." +
"<table><tr><td>Expected:</td><td>{1}</td></tr>" + "<table><tr><td>Expected:</td><td>{1}</td></tr>" +
@@ -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 in class 'C'
// 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'
interface I { interface I {
open fun foo(){} open fun foo(){}
} }
@@ -1,3 +1,3 @@
<!-- conflictingOverloadsClass1 --> <!-- conflictingOverloadsClass1 -->
<html> <html>
'<b>public</b> <b>final</b> <b>fun</b> lol(x: kotlin.Int): kotlin.Int'<br />conflicts with another declaration: <b>public</b> <b>final</b> <b>fun</b> lol(y: kotlin.Int): kotlin.Int</html> '<b>public</b> <b>final</b> <b>fun</b> lol(x: kotlin.Int): kotlin.Int'<br />conflicts with another declaration in class 'conflictingOverloads'</html>
@@ -1,3 +1,3 @@
<!-- conflictingOverloadsClass2 --> <!-- conflictingOverloadsClass2 -->
<html> <html>
'<b>public</b> <b>final</b> <b>fun</b> lol(y: kotlin.Int): kotlin.Int'<br />conflicts with another declaration: <b>public</b> <b>final</b> <b>fun</b> lol(x: kotlin.Int): kotlin.Int</html> '<b>public</b> <b>final</b> <b>fun</b> lol(y: kotlin.Int): kotlin.Int'<br />conflicts with another declaration in class 'conflictingOverloads'</html>
@@ -1,2 +1,2 @@
<!-- conflictingOverloadsDefaultPackage1 --> <!-- conflictingOverloadsDefaultPackage1 -->
'public fun foo(x: kotlin.Int): kotlin.Int' conflicts with another declaration: public fun foo(y: kotlin.Int): kotlin.Int 'public fun foo(x: kotlin.Int): kotlin.Int' conflicts with another declaration in package '<root>'
@@ -1,2 +1,2 @@
<!-- conflictingOverloadsDefaultPackage2 --> <!-- conflictingOverloadsDefaultPackage2 -->
'public fun foo(y: kotlin.Int): kotlin.Int' conflicts with another declaration: public fun foo(x: kotlin.Int): kotlin.Int 'public fun foo(y: kotlin.Int): kotlin.Int' conflicts with another declaration in package '<root>'
@@ -1,2 +1,2 @@
<!-- constructorsRedeclaration1 --> <!-- constructorsRedeclaration1 -->
'public constructor Element(x: kotlin.String)' conflicts with another declaration: public constructor Element(x: kotlin.String) 'public constructor Element(x: kotlin.String)' conflicts with another declaration in class 'Element'
@@ -1,2 +1,2 @@
<!-- constructorsRedeclaration2 --> <!-- constructorsRedeclaration2 -->
'public constructor Element(x: kotlin.String)' conflicts with another declaration: public constructor Element(x: kotlin.String) 'public constructor Element(x: kotlin.String)' conflicts with another declaration in class 'Element'
@@ -1,2 +1,2 @@
<!-- constructorsRedeclarationTopLevel1 --> <!-- constructorsRedeclarationTopLevel1 -->
'public fun Element(x: kotlin.String): kotlin.Unit' conflicts with another declaration: public constructor Element(x: kotlin.String) 'public fun Element(x: kotlin.String): kotlin.Unit' conflicts with another declaration in class 'Element'
@@ -1,2 +1,2 @@
<!-- constructorsRedeclarationTopLevel2 --> <!-- constructorsRedeclarationTopLevel2 -->
'public constructor Element(x: kotlin.String)' conflicts with another declaration: public fun Element(x: kotlin.String): kotlin.Unit 'public constructor Element(x: kotlin.String)' conflicts with another declaration in package '<root>'
@@ -5,7 +5,7 @@ Compiling files:
src/A.kt src/A.kt
End of files End of files
COMPILATION FAILED 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 '<root>'
Cleaning output files: Cleaning output files:
@@ -6,4 +6,4 @@ Compiling files:
src/fun2.kt src/fun2.kt
End of files End of files
COMPILATION FAILED COMPILATION FAILED
'public fun function(): kotlin.Unit' conflicts with another declaration: public fun function(): kotlin.Unit 'public fun function(): kotlin.Unit' conflicts with another declaration in package 'test'
@@ -6,4 +6,4 @@ Compiling files:
src/fun2.kt src/fun2.kt
End of files End of files
COMPILATION FAILED COMPILATION FAILED
'public constructor function()' conflicts with another declaration: public constructor function() 'public constructor function()' conflicts with another declaration in package 'test'