From 8e49eb135ad33a9ff17aecb6c16ec4adb840eb8c Mon Sep 17 00:00:00 2001 From: Evgeny Gerashchenko Date: Fri, 14 Jun 2013 20:06:01 +0400 Subject: [PATCH] Correctly processing inherited methods when checking for SAM interface. --- .../resolve/java/provider/MembersCache.java | 70 ++++++++++++++++--- .../java/resolver/JavaClassResolver.java | 26 +++++-- .../SamSubinterfaceOfTwo.java | 14 ++++ .../SamSubinterfaceOfTwo.txt | 22 ++++++ .../SamSubinterfaceOverridding.java | 5 ++ .../SamSubinterfaceOverridding.txt | 7 ++ .../jvm/compiler/LoadJavaTestGenerated.java | 10 +++ 7 files changed, 139 insertions(+), 15 deletions(-) create mode 100644 compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOfTwo.java create mode 100644 compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOfTwo.txt create mode 100644 compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOverridding.java create mode 100644 compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOverridding.txt diff --git a/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/provider/MembersCache.java b/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/provider/MembersCache.java index 7443865b3d0..f2977da7900 100644 --- a/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/provider/MembersCache.java +++ b/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/provider/MembersCache.java @@ -18,10 +18,12 @@ package org.jetbrains.jet.lang.resolve.java.provider; import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Lists; import com.google.common.collect.Multimap; +import com.intellij.openapi.util.Ref; import com.intellij.psi.*; +import com.intellij.psi.util.MethodSignatureBackedByPsiMethod; import com.intellij.psi.util.PsiFormatUtil; +import com.intellij.util.ArrayUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.resolve.java.*; @@ -37,6 +39,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import static com.intellij.psi.util.MethodSignatureUtil.areSignaturesErasureEqual; import static com.intellij.psi.util.PsiFormatUtilBase.*; public final class MembersCache { @@ -420,17 +423,66 @@ public final class MembersCache { return null; } - List methods = Lists.newArrayList(); - for (PsiMethod method : psiClass.getAllMethods()) { - if (!isObjectMethod(method) && method.hasModifierProperty(PsiModifier.ABSTRACT)) { - methods.add(method); + return findOnlyAbstractMethod(psiClass); + } - if (method.hasTypeParameters()) { - return null; - } + @Nullable + private static PsiMethod findOnlyAbstractMethod(@NotNull PsiClass psiClass) { + Ref foundRef = Ref.create(); + if (findOnlyAbstractMethod(JavaPsiFacade.getElementFactory(psiClass.getProject()).createType(psiClass), foundRef)) { + MethodSignatureBackedByPsiMethod found = foundRef.get(); + return found == null ? null : found.getMethod(); + } + return null; + } + + private static boolean findOnlyAbstractMethod( + @NotNull PsiClassType classType, + @NotNull Ref foundRef + ) { + PsiClassType.ClassResolveResult classResolveResult = classType.resolveGenerics(); + PsiSubstitutor classSubstitutor = classResolveResult.getSubstitutor(); + PsiClass psiClass = classResolveResult.getElement(); + if (psiClass == null) { + return false; // can't resolve class -> not a SAM interface + } + if (CommonClassNames.JAVA_LANG_OBJECT.equals(psiClass.getQualifiedName())) { + return true; + } + for (PsiMethod method : psiClass.getMethods()) { + if (isObjectMethod(method)) { // e.g., ignore toString() declared in interface + continue; + } + if (method.hasTypeParameters()) { + return false; // if interface has generic methods, it is not a SAM interface + } + + MethodSignatureBackedByPsiMethod found = foundRef.get(); + if (found == null) { + foundRef.set((MethodSignatureBackedByPsiMethod) method.getSignature(classSubstitutor)); + continue; + } + if (!found.getName().equals(method.getName())) { + return false; // optimizing heuristic + } + MethodSignatureBackedByPsiMethod current = (MethodSignatureBackedByPsiMethod) method.getSignature(classSubstitutor); + if (!areSignaturesErasureEqual(current, found) || isVarargMethod(method) != isVarargMethod(found.getMethod())) { + return false; // different signatures } } - return methods.size() == 1 ? methods.get(0) : null; + + for (PsiType t : classType.getSuperTypes()) { + if (!findOnlyAbstractMethod((PsiClassType) t, foundRef)) { + return false; + } + } + + return true; + } + + private static boolean isVarargMethod(@NotNull PsiMethod method) { + PsiParameter lastParameter = ArrayUtil.getLastElement(method.getParameterList().getParameters()); + return lastParameter != null && lastParameter.getType() instanceof PsiEllipsisType; } private static abstract class RunOnce implements Runnable { diff --git a/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/resolver/JavaClassResolver.java b/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/resolver/JavaClassResolver.java index f280910e44e..3e914b3e64d 100644 --- a/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/resolver/JavaClassResolver.java +++ b/compiler/frontend.java/src/org/jetbrains/jet/lang/resolve/java/resolver/JavaClassResolver.java @@ -45,6 +45,7 @@ import org.jetbrains.jet.lang.resolve.name.FqNameUnsafe; import org.jetbrains.jet.lang.resolve.name.Name; import org.jetbrains.jet.lang.types.JetType; import org.jetbrains.jet.lang.types.TypeUtils; +import org.jetbrains.jet.lang.types.checker.JetTypeChecker; import javax.inject.Inject; import java.util.ArrayList; @@ -301,15 +302,28 @@ public final class JavaClassResolver { return abstractMethod; } else { - Set supertypes = TypeUtils.getAllSupertypes(samInterface.getDefaultType()); - for (JetType supertype : supertypes) { - List abstractMembers = SingleAbstractMethodUtils.getAbstractMembers(supertype); - if (!abstractMembers.isEmpty()) { - return (SimpleFunctionDescriptor) abstractMembers.get(0); - } + return findFunctionWithMostSpecificReturnType(TypeUtils.getAllSupertypes(samInterface.getDefaultType())); + } + } + + private static SimpleFunctionDescriptor findFunctionWithMostSpecificReturnType(@NotNull Set supertypes) { + List candidates = Lists.newArrayList(); + for (JetType supertype : supertypes) { + List abstractMembers = SingleAbstractMethodUtils.getAbstractMembers(supertype); + if (!abstractMembers.isEmpty()) { + candidates.add((SimpleFunctionDescriptor) abstractMembers.get(0)); } + } + if (candidates.isEmpty()) { throw new IllegalStateException("Couldn't find abstract method in supertypes " + supertypes); } + SimpleFunctionDescriptor currentMostSpecificType = candidates.get(0); + for (SimpleFunctionDescriptor candidate : candidates) { + if (JetTypeChecker.INSTANCE.isSubtypeOf(candidate.getReturnType(), currentMostSpecificType.getReturnType())) { + currentMostSpecificType = candidate; + } + } + return currentMostSpecificType; } private void cache(@NotNull FqNameBase fqName, @Nullable ClassDescriptor classDescriptor) { diff --git a/compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOfTwo.java b/compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOfTwo.java new file mode 100644 index 00000000000..3d4c590d19a --- /dev/null +++ b/compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOfTwo.java @@ -0,0 +1,14 @@ +package test; + +public interface SamSubinterfaceOfTwo { + public interface Super1 { + CharSequence f(); + } + + public interface Super2 { + T f(); + } + + public interface Sub extends Super1, Super2 { + } +} diff --git a/compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOfTwo.txt b/compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOfTwo.txt new file mode 100644 index 00000000000..07f65c706d7 --- /dev/null +++ b/compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOfTwo.txt @@ -0,0 +1,22 @@ +package test + +public trait SamSubinterfaceOfTwo : java.lang.Object { + + public trait Sub : test.SamSubinterfaceOfTwo.Super1, test.SamSubinterfaceOfTwo.Super2 { + public abstract override /*2*/ /*fake_override*/ fun f(): jet.CharSequence? + } + + public trait Super1 : java.lang.Object { + public abstract fun f(): jet.CharSequence? + } + + public trait Super2 : java.lang.Object { + public abstract fun f(): T? + } +} + +package SamSubinterfaceOfTwo { + public /*synthesized*/ fun Sub(/*0*/ function: () -> jet.String?): test.SamSubinterfaceOfTwo.Sub + public /*synthesized*/ fun Super1(/*0*/ function: () -> jet.CharSequence?): test.SamSubinterfaceOfTwo.Super1 + public /*synthesized*/ fun Super2(/*0*/ function: () -> T?): test.SamSubinterfaceOfTwo.Super2 +} diff --git a/compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOverridding.java b/compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOverridding.java new file mode 100644 index 00000000000..d21e0b345ba --- /dev/null +++ b/compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOverridding.java @@ -0,0 +1,5 @@ +package test; + +public interface SamSubinterfaceOverridding extends Runnable { + void run(); +} diff --git a/compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOverridding.txt b/compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOverridding.txt new file mode 100644 index 00000000000..fbdec2aa483 --- /dev/null +++ b/compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOverridding.txt @@ -0,0 +1,7 @@ +package test + +public /*synthesized*/ fun SamSubinterfaceOverridding(/*0*/ function: () -> jet.Unit): test.SamSubinterfaceOverridding + +public trait SamSubinterfaceOverridding : java.lang.Runnable { + public abstract override /*1*/ fun run(): jet.Unit +} diff --git a/compiler/tests/org/jetbrains/jet/jvm/compiler/LoadJavaTestGenerated.java b/compiler/tests/org/jetbrains/jet/jvm/compiler/LoadJavaTestGenerated.java index 36030874f3d..412b26ca6b8 100644 --- a/compiler/tests/org/jetbrains/jet/jvm/compiler/LoadJavaTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/jvm/compiler/LoadJavaTestGenerated.java @@ -1218,6 +1218,16 @@ public class LoadJavaTestGenerated extends AbstractLoadJavaTest { doTestCompiledJava("compiler/testData/loadJava/compiledJava/singleAbstractMethod/Runnable.java"); } + @TestMetadata("SamSubinterfaceOfTwo.java") + public void testSamSubinterfaceOfTwo() throws Exception { + doTestCompiledJava("compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOfTwo.java"); + } + + @TestMetadata("SamSubinterfaceOverridding.java") + public void testSamSubinterfaceOverridding() throws Exception { + doTestCompiledJava("compiler/testData/loadJava/compiledJava/singleAbstractMethod/SamSubinterfaceOverridding.java"); + } + @TestMetadata("SubstitutedSamInterface.java") public void testSubstitutedSamInterface() throws Exception { doTestCompiledJava("compiler/testData/loadJava/compiledJava/singleAbstractMethod/SubstitutedSamInterface.java");