PyArgumentsListInspection now uses new PyCallExpressionHelper.mapArguments() instead of CallArgumentsMapping

We simplified two checks: now we don't detect some cases of
multiple values that resolve to a single positional parameter (we show
a related warning about the wrong order of keyword arguments anyway)
and we stopped telling the user that a sequence literal argument doens't
match against a tuple paramater in Python 2. We might enhance the new
version of arguments-parameters matching to restore this functionality.
This commit is contained in:
Andrey Vlasovskikh
2015-08-21 16:56:51 +03:00
parent 313213c2d3
commit 9960f718e8
5 changed files with 166 additions and 103 deletions
@@ -37,10 +37,8 @@ import com.jetbrains.python.psi.types.TypeEvalContext;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.*;
import java.util.HashMap;
/**
* Looks at argument lists.
@@ -114,7 +112,6 @@ public class PyArgumentListInspection extends PyInspection {
}
final PyResolveContext resolveContext = PyResolveContext.noImplicits().withTypeEvalContext(context);
final PyCallExpression.PyArgumentsMapping mapping = callExpr.mapArguments(resolveContext, implicitOffset);
CallArgumentsMapping result = node.analyzeCall(resolveContext, implicitOffset);
final PyCallExpression.PyMarkedCallee callee = mapping.getMarkedCallee();
if (callee != null) {
final PyCallable callable = callee.getCallable();
@@ -123,7 +120,7 @@ public class PyArgumentListInspection extends PyInspection {
return;
}
}
highlightIncorrectArguments(holder, result, context);
highlightIncorrectArguments(callExpr, holder, mapping);
highlightMissingArguments(node, holder, mapping);
highlightStarArgumentTypeMismatch(node, holder, context);
}
@@ -132,50 +129,91 @@ public class PyArgumentListInspection extends PyInspection {
inspectPyArgumentList(node, holder, context, 0);
}
private static void highlightIncorrectArguments(ProblemsHolder holder, CallArgumentsMapping result, @NotNull TypeEvalContext context) {
for (Map.Entry<PyExpression, EnumSet<CallArgumentsMapping.ArgFlag>> argEntry : result.getArgumentFlags().entrySet()) {
EnumSet<CallArgumentsMapping.ArgFlag> flags = argEntry.getValue();
if (!flags.isEmpty()) { // something's wrong
PyExpression arg = argEntry.getKey();
if (flags.contains(CallArgumentsMapping.ArgFlag.IS_DUP)) {
holder.registerProblem(arg, PyBundle.message("INSP.duplicate.argument"), new PyRemoveArgumentQuickFix());
private enum ArgumentProblem {
OK,
DUPLICATE_KEYWORD_ARGUMENT,
DUPLICATE_KEYWORD_CONTAINER,
DUPLICATE_POSITIONAL_CONTAINER,
CANNOT_APPEAR_AFTER_KEYWORD_OR_CONTAINER,
}
@NotNull
private static Map<PyExpression, ArgumentProblem> analyzeArguments(@NotNull PyCallExpression callExpression) {
final Map<PyExpression, ArgumentProblem> results = new HashMap<PyExpression, ArgumentProblem>();
final Set<String> keywordArgumentNames = new HashSet<String>();
boolean seenKeywordOrContainerArgument = false;
boolean seenKeywordContainer = false;
boolean seenPositionalContainer = false;
for (PyExpression argument : callExpression.getArguments()) {
if (argument instanceof PyKeywordArgument) {
seenKeywordOrContainerArgument = true;
final String keyword = ((PyKeywordArgument)argument).getKeyword();
final ArgumentProblem problem;
if (keywordArgumentNames.contains(keyword)) {
problem = ArgumentProblem.DUPLICATE_KEYWORD_ARGUMENT;
}
if (flags.contains(CallArgumentsMapping.ArgFlag.IS_DUP_KWD)) {
holder.registerProblem(arg, PyBundle.message("INSP.duplicate.doublestar.arg"), new PyRemoveArgumentQuickFix());
else if (seenKeywordContainer) {
problem = ArgumentProblem.CANNOT_APPEAR_AFTER_KEYWORD_OR_CONTAINER;
}
if (flags.contains(CallArgumentsMapping.ArgFlag.IS_DUP_TUPLE)) {
holder.registerProblem(arg, PyBundle.message("INSP.duplicate.star.arg"), new PyRemoveArgumentQuickFix());
else {
problem = ArgumentProblem.OK;
}
if (flags.contains(CallArgumentsMapping.ArgFlag.IS_POS_PAST_KWD)) {
holder.registerProblem(arg, PyBundle.message("INSP.cannot.appear.past.keyword.arg"), ProblemHighlightType.ERROR, new PyRemoveArgumentQuickFix());
results.put(argument, problem);
keywordArgumentNames.add(keyword);
}
else if (argument instanceof PyStarArgument) {
seenKeywordOrContainerArgument = true;
final PyStarArgument starArgument = (PyStarArgument)argument;
if (starArgument.isKeyword()) {
results.put(argument, seenKeywordContainer ? ArgumentProblem.DUPLICATE_KEYWORD_CONTAINER : ArgumentProblem.OK);
seenKeywordContainer = true;
}
if (flags.contains(CallArgumentsMapping.ArgFlag.IS_UNMAPPED)) {
ArrayList<LocalQuickFix> quickFixes = Lists.<LocalQuickFix>newArrayList(new PyRemoveArgumentQuickFix());
if (arg instanceof PyKeywordArgument) {
quickFixes.add(new PyRenameArgumentQuickFix());
}
holder.registerProblem(arg, PyBundle.message("INSP.unexpected.arg"), quickFixes.toArray(new LocalQuickFix[quickFixes.size()-1]));
else {
results.put(argument, seenPositionalContainer ? ArgumentProblem.DUPLICATE_POSITIONAL_CONTAINER : ArgumentProblem.OK);
seenPositionalContainer = true;
}
if (flags.contains(CallArgumentsMapping.ArgFlag.IS_TOO_LONG)) {
final PyCallExpression.PyMarkedCallee markedCallee = result.getMarkedCallee();
String parameterName = null;
if (markedCallee != null) {
final List<PyParameter> parameters = PyUtil.getParameters(markedCallee.getCallable(), context);
for (int i = parameters.size() - 1; i >= 0; --i) {
final PyParameter param = parameters.get(i);
if (param instanceof PyNamedParameter) {
final List<PyNamedParameter> unmappedParams = result.getUnmappedParams();
if (!((PyNamedParameter)param).isPositionalContainer() && !((PyNamedParameter)param).isKeywordContainer() &&
param.getDefaultValue() == null && !unmappedParams.contains(param)) {
parameterName = param.getName();
break;
}
}
}
holder.registerProblem(arg, parameterName != null ? PyBundle.message("INSP.multiple.values.resolve.to.positional.$0", parameterName)
: PyBundle.message("INSP.more.args.that.pos.params"));
}
}
else {
results.put(argument, seenKeywordOrContainerArgument ? ArgumentProblem.CANNOT_APPEAR_AFTER_KEYWORD_OR_CONTAINER : ArgumentProblem.OK);
}
}
return results;
}
private static void highlightIncorrectArguments(@NotNull PyCallExpression callExpr,
@NotNull ProblemsHolder holder,
@NotNull PyCallExpression.PyArgumentsMapping mapping) {
final Set<PyExpression> problematicArguments = new HashSet<PyExpression>();
for (Map.Entry<PyExpression, ArgumentProblem> entry : analyzeArguments(callExpr).entrySet()) {
final PyExpression argument = entry.getKey();
final ArgumentProblem problem = entry.getValue();
switch (problem) {
case OK:
break;
case DUPLICATE_KEYWORD_ARGUMENT:
holder.registerProblem(argument, PyBundle.message("INSP.duplicate.argument"), new PyRemoveArgumentQuickFix());
break;
case DUPLICATE_KEYWORD_CONTAINER:
holder.registerProblem(argument, PyBundle.message("INSP.duplicate.doublestar.arg"), new PyRemoveArgumentQuickFix());
break;
case DUPLICATE_POSITIONAL_CONTAINER:
holder.registerProblem(argument, PyBundle.message("INSP.duplicate.star.arg"), new PyRemoveArgumentQuickFix());
break;
case CANNOT_APPEAR_AFTER_KEYWORD_OR_CONTAINER:
holder.registerProblem(argument, PyBundle.message("INSP.cannot.appear.past.keyword.arg"), ProblemHighlightType.ERROR, new PyRemoveArgumentQuickFix());
}
if (problem != ArgumentProblem.OK) {
problematicArguments.add(argument);
}
}
for (PyExpression argument : mapping.getUnmappedArguments()) {
if (!problematicArguments.contains(argument)) {
final List<LocalQuickFix> quickFixes = Lists.<LocalQuickFix>newArrayList(new PyRemoveArgumentQuickFix());
if (argument instanceof PyKeywordArgument) {
quickFixes.add(new PyRenameArgumentQuickFix());
}
holder.registerProblem(argument, PyBundle.message("INSP.unexpected.arg"), quickFixes.toArray(new LocalQuickFix[quickFixes.size() - 1]));
}
}
}
@@ -210,7 +248,10 @@ public class PyArgumentListInspection extends PyInspection {
ASTNode close_paren = our_node.findChildByType(PyTokenTypes.RPAR);
if (close_paren != null) {
for (PyParameter parameter : mapping.getUnmappedParameters()) {
holder.registerProblem(close_paren.getPsi(), PyBundle.message("INSP.parameter.$0.unfilled", parameter.getName()));
final String name = parameter.getName();
if (name != null) {
holder.registerProblem(close_paren.getPsi(), PyBundle.message("INSP.parameter.$0.unfilled", name));
}
}
}
}
@@ -645,6 +645,7 @@ public class PyCallExpressionHelper {
}
boolean seenSingleStar = false;
boolean mappedVariadicArgumentsToParameters = false;
final TypeEvalContext context = resolveContext.getTypeEvalContext();
final Map<PyExpression, PyNamedParameter> mappedParameters = new LinkedHashMap<PyExpression, PyNamedParameter>();
final List<PyParameter> unmappedParameters = new ArrayList<PyParameter>();
@@ -653,32 +654,34 @@ public class PyCallExpressionHelper {
final List<PyParameter> parameters = dropImplicitParameters(allParameters, markedCallee.getImplicitOffset());
final List<PyExpression> arguments = new ArrayList<PyExpression>(Arrays.asList(argumentList.getArguments()));
final List<PyExpression> positionalArguments = removePositionalElements(arguments, resolveContext);
final List<PyExpression> positionalArguments = filterPositionalElements(arguments);
final List<PyKeywordArgument> keywordArguments = filterKeywordArguments(arguments);
final List<PyExpression> variadicPositionalArguments = filterVariadicPositionalArguments(arguments);
final Pair<List<PyExpression>, List<PyExpression>> variadicPositionalArgumentsAndTheirComponents = filterVariadicPositionalArguments(arguments);
final List<PyExpression> variadicPositionalArguments = variadicPositionalArgumentsAndTheirComponents.getFirst();
final List<PyExpression> positionalComponentsOfVariadicArguments = variadicPositionalArgumentsAndTheirComponents.getSecond();
final List<PyExpression> variadicKeywordArguments = filterVariadicKeywordArguments(arguments);
final List<PyExpression> allPositionalArguments = new ArrayList<PyExpression>();
allPositionalArguments.addAll(positionalArguments);
allPositionalArguments.addAll(positionalComponentsOfVariadicArguments);
for (PyParameter parameter : parameters) {
if (parameter instanceof PyNamedParameter) {
final PyNamedParameter namedParameter = (PyNamedParameter)parameter;
final String parameterName = namedParameter.getName();
if (namedParameter.isPositionalContainer()) {
if (variadicPositionalArguments.size() == 1) {
mappedParameters.put(variadicPositionalArguments.remove(0), namedParameter);
}
else {
positionalArguments.clear();
variadicPositionalArguments.clear();
mappedParameters.put(variadicPositionalArguments.get(0), namedParameter);
}
allPositionalArguments.clear();
variadicPositionalArguments.clear();
}
else if (namedParameter.isKeywordContainer()) {
if (variadicKeywordArguments.size() == 1) {
mappedParameters.put(variadicKeywordArguments.remove(0), namedParameter);
}
else {
keywordArguments.clear();
variadicKeywordArguments.clear();
mappedParameters.put(variadicKeywordArguments.get(0), namedParameter);
}
keywordArguments.clear();
variadicKeywordArguments.clear();
}
else if (seenSingleStar) {
final PyExpression keywordArgument = removeKeywordArgument(keywordArguments, parameterName);
@@ -690,16 +693,7 @@ public class PyCallExpressionHelper {
}
}
else {
if (!positionalArguments.isEmpty()) {
final PyExpression positionalArgument = next(positionalArguments);
if (positionalArgument != null) {
mappedParameters.put(positionalArgument, namedParameter);
}
else if (!namedParameter.hasDefaultValue()) {
unmappedParameters.add(namedParameter);
}
}
else {
if (allPositionalArguments.isEmpty()) {
final PyKeywordArgument keywordArgument = removeKeywordArgument(keywordArguments, parameterName);
if (keywordArgument != null) {
mappedParameters.put(keywordArgument, namedParameter);
@@ -707,11 +701,35 @@ public class PyCallExpressionHelper {
else if (variadicPositionalArguments.isEmpty() && variadicKeywordArguments.isEmpty() && !namedParameter.hasDefaultValue()) {
unmappedParameters.add(namedParameter);
}
else {
mappedVariadicArgumentsToParameters = true;
}
}
else {
final PyExpression positionalArgument = next(allPositionalArguments);
if (positionalArgument != null) {
mappedParameters.put(positionalArgument, namedParameter);
}
else if (!namedParameter.hasDefaultValue()) {
unmappedParameters.add(namedParameter);
}
}
}
}
else if (parameter instanceof PyTupleParameter) {
// TODO: Handle tuple parameters
final PyExpression positionalArgument = next(allPositionalArguments);
// TODO: Python 2: If we found a positional argument, we should match the components of the argument against the components of the
// tuple parameter
if (positionalArgument == null) {
if (variadicPositionalArguments.isEmpty()) {
if (!parameter.hasDefaultValue()) {
unmappedParameters.add(parameter);
}
}
else {
mappedVariadicArgumentsToParameters = true;
}
}
}
else if (parameter instanceof PySingleStarParameter) {
seenSingleStar = true;
@@ -721,8 +739,13 @@ public class PyCallExpressionHelper {
}
}
if (mappedVariadicArgumentsToParameters) {
variadicPositionalArguments.clear();
variadicKeywordArguments.clear();
}
final List<PyExpression> unmappedArguments = new ArrayList<PyExpression>();
unmappedArguments.addAll(positionalArguments);
unmappedArguments.addAll(allPositionalArguments);
unmappedArguments.addAll(keywordArguments);
unmappedArguments.addAll(variadicPositionalArguments);
unmappedArguments.addAll(variadicKeywordArguments);
@@ -747,28 +770,17 @@ public class PyCallExpressionHelper {
}
@NotNull
private static List<PyExpression> removePositionalElements(@NotNull List<PyExpression> arguments,
@NotNull PyResolveContext resolveContext) {
private static List<PyExpression> filterPositionalElements(@NotNull List<PyExpression> arguments) {
final List<PyExpression> results = new ArrayList<PyExpression>();
for (PyExpression argument : new ArrayList<PyExpression>(arguments)) {
boolean seenKeywordOrContainerArgument = false;
for (PyExpression argument : arguments) {
if (isPositionalArgument(argument)) {
results.add(argument);
arguments.remove(argument);
if (!seenKeywordOrContainerArgument) {
results.add(argument);
}
}
else if (isVariadicPositionalArgument(argument)) {
final PsiElement expr = PyPsiUtils.flattenParens(PsiTreeUtil.getChildOfType(argument, PyExpression.class));
final PsiElement element;
if (expr instanceof PyReferenceExpression) {
element = ((PyReferenceExpression)expr).followAssignmentsChain(resolveContext).getElement();
}
else {
element = expr;
}
if (element instanceof PySequenceExpression) {
final PySequenceExpression sequenceExpr = (PySequenceExpression)element;
results.addAll(Arrays.asList(sequenceExpr.getElements()));
arguments.remove(argument);
}
if (argument instanceof PyStarArgument || argument instanceof PyKeywordArgument) {
seenKeywordOrContainerArgument = true;
}
}
return results;
@@ -785,15 +797,27 @@ public class PyCallExpressionHelper {
return results;
}
/**
* Returns a list of variadic positional arguments and a list of components of variadic positional arguments
* if they are sequence literals.
*/
@NotNull
private static List<PyExpression> filterVariadicPositionalArguments(@NotNull List<PyExpression> arguments) {
final List<PyExpression> results = new ArrayList<PyExpression>();
private static Pair<List<PyExpression>, List<PyExpression>> filterVariadicPositionalArguments(@NotNull List<PyExpression> arguments) {
final List<PyExpression> variadicArguments = new ArrayList<PyExpression>();
final List<PyExpression> positionalComponentsOfVariadicArguments = new ArrayList<PyExpression>();
for (PyExpression argument : arguments) {
if (argument != null && isVariadicPositionalArgument(argument)) {
results.add(argument);
final PsiElement expr = PyPsiUtils.flattenParens(PsiTreeUtil.getChildOfType(argument, PyExpression.class));
if (expr instanceof PySequenceExpression) {
final PySequenceExpression sequenceExpr = (PySequenceExpression)expr;
positionalComponentsOfVariadicArguments.addAll(Arrays.asList(sequenceExpr.getElements()));
}
else {
variadicArguments.add(argument);
}
}
}
return results;
return Pair.create(variadicArguments, positionalComponentsOfVariadicArguments);
}
@NotNull
@@ -3,8 +3,8 @@ def f(a, b, c):
f(c=1, *(10, 20))
f(*(10, 20), c=1)
f(*(10, 20, 30), <warning descr="Duplicate argument">c=1</warning>) # fail: duplicate c
f(1, <warning descr="Multiple values resolve to positional parameter 'c'">*(10, 20, 30)</warning>) # fail: tuple too long
f(*(10, 20, 30), <warning descr="Unexpected argument">c=1</warning>) # fail: duplicate c
f(1, *(10, 20, <warning descr="Unexpected argument">30</warning>)) # fail: tuple too long
f(1, <warning descr="Expected an iterable, got int">*(10)</warning>) # fail: wrong type
f(1, *(10,)<warning descr="Parameter 'c' unfilled">)</warning> # fail: tuple too short, c not mapped
@@ -24,15 +24,14 @@ f2(*(1,2), <error descr="Cannot appear past keyword arguments or *arg or **kwarg
def f3(a=1, b=2, c=3, *d):
return a,b,c,d
f3(c=3, <warning descr="Duplicate argument">a=1</warning>, b=2, *(1,2)) # fail: a twice
f3(1, 2, *(3,), <warning descr="Duplicate argument">c=4</warning>) # fail: c twice
f3(c=3, <warning descr="Unexpected argument">a=1</warning>, <warning descr="Unexpected argument">b=2</warning>, *(1,2)) # fail: a and b twice
f3(1, 2, *(3,), <warning descr="Unexpected argument">c=4</warning>) # fail: c twice
f3(1,2,3, *(1,2))
f3(c=3, *(1,2)) #
f3(1, <warning descr="Duplicate argument">c=3</warning>, *(1,2)) # fail: c twice
f3(c=3, <warning descr="Duplicate argument">a=1</warning>, b=2, *(1,2)) # fail: a twice, no positinals
f3(1, <warning descr="Unexpected argument">c=3</warning>, *(1,2)) # fail: c twice
f3(c=3, a=1, b=2, <warning descr="Unexpected argument">d=(1,2)</warning>) # fail: unexpected d
f3(1, c=3, *(10,)) # ZZZ
f3(1, *(10,))
f3(1, *(10,), c=20)
f3(*(1,2), c=20)
f3(*(1,2), <warning descr="Duplicate argument">a=20</warning>) # fail: a twice
f3(*(1,2), <warning descr="Unexpected argument">a=20</warning>) # fail: a twice
@@ -19,9 +19,8 @@ def a23(a, *b, c=1):
pass
a23(1,2,3, c=10) # pass
a23(1,2,3, c=10, <warning descr="Duplicate argument">a=1</warning>) # fail
a23(1,2,3, c=10, <warning descr="Unexpected argument">a=1</warning>) # fail
a23(c=10, a=1) # pass
a23(c=10, <error descr="Cannot appear past keyword arguments or *arg or **kwarg">1</error>) # fail
a23(<warning descr="Multiple values resolve to positional parameter 'a'">*args</warning>, a=1) # fail
a23(c=10, <error descr="Cannot appear past keyword arguments or *arg or **kwarg">1</error><warning descr="Parameter 'a' unfilled">)</warning> # fail
a23(*args, c=1) # pass
@@ -2,4 +2,4 @@ def f20(a, (b, c)):
pass
f20(1, [2, 3]) # ok
f20(1, (2, 3, <warning descr="Unexpected argument">4</warning>)) # fail: 4 is unexpected
f20(1, (2, 3, 4)) # ok: ignore problems with arguments of tuple parameters