Fix for MakeOverriddenMemberOpenFix

Fixes bugs which led to BindingContextUtils#descriptorToDeclaration throwing exception

idea/src/org/jetbrains/jet/plugin/quickfix/MakeOverriddenMemberOpenFix.java

@@ -16,12 +16,15 @@
 
 package org.jetbrains.jet.plugin.quickfix;
 
+import com.google.common.collect.Sets;
 import com.intellij.codeInsight.intention.IntentionAction;
 import com.intellij.openapi.editor.Editor;
 import com.intellij.openapi.project.Project;
+import com.intellij.openapi.util.Condition;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiFile;
 import com.intellij.util.IncorrectOperationException;
+import com.intellij.util.containers.ContainerUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.jet.lang.descriptors.CallableMemberDescriptor;
@@ -29,19 +32,18 @@ import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor;
 import org.jetbrains.jet.lang.diagnostics.Diagnostic;
 import org.jetbrains.jet.lang.psi.JetDeclaration;
 import org.jetbrains.jet.lang.psi.JetFile;
-import org.jetbrains.jet.lang.resolve.BindingContextUtils;
 import org.jetbrains.jet.plugin.JetBundle;
 import org.jetbrains.jet.plugin.project.AnalyzerFacadeWithCache;
 import org.jetbrains.jet.plugin.project.CancelableResolveSession;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
+import java.util.*;
 
+import static org.jetbrains.jet.lang.descriptors.CallableMemberDescriptor.Kind.*;
+import static org.jetbrains.jet.lang.resolve.BindingContextUtils.descriptorToDeclaration;
 import static org.jetbrains.jet.lexer.JetTokens.OPEN_KEYWORD;
 
 public class MakeOverriddenMemberOpenFix extends JetIntentionAction<JetDeclaration> {
-    private final List<PsiElement> overriddenMembers = new ArrayList<PsiElement>();
+    private final List<PsiElement> overriddenNonOverridableMembers = new ArrayList<PsiElement>();
     private final List<String> containingDeclarationsNames = new ArrayList<String>();
 
     public MakeOverriddenMemberOpenFix(@NotNull JetDeclaration declaration) {
@@ -55,32 +57,68 @@ public class MakeOverriddenMemberOpenFix extends JetIntentionAction<JetDeclarati
         }
 
         // When running single test 'isAvailable()' is invoked multiple times, so we need to clear lists.
-        overriddenMembers.clear();
+        overriddenNonOverridableMembers.clear();
         containingDeclarationsNames.clear();
 
         CancelableResolveSession resolveSession = AnalyzerFacadeWithCache.getLazyResolveSessionForFile((JetFile) file);
         DeclarationDescriptor descriptor = resolveSession.resolveToDescriptor(element);
         if (!(descriptor instanceof CallableMemberDescriptor)) return false;
-        CallableMemberDescriptor callableMemberDescriptor = (CallableMemberDescriptor) descriptor;
-        for (CallableMemberDescriptor overriddenDescriptor : callableMemberDescriptor.getOverriddenDescriptors()) {
-            if (!overriddenDescriptor.getModality().isOverridable()) {
-                PsiElement overriddenMember =
-                        BindingContextUtils.descriptorToDeclaration(resolveSession.getBindingContext(), overriddenDescriptor);
-                if (overriddenMember == null || !QuickFixUtil.canModifyElement(overriddenMember)) {
-                    return false;
-                }
-                String containingDeclarationName = overriddenDescriptor.getContainingDeclaration().getName().asString();
-                overriddenMembers.add(overriddenMember);
-                containingDeclarationsNames.add(containingDeclarationName);
+
+        for (CallableMemberDescriptor overriddenDescriptor : getAllDeclaredNonOverridableOverriddenDescriptors(
+                (CallableMemberDescriptor) descriptor)) {
+            assert overriddenDescriptor.getKind() == DECLARATION : "Can only be applied to declarations.";
+            PsiElement overriddenMember = descriptorToDeclaration(resolveSession.getBindingContext(), overriddenDescriptor);
+            if (overriddenMember == null || !QuickFixUtil.canModifyElement(overriddenMember)) {
+                return false;
+            }
+            String containingDeclarationName = overriddenDescriptor.getContainingDeclaration().getName().asString();
+            overriddenNonOverridableMembers.add(overriddenMember);
+            containingDeclarationsNames.add(containingDeclarationName);
+        }
+        return overriddenNonOverridableMembers.size() > 0;
+    }
+
+    @NotNull
+    private static Collection<CallableMemberDescriptor> getAllDeclaredNonOverridableOverriddenDescriptors(
+            @NotNull CallableMemberDescriptor callableMemberDescriptor
+    ) {
+        Set<CallableMemberDescriptor> result = Sets.newHashSet();
+        Collection<CallableMemberDescriptor>
+                nonOverridableOverriddenDescriptors = retainNonOverridableMembers(callableMemberDescriptor.getOverriddenDescriptors());
+        for (CallableMemberDescriptor overriddenDescriptor : nonOverridableOverriddenDescriptors) {
+            CallableMemberDescriptor.Kind kind = overriddenDescriptor.getKind();
+            if (kind == DECLARATION) {
+                result.add(overriddenDescriptor);
+            }
+            else if (kind == FAKE_OVERRIDE || kind == DELEGATION) {
+                result.addAll(getAllDeclaredNonOverridableOverriddenDescriptors(overriddenDescriptor));
+            }
+            else if (kind == SYNTHESIZED) {
+                // do nothing, final synthesized members can't be made open
+            }
+            else {
+                throw new UnsupportedOperationException("Unexpected callable kind " + kind);
             }
         }
-        return overriddenMembers.size() > 0;
+        return result;
+    }
+
+    @NotNull
+    private static Collection<CallableMemberDescriptor> retainNonOverridableMembers(
+            @NotNull Collection<? extends CallableMemberDescriptor> callableMemberDescriptors
+    ) {
+        return ContainerUtil.filter(callableMemberDescriptors, new Condition<CallableMemberDescriptor>() {
+            @Override
+            public boolean value(CallableMemberDescriptor descriptor) {
+                return !descriptor.getModality().isOverridable();
+            }
+        });
     }
 
     @NotNull
     @Override
     public String getText() {
-        if (overriddenMembers.size() == 1) {
+        if (overriddenNonOverridableMembers.size() == 1) {
             return JetBundle.message("make.element.modifier", containingDeclarationsNames.get(0) + "." + element.getName(), OPEN_KEYWORD);
         }
 
@@ -104,7 +142,7 @@ public class MakeOverriddenMemberOpenFix extends JetIntentionAction<JetDeclarati
 
     @Override
     public void invoke(@NotNull Project project, Editor editor, JetFile file) throws IncorrectOperationException {
-        for (PsiElement overriddenMember : overriddenMembers) {
+        for (PsiElement overriddenMember : overriddenNonOverridableMembers) {
             overriddenMember.replace(AddModifierFix.addModifierWithDefaultReplacement(overriddenMember, OPEN_KEYWORD, project, false));
         }
     }

idea/testData/quickfix/override/afterOverridingDelegatedMethod.kt

@@ -0,0 +1,18 @@
+// "Make XX.foo open" "true"
+trait X {
+    fun foo()
+}
+
+trait XX : X {
+    override open fun foo() {
+
+    }
+}
+
+abstract class A(val y: XX) : X, XX by y {
+}
+
+class B(y: XX) : A(y) {
+    override fun foo() {
+    }
+}

idea/testData/quickfix/override/afterOverridingFakeOverride.kt

@@ -0,0 +1,18 @@
+// "Make XX.foo open" "true"
+trait X {
+    fun foo()
+}
+
+trait XX : X {
+    override open fun foo() {
+
+    }
+}
+
+trait Y : X, XX {
+}
+
+class B() : Y {
+    override fun foo() {
+    }
+}

idea/testData/quickfix/override/beforeOverridingDelegatedMethod.kt

@@ -0,0 +1,18 @@
+// "Make XX.foo open" "true"
+trait X {
+    fun foo()
+}
+
+trait XX : X {
+    override final fun foo() {
+
+    }
+}
+
+abstract class A(val y: XX) : X, XX by y {
+}
+
+class B(y: XX) : A(y) {
+    override<caret> fun foo() {
+    }
+}

idea/testData/quickfix/override/beforeOverridingFakeOverride.kt

@@ -0,0 +1,18 @@
+// "Make XX.foo open" "true"
+trait X {
+    fun foo()
+}
+
+trait XX : X {
+    override final fun foo() {
+
+    }
+}
+
+trait Y : X, XX {
+}
+
+class B() : Y {
+    override<caret> fun foo() {
+    }
+}

idea/tests/org/jetbrains/jet/plugin/quickfix/QuickFixTestGenerated.java

@@ -1017,6 +1017,16 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest {
             doTest("idea/testData/quickfix/override/beforeOverriddingMultipleFinalMethods.kt");
         }
         
+        @TestMetadata("beforeOverridingDelegatedMethod.kt")
+        public void testOverridingDelegatedMethod() throws Exception {
+            doTest("idea/testData/quickfix/override/beforeOverridingDelegatedMethod.kt");
+        }
+        
+        @TestMetadata("beforeOverridingFakeOverride.kt")
+        public void testOverridingFakeOverride() throws Exception {
+            doTest("idea/testData/quickfix/override/beforeOverridingFakeOverride.kt");
+        }
+        
         @TestMetadata("beforeOverridingFinalJavaMethod.kt")
         public void testOverridingFinalJavaMethod() throws Exception {
             doTest("idea/testData/quickfix/override/beforeOverridingFinalJavaMethod.kt");