From 8eda017f0fabe8e6fa46f9f00cf78580911c109a Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Mon, 27 Nov 2017 19:19:54 +0300 Subject: [PATCH] PyBinaryExpression#getReceiver() is aware of chained comparisons Otherwise, it might lead to some subtle problems when this method is used for unification of generic types. Also, PyPrefixExpression now extends PyCallSiteExpression since technically it is and should have been marked this way long ago. --- .../python/psi/PyBinaryExpression.java | 23 ++---------- .../python/psi/PyCallSiteExpression.java | 10 ++++++ .../python/psi/PyPrefixExpression.java | 2 +- .../psi/impl/PyBinaryExpressionImpl.java | 35 ++++++++++++++++--- .../psi/impl/PyPrefixExpressionImpl.java | 13 +++++++ .../impl/references/PyOperatorReference.java | 28 ++------------- .../ChainedComparisonsGenericMatching.py | 14 ++++++++ .../Py3TypeCheckerInspectionTest.java | 7 ++++ 8 files changed, 81 insertions(+), 51 deletions(-) create mode 100644 python/testData/inspections/PyTypeCheckerInspection/ChainedComparisonsGenericMatching.py diff --git a/python/psi-api/src/com/jetbrains/python/psi/PyBinaryExpression.java b/python/psi-api/src/com/jetbrains/python/psi/PyBinaryExpression.java index 6b3ba29419f0..b783c683ed25 100644 --- a/python/psi-api/src/com/jetbrains/python/psi/PyBinaryExpression.java +++ b/python/psi-api/src/com/jetbrains/python/psi/PyBinaryExpression.java @@ -16,34 +16,13 @@ package com.jetbrains.python.psi; import com.intellij.psi.PsiElement; -import com.jetbrains.python.PyNames; -import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.Collections; -import java.util.List; - /** * @author yole */ public interface PyBinaryExpression extends PyQualifiedExpression, PyCallSiteExpression, PyReferenceOwner { - @Nullable - @Override - default PyExpression getReceiver(@Nullable PyCallable resolvedCallee) { - return isRightOperator(resolvedCallee) ? getRightExpression() : getLeftExpression(); - } - - @NotNull - @Override - default List getArguments(@Nullable PyCallable resolvedCallee) { - return Collections.singletonList(isRightOperator(resolvedCallee) ? getLeftExpression() : getRightExpression()); - } - - default boolean isRightOperator(@Nullable PyCallable resolvedCallee) { - return resolvedCallee != null && PyNames.isRightOperatorName(getReferencedName(), resolvedCallee.getName()); - } - PyExpression getLeftExpression(); @Nullable PyExpression getRightExpression(); @@ -57,4 +36,6 @@ public interface PyBinaryExpression extends PyQualifiedExpression, PyCallSiteExp PyExpression getOppositeExpression(PyExpression expression) throws IllegalArgumentException; + + boolean isRightOperator(@Nullable PyCallable resolvedCallee); } diff --git a/python/psi-api/src/com/jetbrains/python/psi/PyCallSiteExpression.java b/python/psi-api/src/com/jetbrains/python/psi/PyCallSiteExpression.java index 526f17b79d36..2703e24cb680 100644 --- a/python/psi-api/src/com/jetbrains/python/psi/PyCallSiteExpression.java +++ b/python/psi-api/src/com/jetbrains/python/psi/PyCallSiteExpression.java @@ -27,6 +27,16 @@ import java.util.List; */ public interface PyCallSiteExpression extends PyExpression { + /** + * Returns an expression that is treated as a receiver for this explicit or implicit (read, operator) call. + *

+ * For most operator expressions it returns the result of {@code getOperator()} since it naturally represents + * the object on which a special magic method is called. However for binary expressions that additionally + * can be reversible such as {@code __add__} and {@code __radd__} it also takes into account name of the + * actual callee method and chained comparisons order if any. + * + * @param resolvedCallee optional callee corresponding to the call. Without it the receiver is deduced purely syntactically. + */ @Nullable PyExpression getReceiver(@Nullable PyCallable resolvedCallee); diff --git a/python/psi-api/src/com/jetbrains/python/psi/PyPrefixExpression.java b/python/psi-api/src/com/jetbrains/python/psi/PyPrefixExpression.java index 45d426f01a4d..24a751925177 100644 --- a/python/psi-api/src/com/jetbrains/python/psi/PyPrefixExpression.java +++ b/python/psi-api/src/com/jetbrains/python/psi/PyPrefixExpression.java @@ -20,7 +20,7 @@ import org.jetbrains.annotations.Nullable; /** * @author yole */ -public interface PyPrefixExpression extends PyQualifiedExpression, PyReferenceOwner { +public interface PyPrefixExpression extends PyQualifiedExpression, PyReferenceOwner, PyCallSiteExpression { @Nullable PyExpression getOperand(); PyElementType getOperator(); diff --git a/python/src/com/jetbrains/python/psi/impl/PyBinaryExpressionImpl.java b/python/src/com/jetbrains/python/psi/impl/PyBinaryExpressionImpl.java index f4d0b036f0a3..7c30fbd356fb 100644 --- a/python/src/com/jetbrains/python/psi/impl/PyBinaryExpressionImpl.java +++ b/python/src/com/jetbrains/python/psi/impl/PyBinaryExpressionImpl.java @@ -33,10 +33,9 @@ import com.jetbrains.python.psi.types.*; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; + +import static com.jetbrains.python.psi.PyUtil.as; /** * @author yole @@ -215,6 +214,34 @@ public class PyBinaryExpressionImpl extends PyElementImpl implements PyBinaryExp return op != null ? op.getNode() : null; } + @Nullable + @Override + public PyExpression getReceiver(@Nullable PyCallable resolvedCallee) { + return isRightOperator(resolvedCallee) ? getRightExpression() : getChainedComparisonAwareLeftExpression(); + } + + @NotNull + @Override + public List getArguments(@Nullable PyCallable resolvedCallee) { + return Collections.singletonList(isRightOperator(resolvedCallee) ? getChainedComparisonAwareLeftExpression() : getRightExpression()); + } + + public boolean isRightOperator(@Nullable PyCallable resolvedCallee) { + return resolvedCallee != null && PyNames.isRightOperatorName(getReferencedName(), resolvedCallee.getName()); + } + + @Nullable + private PyExpression getChainedComparisonAwareLeftExpression() { + final PyExpression leftOperand = getLeftExpression(); + if (PyTokenTypes.COMPARISON_OPERATIONS.contains(getOperator())) { + final PyBinaryExpression leftBinaryExpr = as(leftOperand, PyBinaryExpression.class); + if (leftBinaryExpr != null && PyTokenTypes.COMPARISON_OPERATIONS.contains(leftBinaryExpr.getOperator())) { + return leftBinaryExpr.getRightExpression(); + } + } + return leftOperand; + } + private static boolean operandIsKnown(@Nullable PyExpression operand, @NotNull TypeEvalContext context) { if (operand == null) return false; diff --git a/python/src/com/jetbrains/python/psi/impl/PyPrefixExpressionImpl.java b/python/src/com/jetbrains/python/psi/impl/PyPrefixExpressionImpl.java index 6f2df1f079c5..79047f1b4196 100644 --- a/python/src/com/jetbrains/python/psi/impl/PyPrefixExpressionImpl.java +++ b/python/src/com/jetbrains/python/psi/impl/PyPrefixExpressionImpl.java @@ -20,6 +20,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.ArrayList; +import java.util.Collections; import java.util.List; /** @@ -126,6 +127,18 @@ public class PyPrefixExpressionImpl extends PyElementImpl implements PyPrefixExp return op != null ? op.getNode() : null; } + @Nullable + @Override + public PyExpression getReceiver(@Nullable PyCallable resolvedCallee) { + return getOperand(); + } + + @NotNull + @Override + public List getArguments(@Nullable PyCallable resolvedCallee) { + return Collections.emptyList(); + } + @Nullable private static Ref getGeneratorReturnType(@Nullable PyType type) { if (type instanceof PyClassLikeType && type instanceof PyCollectionType) { diff --git a/python/src/com/jetbrains/python/psi/impl/references/PyOperatorReference.java b/python/src/com/jetbrains/python/psi/impl/references/PyOperatorReference.java index dd38e6864541..438f0d95eeaf 100644 --- a/python/src/com/jetbrains/python/psi/impl/references/PyOperatorReference.java +++ b/python/src/com/jetbrains/python/psi/impl/references/PyOperatorReference.java @@ -18,7 +18,6 @@ package com.jetbrains.python.psi.impl.references; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.PsiElement; import com.jetbrains.python.PyNames; -import com.jetbrains.python.PyTokenTypes; import com.jetbrains.python.psi.*; import com.jetbrains.python.psi.resolve.PyResolveContext; import com.jetbrains.python.psi.resolve.RatedResolveResult; @@ -33,8 +32,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import static com.jetbrains.python.psi.PyUtil.as; - /** * @author vlan */ @@ -92,31 +89,12 @@ public class PyOperatorReference extends PyReferenceImpl { @Nullable public PyExpression getReceiver() { - if (myElement instanceof PyBinaryExpression) { - return getBinaryOperatorReceiver((PyBinaryExpression)myElement); - } - else if (myElement instanceof PySubscriptionExpression) { - return ((PySubscriptionExpression)myElement).getOperand(); - } - else if (myElement instanceof PyPrefixExpression) { - return ((PyPrefixExpression)myElement).getOperand(); + if (myElement instanceof PyCallSiteExpression) { + return ((PyCallSiteExpression)myElement).getReceiver(null); } return null; } - @Nullable - private static PyExpression getBinaryOperatorReceiver(PyBinaryExpression expr) { - final PyExpression leftOperand = expr.getLeftExpression(); - // Chained comparisons - if (PyTokenTypes.COMPARISON_OPERATIONS.contains(expr.getOperator())) { - final PyBinaryExpression leftBinaryExpr = as(leftOperand, PyBinaryExpression.class); - if (leftBinaryExpr != null && PyTokenTypes.COMPARISON_OPERATIONS.contains(leftBinaryExpr.getOperator())) { - return leftBinaryExpr.getRightExpression(); - } - } - return leftOperand; - } - @NotNull private List resolveLeftAndRightOperators(@NotNull PyBinaryExpression expr, @Nullable String name) { final List result = new ArrayList<>(); @@ -125,7 +103,7 @@ public class PyOperatorReference extends PyReferenceImpl { typeEvalContext.trace("Trying to resolve left operator"); typeEvalContext.traceIndent(); try { - result.addAll(resolveMember(getBinaryOperatorReceiver(expr), name)); + result.addAll(resolveMember(expr.getReceiver(null), name)); } finally { typeEvalContext.traceUnindent(); diff --git a/python/testData/inspections/PyTypeCheckerInspection/ChainedComparisonsGenericMatching.py b/python/testData/inspections/PyTypeCheckerInspection/ChainedComparisonsGenericMatching.py new file mode 100644 index 000000000000..3446fba735d7 --- /dev/null +++ b/python/testData/inspections/PyTypeCheckerInspection/ChainedComparisonsGenericMatching.py @@ -0,0 +1,14 @@ +from typing import Generic, TypeVar + +T = TypeVar('T') + + +class MyClass(Generic[T]): + def __init__(self, x: T): + pass + + def __lt__(self, other: 'MyClass[T]'): + pass + + +x = MyClass(1) < MyClass(2) < MyClass('foo') diff --git a/python/testSrc/com/jetbrains/python/inspections/Py3TypeCheckerInspectionTest.java b/python/testSrc/com/jetbrains/python/inspections/Py3TypeCheckerInspectionTest.java index 76c1973e73f8..ec589b6a8587 100644 --- a/python/testSrc/com/jetbrains/python/inspections/Py3TypeCheckerInspectionTest.java +++ b/python/testSrc/com/jetbrains/python/inspections/Py3TypeCheckerInspectionTest.java @@ -273,4 +273,11 @@ public class Py3TypeCheckerInspectionTest extends PyInspectionTestCase { public void testMatchingOpenFunctionCallTypesPy3() { doMultiFileTest(); } + + public void testChainedComparisonsGenericMatching() { + runWithLanguageLevel(LanguageLevel.PYTHON36, () -> { + myFixture.copyDirectoryToProject("typing/typing.py", TEST_DIRECTORY); + doTest(); + }); + } }