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)

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<SimpleFunctionDescriptor> 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);

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;
     }

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;

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<TypeParameterDescriptor> getTypeParameters() {

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;
             }

compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java

@@ -157,7 +157,10 @@ public interface Errors {
     DiagnosticFactory<JetDelegatorToSuperClass> INITIALIZER_WITH_NO_ARGUMENTS = DiagnosticFactory.create(ERROR, "Constructor arguments required");
     DiagnosticFactory<JetDelegationSpecifier> MANY_CALLS_TO_THIS = DiagnosticFactory.create(ERROR, "Only one call to 'this(...)' is allowed");
     DiagnosticFactory1<JetModifierListOwner, CallableMemberDescriptor> NOTHING_TO_OVERRIDE = DiagnosticFactory1.create(ERROR, "{0} overrides nothing", PositioningStrategies.positionModifier(JetTokens.OVERRIDE_KEYWORD), DescriptorRenderer.TEXT);
-    DiagnosticFactory3<PsiNameIdentifierOwner, CallableMemberDescriptor, CallableMemberDescriptor, DeclarationDescriptor> 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<PsiNameIdentifierOwner, CallableMemberDescriptor, CallableMemberDescriptor, DeclarationDescriptor> 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<JetModifierListOwner, CallableMemberDescriptor, CallableDescriptor, DeclarationDescriptor> 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<JetClass, ClassDescriptor> ENUM_ENTRY_SHOULD_BE_INITIALIZED = DiagnosticFactory1.create(ERROR, "Missing delegation specifier ''{0}''", PositioningStrategies.POSITION_NAME_IDENTIFIER, NAME);
     DiagnosticFactory1<JetTypeReference, ClassDescriptor> ENUM_ENTRY_ILLEGAL_TYPE = DiagnosticFactory1.create(ERROR, "The type constructor of enum entry should be ''{0}''", NAME);

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) {

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

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.*;
-import com.google.common.collect.Multimap;
-import com.google.common.collect.Sets;
 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();
+        //all created fake descriptors are stored to resolve visibility on them later
-        checkOverrides();
+        Set<CallableMemberDescriptor> fakeOverrides = Sets.newHashSet();
-        checkVisibilityForOverriddenMembers();
+        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<CallableDescriptor, CallableDescriptor> 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<CallableMemberDescriptor> fakeOverrides) {
         Set<MutableClassDescriptor> ourClasses = new HashSet<MutableClassDescriptor>();
         ourClasses.addAll(context.getClasses().values());
         ourClasses.addAll(context.getObjects().values());
@@ -83,11 +93,12 @@ public class OverrideResolver {
         Set<ClassifierDescriptor> processed = new HashSet<ClassifierDescriptor>();
 
         for (MutableClassDescriptor clazz : ourClasses) {
-            generateOverridesInAClass(clazz, processed, ourClasses);
+            generateOverridesInAClass(clazz, processed, ourClasses, fakeOverrides);
         }
     }
 
-    private void generateOverridesInAClass(final MutableClassDescriptor classDescriptor, Set<ClassifierDescriptor> processed, Set<MutableClassDescriptor> ourClasses) {
+    private void generateOverridesInAClass(@NotNull final MutableClassDescriptor classDescriptor, @NotNull Set<ClassifierDescriptor> processed,
+            @NotNull Set<MutableClassDescriptor> ourClasses, @NotNull Set<CallableMemberDescriptor> 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<CallableMemberDescriptor> fakeOverrides,
             @NotNull Collection<? extends CallableMemberDescriptor> functionsFromSupertypes,
             @NotNull Collection<? extends CallableMemberDescriptor> functionsFromCurrent,
             @NotNull ClassDescriptor current,
             @NotNull DescriptorSink sink) {
         
-        List<CallableMemberDescriptor> fakeOverrides = Lists.newArrayList();
+        List<CallableMemberDescriptor> 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);
-                
+                fakeOverrideList.add(fakeOverride);
-                fakeOverrides.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<CallableDescriptor, CallableDescriptor> invisibleOverriddenDescriptors) {
         for (Map.Entry<JetClass, MutableClassDescriptor> entry : context.getClasses().entrySet()) {
-            checkOverridesInAClass(entry.getValue(), entry.getKey());
+            checkOverridesInAClass(entry.getValue(), entry.getKey(), invisibleOverriddenDescriptors);
         }
         for (Map.Entry<JetObjectDeclaration, MutableClassDescriptor> 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<CallableDescriptor, CallableDescriptor> 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<FunctionDescriptor> allOverriddenFunctions = Sets.newHashSet();
-//        Collection<DeclarationDescriptor> 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<CallableMemberDescriptor> abstractNoImpl, Set<CallableMemberDescriptor> manyImpl) {
@@ -317,21 +299,21 @@ public class OverrideResolver {
             }
         }
         else {
-            Collection<CallableMemberDescriptor> overridenDeclarations = OverridingUtil.getOverridenDeclarations(descriptor);
+            Collection<CallableMemberDescriptor> overriddenDeclarations = OverridingUtil.getOverridenDeclarations(descriptor);
-            if (overridenDeclarations.size() == 0) {
+            if (overriddenDeclarations.size() == 0) {
                 throw new IllegalStateException();
             }
-            else if (overridenDeclarations.size() == 1) {
+            else if (overriddenDeclarations.size() == 1) {
-                CallableMemberDescriptor single = overridenDeclarations.iterator().next();
+                CallableMemberDescriptor single = overriddenDeclarations.iterator().next();
                 if (single.getModality() == Modality.ABSTRACT) {
                     abstractNoImpl.add(single);
                 }
             }
             else {
                 List<CallableMemberDescriptor> nonAbstractManyImpl = Lists.newArrayList();
-                for (CallableMemberDescriptor overriden : overridenDeclarations) {
+                for (CallableMemberDescriptor overridden : overriddenDeclarations) {
-                    if (overriden.getModality() != Modality.ABSTRACT) {
+                    if (overridden.getModality() != Modality.ABSTRACT) {
-                        nonAbstractManyImpl.add(overriden);
+                        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<CallableDescriptor, CallableDescriptor> 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() {
+    private void resolveUnknownVisibilityForMembers(@NotNull Set<CallableMemberDescriptor> fakeOverrides) {
-        for (Map.Entry<JetNamedFunction, SimpleFunctionDescriptor> entry : context.getFunctions().entrySet()) {
+        for (CallableMemberDescriptor override : fakeOverrides) {
-            JetNamedFunction function = entry.getKey();
+            resolveUnknownVisibilityForMember(override);
-            SimpleFunctionDescriptor functionDescriptor = entry.getValue();
-
-            checkVisibilityForMember(functionDescriptor.getVisibility(), function, functionDescriptor.getOverriddenDescriptors());
         }
-        for (Map.Entry<JetProperty, PropertyDescriptor> entry : context.getProperties().entrySet()) {
+        for (CallableMemberDescriptor memberDescriptor : context.getMembers().values()) {
-            JetProperty property = entry.getKey();
+            resolveUnknownVisibilityForMember(memberDescriptor);
-            PropertyDescriptor propertyDescriptor = entry.getValue();
-
-            checkVisibilityForMember(propertyDescriptor.getVisibility(), property, propertyDescriptor.getOverriddenDescriptors());
         }
-        for (PropertyDescriptor propertyDescriptor : context.getPrimaryConstructorParameterProperties()) {
+        for (PropertyDescriptor propertyDescriptor : context.getProperties().values()) {
-            PsiElement parameter = BindingContextUtils.descriptorToDeclaration(trace.getBindingContext(), propertyDescriptor);
+            for (PropertyAccessorDescriptor accessor : propertyDescriptor.getAccessors()) {
-            if (parameter instanceof JetParameter) {
+                if (accessor != null && accessor.getVisibility() == Visibilities.INHERITED) {
-                checkVisibilityForMember(propertyDescriptor.getVisibility(), (JetParameter)parameter, propertyDescriptor.getOverriddenDescriptors());
+                    accessor.setVisibility(propertyDescriptor.getVisibility());
+                }
             }
         }
     }
 
-    private void checkVisibilityForMember(@NotNull Visibility visibility,
+    private void resolveUnknownVisibilityForMember(@NotNull CallableMemberDescriptor memberDescriptor) {
-            @NotNull JetModifierListOwner modifierListOwner,
+        resolveUnknownVisibilityForOverriddenDescriptors(memberDescriptor.getOverriddenDescriptors());
-            @NotNull Collection<? extends CallableMemberDescriptor> descriptors) {
+        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<CallableDescriptor, CallableDescriptor> invisibleOverriddenDescriptors) {
+        for (CallableMemberDescriptor memberDescriptor : context.getMembers().values()) {
+            Set<? extends CallableDescriptor> overriddenDescriptors = memberDescriptor.getOverriddenDescriptors();
+            for (Iterator<? extends CallableDescriptor> 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<? extends CallableMemberDescriptor> descriptors) {
         for (CallableMemberDescriptor descriptor : descriptors) {
+            if (descriptor.getVisibility() == Visibilities.INHERITED) {
+                resolveUnknownVisibilityForMember(descriptor);
+            }
+        }
+    }
+
+    @NotNull
+    private Visibility findMaxVisibility(@NotNull Collection<? extends CallableMemberDescriptor> 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<JetDeclaration, CallableMemberDescriptor> 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) {
+            else if (compare < 0) {
-                trace.report(CANNOT_WEAKEN_ACCESS_PRIVILEGE.on(modifierListOwner, descriptor.getVisibility(), descriptor, descriptor.getContainingDeclaration()));
+                trace.report(CANNOT_WEAKEN_ACCESS_PRIVILEGE.on(declaration, descriptor.getVisibility(), descriptor, descriptor.getContainingDeclaration()));
                 return;
             }
         }

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<JetNamedFunction, SimpleFunctionDescriptor> functions = Maps.newLinkedHashMap();
     private final Map<JetSecondaryConstructor, ConstructorDescriptor> constructors = Maps.newLinkedHashMap();
     private final Map<JetProperty, PropertyDescriptor> properties = Maps.newLinkedHashMap();
-    private final Set<PropertyDescriptor> primaryConstructorParameterProperties = Sets.newHashSet();
+    private final Map<JetParameter, PropertyDescriptor> primaryConstructorParameterProperties = Maps.newHashMap();
+    private Map<JetDeclaration, CallableMemberDescriptor> members = null;
 
     private StringBuilder debugOutput;
 
@@ -114,7 +116,7 @@ public class TopDownAnalysisContext {
         return namespaceDescriptors;
     }
 
-    public Set<PropertyDescriptor> getPrimaryConstructorParameterProperties() {
+    public Map<JetParameter, PropertyDescriptor> getPrimaryConstructorParameterProperties() {
         return primaryConstructorParameterProperties;
     }
 
@@ -134,4 +136,13 @@ public class TopDownAnalysisContext {
         return functions;
     }
 
+    public Map<JetDeclaration, CallableMemberDescriptor> getMembers() {
+        if (members == null) {
+            members = Maps.newHashMap();
+            members.putAll(functions);
+            members.putAll(properties);
+            members.putAll(primaryConstructorParameterProperties);
+        }
+        return members;
+    }
 }

compiler/testData/diagnostics/tests/scopes/VisibilityInheritModifier.jet

@@ -18,7 +18,7 @@ open class D {
 }
 
 class E : D() {
-    internal <!NOTHING_TO_OVERRIDE!>override<!> fun self() = this
+    internal <!CANNOT_OVERRIDE_INVISIBLE_MEMBER!>override<!> fun self() = this
 
     fun test() {
         val <!UNUSED_VARIABLE!>s<!> : E = self()
@@ -31,7 +31,11 @@ open class F {
 }
 
 class G : F() {
-    <!CANNOT_CHANGE_ACCESS_PRIVILEGE!>override<!> fun protected_fun() {}
+    override fun protected_fun() {}
+}
+
+fun test_fun_stays_protected(g: G) {
+    g.<!INVISIBLE_MEMBER!>protected_fun<!>()
 }
 
 //------------
@@ -57,11 +61,11 @@ trait T {
 }
 
 open class L : T {
-    <!CANNOT_WEAKEN_ACCESS_PRIVILEGE!>override<!> fun foo() {}
+    override fun foo() {}
 }
 
 class M : L() {
-    internal override fun foo() {}
+    <!CANNOT_WEAKEN_ACCESS_PRIVILEGE!>internal<!> override fun foo() {}
 }
 //---------------
 trait R {

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.<!INVISIBLE_MEMBER!>x<!>()
+}
+
+
+//more tests
+open class C {
+    internal open fun foo() {}
+}
+
+trait T {
+    protected fun foo() {}
+}
+
+class D : C(), T {
+    <!CANNOT_CHANGE_ACCESS_PRIVILEGE!>protected<!> override fun foo() {}
+}
+
+class E : C(), T {
+    <!CANNOT_CHANGE_ACCESS_PRIVILEGE!>internal<!> override fun foo() {}
+}
+
+class F : C(), T {
+    <!CANNOT_WEAKEN_ACCESS_PRIVILEGE!>private<!> override fun foo() {}
+}
+
+class G : C(), T {
+    override fun foo() {}
+}

idea/testData/quickfix/modifiers/beforeVisibilityModifer2.kt

@@ -4,5 +4,5 @@ open class A {
 }
 
 class B : A() {
-    <caret>override fun run() {}
+    <caret>internal override fun run() {}
 }

idea/testData/quickfix/modifiers/beforeVisibilityModiferParameter.kt

@@ -4,6 +4,6 @@ trait ParseResult<out T> {
     public val value : T
 }
 
-class Success<T>(<caret>override val value : T) : ParseResult<T> {
+class Success<T>(<caret>internal override val value : T) : ParseResult<T> {
     public override val success : Boolean = true
 }