From 55b6e405a6a90c7e1860e1076de2d89c565b5dce Mon Sep 17 00:00:00 2001 From: Dmitry Cheryasov Date: Fri, 7 May 2010 18:55:34 +0300 Subject: [PATCH] Detect parameters of bound static methods (PY-50). Added tests. --- .../python/psi/PyReferenceExpression.java | 3 +- .../psi/impl/PyCallExpressionHelper.java | 55 +++++++++++----- .../psi/impl/PyReferenceExpressionImpl.java | 65 ++++++++++++++----- .../paramInfo/BoundMethodReassigned.py | 11 ++++ .../testData/paramInfo/BoundMethodSimple.py | 7 ++ .../testData/paramInfo/BoundMethodStatic.py | 8 +++ .../jetbrains/python/PyParameterInfoTest.java | 29 ++++++++- 7 files changed, 146 insertions(+), 32 deletions(-) create mode 100644 python/testData/paramInfo/BoundMethodReassigned.py create mode 100644 python/testData/paramInfo/BoundMethodSimple.py create mode 100644 python/testData/paramInfo/BoundMethodStatic.py diff --git a/python/src/com/jetbrains/python/psi/PyReferenceExpression.java b/python/src/com/jetbrains/python/psi/PyReferenceExpression.java index b37a3d8b709b..ae403e042e45 100644 --- a/python/src/com/jetbrains/python/psi/PyReferenceExpression.java +++ b/python/src/com/jetbrains/python/psi/PyReferenceExpression.java @@ -4,6 +4,7 @@ import com.intellij.psi.PsiPolyVariantReference; import com.intellij.psi.ResolveResult; import com.jetbrains.python.psi.impl.PyQualifiedName; import com.jetbrains.python.psi.resolve.PyResolveContext; +import com.jetbrains.python.psi.resolve.QualifiedResolveResult; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -21,7 +22,7 @@ public interface PyReferenceExpression extends PyQualifiedExpression { * Note: will return null if the assignment chain ends in a target of a non-assignment statement such as 'for'. */ @NotNull - ResolveResult followAssignmentsChain(); + QualifiedResolveResult followAssignmentsChain(); @Nullable PyQualifiedName asQualifiedName(); diff --git a/python/src/com/jetbrains/python/psi/impl/PyCallExpressionHelper.java b/python/src/com/jetbrains/python/psi/impl/PyCallExpressionHelper.java index 9c32f86823f1..33cebd93ee8b 100644 --- a/python/src/com/jetbrains/python/psi/impl/PyCallExpressionHelper.java +++ b/python/src/com/jetbrains/python/psi/impl/PyCallExpressionHelper.java @@ -3,10 +3,10 @@ package com.jetbrains.python.psi.impl; import com.intellij.openapi.util.Pair; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiNamedElement; -import com.intellij.psi.ResolveResult; import com.jetbrains.python.PyNames; import com.jetbrains.python.psi.*; import com.jetbrains.python.psi.resolve.ImplicitResolveResult; +import com.jetbrains.python.psi.resolve.QualifiedResolveResult; import com.jetbrains.python.psi.types.PyClassType; import com.jetbrains.python.psi.types.PyType; import com.jetbrains.python.psi.types.TypeEvalContext; @@ -78,7 +78,7 @@ public class PyCallExpressionHelper { boolean is_constructor_call = false; if (callee instanceof PyReferenceExpression) { PyReferenceExpression ref = (PyReferenceExpression)callee; - ResolveResult resolveResult = ref.followAssignmentsChain(); + QualifiedResolveResult resolveResult = ref.followAssignmentsChain(); PsiElement resolved = resolveResult.getElement(); if (resolved instanceof PyClass) { resolved = ((PyClass)resolved).findInitOrNew(true); // class to constructor call @@ -97,7 +97,14 @@ public class PyCallExpressionHelper { } if (resolved instanceof PyFunction) { EnumSet flags = EnumSet.noneOf(PyFunction.Flag.class); - int implicit_offset = getImplicitArgumentCount(us.getCallee(), (PyFunction) resolved, wrapped_flag, flags); + PyExpression last_qualifier = resolveResult.getLastQualifier(); + final PyExpression call_reference = us.getCallee(); + boolean is_by_instance = isByInstance(call_reference); + if (last_qualifier != null) { + PyType qualifier_type = last_qualifier.getType(TypeEvalContext.fast()); // NOTE: ...or slow()? + is_by_instance |= (qualifier_type != null && qualifier_type instanceof PyClassType && !((PyClassType)qualifier_type).isDefinition()); + } + int implicit_offset = getImplicitArgumentCount(call_reference, (PyFunction) resolved, wrapped_flag, flags, is_by_instance); if (! is_constructor_call && PyNames.NEW.equals(((PyFunction)resolved).getName())) { implicit_offset = Math.min(implicit_offset-1, 0); // case of Class.__new__ } @@ -108,27 +115,45 @@ public class PyCallExpressionHelper { return null; } + /** + * Calls the {@link #getImplicitArgumentCount(PyExpression, PyFunction, PyFunction.Flag, EnumSet, boolean) full version} + * with null flags and with isByInstance inferred directly from call site (won't work with reassigned bound methods). + * @param callReference the call site, where arguments are given. + * @param functionBeingCalled resolved method which is being called; plain functions are OK but make little sense. + * @return a non-negative number of parameters that are implicit to this call. + */ public static int getImplicitArgumentCount(final PyExpression callReference, PyFunction functionBeingCalled) { - return getImplicitArgumentCount(callReference, functionBeingCalled, null, null); + return getImplicitArgumentCount(callReference, functionBeingCalled, null, null, isByInstance(callReference)); } + /** + * Finds how many arguments are implicit in a given call. + * @param callReference the call site, where arguments are given. + * @param method resolved method which is being called; plain functions are OK but make little sense. + * @param wrappedFlag value of {@link PyFunction.Flag#WRAPPED} if known. + * @param flags set of flags to be updated by this call; wrappedFlag's value ends up here, too. + * @param isByInstance true if the call is known to be by instance (not by class). + * @return a non-negative number of parameters that are implicit to this call. E.g. for a typical method call 1 is returned + * because one parameter ('self') is implicit. + */ private static int getImplicitArgumentCount(final PyExpression callReference, PyFunction method, - @Nullable PyFunction.Flag wrapped_flag, - @Nullable EnumSet flags) { + @Nullable PyFunction.Flag wrappedFlag, + @Nullable EnumSet flags, + boolean isByInstance + ) { int implicit_offset = 0; - boolean is_by_instance = isByInstance(callReference); - if (is_by_instance) implicit_offset += 1; + if (isByInstance) implicit_offset += 1; // wrapped flags? - if (wrapped_flag != null) { + if (wrappedFlag != null) { if (flags != null) { - flags.add(wrapped_flag); + flags.add(wrappedFlag); flags.add(PyFunction.Flag.WRAPPED); } - if (wrapped_flag == PyFunction.Flag.STATICMETHOD && implicit_offset > 0) implicit_offset -= 1; // might have marked it as implicit 'self' - if (wrapped_flag == PyFunction.Flag.CLASSMETHOD && ! is_by_instance) implicit_offset += 1; // Both Foo.method() and foo.method() have implicit the first arg + if (wrappedFlag == PyFunction.Flag.STATICMETHOD && implicit_offset > 0) implicit_offset -= 1; // might have marked it as implicit 'self' + if (wrappedFlag == PyFunction.Flag.CLASSMETHOD && ! isByInstance) implicit_offset += 1; // Both Foo.method() and foo.method() have implicit the first arg } - if (! is_by_instance && PyNames.NEW.equals(method.getName())) implicit_offset += 1; // constructor call + if (! isByInstance && PyNames.NEW.equals(method.getName())) implicit_offset += 1; // constructor call // decorators? if (PyNames.INIT.equals(method.getName())) { String refName = callReference instanceof PyReferenceExpression @@ -151,13 +176,13 @@ public class PyCallExpressionHelper { if (flags != null) { flags.add(PyFunction.Flag.STATICMETHOD); } - if (is_by_instance && implicit_offset > 0) implicit_offset -= 1; // might have marked it as implicit 'self' + if (isByInstance && implicit_offset > 0) implicit_offset -= 1; // might have marked it as implicit 'self' } else if (PyNames.CLASSMETHOD.equals(deconame)) { if (flags != null) { flags.add(PyFunction.Flag.CLASSMETHOD); } - if (! is_by_instance) implicit_offset += 1; // Both Foo.method() and foo.method() have implicit the first arg + if (! isByInstance) implicit_offset += 1; // Both Foo.method() and foo.method() have implicit the first arg } // else could be custom decorator processing } diff --git a/python/src/com/jetbrains/python/psi/impl/PyReferenceExpressionImpl.java b/python/src/com/jetbrains/python/psi/impl/PyReferenceExpressionImpl.java index ceb8b515a0b6..37a042db1885 100644 --- a/python/src/com/jetbrains/python/psi/impl/PyReferenceExpressionImpl.java +++ b/python/src/com/jetbrains/python/psi/impl/PyReferenceExpressionImpl.java @@ -16,6 +16,7 @@ import com.jetbrains.python.console.pydev.PydevConsoleCommunication; import com.jetbrains.python.psi.*; import com.jetbrains.python.psi.resolve.PyResolveContext; import com.jetbrains.python.psi.resolve.PyResolveUtil; +import com.jetbrains.python.psi.resolve.QualifiedResolveResult; import com.jetbrains.python.psi.types.*; import com.jetbrains.python.refactoring.PyDefUseUtil; import org.jetbrains.annotations.NotNull; @@ -97,10 +98,13 @@ public class PyReferenceExpressionImpl extends PyElementImpl implements PyRefere } + private final QualifiedResolveResult EMPTY_RESULT = new QualifiedResolveResultEmpty(); + @NotNull - public ResolveResult followAssignmentsChain() { + public QualifiedResolveResult followAssignmentsChain() { PyReferenceExpression seeker = this; - ResolveResult ret = null; + QualifiedResolveResult ret = null; + PyExpression last_qualifier = null; SEARCH: while (ret == null) { ResolveResult[] targets = seeker.getReference().multiResolve(false); @@ -110,28 +114,19 @@ public class PyReferenceExpressionImpl extends PyElementImpl implements PyRefere PyExpression assigned_from = ((PyTargetExpression)elt).findAssignedValue(); if (assigned_from instanceof PyReferenceExpression) { seeker = (PyReferenceExpression)assigned_from; + if (seeker.getQualifier() != null) last_qualifier = seeker.getQualifier(); continue SEARCH; } - else if (assigned_from != null) ret = new PsiElementResolveResult(assigned_from); + else if (assigned_from != null) ret = new QualifiedResolveResultImpl(assigned_from, last_qualifier); } else if (ret == null && elt instanceof PyElement) { // remember this result, but a further reference may be the next resolve result - ret = target; + ret = new QualifiedResolveResultImpl(target.getElement(), target.isValidResult(), last_qualifier); } } // all resolve results checked, reassignment not detected, nothing more to do break; } - if (ret == null) { - ret = new ResolveResult() { - public PsiElement getElement() { - return null; - } - - public boolean isValidResult() { - return false; - } - }; - } + if (ret == null) ret = EMPTY_RESULT; return ret; } @@ -291,4 +286,44 @@ public class PyReferenceExpressionImpl extends PyElementImpl implements PyRefere return null; } + + private static class QualifiedResolveResultImpl extends PsiElementResolveResult implements QualifiedResolveResult { + // a trivial implementation + private PyExpression myLastQualifier; + + QualifiedResolveResultImpl(@NotNull PsiElement element, PyExpression lastQualifier) { + super(element); + myLastQualifier = lastQualifier; + } + + public QualifiedResolveResultImpl(@NotNull PsiElement element, boolean validResult, PyExpression lastQualifier) { + super(element, validResult); + myLastQualifier = lastQualifier; + } + + public PyExpression getLastQualifier() { + return myLastQualifier; + } + } + + private static class QualifiedResolveResultEmpty implements QualifiedResolveResult { + // a trivial implementation + + public QualifiedResolveResultEmpty() { + } + + public PyExpression getLastQualifier() { + return null; + } + + public PsiElement getElement() { + return null; + } + + public boolean isValidResult() { + return false; + } + } + } + diff --git a/python/testData/paramInfo/BoundMethodReassigned.py b/python/testData/paramInfo/BoundMethodReassigned.py new file mode 100644 index 000000000000..73d4069a1c81 --- /dev/null +++ b/python/testData/paramInfo/BoundMethodReassigned.py @@ -0,0 +1,11 @@ +class A(object): + def foo(self, a, b): + pass + + moo = foo + +ff = A().moo + +f = ff + +f(1, 2) diff --git a/python/testData/paramInfo/BoundMethodSimple.py b/python/testData/paramInfo/BoundMethodSimple.py new file mode 100644 index 000000000000..2a899546a1a6 --- /dev/null +++ b/python/testData/paramInfo/BoundMethodSimple.py @@ -0,0 +1,7 @@ +class A(object): + def foo(self, a, b): + pass + +f = A().foo + +f(1, 2) diff --git a/python/testData/paramInfo/BoundMethodStatic.py b/python/testData/paramInfo/BoundMethodStatic.py new file mode 100644 index 000000000000..c863abb06378 --- /dev/null +++ b/python/testData/paramInfo/BoundMethodStatic.py @@ -0,0 +1,8 @@ +class A(object): + @staticmethod + def foo(a, b): + pass + +f = A().foo + +f(1, 2) diff --git a/python/testSrc/com/jetbrains/python/PyParameterInfoTest.java b/python/testSrc/com/jetbrains/python/PyParameterInfoTest.java index a052d4c75a98..9badb041754b 100644 --- a/python/testSrc/com/jetbrains/python/PyParameterInfoTest.java +++ b/python/testSrc/com/jetbrains/python/PyParameterInfoTest.java @@ -271,6 +271,33 @@ public class PyParameterInfoTest extends LightMarkedTestCase { feignCtrlP(marks.get("").getTextOffset()).check("self,one", new String[]{"one"}, new String[]{"self,"}); } + + public void testBoundMethodSimple() throws Exception { + Map marks = loadTest(); + assertEquals("Test data sanity", marks.size(), 2); + + feignCtrlP(marks.get("").getTextOffset()).check("self,a,b", new String[]{"a,"}, new String[]{"self,"}); + feignCtrlP(marks.get("").getTextOffset()).check("self,a,b", new String[]{"b"}, new String[]{"self,"}); + } + + public void testBoundMethodReassigned() throws Exception { + Map marks = loadTest(); + assertEquals("Test data sanity", marks.size(), 2); + + feignCtrlP(marks.get("").getTextOffset()).check("self,a,b", new String[]{"a,"}, new String[]{"self,"}); + feignCtrlP(marks.get("").getTextOffset()).check("self,a,b", new String[]{"b"}, new String[]{"self,"}); + } + + public void testBoundMethodStatic() throws Exception { + Map marks = loadTest(); + assertEquals("Test data sanity", marks.size(), 2); + + feignCtrlP(marks.get("").getTextOffset()).check("a,b", new String[]{"a,"}); + feignCtrlP(marks.get("").getTextOffset()).check("a,b", new String[]{"b"}); + } + + + // TODO: add method tests with decorators when a mock SDK is available /** @@ -433,7 +460,7 @@ public class PyParameterInfoTest extends LightMarkedTestCase { disabled_set.addAll(Arrays.asList(disabled)); for (int i=0; i < myTexts.length; i += 1) { if (myFlags[i].contains(Flag.DISABLE) && !disabled_set.contains(myTexts[i])) { - wrongs.append("Highlighted unexpected '").append(myTexts[i]).append("'. "); + wrongs.append("Highlighted a disabled '").append(myTexts[i]).append("'. "); } } for (int i=0; i < myTexts.length; i += 1) {