Fix KT-10764 IDEA doesn't show overload conflict between constructor and function...

When checking for overloads in package, consider functions and top-level class constructors as possibly conflicting between each other. NB OverloadUtil uses containing package scope from module descriptor.

Change diagnostic message for CONFLICTING_OVERLOAD: it's misleading in case of fun vs constructor conflict.

Add custom multifile test for diagnostics in IDE (probably not the best; should preprocess file content if it's required to check highlighting in multiple files, not only in the first file).

Add test for KT-10765 Incremental compilation misses overload conflict between constructor and function ...
This commit is contained in:
Dmitry Petrov
2016-02-01 16:27:58 +03:00
parent 7dd725f0a4
commit 65f754ffca
36 changed files with 193 additions and 84 deletions
@@ -297,7 +297,7 @@ public interface Errors {
// Members
DiagnosticFactory2<KtDeclaration, CallableMemberDescriptor, String> CONFLICTING_OVERLOADS =
DiagnosticFactory2<KtDeclaration, CallableMemberDescriptor, CallableMemberDescriptor> CONFLICTING_OVERLOADS =
DiagnosticFactory2.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT);
DiagnosticFactory0<KtNamedDeclaration> NON_FINAL_MEMBER_IN_FINAL_CLASS = DiagnosticFactory0.create(WARNING, modifierSetPosition(
@@ -611,7 +611,7 @@ 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}'' is already defined in {1}", COMPACT_WITH_MODIFIERS, STRING);
MAP.put(CONFLICTING_OVERLOADS, "''{0}'' conflicts with another declaration: {1}", COMPACT_WITH_MODIFIERS, COMPACT_WITH_MODIFIERS);
MAP.put(FUNCTION_EXPECTED, "Expression ''{0}''{1} cannot be invoked as a function. " +
"The function '" + OperatorNameConventions.INVOKE.asString() + "()' is not found",
@@ -28,11 +28,10 @@ import org.jetbrains.kotlin.name.Name;
import org.jetbrains.kotlin.psi.*;
import java.util.Collection;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import static org.jetbrains.kotlin.resolve.DescriptorUtils.getFqName;
public class OverloadResolver {
@NotNull private final BindingTrace trace;
@NotNull private final OverloadFilter overloadFilter;
@@ -50,49 +49,43 @@ public class OverloadResolver {
}
private void checkOverloads(@NotNull BodiesResolveContext c) {
MultiMap<ClassDescriptor, ConstructorDescriptor> inClasses = MultiMap.create();
MultiMap<FqNameUnsafe, ConstructorDescriptor> inPackages = MultiMap.create();
fillGroupedConstructors(c, inClasses, inPackages);
MultiMap<ClassDescriptor, ConstructorDescriptor> inClasses = findConstructorsInNestedClasses(c);
for (Map.Entry<KtClassOrObject, ClassDescriptorWithResolutionScopes> entry : c.getDeclaredClasses().entrySet()) {
checkOverloadsInAClass(entry.getValue(), entry.getKey(), inClasses.get(entry.getValue()));
}
checkOverloadsInPackages(c, inPackages);
checkOverloadsInPackages(c);
}
private static void fillGroupedConstructors(
@NotNull BodiesResolveContext c,
@NotNull MultiMap<ClassDescriptor, ConstructorDescriptor> inClasses,
@NotNull MultiMap<FqNameUnsafe, ConstructorDescriptor> inPackages
) {
private static MultiMap<ClassDescriptor, ConstructorDescriptor> findConstructorsInNestedClasses(@NotNull BodiesResolveContext c) {
MultiMap<ClassDescriptor, ConstructorDescriptor> inClasses = MultiMap.create();
for (ClassDescriptorWithResolutionScopes klass : c.getDeclaredClasses().values()) {
if (klass.getKind().isSingleton() || klass.getName().isSpecial()) {
// Constructors of singletons or anonymous object aren't callable from the code, so they shouldn't participate in overload name checking
continue;
}
DeclarationDescriptor containingDeclaration = klass.getContainingDeclaration();
if (containingDeclaration instanceof ClassDescriptor) {
if (containingDeclaration instanceof ScriptDescriptor) {
// TODO: check overload conflicts of functions with constructors in scripts
}
else if (containingDeclaration instanceof ClassDescriptor) {
ClassDescriptor classDescriptor = (ClassDescriptor) containingDeclaration;
inClasses.putValues(classDescriptor, klass.getConstructors());
}
else if (containingDeclaration instanceof PackageFragmentDescriptor) {
inPackages.putValues(getFqName(klass), klass.getConstructors());
}
else if (containingDeclaration instanceof ScriptDescriptor) {
// TODO: check overload conflicts of functions with constructors in scripts
}
else if (!(containingDeclaration instanceof FunctionDescriptor || containingDeclaration instanceof PropertyDescriptor)) {
else if (!(containingDeclaration instanceof FunctionDescriptor ||
containingDeclaration instanceof PropertyDescriptor ||
containingDeclaration instanceof PackageFragmentDescriptor)) {
throw new IllegalStateException("Illegal class container: " + containingDeclaration);
}
}
return inClasses;
}
private void checkOverloadsInPackages(
@NotNull BodiesResolveContext c,
@NotNull MultiMap<FqNameUnsafe, ConstructorDescriptor> inPackages
) {
private void checkOverloadsInPackages(@NotNull BodiesResolveContext c) {
MultiMap<FqNameUnsafe, CallableMemberDescriptor> membersByName =
OverloadUtil.groupModulePackageMembersByFqName(c, inPackages, overloadFilter);
OverloadUtil.groupModulePackageMembersByFqName(c, overloadFilter);
for (Map.Entry<FqNameUnsafe, Collection<CallableMemberDescriptor>> e : membersByName.entrySet()) {
FqNameUnsafe fqName = e.getKey().parent();
@@ -203,22 +196,38 @@ public class OverloadResolver {
return file == null || file2 == null || file != file2;
}
private void reportRedeclarations(@NotNull String functionContainer,
@NotNull Set<Pair<KtDeclaration, CallableMemberDescriptor>> redeclarations) {
private void reportRedeclarations(
@NotNull String functionContainer,
@NotNull Set<Pair<KtDeclaration, CallableMemberDescriptor>> redeclarations
) {
if (redeclarations.isEmpty()) return;
Iterator<Pair<KtDeclaration, CallableMemberDescriptor>> redeclarationsIterator = redeclarations.iterator();
CallableMemberDescriptor firstRedeclarationDescriptor = redeclarationsIterator.next().getSecond();
CallableMemberDescriptor otherRedeclarationDescriptor = redeclarationsIterator.hasNext()
? redeclarationsIterator.next().getSecond()
: null;
for (Pair<KtDeclaration, CallableMemberDescriptor> redeclaration : redeclarations) {
KtDeclaration ktDeclaration = redeclaration.getFirst();
CallableMemberDescriptor memberDescriptor = redeclaration.getSecond();
KtDeclaration ktDeclaration = redeclaration.getFirst();
CallableMemberDescriptor redeclarationDescriptor;
if (otherRedeclarationDescriptor == null) {
redeclarationDescriptor = firstRedeclarationDescriptor;
}
else if (firstRedeclarationDescriptor == memberDescriptor) {
redeclarationDescriptor = otherRedeclarationDescriptor;
}
else {
redeclarationDescriptor = firstRedeclarationDescriptor;
}
if (memberDescriptor instanceof PropertyDescriptor) {
trace.report(Errors.REDECLARATION.on(ktDeclaration, memberDescriptor.getName().asString()));
}
else {
String containingClassName = ktDeclaration instanceof KtSecondaryConstructor ?
((KtSecondaryConstructor) ktDeclaration).getContainingClassOrObject().getName() : null;
trace.report(Errors.CONFLICTING_OVERLOADS.on(
ktDeclaration, memberDescriptor,
containingClassName != null ? containingClassName : functionContainer));
trace.report(Errors.CONFLICTING_OVERLOADS.on(ktDeclaration, memberDescriptor, redeclarationDescriptor));
}
}
}
@@ -82,14 +82,18 @@ object OverloadUtil {
@JvmStatic fun groupModulePackageMembersByFqName(
c: BodiesResolveContext,
constructorsInPackages: MultiMap<FqNameUnsafe, ConstructorDescriptor>,
overloadFilter: OverloadFilter
): MultiMap<FqNameUnsafe, CallableMemberDescriptor> {
val packageMembersByName = MultiMap<FqNameUnsafe, CallableMemberDescriptor>()
collectModulePackageMembersWithSameName(packageMembersByName, c.functions.values, overloadFilter) {
collectModulePackageMembersWithSameName(packageMembersByName, c.functions.values + c.declaredClasses.values, overloadFilter) {
scope, name ->
scope.getContributedFunctions(name, NoLookupLocation.WHEN_CHECK_REDECLARATIONS)
val functions = scope.getContributedFunctions(name, NoLookupLocation.WHEN_CHECK_REDECLARATIONS)
val classifier = scope.getContributedClassifier(name, NoLookupLocation.WHEN_CHECK_REDECLARATIONS)
if (classifier is ClassDescriptor && !classifier.kind.isSingleton)
functions + classifier.constructors
else
functions
}
collectModulePackageMembersWithSameName(packageMembersByName, c.properties.values, overloadFilter) {
@@ -97,15 +101,12 @@ object OverloadUtil {
scope.getContributedVariables(name, NoLookupLocation.WHEN_CHECK_REDECLARATIONS).filterIsInstance<CallableMemberDescriptor>()
}
// TODO handle constructor redeclarations in modules. See also: https://youtrack.jetbrains.com/issue/KT-3632
packageMembersByName.putAllValues(constructorsInPackages)
return packageMembersByName
}
private inline fun collectModulePackageMembersWithSameName(
packageMembersByName: MultiMap<FqNameUnsafe, CallableMemberDescriptor>,
interestingDescriptors: Collection<CallableMemberDescriptor>,
interestingDescriptors: Collection<DeclarationDescriptor>,
overloadFilter: OverloadFilter,
getMembersByName: (MemberScope, Name) -> Collection<CallableMemberDescriptor>
) {
@@ -123,20 +124,25 @@ object OverloadUtil {
}
private inline fun getModulePackageMembersWithSameName(
packageMember: CallableMemberDescriptor,
descriptor: DeclarationDescriptor,
overloadFilter: OverloadFilter,
getMembersByName: (MemberScope, Name) -> Collection<CallableMemberDescriptor>
): Collection<CallableMemberDescriptor> {
val containingPackage = packageMember.containingDeclaration
val containingPackage = descriptor.containingDeclaration
if (containingPackage !is PackageFragmentDescriptor) {
throw AssertionError("$packageMember is not a top-level package member")
throw AssertionError("$descriptor is not a top-level package member")
}
val containingModule = DescriptorUtils.getContainingModuleOrNull(packageMember) ?: return listOf(packageMember)
val containingModule = DescriptorUtils.getContainingModuleOrNull(descriptor) ?:
return when (descriptor) {
is CallableMemberDescriptor -> listOf(descriptor)
is ClassDescriptor -> descriptor.constructors
else -> throw AssertionError("Unexpected descriptor kind: $descriptor")
}
val containingPackageScope = containingModule.getPackage(containingPackage.fqName).memberScope
val possibleOverloads =
getMembersByName(containingPackageScope, packageMember.name).filter {
getMembersByName(containingPackageScope, descriptor.name).filter {
// NB memberScope for PackageViewDescriptor includes module dependencies
DescriptorUtils.getContainingModule(it) == containingModule
}
@@ -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, fromCurrent.getContainingDeclaration().getName().asString()))
trace.report(Errors.CONFLICTING_OVERLOADS.on(declaration, fromCurrent, fromSuper))
}
})
OverrideResolver.resolveUnknownVisibilities(result, trace)