From f3103c6f1f3be8b4e25d24df85306d9ad6a76bdb Mon Sep 17 00:00:00 2001 From: Dmitry Cheryasov Date: Thu, 8 Jan 2009 13:27:28 +0300 Subject: [PATCH] Adds proper inspection of decorator argument lists. Changes parsing slightly, updates call analysis. Fixes PY-97. --- .../com/jetbrains/python/PyBundle.properties | 5 + .../python/PyParameterInfoHandler.java | 7 +- .../inspections/PyArgumentListInspection.java | 97 ++++++++++++------- .../inspections/PyInspectionVisitor.java | 4 + .../python/parsing/FunctionParsing.java | 3 + .../python/psi/PyCallExpression.java | 17 +++- .../python/psi/PyElementVisitor.java | 4 + .../python/psi/impl/PyArgumentListImpl.java | 9 +- .../psi/impl/PyCallExpressionHelper.java | 9 +- .../python/psi/impl/PyDecoratorImpl.java | 5 +- .../python/psi/impl/PyDecoratorListImpl.java | 6 ++ .../PyArgumentListInspection/expected.xml | 40 ++++++++ python/testData/psi/DecoratedFunction.txt | 2 + .../jetbrains/python/PyResolveCalleeTest.java | 4 +- 14 files changed, 157 insertions(+), 55 deletions(-) diff --git a/python/src/com/jetbrains/python/PyBundle.properties b/python/src/com/jetbrains/python/PyBundle.properties index dab08c415620..e3da7f9cebef 100644 --- a/python/src/com/jetbrains/python/PyBundle.properties +++ b/python/src/com/jetbrains/python/PyBundle.properties @@ -28,6 +28,7 @@ INSP.duplicate.doublestar.arg=Duplicate **arg INSP.cannot.appear.past.keyword.arg=Cannot appear past keyword arguments INSP.unexpected.arg=Unexpected argument INSP.parameter.$0.unfilled=Parameter ''{0}'' unfilled +INSP.func.$0.lacks.first.arg=Function ''{0}'' lacks a positional argument # PyMethodParametersInspection INSP.NAME.problematic.first.parameter=Methods having troubles with first parameter @@ -46,6 +47,10 @@ INSP.module.$0.not.found=Module ''{0}'' not found INSP.unresolved.ref.$0=Unresolved reference ''{0}'' INSP.unresolved.ref.$0.for.class.$1=Unresolved attribute reference ''{0}'' for class ''{1}'' +# ReturnValueFromInitInspection +INSP.NAME.init.return=__init__ method that returns a value +INSP.cant.return.value.from.init=Cannot return a value from __init__ + ### Annotators ### ANN.deleting.none=deleting None ANN.assign.to.none=assignment to None diff --git a/python/src/com/jetbrains/python/PyParameterInfoHandler.java b/python/src/com/jetbrains/python/PyParameterInfoHandler.java index 7bede77fe2b5..ccfa12e0df47 100644 --- a/python/src/com/jetbrains/python/PyParameterInfoHandler.java +++ b/python/src/com/jetbrains/python/PyParameterInfoHandler.java @@ -3,7 +3,6 @@ package com.jetbrains.python; import com.intellij.codeInsight.lookup.LookupElement; import com.intellij.lang.parameterInfo.*; import com.jetbrains.python.psi.*; -import static com.jetbrains.python.psi.PyCallExpression.Flag; import static com.jetbrains.python.psi.PyCallExpression.PyMarkedFunction; import org.jetbrains.annotations.NotNull; @@ -103,9 +102,9 @@ public class PyParameterInfoHandler implements ParameterInfoHandler> arg_entry : result.getArgumentFlags().entrySet()) { - EnumSet flags = arg_entry.getValue(); - if (!flags.isEmpty()) { // something's wrong - PyExpression arg = arg_entry.getKey(); - if (flags.contains(PyArgumentList.ArgFlag.IS_DUP)) { - registerProblem(arg, PyBundle.message("INSP.duplicate.argument")); - } - if (flags.contains(PyArgumentList.ArgFlag.IS_DUP_KWD)) { - registerProblem(arg, PyBundle.message("INSP.duplicate.doublestar.arg")); - } - if (flags.contains(PyArgumentList.ArgFlag.IS_DUP_TUPLE)) { - registerProblem(arg, PyBundle.message("INSP.duplicate.star.arg")); - } - if (flags.contains(PyArgumentList.ArgFlag.IS_POS_PAST_KWD)) { - registerProblem(arg, PyBundle.message("INSP.cannot.appear.past.keyword.arg")); - } - if (flags.contains(PyArgumentList.ArgFlag.IS_UNMAPPED)) { - registerProblem(arg, PyBundle.message("INSP.unexpected.arg")); + } + + @Override + public void visitPyDecoratorList(final PyDecoratorList node) { + PyDecorator[] decos = node.getDecorators(); + for (PyDecorator deco : decos) { + if (! deco.hasArgumentList()) { + // empty arglist; deco function must have a non-kwarg first arg + PyCallExpression.PyMarkedFunction mkfunc = deco.resolveCallee(); + if (mkfunc != null) { + PyFunction decofunc = mkfunc.getFunction(); + int first_param_offset = mkfunc.getImplicitOffset(); + PyParameter[] params = decofunc.getParameterList().getParameters(); + if (params.length < first_param_offset || params[first_param_offset-1].isKeywordContainer()) { + // no paramaters left to pass function implicitly, or wrong param type + registerProblem(deco, PyBundle.message("INSP.func.$0.lacks.first.arg", decofunc.getName())); + } + else { + // possible unfilled params + for (int i=first_param_offset; i < params.length; i += 1) { + PyParameter par = params[i]; + if (! par.isKeywordContainer() && ! par.isPositionalContainer() && (par.getDefaultValue() == null)) { + registerProblem(deco, PyBundle.message("INSP.parameter.$0.unfilled", par.getName())); + } + } + } } } + // else: this case is handled by arglist visitor } - // show unfilled params - ASTNode our_node = node.getNode(); - if (our_node != null) { - ASTNode close_paren = our_node.findChildByType(PyTokenTypes.RPAR); - if (close_paren != null) { - for (PyParameter param : result.getUnmappedParams()) { - registerProblem(close_paren.getPsi(), PyBundle.message("INSP.parameter.$0.unfilled", param.getName())); - } + } + + } + + public static void inspectPyArgumentList(PyArgumentList node, ProblemsHolder holder) { + PyArgumentList.AnalysisResult result = node.analyzeCall(); + for (Map.Entry> arg_entry : result.getArgumentFlags().entrySet()) { + EnumSet flags = arg_entry.getValue(); + if (!flags.isEmpty()) { // something's wrong + PyExpression arg = arg_entry.getKey(); + if (flags.contains(PyArgumentList.ArgFlag.IS_DUP)) { + holder.registerProblem(arg, PyBundle.message("INSP.duplicate.argument")); + } + if (flags.contains(PyArgumentList.ArgFlag.IS_DUP_KWD)) { + holder.registerProblem(arg, PyBundle.message("INSP.duplicate.doublestar.arg")); + } + if (flags.contains(PyArgumentList.ArgFlag.IS_DUP_TUPLE)) { + holder.registerProblem(arg, PyBundle.message("INSP.duplicate.star.arg")); + } + if (flags.contains(PyArgumentList.ArgFlag.IS_POS_PAST_KWD)) { + holder.registerProblem(arg, PyBundle.message("INSP.cannot.appear.past.keyword.arg")); + } + if (flags.contains(PyArgumentList.ArgFlag.IS_UNMAPPED)) { + holder.registerProblem(arg, PyBundle.message("INSP.unexpected.arg")); + } + } + } + // show unfilled params + ASTNode our_node = node.getNode(); + if (our_node != null) { + ASTNode close_paren = our_node.findChildByType(PyTokenTypes.RPAR); + if (close_paren != null) { + for (PyParameter param : result.getUnmappedParams()) { + holder.registerProblem(close_paren.getPsi(), PyBundle.message("INSP.parameter.$0.unfilled", param.getName())); } } } diff --git a/python/src/com/jetbrains/python/inspections/PyInspectionVisitor.java b/python/src/com/jetbrains/python/inspections/PyInspectionVisitor.java index 425b0f887aa5..e1ecca74660d 100644 --- a/python/src/com/jetbrains/python/inspections/PyInspectionVisitor.java +++ b/python/src/com/jetbrains/python/inspections/PyInspectionVisitor.java @@ -19,6 +19,10 @@ public class PyInspectionVisitor extends PyElementVisitor { myHolder = holder; } + public ProblemsHolder getHolder() { + return myHolder; + } + protected final void registerProblem(final PsiElement element, final String message){ if (element == null || element.getTextLength() == 0){ diff --git a/python/src/com/jetbrains/python/parsing/FunctionParsing.java b/python/src/com/jetbrains/python/parsing/FunctionParsing.java index c68fea1a87bb..e1749d1fad21 100644 --- a/python/src/com/jetbrains/python/parsing/FunctionParsing.java +++ b/python/src/com/jetbrains/python/parsing/FunctionParsing.java @@ -65,6 +65,9 @@ public class FunctionParsing extends Parsing { if (myBuilder.getTokenType() == PyTokenTypes.LPAR) { getExpressionParser().parseArgumentList(myBuilder); } + else { // empty arglist node, so we always have it + myBuilder.mark().done(PyElementTypes.ARGUMENT_LIST); + } checkMatches(PyTokenTypes.STATEMENT_BREAK, message("PARSE.expected.statement.break")); decoratorMarker.done(PyElementTypes.DECORATOR_CALL); decorated = true; diff --git a/python/src/com/jetbrains/python/psi/PyCallExpression.java b/python/src/com/jetbrains/python/psi/PyCallExpression.java index 58e9f8399097..4456e3681352 100644 --- a/python/src/com/jetbrains/python/psi/PyCallExpression.java +++ b/python/src/com/jetbrains/python/psi/PyCallExpression.java @@ -50,10 +50,6 @@ public interface PyCallExpression extends PyExpression { enum Flag { - /** - * First arg of the call is implicit, drop first parameter. - */ - IMPLICIT_FIRST_ARG, /** * Called function is decorated with @classmethod, first param is the class. */ @@ -70,10 +66,12 @@ public interface PyCallExpression extends PyExpression { class PyMarkedFunction { PyFunction myFunction; EnumSet myFlags; + int myImplicitOffset; - public PyMarkedFunction(@NotNull PyFunction function, EnumSet flags) { + public PyMarkedFunction(@NotNull PyFunction function, EnumSet flags, int offset) { myFunction = function; myFlags = flags; + myImplicitOffset = offset; } public PyFunction getFunction() { @@ -84,6 +82,15 @@ public interface PyCallExpression extends PyExpression { return myFlags; } + /** + * @return number of implicitly passed positional parameters; 0 means no parameters are passed implicitly. + * Note that a *args is never markeg as passed implicitly. + * E.g. for a function like foo(a, b, *args) always holds getImplicitOffset() < 2. + */ + public int getImplicitOffset() { + return myImplicitOffset; + } + } } diff --git a/python/src/com/jetbrains/python/psi/PyElementVisitor.java b/python/src/com/jetbrains/python/psi/PyElementVisitor.java index 4798a4f34e1d..79f8e7b42b7c 100644 --- a/python/src/com/jetbrains/python/psi/PyElementVisitor.java +++ b/python/src/com/jetbrains/python/psi/PyElementVisitor.java @@ -42,6 +42,10 @@ public class PyElementVisitor extends PsiElementVisitor { visitPyExpression(node); } + public void visitPyDecoratorList(final PyDecoratorList node) { + visitElement(node); + } + public void visitPyGeneratorExpression(final PyGeneratorExpression node) { visitPyExpression(node); } diff --git a/python/src/com/jetbrains/python/psi/impl/PyArgumentListImpl.java b/python/src/com/jetbrains/python/psi/impl/PyArgumentListImpl.java index ca2a963fdff6..ecc6e46fb24c 100644 --- a/python/src/com/jetbrains/python/psi/impl/PyArgumentListImpl.java +++ b/python/src/com/jetbrains/python/psi/impl/PyArgumentListImpl.java @@ -258,7 +258,6 @@ public class PyArgumentListImpl extends PyElementImpl implements PyArgumentList ret.my_marked_func = resolved_callee; if (resolved_callee != null) { PyFunction func = resolved_callee.getFunction(); - boolean implicit_self = resolved_callee.getFlags().contains(PyCallExpression.Flag.IMPLICIT_FIRST_ARG); PyParameter[] params = func.getParameterList().getParameters(); // prepare args and slots List unmatched_args = new LinkedList(); @@ -298,11 +297,11 @@ public class PyArgumentListImpl extends PyElementImpl implements PyArgumentList } } } - // rule out 'self' + // rule out 'self' or other implicit params int param_index = 0; - if (implicit_self && (params.length > 0)) { - param_slots.remove(params[0].getName()); // the self param - param_index = 1; + for (int i=0; i < resolved_callee.getImplicitOffset() && i < params.length; i+=1) { + param_slots.remove(params[i].getName()); // the self param + param_index += 1; } boolean seen_tuple_arg = false; boolean seen_kwd_arg = false; diff --git a/python/src/com/jetbrains/python/psi/impl/PyCallExpressionHelper.java b/python/src/com/jetbrains/python/psi/impl/PyCallExpressionHelper.java index 91822de8a1f2..4de9c48e58ec 100644 --- a/python/src/com/jetbrains/python/psi/impl/PyCallExpressionHelper.java +++ b/python/src/com/jetbrains/python/psi/impl/PyCallExpressionHelper.java @@ -64,11 +64,12 @@ public class PyCallExpressionHelper { else resolved = cref.resolve(); if (resolved != null) { EnumSet flags = EnumSet.noneOf(PyCallExpression.Flag.class); + int implicit_offset = 0; //boolean is_inst = isByInstance(); - if (isByInstance(us)) flags.add(PyCallExpression.Flag.IMPLICIT_FIRST_ARG); + if (isByInstance(us)) implicit_offset += 1; if (resolved instanceof PyFunction) { PyFunction meth = (PyFunction)resolved; // constructor call? - if (PyNames.INIT.equals(meth.getName())) flags.add(PyCallExpression.Flag.IMPLICIT_FIRST_ARG); + if (PyNames.INIT.equals(meth.getName())) implicit_offset += 1; // look for closest decorator PyDecoratorList decolist = meth.getDecoratorList(); if (decolist != null) { @@ -80,7 +81,7 @@ public class PyCallExpressionHelper { if (deco.isBuiltin()) { if (PyNames.STATICMETHOD.equals(deconame)) { flags.add(PyCallExpression.Flag.STATICMETHOD); - flags.remove(PyCallExpression.Flag.IMPLICIT_FIRST_ARG); + if (implicit_offset > 0) implicit_offset -= 1; // might have marked it as implicit 'self' } else if (PyNames.CLASSMETHOD.equals(deconame)) { flags.add(PyCallExpression.Flag.CLASSMETHOD); @@ -91,7 +92,7 @@ public class PyCallExpressionHelper { } } if (!(resolved instanceof PyFunction)) return null; // omg, bogus __init__ - return new PyCallExpression.PyMarkedFunction((PyFunction) resolved, flags); + return new PyCallExpression.PyMarkedFunction((PyFunction)resolved, flags, implicit_offset); } } } diff --git a/python/src/com/jetbrains/python/psi/impl/PyDecoratorImpl.java b/python/src/com/jetbrains/python/psi/impl/PyDecoratorImpl.java index 45ed666c7a4a..49cb19f91a3d 100644 --- a/python/src/com/jetbrains/python/psi/impl/PyDecoratorImpl.java +++ b/python/src/com/jetbrains/python/psi/impl/PyDecoratorImpl.java @@ -72,7 +72,8 @@ public class PyDecoratorImpl extends PyPresentableElementImpl i } public boolean hasArgumentList() { - return getNode().findChildByType(PyElementTypes.ARGUMENT_LIST) != null; + ASTNode arglist_node = getNode().findChildByType(PyElementTypes.ARGUMENT_LIST); + return (arglist_node != null) && (arglist_node.findChildByType(PyTokenTypes.LPAR) != null); } public PyExpression getCallee() { @@ -100,7 +101,7 @@ public class PyDecoratorImpl extends PyPresentableElementImpl i PyMarkedFunction callee = PyCallExpressionHelper.resolveCallee(this); if (callee == null) return null; if (! hasArgumentList()) { - callee.getFlags().add(Flag.IMPLICIT_FIRST_ARG); // NOTE: assumes mutability + callee = new PyMarkedFunction(callee.getFunction(), callee.getFlags(), callee.getImplicitOffset() + 1); } return callee; } diff --git a/python/src/com/jetbrains/python/psi/impl/PyDecoratorListImpl.java b/python/src/com/jetbrains/python/psi/impl/PyDecoratorListImpl.java index 84b18ee13f23..3258afd804b9 100644 --- a/python/src/com/jetbrains/python/psi/impl/PyDecoratorListImpl.java +++ b/python/src/com/jetbrains/python/psi/impl/PyDecoratorListImpl.java @@ -5,6 +5,7 @@ import com.jetbrains.python.PyElementTypes; import com.jetbrains.python.psi.PyDecoratorList; import com.jetbrains.python.psi.stubs.PyDecoratorListStub; import com.jetbrains.python.psi.PyDecorator; +import com.jetbrains.python.psi.PyElementVisitor; import org.jetbrains.annotations.NotNull; /** @@ -18,6 +19,11 @@ public class PyDecoratorListImpl extends PyBaseElementImpl super(astNode); } + @Override + protected void acceptPyVisitor(PyElementVisitor pyVisitor) { + pyVisitor.visitPyDecoratorList(this); + } + public PyDecoratorListImpl(final PyDecoratorListStub stub) { super(stub, PyElementTypes.DECORATOR_LIST); } diff --git a/python/testData/inspections/PyArgumentListInspection/expected.xml b/python/testData/inspections/PyArgumentListInspection/expected.xml index 32c2dd139fc1..905655b6b9e2 100644 --- a/python/testData/inspections/PyArgumentListInspection/expected.xml +++ b/python/testData/inspections/PyArgumentListInspection/expected.xml @@ -80,4 +80,44 @@ 66 Unexpected argument + + badarglist.py + 80 + Parameter 'param' unfilled + + + badarglist.py + 84 + Unexpected argument + + + badarglist.py + 90 + Parameter 'p2' unfilled + + + badarglist.py + 94 + Parameter 'p2' unfilled + + + badarglist.py + 119 + Parameter 'param' unfilled + + + badarglist.py + 128 + Parameter 'p1' unfilled + + + badarglist.py + 128 + Parameter 'p2' unfilled + + + badarglist.py + 132 + Parameter 'p2' unfilled + diff --git a/python/testData/psi/DecoratedFunction.txt b/python/testData/psi/DecoratedFunction.txt index df8288f4f518..f1a15f0bff15 100644 --- a/python/testData/psi/DecoratedFunction.txt +++ b/python/testData/psi/DecoratedFunction.txt @@ -5,6 +5,8 @@ PyFile:DecoratedFunction.py PsiElement(Py:AT)('@') PyReferenceExpression: staticmethod PsiElement(Py:IDENTIFIER)('staticmethod') + PyArgumentList + PsiWhiteSpace('\n') PyDecorator: @xmlize PsiElement(Py:AT)('@') diff --git a/python/testSrc/com/jetbrains/python/PyResolveCalleeTest.java b/python/testSrc/com/jetbrains/python/PyResolveCalleeTest.java index f3ff42165bfd..4607d898cdd8 100644 --- a/python/testSrc/com/jetbrains/python/PyResolveCalleeTest.java +++ b/python/testSrc/com/jetbrains/python/PyResolveCalleeTest.java @@ -24,7 +24,7 @@ public class PyResolveCalleeTest extends ResolveTestCase { public void testInstanceCall() throws Exception { PyCallExpression.PyMarkedFunction resolved = resolveCallee(); assertNotNull(resolved.getFunction()); - assertTrue(resolved.getFlags().equals(EnumSet.of(PyCallExpression.Flag.IMPLICIT_FIRST_ARG))); + assertEquals(1, resolved.getImplicitOffset()); } public void testClassCall() throws Exception { @@ -36,7 +36,7 @@ public class PyResolveCalleeTest extends ResolveTestCase { public void testDecoCall() throws Exception { PyCallExpression.PyMarkedFunction resolved = resolveCallee(); assertNotNull(resolved.getFunction()); - assertTrue(resolved.getFlags().equals(EnumSet.of(PyCallExpression.Flag.IMPLICIT_FIRST_ARG))); + assertEquals(1, resolved.getImplicitOffset()); } public void testDecoParamCall() throws Exception {