From 1ed79d534f5b0d0f5cce42bb9972ca7d9fc9deb6 Mon Sep 17 00:00:00 2001 From: Svetlana Isakova Date: Wed, 18 Apr 2012 19:01:26 +0400 Subject: [PATCH] KT-151 Inherit visibility when overriding #KT-151 Fixed Visibilities.INHERITED constant added (occurs only during resolve), changed after overridden descriptors resolve Fake descriptors are created for invisible properties as well (is necessary for better error reporting) --- .../resolve/java/JavaDescriptorResolver.java | 4 +- .../descriptors/FunctionDescriptorImpl.java | 4 + .../PropertyAccessorDescriptor.java | 6 +- .../lang/descriptors/PropertyDescriptor.java | 4 + .../jet/lang/descriptors/Visibilities.java | 15 +- .../jet/lang/diagnostics/Errors.java | 5 +- .../jet/lang/resolve/DeclarationResolver.java | 2 +- .../jet/lang/resolve/DescriptorResolver.java | 3 +- .../jet/lang/resolve/OverrideResolver.java | 221 +++++++++++------- .../lang/resolve/TopDownAnalysisContext.java | 15 +- ...fier.jet => VisibilityInheritModifier.jet} | 12 +- .../diagnostics/tests/scopes/kt151.jet | 40 ++++ .../modifiers/beforeVisibilityModifer2.kt | 2 +- .../beforeVisibilityModiferParameter.kt | 2 +- 14 files changed, 236 insertions(+), 99 deletions(-) rename compiler/testData/diagnostics/tests/scopes/{visibility_inherit_modifier.jet => VisibilityInheritModifier.jet} (78%) create mode 100644 compiler/testData/diagnostics/tests/scopes/kt151.jet diff --git a/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/JavaDescriptorResolver.java b/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/JavaDescriptorResolver.java index e53c9031ffb..b225cf81b40 100644 --- a/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/JavaDescriptorResolver.java +++ b/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/JavaDescriptorResolver.java @@ -1394,7 +1394,7 @@ public class JavaDescriptorResolver { if (owner instanceof ClassDescriptor) { ClassDescriptor classDescriptor = (ClassDescriptor) owner; - OverrideResolver.generateOverridesInFunctionGroup(propertyName, propertiesFromSupertypes, propertiesFromCurrent, classDescriptor, new OverrideResolver.DescriptorSink() { + OverrideResolver.generateOverridesInFunctionGroup(propertyName, null, propertiesFromSupertypes, propertiesFromCurrent, classDescriptor, new OverrideResolver.DescriptorSink() { @Override public void addToScope(@NotNull CallableMemberDescriptor fakeOverride) { properties.add((PropertyDescriptor) fakeOverride); @@ -1433,7 +1433,7 @@ public class JavaDescriptorResolver { Set functionsFromSupertypes = getFunctionsFromSupertypes(scopeData, methodName); - OverrideResolver.generateOverridesInFunctionGroup(methodName, functionsFromSupertypes, functionsFromCurrent, classDescriptor, new OverrideResolver.DescriptorSink() { + OverrideResolver.generateOverridesInFunctionGroup(methodName, null, functionsFromSupertypes, functionsFromCurrent, classDescriptor, new OverrideResolver.DescriptorSink() { @Override public void addToScope(@NotNull CallableMemberDescriptor fakeOverride) { functions.add((FunctionDescriptor) fakeOverride); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/FunctionDescriptorImpl.java b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/FunctionDescriptorImpl.java index 18123e6947e..96c7cae015e 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/FunctionDescriptorImpl.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/FunctionDescriptorImpl.java @@ -106,6 +106,10 @@ public abstract class FunctionDescriptorImpl extends DeclarationDescriptorImpl i return this; } + public void setVisibility(@NotNull Visibility visibility) { + this.visibility = visibility; + } + public void setReturnType(@NotNull JetType unsubstitutedReturnType) { this.unsubstitutedReturnType = unsubstitutedReturnType; } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/PropertyAccessorDescriptor.java b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/PropertyAccessorDescriptor.java index d05a34969c1..8924fc839d9 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/PropertyAccessorDescriptor.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/PropertyAccessorDescriptor.java @@ -34,9 +34,9 @@ public abstract class PropertyAccessorDescriptor extends DeclarationDescriptorIm private final boolean hasBody; private final boolean isDefault; private final Modality modality; - private final Visibility visibility; private final PropertyDescriptor correspondingProperty; private final Kind kind; + private Visibility visibility; public PropertyAccessorDescriptor( @NotNull Modality modality, @@ -99,6 +99,10 @@ public abstract class PropertyAccessorDescriptor extends DeclarationDescriptorIm return visibility; } + public void setVisibility(Visibility visibility) { + this.visibility = visibility; + } + @NotNull public PropertyDescriptor getCorrespondingProperty() { return correspondingProperty; diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/PropertyDescriptor.java b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/PropertyDescriptor.java index c614cca9161..a7edd5d4ea5 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/PropertyDescriptor.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/PropertyDescriptor.java @@ -131,6 +131,10 @@ public class PropertyDescriptor extends VariableDescriptorImpl implements Callab this.setter = setter; } + public void setVisibility(@NotNull Visibility visibility) { + this.visibility = visibility; + } + @NotNull @Override public List getTypeParameters() { diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/Visibilities.java b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/Visibilities.java index 8651ec3b4f3..5fce7230afd 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/Visibilities.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/descriptors/Visibilities.java @@ -32,6 +32,9 @@ public class Visibilities { public static final Visibility PRIVATE = new Visibility("private", false) { @Override protected boolean isVisible(@NotNull DeclarationDescriptorWithVisibility what, @NotNull DeclarationDescriptor from) { + if (what instanceof CallableMemberDescriptor && ((CallableMemberDescriptor)what).getKind() == CallableMemberDescriptor.Kind.FAKE_OVERRIDE) { + return false; + } DeclarationDescriptor parent = what; while (parent != null) { parent = parent.getContainingDeclaration(); @@ -93,7 +96,14 @@ public class Visibilities { public static final Visibility LOCAL = new Visibility("local", false) { @Override protected boolean isVisible(@NotNull DeclarationDescriptorWithVisibility what, @NotNull DeclarationDescriptor from) { - return true; + throw new IllegalStateException(); //This method shouldn't be invoked for LOCAL visibility + } + }; + + public static final Visibility INHERITED = new Visibility("inherited", false) { + @Override + protected boolean isVisible(@NotNull DeclarationDescriptorWithVisibility what, @NotNull DeclarationDescriptor from) { + throw new IllegalStateException("Visibility is unknown yet"); //This method shouldn't be invoked for INHERITED visibility } }; @@ -102,6 +112,9 @@ public class Visibilities { public static boolean isVisible(DeclarationDescriptorWithVisibility what, DeclarationDescriptor from) { DeclarationDescriptorWithVisibility parent = what; while (parent != null) { + if (parent.getVisibility() == LOCAL) { + return true; + } if (!parent.getVisibility().isVisible(parent, from)) { return false; } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java index e6ba102a7eb..42028fc4d2c 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java @@ -157,7 +157,10 @@ public interface Errors { DiagnosticFactory INITIALIZER_WITH_NO_ARGUMENTS = DiagnosticFactory.create(ERROR, "Constructor arguments required"); DiagnosticFactory MANY_CALLS_TO_THIS = DiagnosticFactory.create(ERROR, "Only one call to 'this(...)' is allowed"); DiagnosticFactory1 NOTHING_TO_OVERRIDE = DiagnosticFactory1.create(ERROR, "{0} overrides nothing", PositioningStrategies.positionModifier(JetTokens.OVERRIDE_KEYWORD), DescriptorRenderer.TEXT); - DiagnosticFactory3 VIRTUAL_MEMBER_HIDDEN = DiagnosticFactory3.create(ERROR, "''{0}'' hides ''{1}'' in class {2} and needs 'override' modifier", PositioningStrategies.POSITION_NAME_IDENTIFIER, DescriptorRenderer.TEXT, DescriptorRenderer.TEXT, DescriptorRenderer.TEXT); + DiagnosticFactory3 VIRTUAL_MEMBER_HIDDEN = + DiagnosticFactory3.create(ERROR, "''{0}'' hides ''{1}'' in class {2} and needs 'override' modifier", PositioningStrategies.POSITION_NAME_IDENTIFIER, DescriptorRenderer.TEXT, DescriptorRenderer.TEXT, DescriptorRenderer.TEXT); + DiagnosticFactory3 CANNOT_OVERRIDE_INVISIBLE_MEMBER = + DiagnosticFactory3.create(ERROR, "''{0}'' cannot has no access to ''{1}'' in class {2}, so it cannot override it", PositioningStrategies.positionModifier(JetTokens.OVERRIDE_KEYWORD), DescriptorRenderer.TEXT, DescriptorRenderer.TEXT, DescriptorRenderer.TEXT); DiagnosticFactory1 ENUM_ENTRY_SHOULD_BE_INITIALIZED = DiagnosticFactory1.create(ERROR, "Missing delegation specifier ''{0}''", PositioningStrategies.POSITION_NAME_IDENTIFIER, NAME); DiagnosticFactory1 ENUM_ENTRY_ILLEGAL_TYPE = DiagnosticFactory1.create(ERROR, "The type constructor of enum entry should be ''{0}''", NAME); diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DeclarationResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DeclarationResolver.java index 96a8c8b0a6e..26ce64163e1 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DeclarationResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DeclarationResolver.java @@ -223,7 +223,7 @@ public class DeclarationResolver { parameter, trace ); classDescriptor.addPropertyDescriptor(propertyDescriptor); - context.getPrimaryConstructorParameterProperties().add(propertyDescriptor); + context.getPrimaryConstructorParameterProperties().put(parameter, propertyDescriptor); } } if (constructorDescriptor != null) { diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DescriptorResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DescriptorResolver.java index 50802066804..e40d2cdbcec 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DescriptorResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DescriptorResolver.java @@ -673,7 +673,8 @@ public class DescriptorResolver { @NotNull /*package*/ static Visibility resolveVisibilityFromModifiers(@Nullable JetModifierList modifierList) { - return resolveVisibilityFromModifiers(modifierList, Visibilities.INTERNAL); + Visibility defaultVisibility = modifierList != null && modifierList.hasModifier(JetTokens.OVERRIDE_KEYWORD) ? Visibilities.INHERITED : Visibilities.INTERNAL; + return resolveVisibilityFromModifiers(modifierList, defaultVisibility); } @NotNull diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java index 5cba0b13e25..e971843d3e1 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java @@ -16,14 +16,13 @@ package org.jetbrains.jet.lang.resolve; -import com.google.common.collect.Lists; -import com.google.common.collect.Multimap; -import com.google.common.collect.Sets; +import com.google.common.collect.*; import com.intellij.lang.ASTNode; import com.intellij.psi.PsiElement; import com.intellij.util.containers.LinkedMultiMap; import com.intellij.util.containers.MultiMap; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.*; import org.jetbrains.jet.lang.diagnostics.Errors; import org.jetbrains.jet.lang.psi.*; @@ -67,15 +66,26 @@ public class OverrideResolver { public void process() { - generateOverrides(); - checkOverrides(); - checkVisibilityForOverriddenMembers(); + //all created fake descriptors are stored to resolve visibility on them later + Set fakeOverrides = Sets.newHashSet(); + generateOverrides(fakeOverrides); + + //functions and properties visibility can be inherited when overriding, so for overridden members + //it can be resolved only after overrides resolve is finished + resolveUnknownVisibilityForMembers(fakeOverrides); + + //invisible overridden descriptors are saved for proper error reporting + Multimap invisibleOverriddenDescriptors = LinkedHashMultimap.create(); + removeInvisibleOverriddenDescriptors(invisibleOverriddenDescriptors); + + checkVisibility(); + checkOverrides(invisibleOverriddenDescriptors); } /** - * Generate fake overrides and add overriden descriptors to existing descriptors. + * Generate fake overrides and add overridden descriptors to existing descriptors. */ - private void generateOverrides() { + private void generateOverrides(@NotNull Set fakeOverrides) { Set ourClasses = new HashSet(); ourClasses.addAll(context.getClasses().values()); ourClasses.addAll(context.getObjects().values()); @@ -83,11 +93,12 @@ public class OverrideResolver { Set processed = new HashSet(); for (MutableClassDescriptor clazz : ourClasses) { - generateOverridesInAClass(clazz, processed, ourClasses); + generateOverridesInAClass(clazz, processed, ourClasses, fakeOverrides); } } - private void generateOverridesInAClass(final MutableClassDescriptor classDescriptor, Set processed, Set ourClasses) { + private void generateOverridesInAClass(@NotNull final MutableClassDescriptor classDescriptor, @NotNull Set processed, + @NotNull Set ourClasses, @NotNull Set fakeOverrides) { if (!processed.add(classDescriptor)) { return; } @@ -100,7 +111,7 @@ public class OverrideResolver { for (JetType supertype : classDescriptor.getTypeConstructor().getSupertypes()) { ClassDescriptor superclass = (ClassDescriptor) supertype.getConstructor().getDeclarationDescriptor(); if (superclass instanceof MutableClassDescriptor) { - generateOverridesInAClass((MutableClassDescriptor) superclass, processed, ourClasses); + generateOverridesInAClass((MutableClassDescriptor) superclass, processed, ourClasses, fakeOverrides); } } @@ -115,7 +126,7 @@ public class OverrideResolver { functionNames.addAll(functionsFromCurrentByName.keySet()); for (String functionName : functionNames) { - generateOverridesInFunctionGroup(functionName, + generateOverridesInFunctionGroup(functionName, fakeOverrides, functionsFromSupertypesByName.get(functionName), functionsFromCurrentByName.get(functionName), classDescriptor, @@ -150,18 +161,15 @@ public class OverrideResolver { public static void generateOverridesInFunctionGroup( @NotNull String name, + @Nullable Set fakeOverrides, @NotNull Collection functionsFromSupertypes, @NotNull Collection functionsFromCurrent, @NotNull ClassDescriptor current, @NotNull DescriptorSink sink) { - List fakeOverrides = Lists.newArrayList(); + List fakeOverrideList = Lists.newArrayList(); for (CallableMemberDescriptor functionFromSupertype : functionsFromSupertypes) { - if (!Visibilities.isVisible(functionFromSupertype, current)) { - continue; - } - boolean overrides = false; for (CallableMemberDescriptor functionFromCurrent : functionsFromCurrent) { @@ -175,7 +183,7 @@ public class OverrideResolver { } } - for (CallableMemberDescriptor fakeOverride : fakeOverrides) { + for (CallableMemberDescriptor fakeOverride : fakeOverrideList) { if (OverridingUtil.isOverridableBy(functionFromSupertype, fakeOverride).getResult() == OverridingUtil.OverrideCompatibilityInfo.Result.OVERRIDABLE) { fakeOverride.addOverriddenDescriptor(functionFromSupertype); overrides = true; @@ -185,9 +193,10 @@ public class OverrideResolver { if (!overrides) { CallableMemberDescriptor fakeOverride = functionFromSupertype.copy(current, false, CallableMemberDescriptor.Kind.FAKE_OVERRIDE, false); fakeOverride.addOverriddenDescriptor(functionFromSupertype); - - fakeOverrides.add(fakeOverride); - + fakeOverrideList.add(fakeOverride); + if (fakeOverrides != null) { + fakeOverrides.add(fakeOverride); + } sink.addToScope(fakeOverride); } } @@ -222,21 +231,22 @@ public class OverrideResolver { return r; } - private void checkOverrides() { + private void checkOverrides(@NotNull Multimap invisibleOverriddenDescriptors) { for (Map.Entry entry : context.getClasses().entrySet()) { - checkOverridesInAClass(entry.getValue(), entry.getKey()); + checkOverridesInAClass(entry.getValue(), entry.getKey(), invisibleOverriddenDescriptors); } for (Map.Entry entry : context.getObjects().entrySet()) { - checkOverridesInAClass(entry.getValue(), entry.getKey()); + checkOverridesInAClass(entry.getValue(), entry.getKey(), invisibleOverriddenDescriptors); } } - protected void checkOverridesInAClass(MutableClassDescriptor classDescriptor, JetClassOrObject klass) { + protected void checkOverridesInAClass(@NotNull MutableClassDescriptor classDescriptor, @NotNull JetClassOrObject klass, + @NotNull Multimap invisibleOverriddenDescriptors) { if (topDownAnalysisParameters.isAnalyzingBootstrapLibrary()) return; // Check overrides for internal consistency for (CallableMemberDescriptor member : classDescriptor.getCallableMembers()) { - checkOverride(member); + checkOverrideForMember(member, invisibleOverriddenDescriptors); } // Check if everything that must be overridden, actually is @@ -271,34 +281,6 @@ public class OverrideResolver { trace.report(ABSTRACT_MEMBER_NOT_IMPLEMENTED.on(nameIdentifier, klass, memberDescriptor)); break; } - -// Set allOverriddenFunctions = Sets.newHashSet(); -// Collection allDescriptors = classDescriptor.getDefaultType().getMemberScope().getAllDescriptors(); -// for (DeclarationDescriptor descriptor : allDescriptors) { -// if (descriptor instanceof FunctionDescriptor) { -// FunctionDescriptor functionDescriptor = (FunctionDescriptor) descriptor; -// if (functionDescriptor.getModality() != Modality.ABSTRACT) { -// for (FunctionDescriptor overriddenDescriptor : functionDescriptor.getOverriddenDescriptors()) { -// allOverriddenFunctions.add(overriddenDescriptor.getOriginal()); -// } -// } -// } -// } -// boolean foundError = false; -// for (DeclarationDescriptor descriptor : allDescriptors) { -// if (descriptor instanceof FunctionDescriptor) { -// FunctionDescriptor functionDescriptor = (FunctionDescriptor) descriptor; -// if (functionDescriptor.getModality() == Modality.ABSTRACT && !allOverriddenFunctions.contains(functionDescriptor.getOriginal()) && !foundError && nameIdentifier != null) { -// DeclarationDescriptor declarationDescriptor = functionDescriptor.getContainingDeclaration(); -// if (declarationDescriptor != classDescriptor) { -//// trace.getErrorHandler().genericError(nameIdentifier.getNode(), "Class '" + klass.getName() + "' must be declared abstract or implement abstract method '" + -//// functionDescriptor.getName() + "' declared in " + declarationDescriptor.getName()); -// trace.report(ABSTRACT_MEMBER_NOT_IMPLEMENTED.on(nameIdentifier, klass, functionDescriptor)); -// foundError = true; -// } -// } -// } -// } } public static void collectMissingImplementations(MutableClassDescriptor classDescriptor, Set abstractNoImpl, Set manyImpl) { @@ -317,21 +299,21 @@ public class OverrideResolver { } } else { - Collection overridenDeclarations = OverridingUtil.getOverridenDeclarations(descriptor); - if (overridenDeclarations.size() == 0) { + Collection overriddenDeclarations = OverridingUtil.getOverridenDeclarations(descriptor); + if (overriddenDeclarations.size() == 0) { throw new IllegalStateException(); } - else if (overridenDeclarations.size() == 1) { - CallableMemberDescriptor single = overridenDeclarations.iterator().next(); + else if (overriddenDeclarations.size() == 1) { + CallableMemberDescriptor single = overriddenDeclarations.iterator().next(); if (single.getModality() == Modality.ABSTRACT) { abstractNoImpl.add(single); } } else { List nonAbstractManyImpl = Lists.newArrayList(); - for (CallableMemberDescriptor overriden : overridenDeclarations) { - if (overriden.getModality() != Modality.ABSTRACT) { - nonAbstractManyImpl.add(overriden); + for (CallableMemberDescriptor overridden : overriddenDeclarations) { + if (overridden.getModality() != Modality.ABSTRACT) { + nonAbstractManyImpl.add(overridden); } } if (nonAbstractManyImpl.size() > 1) { @@ -371,7 +353,8 @@ public class OverrideResolver { return factoredMembers; } - private void checkOverride(CallableMemberDescriptor declared) { + private void checkOverrideForMember(@NotNull CallableMemberDescriptor declared, + @NotNull Multimap invisibleOverriddenDescriptors) { JetNamedDeclaration member = (JetNamedDeclaration) BindingContextUtils.descriptorToDeclaration(trace.getBindingContext(), declared); if (member == null) { assert trace.get(DELEGATED, declared); @@ -410,7 +393,13 @@ public class OverrideResolver { } } if (hasOverrideModifier && declared.getOverriddenDescriptors().size() == 0) { - trace.report(NOTHING_TO_OVERRIDE.on(member, declared)); + if (!invisibleOverriddenDescriptors.get(declared).isEmpty()) { + CallableDescriptor descriptor = invisibleOverriddenDescriptors.values().iterator().next(); + trace.report(CANNOT_OVERRIDE_INVISIBLE_MEMBER.on(member, declared, descriptor, descriptor.getContainingDeclaration())); + } + else { + trace.report(NOTHING_TO_OVERRIDE.on(member, declared)); + } } PsiElement nameIdentifier = member.getNameIdentifier(); if (!hasOverrideModifier && declared.getOverriddenDescriptors().size() > 0 && nameIdentifier != null) { @@ -427,38 +416,102 @@ public class OverrideResolver { return false; } - private void checkVisibilityForOverriddenMembers() { - for (Map.Entry entry : context.getFunctions().entrySet()) { - JetNamedFunction function = entry.getKey(); - SimpleFunctionDescriptor functionDescriptor = entry.getValue(); - - checkVisibilityForMember(functionDescriptor.getVisibility(), function, functionDescriptor.getOverriddenDescriptors()); + private void resolveUnknownVisibilityForMembers(@NotNull Set fakeOverrides) { + for (CallableMemberDescriptor override : fakeOverrides) { + resolveUnknownVisibilityForMember(override); } - for (Map.Entry entry : context.getProperties().entrySet()) { - JetProperty property = entry.getKey(); - PropertyDescriptor propertyDescriptor = entry.getValue(); - - checkVisibilityForMember(propertyDescriptor.getVisibility(), property, propertyDescriptor.getOverriddenDescriptors()); + for (CallableMemberDescriptor memberDescriptor : context.getMembers().values()) { + resolveUnknownVisibilityForMember(memberDescriptor); } - for (PropertyDescriptor propertyDescriptor : context.getPrimaryConstructorParameterProperties()) { - PsiElement parameter = BindingContextUtils.descriptorToDeclaration(trace.getBindingContext(), propertyDescriptor); - if (parameter instanceof JetParameter) { - checkVisibilityForMember(propertyDescriptor.getVisibility(), (JetParameter)parameter, propertyDescriptor.getOverriddenDescriptors()); + for (PropertyDescriptor propertyDescriptor : context.getProperties().values()) { + for (PropertyAccessorDescriptor accessor : propertyDescriptor.getAccessors()) { + if (accessor != null && accessor.getVisibility() == Visibilities.INHERITED) { + accessor.setVisibility(propertyDescriptor.getVisibility()); + } } } } - private void checkVisibilityForMember(@NotNull Visibility visibility, - @NotNull JetModifierListOwner modifierListOwner, - @NotNull Collection descriptors) { + private void resolveUnknownVisibilityForMember(@NotNull CallableMemberDescriptor memberDescriptor) { + resolveUnknownVisibilityForOverriddenDescriptors(memberDescriptor.getOverriddenDescriptors()); + if (memberDescriptor.getVisibility() != Visibilities.INHERITED) { + return; + } + + Visibility visibility = findMaxVisibility(memberDescriptor.getOverriddenDescriptors()); + + if (memberDescriptor instanceof PropertyDescriptor) { + ((PropertyDescriptor)memberDescriptor).setVisibility(visibility); + } + else { + assert memberDescriptor instanceof FunctionDescriptorImpl; + ((FunctionDescriptorImpl)memberDescriptor).setVisibility(visibility); + } + } + + private void removeInvisibleOverriddenDescriptors(@NotNull Multimap invisibleOverriddenDescriptors) { + for (CallableMemberDescriptor memberDescriptor : context.getMembers().values()) { + Set overriddenDescriptors = memberDescriptor.getOverriddenDescriptors(); + for (Iterator iterator = overriddenDescriptors.iterator(); iterator.hasNext(); ) { + CallableDescriptor superDescriptor = iterator.next(); + if (!Visibilities.isVisible(superDescriptor, memberDescriptor)) { + invisibleOverriddenDescriptors.put(memberDescriptor, superDescriptor); + iterator.remove(); + } + } + } + } + + private void resolveUnknownVisibilityForOverriddenDescriptors(@NotNull Collection descriptors) { for (CallableMemberDescriptor descriptor : descriptors) { + if (descriptor.getVisibility() == Visibilities.INHERITED) { + resolveUnknownVisibilityForMember(descriptor); + } + } + } + + @NotNull + private Visibility findMaxVisibility(@NotNull Collection descriptors) { + if (descriptors.isEmpty()) { + return Visibilities.INTERNAL; + } + Visibility maxVisibility = null; + for (CallableMemberDescriptor descriptor : descriptors) { + Visibility visibility = descriptor.getVisibility(); + assert visibility != Visibilities.INHERITED; + if (maxVisibility == null) { + maxVisibility = visibility; + continue; + } + Integer compare = Visibilities.compare(visibility, maxVisibility); + if (compare == null) { + maxVisibility = Visibilities.PUBLIC; //todo error or warning when inference only from incomparable visibilities + continue; + } + if (compare > 0) { + maxVisibility = visibility; + } + } + assert maxVisibility != null; + return maxVisibility; + } + + private void checkVisibility() { + for (Map.Entry entry : context.getMembers().entrySet()) { + checkVisibilityForMember(entry.getKey(), entry.getValue()); + } + } + + private void checkVisibilityForMember(@NotNull JetDeclaration declaration, @NotNull CallableMemberDescriptor memberDescriptor) { + Visibility visibility = memberDescriptor.getVisibility(); + for (CallableMemberDescriptor descriptor : memberDescriptor.getOverriddenDescriptors()) { Integer compare = Visibilities.compare(visibility, descriptor.getVisibility()); if (compare == null) { - trace.report(CANNOT_CHANGE_ACCESS_PRIVILEGE.on(modifierListOwner, descriptor.getVisibility(), descriptor, descriptor.getContainingDeclaration())); + trace.report(CANNOT_CHANGE_ACCESS_PRIVILEGE.on(declaration, descriptor.getVisibility(), descriptor, descriptor.getContainingDeclaration())); return; } - if (compare < 0) { - trace.report(CANNOT_WEAKEN_ACCESS_PRIVILEGE.on(modifierListOwner, descriptor.getVisibility(), descriptor, descriptor.getContainingDeclaration())); + else if (compare < 0) { + trace.report(CANNOT_WEAKEN_ACCESS_PRIVILEGE.on(declaration, descriptor.getVisibility(), descriptor, descriptor.getContainingDeclaration())); return; } } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalysisContext.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalysisContext.java index ba63fb3ed5f..6f7c088fae8 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalysisContext.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/TopDownAnalysisContext.java @@ -16,6 +16,7 @@ package org.jetbrains.jet.lang.resolve; +import com.google.common.collect.LinkedHashMultimap; import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.intellij.psi.PsiElement; @@ -46,7 +47,8 @@ public class TopDownAnalysisContext { private final Map functions = Maps.newLinkedHashMap(); private final Map constructors = Maps.newLinkedHashMap(); private final Map properties = Maps.newLinkedHashMap(); - private final Set primaryConstructorParameterProperties = Sets.newHashSet(); + private final Map primaryConstructorParameterProperties = Maps.newHashMap(); + private Map members = null; private StringBuilder debugOutput; @@ -114,7 +116,7 @@ public class TopDownAnalysisContext { return namespaceDescriptors; } - public Set getPrimaryConstructorParameterProperties() { + public Map getPrimaryConstructorParameterProperties() { return primaryConstructorParameterProperties; } @@ -134,4 +136,13 @@ public class TopDownAnalysisContext { return functions; } + public Map getMembers() { + if (members == null) { + members = Maps.newHashMap(); + members.putAll(functions); + members.putAll(properties); + members.putAll(primaryConstructorParameterProperties); + } + return members; + } } diff --git a/compiler/testData/diagnostics/tests/scopes/visibility_inherit_modifier.jet b/compiler/testData/diagnostics/tests/scopes/VisibilityInheritModifier.jet similarity index 78% rename from compiler/testData/diagnostics/tests/scopes/visibility_inherit_modifier.jet rename to compiler/testData/diagnostics/tests/scopes/VisibilityInheritModifier.jet index 345369ca996..db64398e7c0 100644 --- a/compiler/testData/diagnostics/tests/scopes/visibility_inherit_modifier.jet +++ b/compiler/testData/diagnostics/tests/scopes/VisibilityInheritModifier.jet @@ -18,7 +18,7 @@ open class D { } class E : D() { - internal override fun self() = this + internal override fun self() = this fun test() { val s : E = self() @@ -31,7 +31,11 @@ open class F { } class G : F() { - override fun protected_fun() {} + override fun protected_fun() {} +} + +fun test_fun_stays_protected(g: G) { + g.protected_fun() } //------------ @@ -57,11 +61,11 @@ trait T { } open class L : T { - override fun foo() {} + override fun foo() {} } class M : L() { - internal override fun foo() {} + internal override fun foo() {} } //--------------- trait R { diff --git a/compiler/testData/diagnostics/tests/scopes/kt151.jet b/compiler/testData/diagnostics/tests/scopes/kt151.jet new file mode 100644 index 00000000000..6387a2b3810 --- /dev/null +++ b/compiler/testData/diagnostics/tests/scopes/kt151.jet @@ -0,0 +1,40 @@ +//KT-151 Inherit visibility when overriding +package kt151 + +open class A { + protected open fun x() {} +} + +class B : A() { + override fun x() {} // No visibility modifier required +} + +fun test(b: B) { + b.x() +} + + +//more tests +open class C { + internal open fun foo() {} +} + +trait T { + protected fun foo() {} +} + +class D : C(), T { + protected override fun foo() {} +} + +class E : C(), T { + internal override fun foo() {} +} + +class F : C(), T { + private override fun foo() {} +} + +class G : C(), T { + override fun foo() {} +} \ No newline at end of file diff --git a/idea/testData/quickfix/modifiers/beforeVisibilityModifer2.kt b/idea/testData/quickfix/modifiers/beforeVisibilityModifer2.kt index ed204c0fa96..d88c204bde4 100644 --- a/idea/testData/quickfix/modifiers/beforeVisibilityModifer2.kt +++ b/idea/testData/quickfix/modifiers/beforeVisibilityModifer2.kt @@ -4,5 +4,5 @@ open class A { } class B : A() { - override fun run() {} + internal override fun run() {} } diff --git a/idea/testData/quickfix/modifiers/beforeVisibilityModiferParameter.kt b/idea/testData/quickfix/modifiers/beforeVisibilityModiferParameter.kt index d3c57fabf7f..cf8682a0135 100644 --- a/idea/testData/quickfix/modifiers/beforeVisibilityModiferParameter.kt +++ b/idea/testData/quickfix/modifiers/beforeVisibilityModiferParameter.kt @@ -4,6 +4,6 @@ trait ParseResult { public val value : T } -class Success(override val value : T) : ParseResult { +class Success(internal override val value : T) : ParseResult { public override val success : Boolean = true } \ No newline at end of file