diff --git a/python/src/com/jetbrains/python/codeInsight/intentions/ConvertVariadicParamIntention.java b/python/src/com/jetbrains/python/codeInsight/intentions/ConvertVariadicParamIntention.java index a2d867b62d1f..115645658c65 100644 --- a/python/src/com/jetbrains/python/codeInsight/intentions/ConvertVariadicParamIntention.java +++ b/python/src/com/jetbrains/python/codeInsight/intentions/ConvertVariadicParamIntention.java @@ -18,6 +18,7 @@ package com.jetbrains.python.codeInsight.intentions; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Pair; +import com.intellij.openapi.util.Ref; import com.intellij.openapi.util.Trinity; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiFile; @@ -26,6 +27,7 @@ import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.ArrayUtil; import com.intellij.util.IncorrectOperationException; import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.containers.MultiMap; import com.jetbrains.python.PyBundle; import com.jetbrains.python.PyNames; import com.jetbrains.python.psi.*; @@ -123,8 +125,7 @@ public class ConvertVariadicParamIntention extends PyBaseIntentionAction { final PyFunction function = PsiTreeUtil.getParentOfType(element, PyFunction.class); if (function != null) { - replaceKeywordContainerSubscriptions(function, project); - replaceKeywordContainerCalls(function, project); + replaceKeywordContainerUsages(function, project); } } @@ -177,53 +178,63 @@ public class ConvertVariadicParamIntention extends PyBaseIntentionAction { .orElse(null); } - private static void replaceKeywordContainerSubscriptions(@NotNull PyFunction function, @NotNull Project project) { + private static void replaceKeywordContainerUsages(@NotNull PyFunction function, @NotNull Project project) { final PyParameter keywordContainer = getKeywordContainer(function.getParameterList()); if (keywordContainer == null) return; - final PyElementGenerator elementGenerator = PyElementGenerator.getInstance(project); + final Set names = new LinkedHashSet<>(); + final MultiMap subscriptions = MultiMap.create(); + final MultiMap calls = MultiMap.create(); SyntaxTraverser .psiTraverser(function.getStatementList()) - .filter(e -> isKeywordContainerSubscription(e, keywordContainer)) - .filter(PySubscriptionExpression.class) .forEach( - subscription -> { - final String indexValue = getIndexValueToReplace(subscription); - if (indexValue != null) { - final PyExpression parameter = elementGenerator.createExpressionFromText(LanguageLevel.forElement(function), indexValue); + e -> { + if (isKeywordContainerSubscription(e, keywordContainer)) { + final PySubscriptionExpression subscription = (PySubscriptionExpression)e; + final String indexValue = getIndexValueToReplace(subscription); - insertParameter(function.getParameterList(), parameter, false, elementGenerator); - subscription.replace(parameter); + if (indexValue != null) { + names.add(indexValue); + subscriptions.putValue(indexValue, subscription); + } } - } - ); - } + else if (isKeywordContainerCall(e, keywordContainer)) { + final PyCallExpression call = (PyCallExpression)e; + final String indexValue = getIndexValueToReplace(call); - private static void replaceKeywordContainerCalls(@NotNull PyFunction function, @NotNull Project project) { - final PyParameter keywordContainer = getKeywordContainer(function.getParameterList()); - if (keywordContainer == null) return; - - final PyElementGenerator elementGenerator = PyElementGenerator.getInstance(project); - - SyntaxTraverser - .psiTraverser(function.getStatementList()) - .filter(e -> isKeywordContainerCall(e, keywordContainer)) - .filter(PyCallExpression.class) - .forEach( - call -> { - final String indexValue = getIndexValueToReplace(call); - if (indexValue != null) { - final PyNamedParameter parameter = createParameter(elementGenerator, call, indexValue); - if (parameter != null) { - final PyExpression parameterUsage = elementGenerator.createExpressionFromText(LanguageLevel.forElement(function), indexValue); - - insertParameter(function.getParameterList(), parameter, parameter.hasDefaultValue(), elementGenerator); - call.replace(parameterUsage); + if (indexValue != null) { + names.add(indexValue); + calls.putValue(indexValue, call); } } } ); + + final PyElementGenerator elementGenerator = PyElementGenerator.getInstance(project); + for (String name : names) { + final Collection currentSubscriptions = subscriptions.get(name); + final Collection currentCalls = calls.get(name); + + if (!currentSubscriptions.isEmpty()) { + final PyExpression parameter = elementGenerator.createExpressionFromText(LanguageLevel.forElement(function), name); + insertParameter(function.getParameterList(), parameter, false, elementGenerator); + + currentSubscriptions.forEach(e -> e.replace(parameter)); + currentCalls.forEach(e -> e.replace(parameter)); + } + else if (!currentCalls.isEmpty()) { + final Ref defaultValue = getCommonDefaultKeyValue(currentCalls); + if (defaultValue == null) return; + + final LanguageLevel languageLevel = LanguageLevel.forElement(function); + final PyNamedParameter parameter = elementGenerator.createParameter(name, defaultValue.get(), null, languageLevel); + final PyExpression parameterUsage = elementGenerator.createExpressionFromText(languageLevel, name); + + insertParameter(function.getParameterList(), parameter, parameter.hasDefaultValue(), elementGenerator); + currentCalls.forEach(e -> e.replace(parameterUsage)); + } + } } private static boolean isKeywordContainerSubscription(@Nullable PsiElement element, @NotNull PyParameter keywordContainer) { @@ -263,22 +274,40 @@ public class ConvertVariadicParamIntention extends PyBaseIntentionAction { parameterList.addBefore((PsiElement)elementGenerator.createComma(), placeToInsertParameter); } + /** + * @return null when calls is empty or there are different default values. + */ @Nullable - private static PyNamedParameter createParameter(@NotNull PyElementGenerator elementGenerator, - @NotNull PyCallExpression call, - @NotNull String parameterName) { + private static Ref getCommonDefaultKeyValue(@NotNull Collection calls) { + Ref defaultValue = null; + + for (PyCallExpression call : calls) { + final String currentDefaultValue = getDefaultKeyValue(call); + if (defaultValue == null) { + defaultValue = Ref.create(currentDefaultValue); + } + else if (!Objects.equals(defaultValue.get(), currentDefaultValue)) { + return null; + } + } + + return defaultValue; + } + + @Nullable + private static String getDefaultKeyValue(@NotNull PyCallExpression call) { final PyExpression[] arguments = call.getArguments(); if (arguments.length > 1) { final PyExpression argument = PyUtil.peelArgument(arguments[1]); - return argument == null ? null : elementGenerator.createParameter(parameterName + "=" + argument.getText()); + return argument == null ? null : argument.getText(); } final PyQualifiedExpression callee = PyUtil.as(call.getCallee(), PyQualifiedExpression.class); if (callee != null && "get".equals(callee.getReferencedName())) { - return elementGenerator.createParameter(parameterName + "=" + PyNames.NONE); + return PyNames.NONE; } - return elementGenerator.createParameter(parameterName); + return null; } @Nullable diff --git a/python/testData/intentions/convertVariadicParamSeveralCalls.py b/python/testData/intentions/convertVariadicParamSeveralCalls.py new file mode 100644 index 000000000000..2772979d652e --- /dev/null +++ b/python/testData/intentions/convertVariadicParamSeveralCalls.py @@ -0,0 +1,3 @@ +def bar(**kwargs): + b = kwargs.get('foo') + c = kwargs.get('foo') \ No newline at end of file diff --git a/python/testData/intentions/convertVariadicParamSeveralCallsWithDifferentDefaultValue.py b/python/testData/intentions/convertVariadicParamSeveralCallsWithDifferentDefaultValue.py new file mode 100644 index 000000000000..65eb9899668d --- /dev/null +++ b/python/testData/intentions/convertVariadicParamSeveralCallsWithDifferentDefaultValue.py @@ -0,0 +1,3 @@ +def bar(**kwargs): + b = kwargs.get('foo', 0) + c = kwargs.get('foo', 1) \ No newline at end of file diff --git a/python/testData/intentions/convertVariadicParamSeveralCallsWithSameDefaultValue.py b/python/testData/intentions/convertVariadicParamSeveralCallsWithSameDefaultValue.py new file mode 100644 index 000000000000..b7cc8905a14c --- /dev/null +++ b/python/testData/intentions/convertVariadicParamSeveralCallsWithSameDefaultValue.py @@ -0,0 +1,3 @@ +def bar(**kwargs): + b = kwargs.get('foo', 0) + c = kwargs.get('foo', 0) \ No newline at end of file diff --git a/python/testData/intentions/convertVariadicParamSeveralCallsWithSameDefaultValue_after.py b/python/testData/intentions/convertVariadicParamSeveralCallsWithSameDefaultValue_after.py new file mode 100644 index 000000000000..1282c00196c4 --- /dev/null +++ b/python/testData/intentions/convertVariadicParamSeveralCallsWithSameDefaultValue_after.py @@ -0,0 +1,3 @@ +def bar(foo=0, **kwargs): + b = foo + c = foo \ No newline at end of file diff --git a/python/testData/intentions/convertVariadicParamSeveralCalls_after.py b/python/testData/intentions/convertVariadicParamSeveralCalls_after.py new file mode 100644 index 000000000000..660bd59debb2 --- /dev/null +++ b/python/testData/intentions/convertVariadicParamSeveralCalls_after.py @@ -0,0 +1,3 @@ +def bar(foo=None, **kwargs): + b = foo + c = foo \ No newline at end of file diff --git a/python/testData/intentions/convertVariadicParamSeveralSubscriptions.py b/python/testData/intentions/convertVariadicParamSeveralSubscriptions.py new file mode 100644 index 000000000000..6a28bae0d055 --- /dev/null +++ b/python/testData/intentions/convertVariadicParamSeveralSubscriptions.py @@ -0,0 +1,3 @@ +def bar(**kwargs): + a = kwargs['foo'] + b = kwargs['foo'] \ No newline at end of file diff --git a/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCalls.py b/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCalls.py new file mode 100644 index 000000000000..e9a151d92a22 --- /dev/null +++ b/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCalls.py @@ -0,0 +1,4 @@ +def bar(**kwargs): + a = kwargs['foo'] + b = kwargs.get('foo') + c = kwargs.get('foo') \ No newline at end of file diff --git a/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCallsWithDifferentDefaultValue.py b/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCallsWithDifferentDefaultValue.py new file mode 100644 index 000000000000..88d7ebb9d29d --- /dev/null +++ b/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCallsWithDifferentDefaultValue.py @@ -0,0 +1,4 @@ +def bar(**kwargs): + a = kwargs['foo'] + b = kwargs.get('foo', 0) + c = kwargs.get('foo', 1) \ No newline at end of file diff --git a/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCallsWithDifferentDefaultValue_after.py b/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCallsWithDifferentDefaultValue_after.py new file mode 100644 index 000000000000..2183810d3e9f --- /dev/null +++ b/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCallsWithDifferentDefaultValue_after.py @@ -0,0 +1,4 @@ +def bar(foo, **kwargs): + a = foo + b = foo + c = foo \ No newline at end of file diff --git a/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCallsWithSameDefaultValue.py b/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCallsWithSameDefaultValue.py new file mode 100644 index 000000000000..e3e5d9e1fdc8 --- /dev/null +++ b/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCallsWithSameDefaultValue.py @@ -0,0 +1,4 @@ +def bar(**kwargs): + a = kwargs['foo'] + b = kwargs.get('foo', 1) + c = kwargs.get('foo', 1) \ No newline at end of file diff --git a/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCallsWithSameDefaultValue_after.py b/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCallsWithSameDefaultValue_after.py new file mode 100644 index 000000000000..2183810d3e9f --- /dev/null +++ b/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCallsWithSameDefaultValue_after.py @@ -0,0 +1,4 @@ +def bar(foo, **kwargs): + a = foo + b = foo + c = foo \ No newline at end of file diff --git a/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCalls_after.py b/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCalls_after.py new file mode 100644 index 000000000000..2183810d3e9f --- /dev/null +++ b/python/testData/intentions/convertVariadicParamSeveralSubscriptionsAndCalls_after.py @@ -0,0 +1,4 @@ +def bar(foo, **kwargs): + a = foo + b = foo + c = foo \ No newline at end of file diff --git a/python/testData/intentions/convertVariadicParamSeveralSubscriptions_after.py b/python/testData/intentions/convertVariadicParamSeveralSubscriptions_after.py new file mode 100644 index 000000000000..05d90ef428de --- /dev/null +++ b/python/testData/intentions/convertVariadicParamSeveralSubscriptions_after.py @@ -0,0 +1,3 @@ +def bar(foo, **kwargs): + a = foo + b = foo \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/intentions/PyIntentionTest.java b/python/testSrc/com/jetbrains/python/intentions/PyIntentionTest.java index 2461900275a9..0a47cf4ef2b1 100644 --- a/python/testSrc/com/jetbrains/python/intentions/PyIntentionTest.java +++ b/python/testSrc/com/jetbrains/python/intentions/PyIntentionTest.java @@ -239,7 +239,7 @@ public class PyIntentionTest extends PyTestCase { } public void testConvertVariadicParamPositionalContainerInPy3() { - runWithLanguageLevel(LanguageLevel.PYTHON34, () -> doTest(PyBundle.message("INTN.convert.variadic.param"))); + runWithLanguageLevel(LanguageLevel.getLatest(), () -> doTest(PyBundle.message("INTN.convert.variadic.param"))); } // PY-26284 @@ -267,6 +267,41 @@ public class PyIntentionTest extends PyTestCase { doNegativeTest(PyBundle.message("INTN.convert.variadic.param")); } + // PY-26286 + public void testConvertVariadicParamSeveralSubscriptions() { + doTest(PyBundle.message("INTN.convert.variadic.param")); + } + + // PY-26286 + public void testConvertVariadicParamSeveralCalls() { + doTest(PyBundle.message("INTN.convert.variadic.param")); + } + + // PY-26286 + public void testConvertVariadicParamSeveralCallsWithSameDefaultValue() { + doTest(PyBundle.message("INTN.convert.variadic.param")); + } + + // PY-26286 + public void testConvertVariadicParamSeveralCallsWithDifferentDefaultValue() { + doNegativeTest(PyBundle.message("INTN.convert.variadic.param")); + } + + // PY-26286 + public void testConvertVariadicParamSeveralSubscriptionsAndCalls() { + doTest(PyBundle.message("INTN.convert.variadic.param")); + } + + // PY-26286 + public void testConvertVariadicParamSeveralSubscriptionsAndCallsWithSameDefaultValue() { + doTest(PyBundle.message("INTN.convert.variadic.param")); + } + + // PY-26286 + public void testConvertVariadicParamSeveralSubscriptionsAndCallsWithDifferentDefaultValue() { + doTest(PyBundle.message("INTN.convert.variadic.param")); + } + public void testConvertTripleQuotedString() { //PY-2697 doTest(PyBundle.message("INTN.triple.quoted.string")); }