KT-1942 Package local members from Java are visible in subclasses

#KT-1942 fixed
2012-05-21 18:32:01 +04:00
parent 8db0546052
commit c2385407e0
15 changed files with 84 additions and 71 deletions
@@ -1411,7 +1411,7 @@ public class JavaDescriptorResolver {
        if (owner instanceof ClassDescriptor) {
            ClassDescriptor classDescriptor = (ClassDescriptor) owner;

-            OverrideResolver.generateOverridesInFunctionGroup(propertyName, null, propertiesFromSupertypes, propertiesFromCurrent, classDescriptor, new OverrideResolver.DescriptorSink() {
+            OverrideResolver.generateOverridesInFunctionGroup(propertyName, null, propertiesFromSupertypes, propertiesFromCurrent, classDescriptor, null, new OverrideResolver.DescriptorSink() {
                @Override
                public void addToScope(@NotNull CallableMemberDescriptor fakeOverride) {
                    properties.add((PropertyDescriptor) fakeOverride);
@@ -1450,7 +1450,7 @@ public class JavaDescriptorResolver {

            Set<SimpleFunctionDescriptor> functionsFromSupertypes = getFunctionsFromSupertypes(scopeData, methodName);

-            OverrideResolver.generateOverridesInFunctionGroup(methodName, null, functionsFromSupertypes, functionsFromCurrent, classDescriptor, new OverrideResolver.DescriptorSink() {
+            OverrideResolver.generateOverridesInFunctionGroup(methodName, null, functionsFromSupertypes, functionsFromCurrent, classDescriptor, null, new OverrideResolver.DescriptorSink() {
                @Override
                public void addToScope(@NotNull CallableMemberDescriptor fakeOverride) {
                    functions.add((FunctionDescriptor) fakeOverride);
@@ -47,5 +47,5 @@ public interface CallableMemberDescriptor extends CallableDescriptor, MemberDesc
    Kind getKind();

    @NotNull
-    CallableMemberDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, Kind kind, boolean copyOverrides);
+    CallableMemberDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, boolean makeInvisible, Kind kind, boolean copyOverrides);
 }
@@ -110,7 +110,7 @@ public class ConstructorDescriptorImpl extends FunctionDescriptorImpl implements

    @NotNull
    @Override
-    public ConstructorDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, Kind kind, boolean copyOverrides) {
+    public ConstructorDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, boolean makeInvisible, Kind kind, boolean copyOverrides) {
        throw new UnsupportedOperationException("Constructors should not be copied for overriding");
    }
 }
@@ -42,5 +42,5 @@ public interface FunctionDescriptor extends CallableMemberDescriptor {

    @NotNull
    @Override
-    FunctionDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, Kind kind, boolean copyOverrides);
+    FunctionDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, boolean makeInvisible, Kind kind, boolean copyOverrides);
 }
@@ -183,11 +183,11 @@ public abstract class FunctionDescriptorImpl extends DeclarationDescriptorImpl i
        if (originalSubstitutor.isEmpty()) {
            return this;
        }
-        return doSubstitute(originalSubstitutor, getContainingDeclaration(), modality, true, true, getKind());
+        return doSubstitute(originalSubstitutor, getContainingDeclaration(), modality, visibility, true, true, getKind());
    }

    protected FunctionDescriptor doSubstitute(TypeSubstitutor originalSubstitutor,
-            DeclarationDescriptor newOwner, Modality newModality, boolean preserveOriginal, boolean copyOverrides, Kind kind) {
+            DeclarationDescriptor newOwner, Modality newModality, Visibility newVisibility, boolean preserveOriginal, boolean copyOverrides, Kind kind) {
        FunctionDescriptorImpl substitutedDescriptor = createSubstitutedCopy(newOwner, preserveOriginal, kind);

        List<TypeParameterDescriptor> substitutedTypeParameters = Lists.newArrayList();
@@ -227,7 +227,7 @@ public abstract class FunctionDescriptorImpl extends DeclarationDescriptorImpl i
                substitutedValueParameters,
                substitutedReturnType,
                newModality,
-                visibility
+                newVisibility
        );
        if (copyOverrides) {
            for (FunctionDescriptor overriddenFunction : overriddenFunctions) {
@@ -122,7 +122,7 @@ public abstract class PropertyAccessorDescriptor extends DeclarationDescriptorIm

    @NotNull
    @Override
-    public PropertyAccessorDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, Kind kind, boolean copyOverrides) {
+    public PropertyAccessorDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, boolean makeInvisible, Kind kind, boolean copyOverrides) {
        throw new UnsupportedOperationException("Accessors must be copied by the corresponding property");
    }

@@ -206,13 +206,13 @@ public class PropertyDescriptor extends VariableDescriptorImpl implements Callab
        if (originalSubstitutor.isEmpty()) {
            return this;
        }
-        return doSubstitute(originalSubstitutor, getContainingDeclaration(), modality, true, true, getKind());
+        return doSubstitute(originalSubstitutor, getContainingDeclaration(), modality, visibility, true, true, getKind());
    }

    private PropertyDescriptor doSubstitute(TypeSubstitutor originalSubstitutor,
-            DeclarationDescriptor newOwner, Modality newModality, boolean preserveOriginal, boolean copyOverrides, Kind kind) {
+            DeclarationDescriptor newOwner, Modality newModality, Visibility newVisibility, boolean preserveOriginal, boolean copyOverrides, Kind kind) {
        PropertyDescriptor substitutedDescriptor = new PropertyDescriptor(preserveOriginal ? getOriginal() : this, newOwner,
-                getAnnotations(), newModality, getVisibility(), isVar(), isObjectDeclaration(), getName(), kind);
+                getAnnotations(), newModality, newVisibility, isVar(), isObjectDeclaration(), getName(), kind);

        List<TypeParameterDescriptor> substitutedTypeParameters = Lists.newArrayList();
        TypeSubstitutor substitutor = DescriptorSubstitutor.substituteTypeParameters(getTypeParameters(), originalSubstitutor, substitutedDescriptor, substitutedTypeParameters);
@@ -305,8 +305,8 @@ public class PropertyDescriptor extends VariableDescriptorImpl implements Callab

    @NotNull
    @Override
-    public PropertyDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, Kind kind, boolean copyOverrides) {
-        return doSubstitute(TypeSubstitutor.EMPTY, newOwner, DescriptorUtils.convertModality(modality, makeNonAbstract), false, copyOverrides, kind);
+    public PropertyDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, boolean makeInvisible, Kind kind, boolean copyOverrides) {
+        return doSubstitute(TypeSubstitutor.EMPTY, newOwner, DescriptorUtils.convertModality(modality, makeNonAbstract), makeInvisible ? Visibilities.INVISIBLE_FAKE : visibility, false, copyOverrides, kind);
    }
    
    public static PropertyDescriptor createDummy() {
@@ -27,7 +27,7 @@ public interface SimpleFunctionDescriptor extends FunctionDescriptor {

    @NotNull
    @Override
-    SimpleFunctionDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, Kind kind, boolean copyOverrides);
+    SimpleFunctionDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, boolean makeInvisible, Kind kind, boolean copyOverrides);

    @NotNull
    @Override
@@ -93,9 +93,9 @@ public class SimpleFunctionDescriptorImpl extends FunctionDescriptorImpl impleme

    @NotNull
    @Override
-    public SimpleFunctionDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, Kind kind, boolean copyOverrides) {
+    public SimpleFunctionDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, boolean makeInvisible, Kind kind, boolean copyOverrides) {
        SimpleFunctionDescriptorImpl copy = (SimpleFunctionDescriptorImpl)doSubstitute(TypeSubstitutor.EMPTY, newOwner, DescriptorUtils
-            .convertModality(modality, makeNonAbstract), false, copyOverrides, kind);
+            .convertModality(modality, makeNonAbstract), makeInvisible ? Visibilities.INVISIBLE_FAKE : visibility, false, copyOverrides, kind);
        copy.isInline = isInline;
        return copy;
    }
@@ -21,6 +21,7 @@ import com.google.common.collect.Sets;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.jet.lang.resolve.DescriptorUtils;
+import org.jetbrains.jet.lang.resolve.OverridingUtil;

 import java.util.Map;
 import java.util.Set;
@@ -32,14 +33,11 @@ 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();
                if ((parent instanceof ClassDescriptor && !DescriptorUtils.isClassObject(parent)) ||
-                        parent instanceof NamespaceDescriptor) {
+                    parent instanceof NamespaceDescriptor) {
                    break;
                }
            }
@@ -107,6 +105,14 @@ public class Visibilities {
        }
    };

+    /* Visibility for fake override invisible members (they are created for better error reporting) */
+    public static final Visibility INVISIBLE_FAKE = new Visibility("invisible_fake", false) {
+        @Override
+        protected boolean isVisible(@NotNull DeclarationDescriptorWithVisibility what, @NotNull DeclarationDescriptor from) {
+            return false;
+        }
+    };
+
    public static final Set<Visibility> INTERNAL_VISIBILITIES = Sets.newHashSet(PRIVATE, INTERNAL, INTERNAL_PROTECTED, LOCAL);

    private Visibilities() {
@@ -127,6 +133,7 @@ public class Visibilities {
    }

    private static final Map<Visibility, Integer> ORDERED_VISIBILITIES = Maps.newHashMap();
+
    static {
        ORDERED_VISIBILITIES.put(PRIVATE, 0);
        ORDERED_VISIBILITIES.put(INTERNAL_PROTECTED, 1);
@@ -135,7 +142,8 @@ public class Visibilities {
        ORDERED_VISIBILITIES.put(PUBLIC, 3);
    }

-    /*package*/ static Integer compareLocal(@NotNull Visibility first, @NotNull Visibility second) {
+    /*package*/
+    static Integer compareLocal(@NotNull Visibility first, @NotNull Visibility second) {
        if (first == second) return 0;
        Integer firstIndex = ORDERED_VISIBILITIES.get(first);
        Integer secondIndex = ORDERED_VISIBILITIES.get(second);
@@ -71,7 +71,7 @@ public class DelegationResolver {
                        if (declarationDescriptor instanceof PropertyDescriptor) {
                            PropertyDescriptor propertyDescriptor = (PropertyDescriptor) declarationDescriptor;
                            if (propertyDescriptor.getModality().isOverridable()) {
-                                PropertyDescriptor copy = propertyDescriptor.copy(classDescriptor, true, CallableMemberDescriptor.Kind.DELEGATION, true);
+                                PropertyDescriptor copy = propertyDescriptor.copy(classDescriptor, true, false, CallableMemberDescriptor.Kind.DELEGATION, true);
                                classDescriptor.getBuilder().addPropertyDescriptor(copy);
                                trace.record(DELEGATED, copy);
                            }
@@ -79,7 +79,7 @@ public class DelegationResolver {
                        else if (declarationDescriptor instanceof SimpleFunctionDescriptor) {
                            SimpleFunctionDescriptor functionDescriptor = (SimpleFunctionDescriptor) declarationDescriptor;
                            if (functionDescriptor.getModality().isOverridable()) {
-                                SimpleFunctionDescriptor copy = functionDescriptor.copy(classDescriptor, true, CallableMemberDescriptor.Kind.DELEGATION, true);
+                                SimpleFunctionDescriptor copy = functionDescriptor.copy(classDescriptor, true, false, CallableMemberDescriptor.Kind.DELEGATION, true);
                                classDescriptor.getBuilder().addFunctionDescriptor(copy);
                                trace.record(DELEGATED, copy);
                            }
@@ -70,16 +70,8 @@ public class OverrideResolver {

    public void process() {
        //all created fake descriptors are stored to resolve visibility on them later
-        Set<CallableMemberDescriptor> 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<CallableDescriptor, CallableDescriptor> invisibleOverriddenDescriptors = LinkedHashMultimap.create();
-        removeInvisibleOverriddenDescriptors(invisibleOverriddenDescriptors);
+        generateOverrides(invisibleOverriddenDescriptors);

        checkVisibility();
        checkOverrides(invisibleOverriddenDescriptors);
@@ -89,7 +81,7 @@ public class OverrideResolver {
    /**
     * Generate fake overrides and add overridden descriptors to existing descriptors.
     */
-    private void generateOverrides(@NotNull Set<CallableMemberDescriptor> fakeOverrides) {
+    private void generateOverrides(@NotNull Multimap<CallableDescriptor, CallableDescriptor> invisibleOverriddenDescriptors) {
        Set<MutableClassDescriptor> ourClasses = new HashSet<MutableClassDescriptor>();
        ourClasses.addAll(context.getClasses().values());
        ourClasses.addAll(context.getObjects().values());
@@ -97,12 +89,12 @@ public class OverrideResolver {
        Set<ClassifierDescriptor> processed = new HashSet<ClassifierDescriptor>();

        for (MutableClassDescriptor clazz : ourClasses) {
-            generateOverridesInAClass(clazz, processed, ourClasses, fakeOverrides);
+            generateOverridesInAClass(clazz, processed, ourClasses, invisibleOverriddenDescriptors);
        }
    }

    private void generateOverridesInAClass(@NotNull final MutableClassDescriptor classDescriptor, @NotNull Set<ClassifierDescriptor> processed,
-            @NotNull Set<MutableClassDescriptor> ourClasses, @NotNull Set<CallableMemberDescriptor> fakeOverrides) {
+            @NotNull Set<MutableClassDescriptor> ourClasses, @NotNull Multimap<CallableDescriptor, CallableDescriptor> invisibleOverriddenDescriptors) {
        if (!processed.add(classDescriptor)) {
            return;
        }
@@ -115,7 +107,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, fakeOverrides);
+                generateOverridesInAClass((MutableClassDescriptor) superclass, processed, ourClasses, invisibleOverriddenDescriptors);
            }
        }

@@ -128,12 +120,14 @@ public class OverrideResolver {
        Set<String> functionNames = new LinkedHashSet<String>();
        functionNames.addAll(functionsFromSupertypesByName.keySet());
        functionNames.addAll(functionsFromCurrentByName.keySet());
-        
+
+        Set<CallableMemberDescriptor> fakeOverrides = Sets.newHashSet();
        for (String functionName : functionNames) {
            generateOverridesInFunctionGroup(functionName, fakeOverrides,
                    functionsFromSupertypesByName.get(functionName),
                    functionsFromCurrentByName.get(functionName),
                    classDescriptor,
+                    invisibleOverriddenDescriptors,
                    new DescriptorSink() {
                        @Override
                        public void addToScope(@NotNull CallableMemberDescriptor fakeOverride) {
@@ -155,6 +149,16 @@ public class OverrideResolver {
                        }
                    });
        }
+        for (CallableMemberDescriptor memberDescriptor : classDescriptor.getAllCallableMembers()) {
+            JetDeclaration declaration = null;
+            if (memberDescriptor.getKind() != CallableMemberDescriptor.Kind.FAKE_OVERRIDE) {
+                PsiElement element = BindingContextUtils.descriptorToDeclaration(trace.getBindingContext(), memberDescriptor);
+                if (element instanceof JetDeclaration) {
+                    declaration = (JetDeclaration) element;
+                }
+            }
+            resolveUnknownVisibilityForMember(declaration, memberDescriptor);
+        }
    }
    
    public interface DescriptorSink {
@@ -169,17 +173,28 @@ public class OverrideResolver {
            @NotNull Collection<? extends CallableMemberDescriptor> functionsFromSupertypes,
            @NotNull Collection<? extends CallableMemberDescriptor> functionsFromCurrent,
            @NotNull ClassDescriptor current,
+            @Nullable Multimap<CallableDescriptor, CallableDescriptor> invisibleOverriddenDescriptors,
            @NotNull DescriptorSink sink) {
        
        List<CallableMemberDescriptor> fakeOverrideList = Lists.newArrayList();

        for (CallableMemberDescriptor functionFromSupertype : functionsFromSupertypes) {
            boolean overrides = false;
+
+            boolean isVisible = Visibilities.isVisible(functionFromSupertype, current);
            
            for (CallableMemberDescriptor functionFromCurrent : functionsFromCurrent) {
                OverridingUtil.OverrideCompatibilityInfo.Result result = OverridingUtil.isOverridableBy(functionFromSupertype, functionFromCurrent).getResult();
                if (result == OverridingUtil.OverrideCompatibilityInfo.Result.OVERRIDABLE) {
-                    OverridingUtil.bindOverride(functionFromCurrent, functionFromSupertype);
+
+                    if (!isVisible) {
+                        if (invisibleOverriddenDescriptors != null) {
+                            invisibleOverriddenDescriptors.put(functionFromCurrent, functionFromSupertype);
+                        }
+                    }
+                    else {
+                        OverridingUtil.bindOverride(functionFromCurrent, functionFromSupertype);
+                    }
                    overrides = true;
                }
                else if (result == OverridingUtil.OverrideCompatibilityInfo.Result.CONFLICT) {
@@ -195,7 +210,7 @@ public class OverrideResolver {
            }

            if (!overrides) {
-                CallableMemberDescriptor fakeOverride = functionFromSupertype.copy(current, false, CallableMemberDescriptor.Kind.FAKE_OVERRIDE, false);
+                CallableMemberDescriptor fakeOverride = functionFromSupertype.copy(current, false, !isVisible, CallableMemberDescriptor.Kind.FAKE_OVERRIDE, false);
                OverridingUtil.bindOverride(fakeOverride, functionFromSupertype);
                fakeOverrideList.add(fakeOverride);
                if (fakeOverrides != null) {
@@ -480,22 +495,6 @@ public class OverrideResolver {
        return false;
    }

-    private void resolveUnknownVisibilityForMembers(@NotNull Set<CallableMemberDescriptor> fakeOverrides) {
-        for (CallableMemberDescriptor override : fakeOverrides) {
-            resolveUnknownVisibilityForMember(null, override);
-        }
-        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()) {
-                if (accessor != null && accessor.getVisibility() == Visibilities.INHERITED) {
-                    accessor.setVisibility(propertyDescriptor.getVisibility());
-                }
-            }
-        }
-    }
-
    private void resolveUnknownVisibilityForMember(@Nullable JetDeclaration member, @NotNull CallableMemberDescriptor memberDescriptor) {
        resolveUnknownVisibilityForOverriddenDescriptors(memberDescriptor.getOverriddenDescriptors());
        if (memberDescriptor.getVisibility() != Visibilities.INHERITED) {
@@ -519,19 +518,6 @@ public class OverrideResolver {
        }
    }

-    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) {
@@ -39,7 +39,7 @@ public class ExpressionAsFunctionDescriptor extends FunctionDescriptorImpl {

    @NotNull
    @Override
-    public FunctionDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, Kind kind, boolean copyOverrides) {
+    public FunctionDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, boolean makeInvisible, Kind kind, boolean copyOverrides) {
        throw new IllegalStateException();
    }
 }
@@ -44,7 +44,7 @@ public class ErrorSimpleFunctionDescriptorImpl extends SimpleFunctionDescriptorI

    @NotNull
    @Override
-    public SimpleFunctionDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, Kind kind, boolean copyOverrides) {
+    public SimpleFunctionDescriptor copy(DeclarationDescriptor newOwner, boolean makeNonAbstract, boolean makeInvisible, Kind kind, boolean copyOverrides) {
        return this;
    }

@@ -0,0 +1,19 @@
+//FILE:a/C.java
+//KT-1942 Package local members from Java are visible in subclasses
+package a;
+
+public class C {
+    int myValue;
+}
+
+//FILE:d.kt
+
+package d
+
+import a.C
+
+class A : C() {
+    fun test() {
+        val <!UNUSED_VARIABLE!>v<!> = <!INVISIBLE_MEMBER!>myValue<!>
+    }
+}