ACCIDENTAL_OVERRIDE diagnostics supported

#KT-1 In Progress
2014-06-05 20:53:31 +04:00
parent d2e464a02b
commit 27f76630e6
22 changed files with 251 additions and 25 deletions
@@ -31,6 +31,7 @@ public abstract class SignatureCollectingClassBuilderFactory(
 ) : ClassBuilderFactory by delegate {

    protected abstract fun handleClashingSignatures(data: ConflictingJvmDeclarationsData)
+    protected abstract fun onClassDone(classOrigin: JvmDeclarationOrigin, classInternalName: String?, hasDuplicateSignatures: Boolean)

    override fun newClassBuilder(origin: JvmDeclarationOrigin): SignatureCollectingClassBuilder {
        return SignatureCollectingClassBuilder(origin, delegate.newClassBuilder(origin))
@@ -67,6 +68,7 @@ public abstract class SignatureCollectingClassBuilderFactory(
        }

        override fun done() {
+            var hasDuplicateSignatures = false
            for ((signature, elementsAndDescriptors) in signatures.entrySet()!!) {
                if (elementsAndDescriptors.size == 1) continue // no clash
                handleClashingSignatures(ConflictingJvmDeclarationsData(
@@ -75,7 +77,9 @@ public abstract class SignatureCollectingClassBuilderFactory(
                        signature,
                        elementsAndDescriptors
                ))
+                hasDuplicateSignatures = true
            }
+            onClassDone(classCreatedFor, classInternalName, hasDuplicateSignatures)
            super.done()
        }

@@ -18,6 +18,7 @@ package org.jetbrains.jet.codegen.state;

 import com.intellij.openapi.project.Project;
 import com.intellij.psi.PsiElement;
+import com.intellij.util.containers.MultiMap;
 import kotlin.Function1;
 import kotlin.KotlinPackage;
 import org.jetbrains.annotations.NotNull;
@@ -26,25 +27,23 @@ import org.jetbrains.jet.codegen.*;
 import org.jetbrains.jet.codegen.binding.CodegenBinding;
 import org.jetbrains.jet.codegen.inline.InlineCodegenUtil;
 import org.jetbrains.jet.codegen.intrinsics.IntrinsicMethods;
-import org.jetbrains.jet.lang.descriptors.ModuleDescriptor;
-import org.jetbrains.jet.lang.descriptors.ScriptDescriptor;
+import org.jetbrains.jet.lang.descriptors.*;
 import org.jetbrains.jet.lang.diagnostics.DiagnosticHolder;
 import org.jetbrains.jet.lang.psi.JetClassOrObject;
 import org.jetbrains.jet.lang.psi.JetFile;
 import org.jetbrains.jet.lang.reflect.ReflectionTypes;
 import org.jetbrains.jet.lang.resolve.BindingContext;
+import org.jetbrains.jet.lang.resolve.BindingContextUtils;
 import org.jetbrains.jet.lang.resolve.BindingTrace;
 import org.jetbrains.jet.lang.resolve.DelegatingBindingTrace;
-import org.jetbrains.jet.lang.resolve.java.diagnostics.ConflictingJvmDeclarationsData;
-import org.jetbrains.jet.lang.resolve.java.diagnostics.ErrorsJvm;
-import org.jetbrains.jet.lang.resolve.java.diagnostics.JvmDeclarationOrigin;
+import org.jetbrains.jet.lang.resolve.java.diagnostics.*;
+import org.jetbrains.jet.lang.resolve.java.jvmSignature.JvmMethodSignature;
 import org.jetbrains.jet.lang.resolve.name.FqName;
-import org.jetbrains.jet.lang.resolve.java.diagnostics.JvmDeclarationOriginKind;

-import java.util.Collection;
-import java.util.Collections;
-import java.util.LinkedHashSet;
-import java.util.List;
+import java.util.*;
+
+import static org.jetbrains.jet.lang.descriptors.CallableMemberDescriptor.Kind.DELEGATION;
+import static org.jetbrains.jet.lang.descriptors.CallableMemberDescriptor.Kind.FAKE_OVERRIDE;

 public class GenerationState {
    public interface GenerateClassFilter {
@@ -275,7 +274,7 @@ public class GenerationState {
        return moduleId;
    }

-    private static class BuilderfactoryForDuplicateSignatureDiagnostics extends SignatureCollectingClassBuilderFactory {
+    private class BuilderfactoryForDuplicateSignatureDiagnostics extends SignatureCollectingClassBuilderFactory {

        private final DiagnosticHolder diagnostics;

@@ -313,5 +312,98 @@ public class GenerationState {
                diagnostics.report(ErrorsJvm.CONFLICTING_JVM_DECLARATIONS.on(element, data));
            }
        }
+
+        @Override
+        protected void onClassDone(
+                @NotNull JvmDeclarationOrigin classOrigin,
+                @Nullable String classInternalName,
+                boolean hasDuplicateSignatures
+        ) {
+            DeclarationDescriptor descriptor = classOrigin.getDescriptor();
+            if (!(descriptor instanceof ClassDescriptor)) return;
+
+            ClassDescriptor classDescriptor = (ClassDescriptor) descriptor;
+
+            MultiMap<RawSignature, CallableMemberDescriptor> groupedBySignature = MultiMap.create();
+            Queue<DeclarationDescriptor> queue =
+                    new LinkedList<DeclarationDescriptor>(classDescriptor.getDefaultType().getMemberScope().getAllDescriptors());
+            while (!queue.isEmpty()) {
+                DeclarationDescriptor member = queue.poll();
+                if (member instanceof DeclarationDescriptorWithVisibility &&
+                    ((DeclarationDescriptorWithVisibility) member).getVisibility() == Visibilities.INVISIBLE_FAKE) {
+                    // a member of super is not visible: no override
+                    continue;
+                }
+                if (member instanceof PropertyDescriptor) {
+                    PropertyDescriptor propertyDescriptor = (PropertyDescriptor) member;
+
+                    PropertyGetterDescriptor getter = propertyDescriptor.getGetter();
+                    if (getter != null) {
+                        queue.add(getter);
+                    }
+                    PropertySetterDescriptor setter = propertyDescriptor.getSetter();
+                    if (setter != null) {
+                        queue.add(setter);
+                    }
+                }
+                else if (member instanceof FunctionDescriptor) {
+                    FunctionDescriptor functionDescriptor = (FunctionDescriptor) member;
+
+                    JvmMethodSignature methodSignature = typeMapper.mapSignature(functionDescriptor);
+                    RawSignature rawSignature = new RawSignature(
+                            methodSignature.getAsmMethod().getName(),
+                            methodSignature.getAsmMethod().getDescriptor(),
+                            MemberKind.METHOD
+                    );
+                    groupedBySignature.putValue(rawSignature, functionDescriptor);
+                }
+            }
+
+            signatures:
+            for (Map.Entry<RawSignature, Collection<CallableMemberDescriptor>> entry : groupedBySignature.entrySet()) {
+                RawSignature rawSignature = entry.getKey();
+                Collection<CallableMemberDescriptor> members = entry.getValue();
+
+                if (members.size() <= 1) continue;
+
+                PsiElement memberElement = null;
+                int nonFakeCount = 0;
+                for (CallableMemberDescriptor member : members) {
+                    //
+                    if (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
+                        if (nonFakeCount > 1) continue signatures;
+
+                        if (member.getKind() != DELEGATION) {
+                            // Delegates don't have declarations in the code
+                            memberElement = BindingContextUtils.callableDescriptorToDeclaration(getBindingContext(), member);
+                            if (memberElement == null && member instanceof PropertyAccessorDescriptor) {
+                                memberElement = BindingContextUtils.callableDescriptorToDeclaration(
+                                        getBindingContext(),
+                                        ((PropertyAccessorDescriptor) member).getCorrespondingProperty()
+                                );
+                            }
+                        }
+                    }
+                }
+
+                PsiElement elementToReportOn = memberElement == null ? classOrigin.getElement() : memberElement;
+                if (elementToReportOn == null) return;
+
+                List<JvmDeclarationOrigin> origins = KotlinPackage.map(
+                        members,
+                        new Function1<CallableMemberDescriptor, JvmDeclarationOrigin>() {
+                            @Override
+                            public JvmDeclarationOrigin invoke(CallableMemberDescriptor descriptor) {
+                                return DiagnosticsPackage.OtherOrigin(descriptor);
+                            }
+                        });
+                ConflictingJvmDeclarationsData data =
+                        new ConflictingJvmDeclarationsData(classInternalName, classOrigin, rawSignature, origins);
+                diagnostics.report(ErrorsJvm.ACCIDENTAL_OVERRIDE.on(elementToReportOn, data));
+            }
+        }
    }
 }
@@ -34,6 +34,7 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension {
    public static final DiagnosticFactoryToRendererMap MAP = new DiagnosticFactoryToRendererMap();
    static {
        MAP.put(ErrorsJvm.CONFLICTING_JVM_DECLARATIONS, "Platform declaration clash: ''{0}''", CONFLICTING_JVM_DECLARATIONS_DATA);
+        MAP.put(ErrorsJvm.ACCIDENTAL_OVERRIDE, "Accidental override: ''{0}''", CONFLICTING_JVM_DECLARATIONS_DATA);
    }


@@ -25,6 +25,7 @@ import static org.jetbrains.jet.lang.diagnostics.Severity.ERROR;

 public interface ErrorsJvm {
    DiagnosticFactory1<PsiElement, ConflictingJvmDeclarationsData> CONFLICTING_JVM_DECLARATIONS = DiagnosticFactory1.create(ERROR, DECLARATION_OR_DEFAULT);
+    DiagnosticFactory1<PsiElement, ConflictingJvmDeclarationsData> ACCIDENTAL_OVERRIDE = DiagnosticFactory1.create(ERROR, DECLARATION_OR_DEFAULT);

    @SuppressWarnings("UnusedDeclaration")
    Object _initializer = new Object() {
@@ -28,6 +28,7 @@ import org.jetbrains.jet.lang.resolve.java.diagnostics.ErrorsJvm.*
 import org.jetbrains.jet.lang.resolve.java.diagnostics.ConflictingJvmDeclarationsData
 import org.jetbrains.jet.lang.resolve.java.diagnostics.JvmDeclarationOriginKind.*
 import org.jetbrains.jet.lang.diagnostics.Errors.*
+import org.jetbrains.jet.lang.diagnostics.DiagnosticFactory.*

 public fun getJvmSignatureDiagnostics(element: PsiElement, otherDiagnostics: Diagnostics): Diagnostics? {
    fun doGetDiagnostics(): Diagnostics? {
@@ -64,7 +65,8 @@ public fun getJvmSignatureDiagnostics(element: PsiElement, otherDiagnostics: Dia
        }

        override fun forElement(psiElement: PsiElement): Collection<Diagnostic> {
-            val (conflicting, other) = result.forElement(element).partition { it.getFactory() == CONFLICTING_JVM_DECLARATIONS }
+            val jvmDiagnostics = setOf(CONFLICTING_JVM_DECLARATIONS, ACCIDENTAL_OVERRIDE)
+            val (conflicting, other) = result.forElement(element).partition { it.getFactory() in jvmDiagnostics }
            if (alreadyReported(psiElement)) {
                // CONFLICTING_OVERLOADS already reported, no need to duplicate it
                return other
@@ -72,7 +74,7 @@ public fun getJvmSignatureDiagnostics(element: PsiElement, otherDiagnostics: Dia

            val filtered = arrayListOf<Diagnostic>()
            conflicting.groupBy {
-                CONFLICTING_JVM_DECLARATIONS.cast(it).getA().signature.name
+                cast(it, jvmDiagnostics).getA().signature.name
            }.forEach {
                val diagnostics = it.getValue()
                if (diagnostics.size <= 1) {
@@ -84,8 +86,12 @@ public fun getJvmSignatureDiagnostics(element: PsiElement, otherDiagnostics: Dia
                                me ->
                                diagnostics.none {
                                    other ->
-                                    me != other &&
-                                            CONFLICTING_JVM_DECLARATIONS.cast(other).getA() higherThan CONFLICTING_JVM_DECLARATIONS.cast(me).getA()
+                                    me != other && (
+                                            // in case of implementation copied from a super trait there will be both diagnostics on the same signature
+                                            me.getFactory() == ACCIDENTAL_OVERRIDE && other.getFactory() == CONFLICTING_JVM_DECLARATIONS
+                                               // there are paris of corresponding signatures that frequently clash simultaneously: package facade & part, trait and trait-impl
+                                               || cast(other, jvmDiagnostics).getA() higherThan cast(me, jvmDiagnostics).getA()
+                                            )
                                }
                            }
                    )
@@ -1,4 +1,10 @@
-class A {
+open class B {
+    val x = 1
+}
+
+class A : B() {
+    fun getX() = 1
+
    fun getA(): Int = 1
    val a: Int = 1
 }
@@ -1,5 +1,6 @@
-ERROR: $TESTDATA_DIR$/signatureClash.kt: (2, 5) Platform declaration clash: 'getA()I'
-ERROR: $TESTDATA_DIR$/signatureClash.kt: (3, 5) Platform declaration clash: 'getA()I'
-ERROR: $TESTDATA_DIR$/signatureClash.kt: (6, 1) Platform declaration clash: 'getB()I'
-ERROR: $TESTDATA_DIR$/signatureClash.kt: (7, 1) Platform declaration clash: 'getB()I'
+ERROR: $TESTDATA_DIR$/signatureClash.kt: (6, 5) Accidental override: 'getX()I'
+ERROR: $TESTDATA_DIR$/signatureClash.kt: (8, 5) Platform declaration clash: 'getA()I'
+ERROR: $TESTDATA_DIR$/signatureClash.kt: (9, 5) Platform declaration clash: 'getA()I'
+ERROR: $TESTDATA_DIR$/signatureClash.kt: (12, 1) Platform declaration clash: 'getB()I'
+ERROR: $TESTDATA_DIR$/signatureClash.kt: (13, 1) Platform declaration clash: 'getB()I'
 COMPILATION_ERROR
@@ -4,5 +4,5 @@ open class B {

 class C : B() {
    val x: Int
-        <!CONFLICTING_JVM_DECLARATIONS!>get()<!> = 1
+        <!ACCIDENTAL_OVERRIDE!>get()<!> = 1
 }
@@ -0,0 +1,5 @@
+open class B {
+    fun getX() = 1
+}
+
+class C(<!ACCIDENTAL_OVERRIDE!>val x: Int<!>) : B()
@@ -0,0 +1,7 @@
+open class B {
+    fun getX() = 1
+}
+
+class C : B() {
+    <!ACCIDENTAL_OVERRIDE!>val x: Int<!> = 1
+}
@@ -4,5 +4,5 @@ open class B {
 }

 class C : B() {
-    <!CONFLICTING_JVM_DECLARATIONS!>fun getX()<!> = 1
+    <!ACCIDENTAL_OVERRIDE!>fun getX()<!> = 1
 }
@@ -0,0 +1,10 @@
+trait B {
+    fun getX() = 1
+}
+
+trait D {
+    val x: Int
+}
+
+class <!ACCIDENTAL_OVERRIDE!>C(d: D)<!> : D by d, B {
+}
@@ -0,0 +1,9 @@
+// !DIAGNOSTICS: -UNUSED_PARAMETER
+
+open class B {
+    fun foo(l: List<String>) {}
+}
+
+class C : B() {
+    <!ACCIDENTAL_OVERRIDE!>fun foo(l: List<Int>)<!> {}
+}
@@ -0,0 +1,8 @@
+open class B {
+    private fun getX() = 1
+}
+
+class C : B() {
+    val x: Int
+        get() = 1
+}
@@ -4,5 +4,5 @@ trait T {

 class C : T {
    val x: Int
-        <!CONFLICTING_JVM_DECLARATIONS!>get() = 1<!>
+        <!CONFLICTING_JVM_DECLARATIONS!>get()<!> = 1
 }
@@ -0,0 +1,8 @@
+trait T {
+    fun getX() = 1
+}
+
+trait C : T {
+    val x: Int
+        <!ACCIDENTAL_OVERRIDE!>get()<!> = 1
+}
@@ -0,0 +1,8 @@
+trait T {
+    fun getX(): Int
+}
+
+abstract class C : T {
+    val x: Int
+        <!ACCIDENTAL_OVERRIDE!>get()<!> = 1
+}
@@ -0,0 +1,8 @@
+trait T {
+    val x: Int
+        get() = 1
+}
+
+trait C : T {
+    <!ACCIDENTAL_OVERRIDE!>fun getX()<!> = 1
+}
@@ -0,0 +1,7 @@
+trait T {
+    val x: Int
+}
+
+abstract class C : T {
+    <!ACCIDENTAL_OVERRIDE!>fun getX()<!> = 1
+}
@@ -42,7 +42,7 @@ abstract class MyAbstractClass1 : MyTrait<Int>, MyAbstractClass<String>() {

 class <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>MyIllegalGenericClass1<!><T> : MyTrait<T>, MyAbstractClass<T>() {}
 class <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>MyIllegalGenericClass2<!><T, R>(r : R) : MyTrait<T>, MyAbstractClass<R>() {
-    <!NOTHING_TO_OVERRIDE!>override<!> fun foo(r: R) = r
+    <!ACCIDENTAL_OVERRIDE!><!NOTHING_TO_OVERRIDE!>override<!> fun foo(r: R)<!> = r
    <!CONFLICTING_OVERLOADS!><!NOTHING_TO_OVERRIDE!>override<!> val <T> pr : R<!> = r
 }
 class <!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>MyIllegalClass1<!> : MyTrait<Int>, MyAbstractClass<String>() {}
@@ -2568,21 +2568,66 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest {
                    doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/accidentalOverrides/classFunctionOverriddenByProperty.kt");
                }
                
+                @TestMetadata("classFunctionOverriddenByPropertyInConstructor.kt")
+                public void testClassFunctionOverriddenByPropertyInConstructor() throws Exception {
+                    doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/accidentalOverrides/classFunctionOverriddenByPropertyInConstructor.kt");
+                }
+                
+                @TestMetadata("classFunctionOverriddenByPropertyNoGetter.kt")
+                public void testClassFunctionOverriddenByPropertyNoGetter() throws Exception {
+                    doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/accidentalOverrides/classFunctionOverriddenByPropertyNoGetter.kt");
+                }
+                
                @TestMetadata("classPropertyOverriddenByFunction.kt")
                public void testClassPropertyOverriddenByFunction() throws Exception {
                    doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/accidentalOverrides/classPropertyOverriddenByFunction.kt");
                }
                
+                @TestMetadata("delegatedFunctionOverriddenByProperty.kt")
+                public void testDelegatedFunctionOverriddenByProperty() throws Exception {
+                    doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/accidentalOverrides/delegatedFunctionOverriddenByProperty.kt");
+                }
+                
+                @TestMetadata("genericClassFunction.kt")
+                public void testGenericClassFunction() throws Exception {
+                    doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/accidentalOverrides/genericClassFunction.kt");
+                }
+                
+                @TestMetadata("privateClassFunctionOverriddenByProperty.kt")
+                public void testPrivateClassFunctionOverriddenByProperty() throws Exception {
+                    doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/accidentalOverrides/privateClassFunctionOverriddenByProperty.kt");
+                }
+                
                @TestMetadata("traitFunctionOverriddenByProperty.kt")
                public void testTraitFunctionOverriddenByProperty() throws Exception {
                    doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/accidentalOverrides/traitFunctionOverriddenByProperty.kt");
                }
                
+                @TestMetadata("traitFunctionOverriddenByPropertyInTrait.kt")
+                public void testTraitFunctionOverriddenByPropertyInTrait() throws Exception {
+                    doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/accidentalOverrides/traitFunctionOverriddenByPropertyInTrait.kt");
+                }
+                
+                @TestMetadata("traitFunctionOverriddenByPropertyNoImpl.kt")
+                public void testTraitFunctionOverriddenByPropertyNoImpl() throws Exception {
+                    doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/accidentalOverrides/traitFunctionOverriddenByPropertyNoImpl.kt");
+                }
+                
                @TestMetadata("traitPropertyOverriddenByFunction.kt")
                public void testTraitPropertyOverriddenByFunction() throws Exception {
                    doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/accidentalOverrides/traitPropertyOverriddenByFunction.kt");
                }
                
+                @TestMetadata("traitPropertyOverriddenByFunctionInTrait.kt")
+                public void testTraitPropertyOverriddenByFunctionInTrait() throws Exception {
+                    doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/accidentalOverrides/traitPropertyOverriddenByFunctionInTrait.kt");
+                }
+                
+                @TestMetadata("traitPropertyOverriddenByFunctionNoImpl.kt")
+                public void testTraitPropertyOverriddenByFunctionNoImpl() throws Exception {
+                    doTest("compiler/testData/diagnostics/tests/duplicateJvmSignature/accidentalOverrides/traitPropertyOverriddenByFunctionNoImpl.kt");
+                }
+                
            }
            
            @TestMetadata("compiler/testData/diagnostics/tests/duplicateJvmSignature/erasure")
@@ -25,7 +25,7 @@

    class <error>MyIllegalGenericClass1</error><T> : MyTrait<T>, MyAbstractClass<T>() {}
    class <error>MyIllegalGenericClass2</error><T, R> : MyTrait<T>, MyAbstractClass<R>() {
-        <error>override</error> fun foo(r: R) = r
+        <error><error>override</error> fun foo(r: R)</error> = r
    }
    class <error>MyIllegalClass1</error> : MyTrait<Int>, MyAbstractClass<String>() {}