From 51d3a547c82c2809e6d3c365de15ef69fdcd98bb Mon Sep 17 00:00:00 2001 From: Andrey Vlasovskikh Date: Fri, 13 Sep 2013 19:21:15 +0400 Subject: [PATCH] Use parameters provided by function type for method signature checks in inspections --- .../jetbrains/python/psi/PyParameterList.java | 8 -- .../PyInitNewSignatureInspection.java | 12 ++- .../PyMethodOverridingInspection.java | 3 +- .../PyPropertyDefinitionInspection.java | 14 ++-- .../src/com/jetbrains/python/psi/PyUtil.java | 73 +++++++++++++++++++ .../python/psi/impl/PyParameterListImpl.java | 60 --------------- .../InstanceCheck.py | 13 ++++ .../PyMethodOverridingInspectionTest.java | 5 ++ 8 files changed, 104 insertions(+), 84 deletions(-) create mode 100644 python/testData/inspections/PyMethodOverridingInspection/InstanceCheck.py diff --git a/python/psi-api/src/com/jetbrains/python/psi/PyParameterList.java b/python/psi-api/src/com/jetbrains/python/psi/PyParameterList.java index 0207bb357d18..04e6c1f72219 100644 --- a/python/psi-api/src/com/jetbrains/python/psi/PyParameterList.java +++ b/python/psi-api/src/com/jetbrains/python/psi/PyParameterList.java @@ -39,14 +39,6 @@ public interface PyParameterList extends PyElement, StubBasedPsiElement myStringClasses; - private PyParameterList myOneParamList; - private PyParameterList myTwoParamList; // arglist with two args, 'self' and 'value' + private PyFunction myOneParamFunction; + private PyFunction myTwoParamFunction; // arglist with two args, 'self' and 'value' public Visitor(final ProblemsHolder holder, LocalInspectionToolSession session) { super(holder, session); @@ -83,9 +83,9 @@ public class PyPropertyDefinitionInspection extends PyInspection { PyClass object_class = builtins.getClass("object"); if (object_class != null) { final PyFunction method_repr = object_class.findMethodByName("__repr__", false); - if (method_repr != null) myOneParamList = method_repr.getParameterList(); + if (method_repr != null) myOneParamFunction = method_repr; final PyFunction method_delattr = object_class.findMethodByName("__delattr__", false); - if (method_delattr != null) myTwoParamList = method_delattr.getParameterList(); + if (method_delattr != null) myTwoParamFunction = method_delattr; } } @@ -235,8 +235,7 @@ public class PyPropertyDefinitionInspection extends PyInspection { if (callable != null) { // signature: at least two params, more optionals ok; first arg 'self' final PyParameterList param_list = callable.getParameterList(); - final PyParameterList two_parameters_list = myTwoParamList; - if (two_parameters_list != null && !param_list.isCompatibleTo(two_parameters_list)) { + if (myTwoParamFunction != null && !PyUtil.isSignatureCompatibleTo(callable, myTwoParamFunction, myTypeEvalContext)) { registerProblem(being_checked, PyBundle.message("INSP.setter.signature.advice")); } checkForSelf(param_list); @@ -254,8 +253,7 @@ public class PyPropertyDefinitionInspection extends PyInspection { private void checkOneParameter(Callable callable, PsiElement being_checked, boolean is_getter) { final PyParameterList param_list = callable.getParameterList(); - final PyParameterList one_parameter_list = myOneParamList; - if (one_parameter_list != null && ! param_list.isCompatibleTo(one_parameter_list)) { + if (myOneParamFunction != null && !PyUtil.isSignatureCompatibleTo(callable, myOneParamFunction, myTypeEvalContext)) { if (is_getter) registerProblem(being_checked, PyBundle.message("INSP.getter.signature.advice")); else registerProblem(being_checked, PyBundle.message("INSP.deleter.signature.advice")); } diff --git a/python/src/com/jetbrains/python/psi/PyUtil.java b/python/src/com/jetbrains/python/psi/PyUtil.java index 92b2d02c68d9..816ce090f698 100644 --- a/python/src/com/jetbrains/python/psi/PyUtil.java +++ b/python/src/com/jetbrains/python/psi/PyUtil.java @@ -1298,4 +1298,77 @@ public class PyUtil { } return Arrays.asList(callable.getParameterList().getParameters()); } + + public static boolean isSignatureCompatibleTo(@NotNull Callable callable, @NotNull Callable otherCallable, + @NotNull TypeEvalContext context) { + final List parameters = getParameters(callable, context); + final List otherParameters = getParameters(otherCallable, context); + final int optionalCount = optionalParametersCount(parameters); + final int otherOptionalCount = optionalParametersCount(otherParameters); + final int requiredCount = requiredParametersCount(callable, parameters); + final int otherRequiredCount = requiredParametersCount(otherCallable, otherParameters); + if (hasPositionalContainer(otherParameters) || hasKeywordContainer(otherParameters)) { + if (otherParameters.size() == specialParametersCount(otherCallable, otherParameters)) { + return true; + } + } + if (hasPositionalContainer(parameters) || hasKeywordContainer(parameters)) { + return requiredCount <= otherRequiredCount; + } + return requiredCount <= otherRequiredCount && parameters.size() >= otherParameters.size() && optionalCount >= otherOptionalCount; + } + + private static int optionalParametersCount(@NotNull List parameters) { + int n = 0; + for (PyParameter parameter : parameters) { + if (parameter.getDefaultValue() != null) { + n++; + } + } + return n; + } + + private static int requiredParametersCount(@NotNull Callable callable, @NotNull List parameters) { + return parameters.size() - optionalParametersCount(parameters) - specialParametersCount(callable, parameters); + } + + private static int specialParametersCount(@NotNull Callable callable, @NotNull List parameters) { + int n = 0; + if (hasPositionalContainer(parameters)) { + n++; + } + if (hasKeywordContainer(parameters)) { + n++; + } + if (callable.asMethod() != null) { + n++; + } + else { + if (parameters.size() > 0) { + final PyParameter first = parameters.get(0); + if (PyNames.CANONICAL_SELF.equals(first.getName())) { + n++; + } + } + } + return n; + } + + private static boolean hasPositionalContainer(@NotNull List parameters) { + for (PyParameter parameter : parameters) { + if (parameter instanceof PyNamedParameter && ((PyNamedParameter)parameter).isPositionalContainer()) { + return true; + } + } + return false; + } + + private static boolean hasKeywordContainer(@NotNull List parameters) { + for (PyParameter parameter : parameters) { + if (parameter instanceof PyNamedParameter && ((PyNamedParameter)parameter).isKeywordContainer()) { + return true; + } + } + return false; + } } diff --git a/python/src/com/jetbrains/python/psi/impl/PyParameterListImpl.java b/python/src/com/jetbrains/python/psi/impl/PyParameterListImpl.java index 0038dfd9d256..52cfe66f6da2 100644 --- a/python/src/com/jetbrains/python/psi/impl/PyParameterListImpl.java +++ b/python/src/com/jetbrains/python/psi/impl/PyParameterListImpl.java @@ -5,7 +5,6 @@ import com.intellij.openapi.util.Ref; import com.intellij.psi.PsiElement; import com.intellij.psi.stubs.IStubElementType; import com.jetbrains.python.PyElementTypes; -import com.jetbrains.python.PyNames; import com.jetbrains.python.PyTokenTypes; import com.jetbrains.python.PythonDialectsTokenSetProvider; import com.jetbrains.python.psi.*; @@ -85,65 +84,6 @@ public class PyParameterListImpl extends PyBaseElementImpl return false; } - public boolean isCompatibleTo(@NotNull PyParameterList other) { - PyParameter[] params = getParameters(); - final PyParameter[] otherParams = other.getParameters(); - final int optionalCount = optionalParametersCount(params); - final int otherOptionalCount = optionalParametersCount(otherParams); - final int requiredCount = requiredParametersCount(this); - final int otherRequiredCount = requiredParametersCount(other); - if (other.hasPositionalContainer() || other.hasKeywordContainer()) { - if (otherParams.length == specialParametersCount(other)) { - return true; - } - } - if (hasPositionalContainer() || hasKeywordContainer()) { - return requiredCount <= otherRequiredCount; - } - return requiredCount <= otherRequiredCount && params.length >= otherParams.length && optionalCount >= otherOptionalCount; - } - - private static int optionalParametersCount(@NotNull PyParameter[] parameters) { - int n = 0; - for (PyParameter parameter : parameters) { - if (parameter.getDefaultValue() != null) { - n++; - } - } - return n; - } - - private static int specialParametersCount(@NotNull PyParameterList parameterList) { - int n = 0; - if (parameterList.hasPositionalContainer()) { - n++; - } - if (parameterList.hasKeywordContainer()) { - n++; - } - final PyFunction function = parameterList.getContainingFunction(); - if (function != null) { - if (function.asMethod() != null) { - n++; - } - } - else { - final PyParameter[] parameters = parameterList.getParameters(); - if (parameters.length > 0) { - final PyParameter first = parameters[0]; - if (PyNames.CANONICAL_SELF.equals(first.getName())) { - n++; - } - } - } - return n; - } - - private static int requiredParametersCount(@NotNull PyParameterList parameterList) { - final PyParameter[] parameters = parameterList.getParameters(); - return parameters.length - optionalParametersCount(parameters) - specialParametersCount(parameterList); - } - @Override @Nullable public PyNamedParameter findParameterByName(@NotNull final String name) { diff --git a/python/testData/inspections/PyMethodOverridingInspection/InstanceCheck.py b/python/testData/inspections/PyMethodOverridingInspection/InstanceCheck.py new file mode 100644 index 000000000000..5863d5a0bd56 --- /dev/null +++ b/python/testData/inspections/PyMethodOverridingInspection/InstanceCheck.py @@ -0,0 +1,13 @@ +class MyType1(type): + def __instancecheck__(cls, instance): + return True + + +class MyType2(type): + def __instancecheck__(cls): + return True + + +class MyType3(type): + def __instancecheck__(cls, foo, bar): + return True diff --git a/python/testSrc/com/jetbrains/python/PyMethodOverridingInspectionTest.java b/python/testSrc/com/jetbrains/python/PyMethodOverridingInspectionTest.java index 4d140b1ed40f..c03a8177bd73 100644 --- a/python/testSrc/com/jetbrains/python/PyMethodOverridingInspectionTest.java +++ b/python/testSrc/com/jetbrains/python/PyMethodOverridingInspectionTest.java @@ -64,6 +64,11 @@ public class PyMethodOverridingInspectionTest extends PyTestCase { doTest(); } + // PY-10229 + public void testInstanceCheck() { + doTest(); + } + private void doTest() { myFixture.configureByFile(TEST_DIRECTORY + getTestName(false) + ".py"); myFixture.enableInspections(PyMethodOverridingInspection.class);