KT-1822 Error 'cannot infer visibility' required

#KT-1822 fixed

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

@@ -156,11 +156,12 @@ public interface Errors {
     DiagnosticFactory<JetDelegatorByExpressionSpecifier> BY_IN_SECONDARY_CONSTRUCTOR = DiagnosticFactory.create(ERROR, "'by'-clause is only supported for primary constructors");
     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);
+    DiagnosticFactory1<JetModifierListOwner, CallableMemberDescriptor> NOTHING_TO_OVERRIDE = DiagnosticFactory1.create(ERROR, "{0} overrides nothing", PositioningStrategies.POSITION_OVERRIDE_MODIFIER, 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);
+            DiagnosticFactory3.create(ERROR, "''{0}'' cannot has no access to ''{1}'' in class {2}, so it cannot override it", PositioningStrategies.POSITION_OVERRIDE_MODIFIER, DescriptorRenderer.TEXT, DescriptorRenderer.TEXT, DescriptorRenderer.TEXT);
+    DiagnosticFactory<JetModifierListOwner> CANNOT_INFER_VISIBILITY = DiagnosticFactory.create(ERROR, "Cannot infer visibility. Please specify it explicitly", PositioningStrategies.POSITION_OVERRIDE_MODIFIER);
 
     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/diagnostics/PositioningStrategies.java

@@ -88,6 +88,8 @@ public class PositioningStrategies {
 
     public static final PositioningStrategy<JetModifierListOwner> POSITION_ABSTRACT_MODIFIER = positionModifier(JetTokens.ABSTRACT_KEYWORD);
 
+    public static final PositioningStrategy<JetModifierListOwner> POSITION_OVERRIDE_MODIFIER = positionModifier(JetTokens.OVERRIDE_KEYWORD);
+
     public static PositioningStrategy<JetModifierListOwner> positionModifier(final JetKeywordToken token) {
         return new PositioningStrategy<JetModifierListOwner>() {
             @NotNull

compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java

@@ -36,6 +36,7 @@ import javax.inject.Inject;
 import java.util.*;
 
 import static org.jetbrains.jet.lang.diagnostics.Errors.*;
+import static org.jetbrains.jet.lang.resolve.BindingContext.DECLARATION_TO_DESCRIPTOR;
 import static org.jetbrains.jet.lang.resolve.BindingContext.DELEGATED;
 
 /**
@@ -418,10 +419,10 @@ public class OverrideResolver {
 
     private void resolveUnknownVisibilityForMembers(@NotNull Set<CallableMemberDescriptor> fakeOverrides) {
         for (CallableMemberDescriptor override : fakeOverrides) {
-            resolveUnknownVisibilityForMember(override);
+            resolveUnknownVisibilityForMember(null, override);
         }
-        for (CallableMemberDescriptor memberDescriptor : context.getMembers().values()) {
-            resolveUnknownVisibilityForMember(memberDescriptor);
+        for (Map.Entry<JetDeclaration, CallableMemberDescriptor> entry : context.getMembers().entrySet()) {
+            resolveUnknownVisibilityForMember(entry.getKey(), entry.getValue());
         }
         for (PropertyDescriptor propertyDescriptor : context.getProperties().values()) {
             for (PropertyAccessorDescriptor accessor : propertyDescriptor.getAccessors()) {
@@ -432,13 +433,19 @@ public class OverrideResolver {
         }
     }
 
-    private void resolveUnknownVisibilityForMember(@NotNull CallableMemberDescriptor memberDescriptor) {
+    private void resolveUnknownVisibilityForMember(@Nullable JetDeclaration member, @NotNull CallableMemberDescriptor memberDescriptor) {
         resolveUnknownVisibilityForOverriddenDescriptors(memberDescriptor.getOverriddenDescriptors());
         if (memberDescriptor.getVisibility() != Visibilities.INHERITED) {
             return;
         }
 
         Visibility visibility = findMaxVisibility(memberDescriptor.getOverriddenDescriptors());
+        if (visibility == null) {
+            if (member != null) {
+                trace.report(CANNOT_INFER_VISIBILITY.on(member));
+            }
+            visibility = Visibilities.PUBLIC;
+        }
 
         if (memberDescriptor instanceof PropertyDescriptor) {
             ((PropertyDescriptor)memberDescriptor).setVisibility(visibility);
@@ -465,12 +472,14 @@ public class OverrideResolver {
     private void resolveUnknownVisibilityForOverriddenDescriptors(@NotNull Collection<? extends CallableMemberDescriptor> descriptors) {
         for (CallableMemberDescriptor descriptor : descriptors) {
             if (descriptor.getVisibility() == Visibilities.INHERITED) {
-                resolveUnknownVisibilityForMember(descriptor);
+                PsiElement element = BindingContextUtils.descriptorToDeclaration(trace.getBindingContext(), descriptor);
+                JetDeclaration declaration = (element instanceof JetDeclaration) ? (JetDeclaration) element : null;
+                resolveUnknownVisibilityForMember(declaration, descriptor);
             }
         }
     }
 
-    @NotNull
+    @Nullable
     private Visibility findMaxVisibility(@NotNull Collection<? extends CallableMemberDescriptor> descriptors) {
         if (descriptors.isEmpty()) {
             return Visibilities.INTERNAL;
@@ -483,16 +492,23 @@ public class OverrideResolver {
                 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;
+            Integer compareResult = Visibilities.compare(visibility, maxVisibility);
+            if (compareResult == null) {
+                maxVisibility = null;
             }
-            if (compare > 0) {
+            else if (compareResult > 0) {
                 maxVisibility = visibility;
             }
         }
-        assert maxVisibility != null;
+        if (maxVisibility == null) {
+            return null;
+        }
+        for (CallableMemberDescriptor descriptor : descriptors) {
+            Integer compareResult = Visibilities.compare(maxVisibility, descriptor.getVisibility());
+            if (compareResult == null || compareResult < 0) {
+                return null;
+            }
+        }
         return maxVisibility;
     }
 

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

@@ -36,5 +36,5 @@ class F : C(), T {
 }
 
 class G : C(), T {
-    override fun foo() {}
+    public override fun foo() {}
 }

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

@@ -0,0 +1,30 @@
+//KT-1822 Error 'cannot infer visibility' required
+package kt1822
+
+open class C {
+    internal open fun foo() {}
+}
+
+trait T {
+    protected fun foo() {}
+}
+
+class G : C(), T {
+    <!CANNOT_INFER_VISIBILITY!>override<!> fun foo() {} //should be an error "cannot infer visibility"; for now 'public' is inferred in such cases
+}
+
+open class A {
+    internal open fun foo() {}
+}
+
+trait B {
+    protected fun foo() {}
+}
+
+trait D {
+    public fun foo() {}
+}
+
+class E : A(), B, D {
+    override fun foo() {}
+}