Reporting ACCIDENTAL_OVERRIDE on jvm signatures that have no descriptors

(marking them SYNTHETIC)
2014-09-10 13:21:24 +04:00
parent 9b7bccce22
commit b8b0316d97
19 changed files with 189 additions and 27 deletions
@@ -71,6 +71,7 @@ import static org.jetbrains.jet.lang.resolve.DescriptorUtils.isTrait;
 import static org.jetbrains.jet.lang.resolve.java.AsmTypeConstants.OBJECT_TYPE;
 import static org.jetbrains.jet.lang.resolve.java.JvmAnnotationNames.OLD_JET_VALUE_PARAMETER_ANNOTATION;
 import static org.jetbrains.jet.lang.resolve.java.diagnostics.DiagnosticsPackage.OtherOrigin;
+import static org.jetbrains.jet.lang.resolve.java.diagnostics.DiagnosticsPackage.Synthetic;
 import static org.jetbrains.org.objectweb.asm.Opcodes.*;

 public class FunctionCodegen extends ParentCodegenAware {
@@ -577,7 +578,7 @@ public class FunctionCodegen extends ParentCodegenAware {

        Method defaultMethod = typeMapper.mapDefaultMethod(functionDescriptor, kind, owner);

-        MethodVisitor mv = v.newMethod(OtherOrigin(functionDescriptor), flags | (isConstructor ? 0 : ACC_STATIC),
+        MethodVisitor mv = v.newMethod(Synthetic(function, functionDescriptor), flags | (isConstructor ? 0 : ACC_STATIC),
                                       defaultMethod.getName(),
                                       defaultMethod.getDescriptor(), null,
                                       getThrownExceptions(functionDescriptor, typeMapper));
@@ -79,6 +79,7 @@ import static org.jetbrains.jet.lang.resolve.java.AsmTypeConstants.*;
 import static org.jetbrains.jet.lang.resolve.java.JvmAnnotationNames.KotlinSyntheticClass;
 import static org.jetbrains.jet.lang.resolve.java.diagnostics.DiagnosticsPackage.DelegationToTraitImpl;
 import static org.jetbrains.jet.lang.resolve.java.diagnostics.DiagnosticsPackage.OtherOrigin;
+import static org.jetbrains.jet.lang.resolve.java.diagnostics.DiagnosticsPackage.Synthetic;
 import static org.jetbrains.jet.lang.resolve.java.diagnostics.JvmDeclarationOrigin.NO_ORIGIN;
 import static org.jetbrains.org.objectweb.asm.Opcodes.*;

@@ -977,7 +978,7 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen {
            FunctionDescriptor bridge = (FunctionDescriptor) entry.getValue();
            final FunctionDescriptor original = (FunctionDescriptor) entry.getKey();
            functionCodegen.generateMethod(
-                    OtherOrigin(original), typeMapper.mapSignature(bridge), bridge,
+                    Synthetic(null, original), typeMapper.mapSignature(bridge), bridge,
                    new FunctionGenerationStrategy.CodegenBased<FunctionDescriptor>(state, bridge) {
                        @Override
                        public void doGenerateBody(@NotNull ExpressionCodegen codegen, @NotNull JvmMethodSignature signature) {
@@ -1024,7 +1025,9 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen {

            PropertyGetterDescriptor getter = bridge.getGetter();
            assert getter != null;
-            functionCodegen.generateMethod(OtherOrigin(original.getGetter()), typeMapper.mapSignature(getter), getter,
+            functionCodegen.generateMethod(Synthetic(null, original.getGetter() != null ? original.getGetter() : original),
+                                           typeMapper.mapSignature(getter),
+                                           getter,
                                           new PropertyAccessorStrategy(getter));


@@ -1032,7 +1035,9 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen {
                PropertySetterDescriptor setter = bridge.getSetter();
                assert setter != null;

-                functionCodegen.generateMethod(OtherOrigin(original.getSetter()), typeMapper.mapSignature(setter), setter,
+                functionCodegen.generateMethod(Synthetic(null, original.getSetter() != null ? original.getSetter() : original),
+                                               typeMapper.mapSignature(setter),
+                                               setter,
                                               new PropertyAccessorStrategy(setter));
            }
        }
@@ -17,18 +17,12 @@
 package org.jetbrains.jet.codegen

 import org.jetbrains.jet.lang.descriptors.FunctionDescriptor
-import org.jetbrains.jet.lang.psi.JetNamedFunction
-import org.jetbrains.jet.codegen.state.JetTypeMapper
-import org.jetbrains.jet.codegen.context.CodegenContext
 import org.jetbrains.org.objectweb.asm.Opcodes
-import org.jetbrains.jet.lang.resolve.java.diagnostics.OtherOrigin
 import org.jetbrains.jet.codegen.state.GenerationState
-import org.jetbrains.jet.lang.resolve.java.jvmSignature.JvmMethodParameterKind
-import org.jetbrains.org.objectweb.asm.Type
 import org.jetbrains.org.objectweb.asm.commons.InstructionAdapter
-import org.jetbrains.jet.lang.psi.JetClassOrObject
 import org.jetbrains.jet.lang.descriptors.ClassDescriptor
 import org.jetbrains.jet.lang.resolve.java.diagnostics.JvmDeclarationOrigin
+import org.jetbrains.jet.lang.resolve.java.diagnostics.Synthetic

 class PlatformStaticGenerator(
        val descriptor: FunctionDescriptor,
@@ -40,7 +34,12 @@ class PlatformStaticGenerator(
        val typeMapper = state.getTypeMapper()
        val callable = typeMapper.mapToCallableMethod(descriptor, false, p1.getContext())
        val asmMethod = callable.getAsmMethod()
-        val methodVisitor = p2.newMethod(OtherOrigin(declarationOrigin.element, declarationOrigin.descriptor), Opcodes.ACC_STATIC or AsmUtil.getMethodAsmFlags(descriptor, OwnerKind.IMPLEMENTATION), asmMethod.getName()!!, asmMethod.getDescriptor()!!, null, null)
+        val methodVisitor = p2.newMethod(
+                Synthetic(declarationOrigin.element, descriptor),
+                Opcodes.ACC_STATIC or AsmUtil.getMethodAsmFlags(descriptor, OwnerKind.IMPLEMENTATION),
+                asmMethod.getName()!!,
+                asmMethod.getDescriptor()!!,
+                null, null)

        AnnotationCodegen.forMethod(methodVisitor, typeMapper)!!.genAnnotations(descriptor, asmMethod.getReturnType())

@@ -31,7 +31,9 @@ public abstract class SignatureCollectingClassBuilderFactory(
 ) : ClassBuilderFactory by delegate {

    protected abstract fun handleClashingSignatures(data: ConflictingJvmDeclarationsData)
-    protected abstract fun onClassDone(classOrigin: JvmDeclarationOrigin, classInternalName: String?, hasDuplicateSignatures: Boolean)
+    protected abstract fun onClassDone(classOrigin: JvmDeclarationOrigin,
+                                       classInternalName: String?,
+                                       signatures: MultiMap<RawSignature, JvmDeclarationOrigin>)

    override fun newClassBuilder(origin: JvmDeclarationOrigin): SignatureCollectingClassBuilder {
        return SignatureCollectingClassBuilder(origin, delegate.newClassBuilder(origin))
@@ -83,7 +85,7 @@ public abstract class SignatureCollectingClassBuilderFactory(
                ))
                hasDuplicateSignatures = true
            }
-            onClassDone(classCreatedFor, classInternalName, hasDuplicateSignatures)
+            onClassDone(classCreatedFor, classInternalName, signatures)
            super.done()
        }

@@ -23,7 +23,6 @@ import org.jetbrains.jet.codegen.SignatureCollectingClassBuilderFactory
 import org.jetbrains.jet.lang.descriptors.*
 import org.jetbrains.jet.lang.diagnostics.DiagnosticHolder
 import org.jetbrains.jet.lang.resolve.BindingContext
-import org.jetbrains.jet.lang.resolve.BindingContextUtils
 import org.jetbrains.jet.lang.resolve.java.diagnostics.*
 import java.util.*
 import org.jetbrains.jet.lang.descriptors.CallableMemberDescriptor.Kind.DELEGATION
@@ -59,20 +58,31 @@ class BuilderFactoryForDuplicateSignatureDiagnostics(
        }
    }

-    override fun onClassDone(classOrigin: JvmDeclarationOrigin, classInternalName: String?, hasDuplicateSignatures: Boolean) {
+    override fun onClassDone(
+            classOrigin: JvmDeclarationOrigin,
+            classInternalName: String?,
+            signatures: MultiMap<RawSignature, JvmDeclarationOrigin>
+    ) {
        val descriptor = classOrigin.descriptor
        if (descriptor !is ClassDescriptor) return

-        val groupedBySignature = groupDescriptorsBySignature(descriptor)
+        val groupedBySignature = groupMembersDescriptorsBySignature(descriptor)
+        for ((rawSignature, origins) in signatures.entrySet()) {
+            for (origin in origins) {
+                if (origin.originKind == JvmDeclarationOriginKind.SYNTHETIC) {
+                    groupedBySignature.putValue(rawSignature, origin)
+                }
+            }
+        }

        @signatures
-        for ((rawSignature, members) in groupedBySignature.entrySet()!!) {
-            if (members.size() <= 1) continue
+        for ((rawSignature, origins) in groupedBySignature.entrySet()) {
+            if (origins.size() <= 1) continue

            var memberElement: PsiElement? = null
            var nonFakeCount = 0
-            for (member in members) {
-                if (member.getKind() != FAKE_OVERRIDE) {
+            for (member in origins.stream().map { it.descriptor as? CallableMemberDescriptor? }) {
+                if (member != null && member.getKind() != FAKE_OVERRIDE) {
                    nonFakeCount++
                    // If there's more than one real element, the clashing signature is already reported.
                    // Only clashes between fake overrides are interesting here
@@ -91,13 +101,13 @@ class BuilderFactoryForDuplicateSignatureDiagnostics(
            val elementToReportOn = memberElement ?: classOrigin.element
            if (elementToReportOn == null) return // TODO: it'd be better to report this error without any element at all

-            val data = ConflictingJvmDeclarationsData(classInternalName, classOrigin, rawSignature, members.map { OtherOrigin(it) })
+            val data = ConflictingJvmDeclarationsData(classInternalName, classOrigin, rawSignature, origins)
            diagnostics.report(ErrorsJvm.ACCIDENTAL_OVERRIDE.on(elementToReportOn, data))
        }
    }

-    private fun groupDescriptorsBySignature(descriptor: ClassDescriptor): MultiMap<RawSignature, CallableMemberDescriptor> {
-        val groupedBySignature = MultiMap.create<RawSignature, CallableMemberDescriptor>()
+    private fun groupMembersDescriptorsBySignature(descriptor: ClassDescriptor): MultiMap<RawSignature, JvmDeclarationOrigin> {
+        val groupedBySignature = MultiMap.create<RawSignature, JvmDeclarationOrigin>()

        fun processMember(member: DeclarationDescriptor?) {
            // a member of super is not visible: no override
@@ -113,7 +123,7 @@ class BuilderFactoryForDuplicateSignatureDiagnostics(
                val methodSignature = typeMapper.mapSignature(member)
                val rawSignature = RawSignature(
                        methodSignature.getAsmMethod().getName()!!, methodSignature.getAsmMethod().getDescriptor()!!, MemberKind.METHOD)
-                groupedBySignature.putValue(rawSignature, member)
+                groupedBySignature.putValue(rawSignature, OtherOrigin(member))
            }
        }

@@ -25,6 +25,7 @@ import org.jetbrains.jet.lang.psi.JetClassOrObject
 import org.jetbrains.jet.lang.psi.JetDeclaration
 import org.jetbrains.jet.lang.descriptors.FunctionDescriptor
 import org.jetbrains.jet.lang.resolve.java.diagnostics.JvmDeclarationOriginKind.*
+import org.jetbrains.jet.lang.descriptors.CallableMemberDescriptor

 public enum class MemberKind { FIELD; METHOD }

@@ -36,6 +37,7 @@ public enum class JvmDeclarationOriginKind {
    PACKAGE_PART
    TRAIT_IMPL
    DELEGATION_TO_TRAIT_IMPL
+    SYNTHETIC // this means that there's no proper descriptor for this jvm declaration
 }

 public class JvmDeclarationOrigin(
@@ -63,3 +65,5 @@ public fun PackagePart(file: JetFile, descriptor: PackageFragmentDescriptor): Jv
 public fun TraitImpl(element: JetClassOrObject, descriptor: ClassDescriptor): JvmDeclarationOrigin = JvmDeclarationOrigin(TRAIT_IMPL, element, descriptor)
 public fun DelegationToTraitImpl(element: PsiElement?, descriptor: FunctionDescriptor): JvmDeclarationOrigin = JvmDeclarationOrigin(DELEGATION_TO_TRAIT_IMPL, element, descriptor)

+public fun Synthetic(element: PsiElement?, descriptor: CallableMemberDescriptor): JvmDeclarationOrigin = JvmDeclarationOrigin(SYNTHETIC, element, descriptor)
+
@@ -0,0 +1,3 @@
+$TESTDATA_DIR$/syntheticAccessorSignatureClash.kt
+-d
+$TEMP_DIR$
@@ -0,0 +1,38 @@
+open class Base {
+    open fun `foo$b$0`(d: Derived) {}
+
+    open fun `getBar$b$1`(d: Derived): Int = 1
+    open fun `setBar$b$1`(d: Derived, i: Int) {}
+
+    open fun `getBaz$b$2`(d: Derived): Int = 1
+
+    open fun `getBoo$b$3`(d: Derived): Int = 1
+
+    open fun `setBar1$b$4`(d: Derived, i: Int) {}
+}
+
+class Derived : Base() {
+    private fun foo() {}
+
+    private var bar = 1
+        get
+        set
+
+    private var baz = 1
+
+    private val boo = 1
+
+    private var bar1 = 1
+        get
+        set
+
+    inner class Nested {
+        fun test() {
+            foo()
+            bar += 1
+            baz += 1
+            val s = boo
+            bar1 += 1
+        }
+    }
+}
@@ -0,0 +1,20 @@
+WARNING: compiler/testData/cli/jvm/syntheticAccessorSignatureClash.kt: (34, 17) Variable 's' is never used
+ERROR: compiler/testData/cli/jvm/syntheticAccessorSignatureClash.kt: (15, 5) Accidental override: The following declarations have the same JVM signature (foo$b$0(LDerived;)V):
+    fun foo$b$0(d: Derived): kotlin.Unit
+    fun foo(): kotlin.Unit
+ERROR: compiler/testData/cli/jvm/syntheticAccessorSignatureClash.kt: (18, 9) Accidental override: The following declarations have the same JVM signature (getBar$b$1(LDerived;)I):
+    fun getBar$b$1(d: Derived): kotlin.Int
+    fun <get-bar>(): kotlin.Int
+ERROR: compiler/testData/cli/jvm/syntheticAccessorSignatureClash.kt: (19, 9) Accidental override: The following declarations have the same JVM signature (setBar$b$1(LDerived;I)V):
+    fun setBar$b$1(d: Derived, i: kotlin.Int): kotlin.Unit
+    fun <set-bar>(<set-?>: kotlin.Int): kotlin.Unit
+ERROR: compiler/testData/cli/jvm/syntheticAccessorSignatureClash.kt: (21, 5) Accidental override: The following declarations have the same JVM signature (getBaz$b$2(LDerived;)I):
+    fun getBaz$b$2(d: Derived): kotlin.Int
+    fun <get-baz>(): kotlin.Int
+ERROR: compiler/testData/cli/jvm/syntheticAccessorSignatureClash.kt: (23, 5) Accidental override: The following declarations have the same JVM signature (getBoo$b$3(LDerived;)I):
+    fun getBoo$b$3(d: Derived): kotlin.Int
+    fun <get-boo>(): kotlin.Int
+ERROR: compiler/testData/cli/jvm/syntheticAccessorSignatureClash.kt: (27, 9) Accidental override: The following declarations have the same JVM signature (setBar1$b$4(LDerived;I)V):
+    fun setBar1$b$4(d: Derived, i: kotlin.Int): kotlin.Unit
+    fun <set-bar1>(<set-?>: kotlin.Int): kotlin.Unit
+COMPILATION_ERROR
@@ -0,0 +1,7 @@
+package test;
+
+public class referenceToJavaFieldViaBridge {
+
+    protected String field = "OK";
+
+}
@@ -0,0 +1,13 @@
+
+import test.referenceToJavaFieldViaBridge
+
+class A : referenceToJavaFieldViaBridge() {
+
+    fun a(): String {
+        return {field!!}()
+    }
+}
+
+fun box(): String {
+    return A().a()
+}
@@ -0,0 +1,9 @@
+// !DIAGNOSTICS: -UNUSED_PARAMETER
+
+open class Base {
+    open fun `foo$default`(d: Derived, i: Int, mask: Int) {}
+}
+
+class Derived : Base() {
+    <!ACCIDENTAL_OVERRIDE!>fun foo(i: Int = 0)<!> {}
+}
@@ -1,6 +1,6 @@
 // !DIAGNOSTICS: -UNUSED_PARAMETER

-class <!CONFLICTING_JVM_DECLARATIONS!>C<!> {
+class C {
    <!CONFLICTING_JVM_DECLARATIONS!>fun `a$default`(c: C, x: Int, m: Int)<!> {}
-    fun a(x: Int = 1) {}
+    <!CONFLICTING_JVM_DECLARATIONS!>fun a(x: Int = 1)<!> {}
 }
@@ -0,0 +1,11 @@
+import kotlin.platform.platformStatic
+
+open class Base {
+    fun foo() {}
+}
+
+class Derived : Base() {
+    class object {
+        <!ACCIDENTAL_OVERRIDE!>platformStatic fun foo()<!> {}
+    }
+}
@@ -0,0 +1,10 @@
+// !DIAGNOSTICS: -UNUSED_PARAMETER
+import kotlin.platform.platformStatic
+
+open class Base {
+    fun `foo$default`(i: Int, mask: Int) {}
+}
+
+object Derived : Base() {
+    <!ACCIDENTAL_OVERRIDE!>platformStatic fun foo(i: Int = 0)<!> {}
+}
@@ -3235,6 +3235,12 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest {
                    doTest(fileName);
                }
                
+                @TestMetadata("defaultFunction.kt")
+                public void testDefaultFunction() throws Exception {
+                    String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/duplicateJvmSignature/accidentalOverrides/defaultFunction.kt");
+                    doTest(fileName);
+                }
+                
                @TestMetadata("delegatedFunctionOverriddenByProperty.kt")
                public void testDelegatedFunctionOverriddenByProperty() throws Exception {
                    String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/duplicateJvmSignature/accidentalOverrides/delegatedFunctionOverriddenByProperty.kt");
@@ -547,6 +547,18 @@ public class JetDiagnosticsTestWithStdLibGenerated extends AbstractJetDiagnostic
            doTest(fileName);
        }
        
+        @TestMetadata("platformStaticInClassObject.kt")
+        public void testPlatformStaticInClassObject() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/duplicateJvmSignature/platformStaticInClassObject.kt");
+            doTest(fileName);
+        }
+        
+        @TestMetadata("platformStaticInObject.kt")
+        public void testPlatformStaticInObject() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/duplicateJvmSignature/platformStaticInObject.kt");
+            doTest(fileName);
+        }
+        
    }
    
    @TestMetadata("compiler/testData/diagnostics/testsWithStdLib/functionLiterals")
@@ -120,6 +120,12 @@ public class KotlincExecutableTestGenerated extends AbstractKotlincExecutableTes
            doJvmTest(fileName);
        }
        
+        @TestMetadata("syntheticAccessorSignatureClash.args")
+        public void testSyntheticAccessorSignatureClash() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("compiler/testData/cli/jvm/syntheticAccessorSignatureClash.args");
+            doJvmTest(fileName);
+        }
+        
        @TestMetadata("wrongAbiVersion.args")
        public void testWrongAbiVersion() throws Exception {
            String fileName = JetTestUtils.navigationMetadata("compiler/testData/cli/jvm/wrongAbiVersion.args");
@@ -279,6 +279,12 @@ public class BlackBoxAgainstJavaCodegenTestGenerated extends AbstractBlackBoxCod
            doTestAgainstJava(fileName);
        }
        
+        @TestMetadata("referenceToJavaFieldViaBridge.kt")
+        public void testReferenceToJavaFieldViaBridge() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxAgainstJava/property/referenceToJavaFieldViaBridge.kt");
+            doTestAgainstJava(fileName);
+        }
+        
    }
    
    @TestMetadata("compiler/testData/codegen/boxAgainstJava/reflection")