Commit 6fec0a57 authored by Semyon Proshev's avatar Semyon Proshev
Browse files

Update PyArgumentListInspection to use PyCallExpression.multiMapArguments(PyResolveContext, int)

Unexpected arguments and unfilled parameters are not registered when there is at least one appropriate function.
In case of only one failed arguments mapping quickfixes are suggested, otherwise possible callees are shown.
In case of only one failed parameters mapping parameters' names are mentioned, otherwise possible callees are shown.
parent 77fae10b
Showing with 222 additions and 59 deletions
+222 -59
......@@ -280,8 +280,11 @@ public interface PyCallExpression extends PyCallSiteExpression {
* @param implicitOffset implicit offset which is known from the context
* @return an object which contains callable and mappings.
* Returns mapping created by {@link PyArgumentsMapping#empty(PyCallExpression)} if the callee cannot be resolved.
* @deprecated Use {@link PyCallExpression#multiMapArguments(PyResolveContext, int)} instead.
* This method will be removed in 2018.1.
*/
@NotNull
@Deprecated
default PyArgumentsMapping mapArguments(@NotNull PyResolveContext resolveContext, int implicitOffset) {
return Optional
.of(multiMapArguments(resolveContext, implicitOffset))
......
......@@ -328,7 +328,10 @@ INSP.GROUP.mako=Mako
INSP.NAME.incorrect.call.arguments=Incorrect call arguments
INSP.cannot.appear.past.keyword.arg=Cannot appear past keyword arguments or *arg or **kwarg
INSP.unexpected.arg=Unexpected argument
INSP.unexpected.arg(s)=Unexpected argument(s)
INSP.parameter.$0.unfilled=Parameter ''{0}'' unfilled
INSP.parameter(s).unfilled=Parameter(s) unfilled
INSP.possible.callees=Possible callees
INSP.func.$0.lacks.first.arg=Function ''{0}'' lacks a positional argument
INSP.expected.dict.got.$0=Expected a dictionary, got {0}
INSP.expected.iter.got.$0=Expected an iterable, got {0}
......
/*
* Copyright 2000-2016 JetBrains s.r.o.
* Copyright 2000-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -24,6 +24,7 @@ import com.intellij.psi.PsiElementVisitor;
import com.intellij.psi.PsiPolyVariantReference;
import com.intellij.psi.ResolveResult;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.xml.util.XmlStringUtil;
import com.jetbrains.python.PyBundle;
import com.jetbrains.python.PyNames;
import com.jetbrains.python.PyTokenTypes;
......@@ -35,19 +36,14 @@ import com.jetbrains.python.psi.types.PyABCUtil;
import com.jetbrains.python.psi.types.PyType;
import com.jetbrains.python.psi.types.PyTypeChecker;
import com.jetbrains.python.psi.types.TypeEvalContext;
import one.util.streamex.StreamEx;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.*;
import java.util.stream.Collectors;
/**
* Looks at argument lists.
* @author dcheryasov
*/
public class PyArgumentListInspection extends PyInspection {
@Nls
@NotNull
......@@ -57,19 +53,25 @@ public class PyArgumentListInspection extends PyInspection {
@NotNull
@Override
public PsiElementVisitor buildVisitor(@NotNull final ProblemsHolder holder, final boolean isOnTheFly, @NotNull LocalInspectionToolSession session) {
public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly, @NotNull LocalInspectionToolSession session) {
return new Visitor(holder, session);
}
public static class Visitor extends PyInspectionVisitor {
private static class Visitor extends PyInspectionVisitor {
public Visitor(final ProblemsHolder holder, LocalInspectionToolSession session) {
public Visitor(@NotNull ProblemsHolder holder, @NotNull LocalInspectionToolSession session) {
super(holder, session);
}
@NotNull
@Override
protected ProblemsHolder getHolder() {
//noinspection ConstantConditions, see Visitor#Visitor(ProblemsHolder, LocalInspectionToolSession)
return super.getHolder();
}
@Override
public void visitPyArgumentList(final PyArgumentList node) {
// analyze
inspectPyArgumentList(node, getHolder(), myTypeEvalContext);
}
......@@ -108,31 +110,38 @@ public class PyArgumentListInspection extends PyInspection {
}
public static void inspectPyArgumentList(PyArgumentList node, ProblemsHolder holder, final TypeEvalContext context, int implicitOffset) {
if (node.getParent() instanceof PyClass) return; // class Foo(object) is also an arg list
final PyCallExpression callExpr = node.getCallExpression();
if (callExpr == null) {
return;
}
public static void inspectPyArgumentList(@NotNull PyArgumentList node,
@NotNull ProblemsHolder holder,
@NotNull TypeEvalContext context,
int implicitOffset) {
if (node.getParent() instanceof PyClass) return; // `(object)` in `class Foo(object)` is also an arg list
final PyCallExpression call = node.getCallExpression();
if (call == null) return;
final PyResolveContext resolveContext = PyResolveContext.noImplicits().withTypeEvalContext(context);
final PyCallExpression.PyArgumentsMapping mapping = callExpr.mapArguments(resolveContext, implicitOffset);
final PyCallExpression.PyMarkedCallee callee = mapping.getMarkedCallee();
if (callee != null) {
final PyCallable callable = callee.getCallable();
if (callable instanceof PyFunction) {
final PyFunction function = (PyFunction)callable;
// Decorate functions may have different parameter lists. We don't match arguments with parameters of decorators yet
if (PyUtil.hasCustomDecorators(function) || decoratedClassInitCall(callExpr.getCallee(), function)) {
return;
final List<PyCallExpression.PyArgumentsMapping> mappings = call.multiMapArguments(resolveContext, implicitOffset);
for (PyCallExpression.PyArgumentsMapping mapping : mappings) {
final PyCallExpression.PyMarkedCallee callee = mapping.getMarkedCallee();
if (callee != null) {
final PyCallable callable = callee.getCallable();
if (callable instanceof PyFunction) {
final PyFunction function = (PyFunction)callable;
// Decorate functions may have different parameter lists. We don't match arguments with parameters of decorators yet
if (PyUtil.hasCustomDecorators(function) || decoratedClassInitCall(call.getCallee(), function)) {
return;
}
}
}
}
highlightParametersMismatch(node, holder, mapping);
highlightUnexpectedArguments(node, holder, mappings);
highlightUnfilledParameters(node, holder, mappings);
highlightStarArgumentTypeMismatch(node, holder, context);
}
public static void inspectPyArgumentList(PyArgumentList node, ProblemsHolder holder, final TypeEvalContext context) {
public static void inspectPyArgumentList(@NotNull PyArgumentList node, @NotNull ProblemsHolder holder, @NotNull TypeEvalContext context) {
inspectPyArgumentList(node, holder, context, 0);
}
......@@ -187,32 +196,87 @@ public class PyArgumentListInspection extends PyInspection {
return results;
}
private static void highlightParametersMismatch(@NotNull PyArgumentList node, @NotNull ProblemsHolder holder,
@NotNull PyCallExpression.PyArgumentsMapping mapping) {
final Set<String> duplicateKeywords = getDuplicateKeywordArguments(node);
for (PyExpression argument : mapping.getUnmappedArguments()) {
final List<LocalQuickFix> quickFixes = Lists.<LocalQuickFix>newArrayList(new PyRemoveArgumentQuickFix());
if (argument instanceof PyKeywordArgument) {
if (duplicateKeywords.contains(((PyKeywordArgument)argument).getKeyword())) {
continue;
private static void highlightUnexpectedArguments(@NotNull PyArgumentList node,
@NotNull ProblemsHolder holder,
@NotNull List<PyCallExpression.PyArgumentsMapping> mappings) {
if (mappings.isEmpty() || mappings.stream().anyMatch(mapping -> mapping.getUnmappedArguments().isEmpty())) return;
if (mappings.size() == 1) {
// if there is only one mapping, we could suggest quick fixes
final Set<String> duplicateKeywords = getDuplicateKeywordArguments(node);
for (PyExpression argument : mappings.get(0).getUnmappedArguments()) {
final List<LocalQuickFix> quickFixes = Lists.newArrayList(new PyRemoveArgumentQuickFix());
if (argument instanceof PyKeywordArgument) {
if (duplicateKeywords.contains(((PyKeywordArgument)argument).getKeyword())) {
continue;
}
quickFixes.add(new PyRenameArgumentQuickFix());
}
quickFixes.add(new PyRenameArgumentQuickFix());
holder.registerProblem(argument,
PyBundle.message("INSP.unexpected.arg"),
quickFixes.toArray(new LocalQuickFix[quickFixes.size() - 1]));
}
holder.registerProblem(argument, PyBundle.message("INSP.unexpected.arg"),
quickFixes.toArray(new LocalQuickFix[quickFixes.size() - 1]));
}
else {
// all mappings have unmapped arguments so we couldn't determine desired argument list and suggest appropriate quick fixes
holder.registerProblem(node, addPossibleCalleesRepresentationAndWrapInHtml(PyBundle.message("INSP.unexpected.arg(s)"), mappings));
}
}
private static void highlightUnfilledParameters(@NotNull PyArgumentList node,
@NotNull ProblemsHolder holder,
@NotNull List<PyCallExpression.PyArgumentsMapping> mappings) {
if (mappings.isEmpty() || mappings.stream().anyMatch(mapping -> mapping.getUnmappedParameters().isEmpty())) return;
ASTNode our_node = node.getNode();
if (our_node != null) {
ASTNode close_paren = our_node.findChildByType(PyTokenTypes.RPAR);
if (close_paren != null) {
for (PyParameter parameter : mapping.getUnmappedParameters()) {
final String name = parameter.getName();
if (name != null) {
holder.registerProblem(close_paren.getPsi(), PyBundle.message("INSP.parameter.$0.unfilled", name));
Optional
.ofNullable(node.getNode())
.map(astNode -> astNode.findChildByType(PyTokenTypes.RPAR))
.map(ASTNode::getPsi)
.ifPresent(
psi -> {
if (mappings.size() == 1) {
StreamEx
.of(mappings.get(0).getUnmappedParameters())
.map(PyParameter::getName)
.filter(Objects::nonNull)
.forEach(name -> holder.registerProblem(psi, PyBundle.message("INSP.parameter.$0.unfilled", name)));
}
else {
holder.registerProblem(psi,
addPossibleCalleesRepresentationAndWrapInHtml(PyBundle.message("INSP.parameter(s).unfilled"), mappings));
}
}
}
}
);
}
@NotNull
private static String addPossibleCalleesRepresentationAndWrapInHtml(@NotNull String prefix,
@NotNull List<PyCallExpression.PyArgumentsMapping> mappings) {
final String possibleCalleesRepresentation = calculatePossibleCalleesRepresentation(mappings);
return XmlStringUtil.wrapInHtml(prefix + "<br>" + PyBundle.message("INSP.possible.callees") + ":<br>" + possibleCalleesRepresentation);
}
@NotNull
private static String calculatePossibleCalleesRepresentation(@NotNull List<PyCallExpression.PyArgumentsMapping> mappings) {
return StreamEx
.of(mappings)
.map(PyCallExpression.PyArgumentsMapping::getMarkedCallee)
.nonNull()
.map(markedCallee -> calculatePossibleCalleeRepresentation(markedCallee.getCallable()))
.nonNull()
.collect(Collectors.joining("<br>"));
}
@Nullable
private static String calculatePossibleCalleeRepresentation(@NotNull PyCallable callable) {
final String callableNameAndParameters = callable.getName() + callable.getParameterList().getPresentableText(true);
return Optional
.ofNullable(PyUtil.as(callable, PyFunction.class))
.map(PyFunction::getContainingClass)
.map(PyClass::getName)
.map(className -> className + "." + callableNameAndParameters)
.orElse(callableNameAndParameters);
}
}
class C1:
def foo(self, x):
return self
class C2:
def foo(self, x, y):
return self
def f():
"""
:rtype: C1 | C2
"""
pass
f().foo<warning descr="Unexpected argument(s)Possible callees:C1.foo(self, x)C2.foo(self, x, y)">(1, 2, 3)</warning>
class C1:
def foo(self, x):
return self
class C2:
def foo(self, x, y):
return self
def f():
"""
:rtype: C1 | C2
"""
pass
f().foo(<warning descr="Parameter(s) unfilledPossible callees:C1.foo(self, x)C2.foo(self, x, y)">)</warning>
class C1:
def foo(self, x):
return self
class C2:
def foo(self, x, y):
return self
def f():
"""
:rtype: C1 | C2
"""
pass
f().foo(1, 2)
\ No newline at end of file
class C1:
def foo(self, x):
return self
class C2:
def foo(self, x, y):
return self
def f():
"""
:rtype: C1 | C2
"""
pass
f().foo(1)
\ No newline at end of file
......@@ -4,6 +4,7 @@ class C1:
class C2:
@decorated
def foo(self, x, y):
return self
......@@ -15,7 +16,7 @@ def f():
pass
f().foo() # XXX: Ignore this false negative as for now
f().foo()
f().foo(1)
f().foo(1, 2)
f().foo(1, 2, 3) # XXX: Ignore this false negative as well
f().foo(1, 2, 3)
/*
* Copyright 2000-2016 JetBrains s.r.o.
* Copyright 2000-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -198,10 +198,6 @@ public class PyArgumentListInspectionTest extends PyTestCase {
doTest();
}
public void testUnionTypeAttributeCall() {
doTest();
}
// PY-18275
public void testStrFormat() {
doTest();
......@@ -222,6 +218,30 @@ public class PyArgumentListInspectionTest extends PyTestCase {
doTest();
}
public void testMultiResolveWhenOneResultIsDecoratedFunction() {
doTest();
}
public void testMultiResolveWhenOneResultIsDunderInitInDecoratedClass() {
// Implement after fixing PY-20057
}
public void testMultiResolveWhenOneResultDoesNotHaveUnmappedArguments() {
doTest();
}
public void testMultiResolveWhenOneResultDoesNotHaveUnmappedParameters() {
doTest();
}
public void testMultiResolveWhenAllResultsHaveUnmappedArguments() {
doTest();
}
public void testMultiResolveWhenAllResultsHaveUnmappedParameters() {
doTest();
}
private void doMultiFileTest() {
final String folderPath = "inspections/PyArgumentListInspection/" + getTestName(false) + "/";
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment