From ef60722aead81e020d4effddba8ea6db97b33502 Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Mon, 5 Jun 2017 14:14:31 +0300 Subject: [PATCH] PY-8174 Always use actual arguments as default values for new parameters even if names referenced in them are not available at the places of some usages and the definition. It's user responsibility to review them and ensure that refactoring won't cause any errors. This approach follows the one used for Java. --- .../quickfix/PyChangeSignatureQuickFix.java | 19 ++++++++----------- .../python/refactoring/PyRefactoringUtil.java | 14 -------------- .../inspections/AddParameterDefaultValues.py | 6 ++++++ .../AddParameterDefaultValues_after.py | 6 ++++++ .../com/jetbrains/python/PyQuickFixTest.java | 5 +++++ 5 files changed, 25 insertions(+), 25 deletions(-) create mode 100644 python/testData/inspections/AddParameterDefaultValues.py create mode 100644 python/testData/inspections/AddParameterDefaultValues_after.py diff --git a/python/src/com/jetbrains/python/inspections/quickfix/PyChangeSignatureQuickFix.java b/python/src/com/jetbrains/python/inspections/quickfix/PyChangeSignatureQuickFix.java index f5ceff9d9893..ec2f531832ba 100644 --- a/python/src/com/jetbrains/python/inspections/quickfix/PyChangeSignatureQuickFix.java +++ b/python/src/com/jetbrains/python/inspections/quickfix/PyChangeSignatureQuickFix.java @@ -76,20 +76,14 @@ public class PyChangeSignatureQuickFix extends LocalQuickFixOnPsiElement { final Set usedParamNames = new HashSet<>(); for (PyExpression arg : mapping.getUnmappedArguments()) { if (arg instanceof PyKeywordArgument) { - final String defaultValueText; final PyExpression value = ((PyKeywordArgument)arg).getValueExpression(); - if (value != null && PyRefactoringUtil.isSimpleExpression(value) && !value.textContains('\n')) { - defaultValueText = value.getText(); - } - else { - defaultValueText = ApplicationManager.getApplication().isUnitTestMode() ? "None" : ""; - } + final String valueText = value != null ? value.getText() : ""; newParameters.add(Pair.create(parameters.length - 1, - new PyParameterInfo(-1, ((PyKeywordArgument)arg).getKeyword(), defaultValueText, true))); + new PyParameterInfo(-1, ((PyKeywordArgument)arg).getKeyword(), valueText, true))); } else { final String paramName = generateParameterName(arg, function, usedParamNames, context); - newParameters.add(Pair.create(positionalParamAnchor, new PyParameterInfo(-1, paramName, "", false))); + newParameters.add(Pair.create(positionalParamAnchor, new PyParameterInfo(-1, paramName, arg.getText(), false))); usedParamNames.add(paramName); } } @@ -109,12 +103,15 @@ public class PyChangeSignatureQuickFix extends LocalQuickFixOnPsiElement { } return new PyChangeSignatureQuickFix(function, extraParams, null); } - - + private final List> myExtraParameters; private final SmartPsiElementPointer myOriginalCallExpression; + /** + * @param extraParameters new parameters anchored by indexes of the existing parameters they should be inserted after + * (-1 in case they should precede the first parameter) + */ public PyChangeSignatureQuickFix(@NotNull PyFunction function, @NotNull List> extraParameters, @Nullable PyCallExpression expression) { diff --git a/python/src/com/jetbrains/python/refactoring/PyRefactoringUtil.java b/python/src/com/jetbrains/python/refactoring/PyRefactoringUtil.java index 14592a8abbe1..4448804a780c 100644 --- a/python/src/com/jetbrains/python/refactoring/PyRefactoringUtil.java +++ b/python/src/com/jetbrains/python/refactoring/PyRefactoringUtil.java @@ -17,7 +17,6 @@ package com.jetbrains.python.refactoring; import com.intellij.codeInsight.PsiEquivalenceUtil; import com.intellij.find.findUsages.FindUsagesHandler; -import com.intellij.lang.ASTNode; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Comparing; import com.intellij.openapi.util.Pair; @@ -29,7 +28,6 @@ import com.intellij.usageView.UsageInfo; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.HashSet; import com.jetbrains.python.PyNames; -import com.jetbrains.python.PyTokenTypes; import com.jetbrains.python.findUsages.PyFindUsagesHandlerFactory; import com.jetbrains.python.psi.*; import com.jetbrains.python.refactoring.introduce.IntroduceValidator; @@ -399,16 +397,4 @@ public class PyRefactoringUtil { public static boolean isValidNewName(@NotNull String name, @NotNull PsiElement scopeAnchor) { return !(IntroduceValidator.isDefinedInScope(name, scopeAnchor) || PyNames.isReserved(name)); } - - public static boolean isSimpleExpression(@NotNull PyExpression value) { - if (value instanceof PyLiteralExpression) { - final ASTNode node = value.getNode(); - // Check that string literal doesn't contain multiple glued nodes - return node.getChildren(null).length == 1 && PyTokenTypes.SCALAR_LITERALS.contains(node.getFirstChildNode().getElementType()); - } - else if (value instanceof PyReferenceExpression) { - return PyUtil.isPy2ReservedWord((PyReferenceExpression)value); - } - return false; - } } diff --git a/python/testData/inspections/AddParameterDefaultValues.py b/python/testData/inspections/AddParameterDefaultValues.py new file mode 100644 index 000000000000..eceee62d0ccc --- /dev/null +++ b/python/testData/inspections/AddParameterDefaultValues.py @@ -0,0 +1,6 @@ +def func(): + pass + + +func(42, foo='spam') +func() diff --git a/python/testData/inspections/AddParameterDefaultValues_after.py b/python/testData/inspections/AddParameterDefaultValues_after.py new file mode 100644 index 000000000000..046a292bc48c --- /dev/null +++ b/python/testData/inspections/AddParameterDefaultValues_after.py @@ -0,0 +1,6 @@ +def func(i, foo='spam'): + pass + + +func(42, foo='spam') +func(42) diff --git a/python/testSrc/com/jetbrains/python/PyQuickFixTest.java b/python/testSrc/com/jetbrains/python/PyQuickFixTest.java index 2c18d8f19bce..2f07cca791a7 100644 --- a/python/testSrc/com/jetbrains/python/PyQuickFixTest.java +++ b/python/testSrc/com/jetbrains/python/PyQuickFixTest.java @@ -657,6 +657,11 @@ public class PyQuickFixTest extends PyTestCase { doInspectionTest(PyArgumentListInspection.class, "Change signature of", true, true); } + // PY-8174 + public void testAddParameterDefaultValues() { + doInspectionTest(PyArgumentListInspection.class, "Change signature of", true, true); + } + @Override @NonNls protected String getTestDataPath() {