KT-1018: better handling of property visibility

compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java

@@ -965,14 +965,16 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> {
                 final boolean directToField = expression.getReferencedNameElementType() == JetTokens.FIELD_IDENTIFIER && contextKind() != OwnerKind.TRAIT_IMPL ;
                 JetExpression r = getReceiverForSelector(expression);
                 final boolean isSuper = r instanceof JetSuperExpression;
-                if(propertyDescriptor.getVisibility() == Visibility.PRIVATE && context.getClassOrNamespaceDescriptor() != propertyDescriptor.getContainingDeclaration()) {
+                if(propertyDescriptor.getVisibility() == Visibility.PRIVATE && !CodegenUtil.isClassObject(propertyDescriptor.getContainingDeclaration())) {
-                    DeclarationDescriptor enclosed = propertyDescriptor.getContainingDeclaration();
+                    if(context.getClassOrNamespaceDescriptor() != propertyDescriptor.getContainingDeclaration()) {
-                    if(enclosed != null && enclosed != context.getThisDescriptor()) {
+                        DeclarationDescriptor enclosed = propertyDescriptor.getContainingDeclaration();
-                        CodegenContext c = context;
+                        if(enclosed != null && enclosed != context.getThisDescriptor()) {
-                        while(c.getContextDescriptor() != enclosed) {
+                            CodegenContext c = context;
-                            c = c.getParentContext();
+                            while(c.getContextDescriptor() != enclosed) {
+                                c = c.getParentContext();
+                            }
+                            propertyDescriptor = (PropertyDescriptor) c.getAccessor(propertyDescriptor);
                         }
-                        propertyDescriptor = (PropertyDescriptor) c.getAccessor(propertyDescriptor);
                     }
                 }
                 final StackValue.Property iValue = intermediateValueForProperty(propertyDescriptor, directToField, isSuper ? (JetSuperExpression)r : null);
@@ -1080,12 +1082,7 @@ public class ExpressionCodegen extends JetVisitor<StackValue, StackValue> {
         }
         else {
             owner = typeMapper.getOwner(functionDescriptor, OwnerKind.IMPLEMENTATION);
-            if (containingDeclaration instanceof ClassDescriptor && ((ClassDescriptor) containingDeclaration).getKind() == ClassKind.TRAIT) {
+            isInterface = CodegenUtil.isInterface(containingDeclaration);
-                isInterface = true;
-            }
-            else {
-                isInterface = false;
-            }
         }
 
         v.visitMethodInsn(isStatic ? Opcodes.INVOKESTATIC : isInterface ? Opcodes.INVOKEINTERFACE : Opcodes.INVOKEVIRTUAL, owner, functionDescriptor.getName(), typeMapper.mapSignature(functionDescriptor.getName(),functionDescriptor).getAsmMethod().getDescriptor());

compiler/backend/src/org/jetbrains/jet/codegen/JetTypeMapper.java

@@ -684,18 +684,26 @@ public class JetTypeMapper {
         return new CallableMethod(owner, method, INVOKESPECIAL);
     }
 
-    static int getAccessModifiers(JetDeclaration p, int defaultFlags) {
+    static int getAccessModifiers(DeclarationDescriptorWithVisibility p, int defaultFlags) {
-        int flags = 0;
+        DeclarationDescriptor declaration = p.getContainingDeclaration();
-        if (p.hasModifier(JetTokens.PUBLIC_KEYWORD)) {
+        if(CodegenUtil.isInterface(declaration)) {
-            flags |= ACC_PUBLIC;
+            return ACC_PUBLIC;
         }
-        else if (p.hasModifier(JetTokens.PRIVATE_KEYWORD)) {
+        if (p.getVisibility() == Visibility.PUBLIC) {
-            flags |= ACC_PRIVATE;
+            return ACC_PUBLIC;
+        }
+        else if (p.getVisibility() == Visibility.PROTECTED) {
+            return ACC_PROTECTED;
+        }
+        else if (p.getVisibility() == Visibility.PRIVATE) {
+            if(CodegenUtil.isClassObject(declaration)) {
+                return defaultFlags;
+            }
+            return ACC_PRIVATE;
         }
         else {
-            flags |= defaultFlags;
+            return defaultFlags;
         }
-        return flags;
     }
 
     String classNameForAnonymousClass(JetExpression expression) {

compiler/backend/src/org/jetbrains/jet/codegen/PropertyCodegen.java

@@ -3,10 +3,7 @@ package org.jetbrains.jet.codegen;
 import com.intellij.openapi.util.text.StringUtil;
 import com.intellij.psi.PsiElement;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor;
+import org.jetbrains.jet.lang.descriptors.*;
-import org.jetbrains.jet.lang.descriptors.PropertyDescriptor;
-import org.jetbrains.jet.lang.descriptors.PropertySetterDescriptor;
-import org.jetbrains.jet.lang.descriptors.VariableDescriptor;
 import org.jetbrains.jet.lang.psi.*;
 import org.jetbrains.jet.lang.resolve.BindingContext;
 import org.jetbrains.jet.lang.resolve.constants.CompileTimeConstant;
@@ -70,7 +67,7 @@ public class PropertyCodegen {
             }
             int modifiers;
             if (kind == OwnerKind.NAMESPACE) {
-                int access = isExternallyAccessible(p) ? Opcodes.ACC_PUBLIC : Opcodes.ACC_PRIVATE;
+                int access = JetTypeMapper.getAccessModifiers(propertyDescriptor, 0);
                 modifiers = access | Opcodes.ACC_STATIC;
             }
             else {
@@ -90,17 +87,17 @@ public class PropertyCodegen {
                 JvmPropertyAccessorSignature signature = state.getTypeMapper().mapGetterSignature(propertyDescriptor, kind);
                 functionCodegen.generateMethod(getter, signature.getJvmMethodSignature(), true, signature.getPropertyTypeKotlinSignature(), propertyDescriptor.getGetter());
             }
-            else if (!getter.hasModifier(JetTokens.PRIVATE_KEYWORD)) {
+            else if (isExternallyAccessible(propertyDescriptor)) {
-                generateDefaultGetter(p, getter);
+                generateDefaultGetter(p);
             }
         }
-        else if (isExternallyAccessible(p)) {
+        else if (isExternallyAccessible(propertyDescriptor)) {
-            generateDefaultGetter(p, p);
+            generateDefaultGetter(p);
         }
     }
 
-    private static boolean isExternallyAccessible(JetProperty p) {
+    private static boolean isExternallyAccessible(PropertyDescriptor p) {
-        return !p.hasModifier(JetTokens.PRIVATE_KEYWORD);
+        return p.getVisibility() != Visibility.PRIVATE || CodegenUtil.isClassObject(p.getContainingDeclaration());
     }
 
     private void generateSetter(JetProperty p, PropertyDescriptor propertyDescriptor) {
@@ -112,18 +109,18 @@ public class PropertyCodegen {
                 JvmPropertyAccessorSignature signature = state.getTypeMapper().mapSetterSignature(propertyDescriptor, kind);
                 functionCodegen.generateMethod(setter, signature.getJvmMethodSignature(), true, signature.getPropertyTypeKotlinSignature(), setterDescriptor);
             }
-            else if (!p.hasModifier(JetTokens.PRIVATE_KEYWORD)) {
+            else if (isExternallyAccessible(propertyDescriptor)) {
-                generateDefaultSetter(p, setter);
+                generateDefaultSetter(p);
             }
         }
-        else if (isExternallyAccessible(p) && p.isVar()) {
+        else if (isExternallyAccessible(propertyDescriptor) && propertyDescriptor.isVar()) {
-            generateDefaultSetter(p, p);
+            generateDefaultSetter(p);
         }
     }
 
-    private void generateDefaultGetter(JetProperty p, JetDeclaration declaration) {
+    private void generateDefaultGetter(JetProperty p) {
         final PropertyDescriptor propertyDescriptor = (PropertyDescriptor) state.getBindingContext().get(BindingContext.VARIABLE, p);
-        int flags = JetTypeMapper.getAccessModifiers(declaration, Opcodes.ACC_PUBLIC);
+        int flags = JetTypeMapper.getAccessModifiers(propertyDescriptor, 0);
         generateDefaultGetter(propertyDescriptor, flags, p);
     }
 
@@ -176,9 +173,13 @@ public class PropertyCodegen {
         aw.visitEnd();
     }
 
-    private void generateDefaultSetter(JetProperty p, JetDeclaration declaration) {
+    private void generateDefaultSetter(JetProperty p) {
         final PropertyDescriptor propertyDescriptor = (PropertyDescriptor) state.getBindingContext().get(BindingContext.VARIABLE, p);
-        int flags = JetTypeMapper.getAccessModifiers(declaration, Opcodes.ACC_PUBLIC);
+        assert propertyDescriptor != null;
+
+        int modifiers = JetTypeMapper.getAccessModifiers(propertyDescriptor, 0);
+        PropertySetterDescriptor setter = propertyDescriptor.getSetter();
+        int flags = setter == null ? modifiers : JetTypeMapper.getAccessModifiers(setter, modifiers);
         generateDefaultSetter(propertyDescriptor, flags, p);
     }
 

compiler/testData/codegen/regressions/kt1018.kt

@@ -0,0 +1,12 @@
+public class StockMarketTableModel() {
+
+    public fun getColumnCount() : Int {
+        return COLUMN_TITLES?.size.sure()
+    }
+
+    class object {
+        private val COLUMN_TITLES : Array<Int?> = Array<Int?>(10)
+    }
+}
+
+fun box() : String = if(StockMarketTableModel().getColumnCount()==10) "OK" else "fail"

compiler/testData/codegen/super/traitproperty.jet

@@ -8,7 +8,7 @@ trait M {
 }
 
 class N() : M {
-    override var backingB : Int = 0
+    public override var backingB : Int = 0
 
     val a : Int
         get() {

compiler/tests/org/jetbrains/jet/codegen/ClassGenTest.java

@@ -199,7 +199,7 @@ public class ClassGenTest extends CodegenTestCase {
 
     public void testKt508 () throws Exception {
         loadFile("regressions/kt508.jet");
-//        System.out.println(generateToText());
+        System.out.println(generateToText());
         blackBox();
     }
 
@@ -245,4 +245,8 @@ public class ClassGenTest extends CodegenTestCase {
     public void testKt940 () throws Exception {
         blackBoxFile("regressions/kt940.kt");
     }
+
+    public void testKt1018 () throws Exception {
+        blackBoxFile("regressions/kt1018.kt");
+    }
 }

compiler/tests/org/jetbrains/jet/codegen/PropertyGenTest.java

@@ -50,7 +50,7 @@ public class PropertyGenTest extends CodegenTestCase {
         assertNotNull(findMethodByName(aClass, "setFoo"));
     }
 
-    public void testPropertyInNamespace() throws Exception {
+    public void testPrivatePropertyInNamespace() throws Exception {
         loadText("private val x = 239");
         final Class nsClass = generateNamespaceClass();
         final Field[] fields = nsClass.getDeclaredFields();
@@ -62,6 +62,18 @@ public class PropertyGenTest extends CodegenTestCase {
         assertEquals(239, field.get(null));
     }
 
+    public void testProtectedPropertyInNamespace() throws Exception {
+        loadText("protected val x : Int = 239");
+        final Class nsClass = generateNamespaceClass();
+        final Field[] fields = nsClass.getDeclaredFields();
+        assertEquals(1, fields.length);
+        final Field field = fields[0];
+        field.setAccessible(true);
+        assertEquals("x", field.getName());
+        assertEquals(Modifier.PROTECTED | Modifier.STATIC | Modifier.FINAL, field.getModifiers());
+        assertEquals(239, field.get(null));
+    }
+
     public void testFieldPropertyAccess() throws Exception {
         loadFile("properties/fieldPropertyAccess.jet");
 //        System.out.println(generateToText());
@@ -96,12 +108,16 @@ public class PropertyGenTest extends CodegenTestCase {
     }
 
     public void testAccessorsWithoutBody() throws Exception {
-        loadText("class AccessorsWithoutBody() { public var foo: Int = 349\n  get\n  private set\n fun setter() { foo = 610; } } ");
+        loadText("class AccessorsWithoutBody() { protected var foo: Int = 349\n get\n  private set\n fun setter() { foo = 610; } } ");
+        System.out.println(generateToText());
         final Class aClass = loadImplementationClass(generateClassesInFile(), "AccessorsWithoutBody");
         final Object instance = aClass.newInstance();
         final Method getFoo = findMethodByName(aClass, "getFoo");
+        getFoo.setAccessible(true);
+        assertTrue((getFoo.getModifiers() & Modifier.PROTECTED) != 0);
         assertEquals(349, getFoo.invoke(instance));
         final Method setFoo = findMethodByName(aClass, "setFoo");
+        setFoo.setAccessible(true);
         assertTrue((setFoo.getModifiers() & Modifier.PRIVATE) != 0);
         final Method setter = findMethodByName(aClass,  "setter");
         setter.invoke(instance);
@@ -111,6 +127,7 @@ public class PropertyGenTest extends CodegenTestCase {
     public void testInitializersForNamespaceProperties() throws Exception {
         loadText("val x = System.currentTimeMillis()");
         final Method method = generateFunction("getX");
+        method.setAccessible(true);
         assertIsCurrentTime((Long) method.invoke(null));
     }
 
@@ -144,6 +161,7 @@ public class PropertyGenTest extends CodegenTestCase {
     public void testKt160() throws Exception {
         loadText("internal val s = java.lang.Double.toString(1.0)");
         final Method method = generateFunction("getS");
+        method.setAccessible(true);
         assertEquals(method.invoke(null), "1.0");
     }
 }