From fc59d948bf180fa04f8554bfdd73ae4a556ecae4 Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Mon, 5 Jun 2017 01:12:17 +0300 Subject: [PATCH] PY-8174 New parameters names are unique if arguments have the same type Incrementally increasing numbers are appended to the names derived from the types of the corresponding arguments to make them sufficiently different. --- .../quickfix/PyChangeSignatureQuickFix.java | 50 ++++++++++++++----- ...ositionalParametersWithSameArgumentType.py | 6 +++ ...nalParametersWithSameArgumentType_after.py | 6 +++ .../com/jetbrains/python/PyQuickFixTest.java | 5 ++ 4 files changed, 55 insertions(+), 12 deletions(-) create mode 100644 python/testData/inspections/AddSeveralPositionalParametersWithSameArgumentType.py create mode 100644 python/testData/inspections/AddSeveralPositionalParametersWithSameArgumentType_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 1900f4ff5eed..f5ceff9d9893 100644 --- a/python/src/com/jetbrains/python/inspections/quickfix/PyChangeSignatureQuickFix.java +++ b/python/src/com/jetbrains/python/inspections/quickfix/PyChangeSignatureQuickFix.java @@ -20,6 +20,7 @@ import com.google.common.collect.PeekingIterator; import com.intellij.codeInspection.LocalQuickFixOnPsiElement; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.Conditions; import com.intellij.openapi.util.Key; import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.text.StringUtil; @@ -28,12 +29,16 @@ import com.intellij.psi.PsiFile; import com.intellij.psi.SmartPointerManager; import com.intellij.psi.SmartPsiElementPointer; import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.containers.HashSet; +import com.intellij.xml.util.XmlStringUtil; import com.jetbrains.python.PyBundle; -import com.jetbrains.python.PyNames; import com.jetbrains.python.psi.*; import com.jetbrains.python.psi.PyCallExpression.PyArgumentsMapping; +import com.jetbrains.python.psi.types.PyClassType; import com.jetbrains.python.psi.types.PyType; +import com.jetbrains.python.psi.types.PyUnionType; import com.jetbrains.python.psi.types.TypeEvalContext; +import com.jetbrains.python.refactoring.NameSuggesterUtil; import com.jetbrains.python.refactoring.PyRefactoringUtil; import com.jetbrains.python.refactoring.changeSignature.PyChangeSignatureDialog; import com.jetbrains.python.refactoring.changeSignature.PyMethodDescriptor; @@ -42,10 +47,7 @@ import one.util.streamex.StreamEx; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.List; +import java.util.*; import static com.jetbrains.python.psi.PyUtil.as; @@ -70,6 +72,8 @@ public class PyChangeSignatureQuickFix extends LocalQuickFixOnPsiElement { positionalParamAnchor++; } final List> newParameters = new ArrayList<>(); + final TypeEvalContext context = TypeEvalContext.userInitiated(function.getProject(), callExpression.getContainingFile()); + final Set usedParamNames = new HashSet<>(); for (PyExpression arg : mapping.getUnmappedArguments()) { if (arg instanceof PyKeywordArgument) { final String defaultValueText; @@ -84,11 +88,9 @@ public class PyChangeSignatureQuickFix extends LocalQuickFixOnPsiElement { new PyParameterInfo(-1, ((PyKeywordArgument)arg).getKeyword(), defaultValueText, true))); } else { - final TypeEvalContext context = TypeEvalContext.userInitiated(function.getProject(), callExpression.getContainingFile()); - final PyType type = context.getType(arg); - final String typeName = type != null && type.getName() != null ? type.getName() : PyNames.OBJECT; - final String paramName = PyRefactoringUtil.selectUniqueNameFromType(typeName, function.getStatementList()); + final String paramName = generateParameterName(arg, function, usedParamNames, context); newParameters.add(Pair.create(positionalParamAnchor, new PyParameterInfo(-1, paramName, "", false))); + usedParamNames.add(paramName); } } return new PyChangeSignatureQuickFix(function, newParameters, mapping.getCallExpression()); @@ -141,9 +143,9 @@ public class PyChangeSignatureQuickFix extends LocalQuickFixOnPsiElement { final String params = StringUtil.join(createMethodDescriptor(function).getParameters(), info -> { return info.getOldIndex() == -1 ? "" + info.getName() + "" : info.getName(); }, ", "); - return "" + - PyBundle.message("QFIX.change.signature.of", StringUtil.notNullize(function.getName()) + "(" + params + ")") + - ""; + + final String message = PyBundle.message("QFIX.change.signature.of", StringUtil.notNullize(function.getName()) + "(" + params + ")"); + return XmlStringUtil.wrapInHtml(message); } @Nullable @@ -183,6 +185,30 @@ public class PyChangeSignatureQuickFix extends LocalQuickFixOnPsiElement { } } + @NotNull + private static String generateParameterName(@NotNull PyExpression argumentValue, + @NotNull PyFunction function, + @NotNull Set usedParameterNames, + @NotNull TypeEvalContext context) { + PyType type = context.getType(argumentValue); + if (type instanceof PyUnionType) { + type = ContainerUtil.find(((PyUnionType)type).getMembers(), Conditions.instanceOf(PyClassType.class)); + } + final String typeName = type != null && type.getName() != null ? type.getName() : "object"; + + final Collection suggestions = NameSuggesterUtil.generateNamesByType(typeName); + final String shortestName = ContainerUtil.getFirstItem(suggestions); + assert shortestName != null; + + String result = shortestName; + int counter = 1; + while (!PyRefactoringUtil.isValidNewName(result, function.getStatementList()) || usedParameterNames.contains(result)) { + result = shortestName + counter; + counter++; + } + return result; + } + @NotNull private PyMethodDescriptor createMethodDescriptor(final PyFunction function) { return new PyMethodDescriptor(function) { diff --git a/python/testData/inspections/AddSeveralPositionalParametersWithSameArgumentType.py b/python/testData/inspections/AddSeveralPositionalParametersWithSameArgumentType.py new file mode 100644 index 000000000000..fc6951765467 --- /dev/null +++ b/python/testData/inspections/AddSeveralPositionalParametersWithSameArgumentType.py @@ -0,0 +1,6 @@ +def func(i1): + i2 = 'Spam' + + + +func(1, 2, 3) \ No newline at end of file diff --git a/python/testData/inspections/AddSeveralPositionalParametersWithSameArgumentType_after.py b/python/testData/inspections/AddSeveralPositionalParametersWithSameArgumentType_after.py new file mode 100644 index 000000000000..6fc392ed7552 --- /dev/null +++ b/python/testData/inspections/AddSeveralPositionalParametersWithSameArgumentType_after.py @@ -0,0 +1,6 @@ +def func(i1, i, i3): + i2 = 'Spam' + + + +func(1, 2, 3) \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/PyQuickFixTest.java b/python/testSrc/com/jetbrains/python/PyQuickFixTest.java index 50dbf9511cd4..2c18d8f19bce 100644 --- a/python/testSrc/com/jetbrains/python/PyQuickFixTest.java +++ b/python/testSrc/com/jetbrains/python/PyQuickFixTest.java @@ -652,6 +652,11 @@ public class PyQuickFixTest extends PyTestCase { doInspectionTest(PyArgumentListInspection.class, "Change signature of", true, true); } + // PY-8174 + public void testAddSeveralPositionalParametersWithSameArgumentType() { + doInspectionTest(PyArgumentListInspection.class, "Change signature of", true, true); + } + @Override @NonNls protected String getTestDataPath() {