From 230c6fd84bc2c164826e20a0bbf378bc014a44f8 Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Wed, 14 Aug 2013 18:46:01 +0400 Subject: [PATCH] moreSpecific: use site info to check assignability, prefer concrete to abstract methods (IDEA-57569) --- .../psi/infos/MethodCandidateInfo.java | 4 ++ .../JavaMethodsConflictResolver.java | 55 +++++++++++++------ .../pck/AmbiguousMethodCall.java | 11 ++++ .../AmbiguousTypeParamVsConcrete.java | 22 ++++++++ .../daemon/AdvHighlightingJdk7Test.java | 4 ++ .../daemon/GenericsHighlightingTest.java | 1 + 6 files changed, 80 insertions(+), 17 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting7/ambiguousIDEA57569/pck/AmbiguousMethodCall.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/genericsHighlighting/AmbiguousTypeParamVsConcrete.java diff --git a/java/java-psi-api/src/com/intellij/psi/infos/MethodCandidateInfo.java b/java/java-psi-api/src/com/intellij/psi/infos/MethodCandidateInfo.java index 94f4cd30ffd0..299575ef5632 100644 --- a/java/java-psi-api/src/com/intellij/psi/infos/MethodCandidateInfo.java +++ b/java/java-psi-api/src/com/intellij/psi/infos/MethodCandidateInfo.java @@ -94,6 +94,10 @@ public class MethodCandidateInfo extends CandidateInfo{ return myApplicabilityLevel; } + public PsiSubstitutor getSiteSubstitutor() { + return super.getSubstitutor(); + } + @Override public PsiSubstitutor getSubstitutor() { if (myCalcedSubstitutor == null) { diff --git a/java/java-psi-impl/src/com/intellij/psi/scope/conflictResolvers/JavaMethodsConflictResolver.java b/java/java-psi-impl/src/com/intellij/psi/scope/conflictResolvers/JavaMethodsConflictResolver.java index c3906e137fd1..3d1cac85cd1c 100644 --- a/java/java-psi-impl/src/com/intellij/psi/scope/conflictResolvers/JavaMethodsConflictResolver.java +++ b/java/java-psi-impl/src/com/intellij/psi/scope/conflictResolvers/JavaMethodsConflictResolver.java @@ -17,6 +17,7 @@ package com.intellij.psi.scope.conflictResolvers; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.progress.ProgressManager; +import com.intellij.openapi.project.Project; import com.intellij.openapi.projectRoots.JavaSdkVersion; import com.intellij.openapi.projectRoots.JavaVersionService; import com.intellij.openapi.util.Comparing; @@ -39,8 +40,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import static com.sun.tools.javac.code.Flags.ABSTRACT; - /** * Created by IntelliJ IDEA. * User: ik @@ -502,15 +501,21 @@ public class JavaMethodsConflictResolver implements PsiConflictResolver{ if (boxingHappened[0] > 0 && boxingHappened[1] == 0) return Specifics.SECOND; if (sameBoxing) { - final PsiResolveHelper resolveHelper = PsiResolveHelper.SERVICE.getInstance(myArgumentsList.getProject()); - int level1 = getLevel(applicabilityLevel, languageLevel, method1, typeParameters1, types2, types1, resolveHelper); - int level2 = getLevel(applicabilityLevel, languageLevel, method2, typeParameters2, types1, types2, resolveHelper); + final PsiSubstitutor siteSubstitutor1 = ((MethodCandidateInfo)info1).getSiteSubstitutor(); + final PsiSubstitutor siteSubstitutor2 = ((MethodCandidateInfo)info2).getSiteSubstitutor(); + final int level1 = getLevel(applicabilityLevel, languageLevel, method1, typeParameters1, types2, types1, siteSubstitutor2, siteSubstitutor1); + final int level2 = getLevel(applicabilityLevel, languageLevel, method2, typeParameters2, types1, types2, siteSubstitutor1, siteSubstitutor2); if (level1 > level2) return Specifics.SECOND; if (level2 > level1) return Specifics.FIRST; - final boolean raw1 = PsiUtil.isRawSubstitutor(method1, classSubstitutor1); - final boolean raw2 = PsiUtil.isRawSubstitutor(method2, classSubstitutor2); - if (raw1 ^ raw2) { - return raw1 ? Specifics.SECOND : Specifics.FIRST; + if (level1 > MethodCandidateInfo.ApplicabilityLevel.NOT_APPLICABLE) { + final boolean abstract1 = method1.hasModifierProperty(PsiModifier.ABSTRACT); + final boolean abstract2 = method2.hasModifierProperty(PsiModifier.ABSTRACT); + if (abstract1 && !abstract2) { + return Specifics.SECOND; + } + if (abstract2 && !abstract1) { + return Specifics.FIRST; + } } } else if (applicabilityLevel == MethodCandidateInfo.ApplicabilityLevel.VARARGS) { @@ -546,6 +551,12 @@ public class JavaMethodsConflictResolver implements PsiConflictResolver{ } } + final boolean raw1 = PsiUtil.isRawSubstitutor(method1, classSubstitutor1); + final boolean raw2 = PsiUtil.isRawSubstitutor(method2, classSubstitutor2); + if (raw1 ^ raw2) { + return raw1 ? Specifics.SECOND : Specifics.FIRST; + } + return Specifics.NEITHER; } @@ -553,9 +564,17 @@ public class JavaMethodsConflictResolver implements PsiConflictResolver{ LanguageLevel languageLevel, PsiMethod method2, PsiTypeParameter[] typeParameters2, - PsiType[] types1, PsiType[] types2, PsiResolveHelper resolveHelper) { - final PsiSubstitutor methodSubstitutor2 = calculateMethodSubstitutor(typeParameters2, types2, types1, resolveHelper, languageLevel); - final int level = Math.min(applicabilityLevel, PsiUtil.getApplicabilityLevel(method2, methodSubstitutor2, types1, languageLevel, false)); + PsiType[] types1, + PsiType[] types2, + PsiSubstitutor siteSubstitutor1, + PsiSubstitutor siteSubstitutor2) { + final int argsLength = types1.length; + final PsiType[] nTypes1 = new PsiType[argsLength]; + for (int i = 0; i < argsLength; i++) { + nTypes1[i] = siteSubstitutor1.substitute(types1[i]); + } + final PsiSubstitutor methodSubstitutor2 = calculateMethodSubstitutor(typeParameters2, method2, siteSubstitutor2, types2, nTypes1, languageLevel); + final int level = Math.min(applicabilityLevel, PsiUtil.getApplicabilityLevel(method2, methodSubstitutor2, nTypes1, languageLevel, false)); if (level > MethodCandidateInfo.ApplicabilityLevel.NOT_APPLICABLE) { for (PsiTypeParameter typeParameter : typeParameters2) { final PsiType substituted = methodSubstitutor2.substitute(typeParameter); @@ -566,7 +585,7 @@ public class JavaMethodsConflictResolver implements PsiConflictResolver{ } } if (level == MethodCandidateInfo.ApplicabilityLevel.VARARGS) { - if (!TypeConversionUtil.isAssignable(methodSubstitutor2.substitute(types2[types1.length - 1]), types1[types1.length - 1])) { + if (!TypeConversionUtil.isAssignable(methodSubstitutor2.substitute(types2[argsLength - 1]), nTypes1[argsLength - 1])) { return MethodCandidateInfo.ApplicabilityLevel.NOT_APPLICABLE; } } @@ -575,16 +594,18 @@ public class JavaMethodsConflictResolver implements PsiConflictResolver{ } private static PsiSubstitutor calculateMethodSubstitutor(final PsiTypeParameter[] typeParameters, + final PsiMethod method, + final PsiSubstitutor siteSubstitutor, final PsiType[] types1, final PsiType[] types2, - final PsiResolveHelper resolveHelper, @NotNull LanguageLevel languageLevel) { - PsiSubstitutor substitutor = resolveHelper.inferTypeArguments(typeParameters, types1, types2, languageLevel); - for (PsiTypeParameter typeParameter : typeParameters) { + PsiSubstitutor substitutor = PsiResolveHelper.SERVICE.getInstance(method.getProject()) + .inferTypeArguments(typeParameters, types1, types2, languageLevel); + for (PsiTypeParameter typeParameter : PsiUtil.typeParametersIterable(method)) { ProgressManager.checkCanceled(); LOG.assertTrue(typeParameter != null); if (!substitutor.getSubstitutionMap().containsKey(typeParameter)) { - substitutor = substitutor.put(typeParameter, TypeConversionUtil.typeParameterErasure(typeParameter)); + substitutor = substitutor.put(typeParameter, TypeConversionUtil.erasure(siteSubstitutor.substitute(typeParameter))); } } return substitutor; diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting7/ambiguousIDEA57569/pck/AmbiguousMethodCall.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting7/ambiguousIDEA57569/pck/AmbiguousMethodCall.java new file mode 100644 index 000000000000..ac6a0cea0ead --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting7/ambiguousIDEA57569/pck/AmbiguousMethodCall.java @@ -0,0 +1,11 @@ +package pck; +abstract class C { + abstract Object foo(T x); + String foo(String x) { return null; } +} + +class D extends C{ + { + foo(""); + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/genericsHighlighting/AmbiguousTypeParamVsConcrete.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/genericsHighlighting/AmbiguousTypeParamVsConcrete.java new file mode 100644 index 000000000000..272fb292d2a3 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/genericsHighlighting/AmbiguousTypeParamVsConcrete.java @@ -0,0 +1,22 @@ +public class CssPropertyValueImpl extends CssTableValueBase implements CssPropertyValue { + public CssPropertyValueImpl(final Type type) { + super(type); + } +} + +public abstract class CssTableValueBase implements CssTableValue { + + protected CssTableValueBase(final Type type) { + } + + protected CssTableValueBase(final T value) { + } +} + +enum Type {} + +interface CssTableValue { +} + +interface CssPropertyValue extends CssTableValue { +} diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/AdvHighlightingJdk7Test.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/AdvHighlightingJdk7Test.java index 46c04c575bf6..a3779d7310c2 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/AdvHighlightingJdk7Test.java +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/AdvHighlightingJdk7Test.java @@ -215,6 +215,10 @@ public class AdvHighlightingJdk7Test extends DaemonAnalyzerTestCase { doTestAmbiguous(); } + public void testAmbiguousIDEA57569() throws Exception { + doTestAmbiguous(); + } + public void testAmbiguousMethodsFromSameClassAccess() throws Exception { doTestAmbiguous(); } diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/GenericsHighlightingTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/GenericsHighlightingTest.java index 88217e92ef2b..3fef647dbaa3 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/GenericsHighlightingTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/GenericsHighlightingTest.java @@ -312,6 +312,7 @@ public class GenericsHighlightingTest extends LightDaemonAnalyzerTestCase { public void testIDEA67865() { doTest5(false); } public void testBoxingSpecific() { doTest5(false); } public void testIDEA67843() { doTest5(false); } + public void testAmbiguousTypeParamVsConcrete() { doTest5(false); } public void testJavaUtilCollections_NoVerify() throws Exception { PsiClass collectionsClass = getJavaFacade().findClass("java.util.Collections", GlobalSearchScope.moduleWithLibrariesScope(getModule()));