fixed PY-8401 Change Signature: breaks code when chaging default value of an argument before star in case keyword-only arguments

This commit is contained in:
Ekaterina Tuzova
2013-01-11 12:45:10 +04:00
parent 17dfe42c7e
commit 27ce105ee9
10 changed files with 108 additions and 32 deletions

View File

@@ -91,7 +91,7 @@ public class PyChangeSignatureUsageProcessor implements ChangeSignatureUsageProc
final PyArgumentList argumentList = call.getArgumentList();
if (argumentList != null) {
final PyElementGenerator elementGenerator = PyElementGenerator.getInstance(element.getProject());
StringBuilder builder = getSignature(changeInfo, call);
StringBuilder builder = buildSignature((PyChangeInfo)changeInfo, call);
final PyExpression newCall =
elementGenerator.createExpressionFromText(LanguageLevel.forElement(element), builder.toString());
@@ -106,13 +106,13 @@ public class PyChangeSignatureUsageProcessor implements ChangeSignatureUsageProc
return false;
}
private StringBuilder getSignature(ChangeInfo changeInfo, PyCallExpression call) {
private StringBuilder buildSignature(PyChangeInfo changeInfo, PyCallExpression call) {
final PyArgumentList argumentList = call.getArgumentList();
final PyExpression callee = call.getCallee();
String name = callee != null ? callee.getText() : changeInfo.getNewName();
StringBuilder builder = new StringBuilder(name + "(");
if (argumentList != null) {
final ParameterInfo[] newParameters = changeInfo.getNewParameters();
final PyParameterInfo[] newParameters = changeInfo.getNewParameters();
List<String> params = collectParameters(newParameters, argumentList);
builder.append(StringUtil.join(params, ","));
}
@@ -121,7 +121,7 @@ public class PyChangeSignatureUsageProcessor implements ChangeSignatureUsageProc
}
private List<String> collectParameters(final ParameterInfo[] newParameters,
private List<String> collectParameters(final PyParameterInfo[] newParameters,
@NotNull final PyArgumentList argumentList) {
useKeywords = false;
isMethod = false;
@@ -131,30 +131,26 @@ public class PyChangeSignatureUsageProcessor implements ChangeSignatureUsageProc
int currentIndex = 0;
final PyExpression[] arguments = argumentList.getArguments();
for (ParameterInfo info : newParameters) {
for (PyParameterInfo info : newParameters) {
int oldIndex = calculateOldIndex(info);
final String parameterName = info.getName();
if (parameterName.equals(PyNames.CANONICAL_SELF) || parameterName.equals("*")) {
currentIndex += 1;
continue;
}
if (parameterName.startsWith("**")) {
addKwArgs(params, arguments, currentIndex);
currentIndex = addKwArgs(params, arguments, currentIndex);
}
else if (parameterName.startsWith("*")) {
addPositionalContainer(params, arguments, currentIndex);
}
else if (oldIndex == currentIndex && currentIndex < arguments.length) {
addOldPositionParameter(params, arguments[currentIndex], info);
currentIndex = addPositionalContainer(params, arguments, currentIndex);
}
else if (oldIndex < 0) {
addNewParameter(params, info);
currentIndex += 1;
}
else {
moveParameter(params, argumentList, info, currentIndex, oldIndex, arguments);
currentIndex = moveParameter(params, argumentList, info, currentIndex, oldIndex, arguments);
}
currentIndex += 1;
}
return params;
}
@@ -174,26 +170,30 @@ public class PyChangeSignatureUsageProcessor implements ChangeSignatureUsageProc
}
private static void addPositionalContainer(List<String> params,
PyExpression[] arguments,
int index) {
private static int addPositionalContainer(List<String> params,
PyExpression[] arguments,
int index) {
for (int i = index; i != arguments.length; ++i) {
if (!(arguments[i] instanceof PyKeywordArgument)) {
params.add(arguments[i].getText());
index += 1;
}
}
return index;
}
private static void addKwArgs(List<String> params, PyExpression[] arguments, int index) {
private static int addKwArgs(List<String> params, PyExpression[] arguments, int index) {
for (int i = index; i < arguments.length; ++i) {
if (arguments[i] instanceof PyKeywordArgument) {
params.add(arguments[i].getText());
index += 1;
}
}
return index;
}
private void addNewParameter(List<String> params, ParameterInfo info) {
if (((PyParameterInfo)info).getDefaultInSignature()) {
private void addNewParameter(List<String> params, PyParameterInfo info) {
if (info.getDefaultInSignature()) {
useKeywords = true;
}
else {
@@ -201,46 +201,60 @@ public class PyChangeSignatureUsageProcessor implements ChangeSignatureUsageProc
}
}
private void moveParameter(List<String> params,
/**
* @return current index in argument list
*/
private int moveParameter(List<String> params,
PyArgumentList argumentList,
ParameterInfo info,
PyParameterInfo info,
int currentIndex,
int oldIndex,
PyExpression[] arguments) {
final PyKeywordArgument keywordArgument = argumentList.getKeywordArgument(info.getName());
final String paramName = info.getName();
final PyKeywordArgument keywordArgument = argumentList.getKeywordArgument(paramName);
if (keywordArgument != null) {
params.add(keywordArgument.getText());
return currentIndex + 1;
}
else if (currentIndex < arguments.length) {
final PyExpression currentParameter = arguments[currentIndex];
if (currentParameter instanceof PyKeywordArgument &&
!info.getName().equals(((PyKeywordArgument)currentParameter).getKeyword())) {
if (currentParameter instanceof PyKeywordArgument && info.isRenamed()) {
params.add(currentParameter.getText());
}
else if (oldIndex < arguments.length) {
else if (oldIndex < arguments.length && !info.getDefaultInSignature() || !(currentParameter instanceof PyKeywordArgument)) {
addOldPositionParameter(params, arguments[oldIndex], info);
}
else
return currentIndex;
}
else if (oldIndex < arguments.length) {
addOldPositionParameter(params, arguments[oldIndex], info);
}
else if (!((PyParameterInfo)info).getDefaultInSignature()) {
params.add( useKeywords ? info.getName() + " = " + info.getDefaultValue()
else if (!info.getDefaultInSignature()) {
params.add( useKeywords ? paramName + " = " + info.getDefaultValue()
: info.getDefaultValue());
}
else {
useKeywords = true;
return currentIndex;
}
return currentIndex + 1;
}
private void addOldPositionParameter(List<String> params,
PyExpression argument,
ParameterInfo info) {
PyParameterInfo info) {
final String paramName = info.getName();
if (argument instanceof PyKeywordArgument) {
final PyExpression valueExpression = ((PyKeywordArgument)argument).getValueExpression();
params.add(valueExpression == null ? paramName : paramName + " = " + valueExpression.getText());
useKeywords = true;
if (!paramName.equals(argument.getName()) && !StringUtil.isEmptyOrSpaces(info.getDefaultValue())
&& !info.getDefaultInSignature())
params.add(useKeywords ? info.getName() + " = " + info.getDefaultValue() : info.getDefaultValue());
else {
params.add(valueExpression == null ? paramName : paramName + " = " + valueExpression.getText());
useKeywords = true;
}
}
else {
params.add(useKeywords ? paramName + " = " + argument.getText() : argument.getText());

View File

@@ -12,6 +12,7 @@ public class PyParameterInfo implements ParameterInfo {
private final int myOldIndex;
private String myName = "";
private String myOldName = "";
private String myDefaultValue = null;
private boolean myDefaultInSignature = false;
@@ -22,6 +23,7 @@ public class PyParameterInfo implements ParameterInfo {
public PyParameterInfo(int oldIndex, String name, @Nullable String defaultValue, boolean defaultInSignature) {
myOldIndex = oldIndex;
myName = name;
myOldName = name;
myDefaultValue = defaultValue;
myDefaultInSignature = defaultInSignature;
}
@@ -32,6 +34,11 @@ public class PyParameterInfo implements ParameterInfo {
return myName;
}
@NotNull
public String getOldName() {
return myOldName;
}
@Override
public int getOldIndex() {
return myOldIndex;
@@ -74,4 +81,8 @@ public class PyParameterInfo implements ParameterInfo {
public void setDefaultInSignature(boolean defaultInSignature) {
myDefaultInSignature = defaultInSignature;
}
public boolean isRenamed() {
return !myOldName.equals(myName);
}
}

View File

@@ -0,0 +1,5 @@
def f(my, *, param):
pass
f(None, param=1)

View File

@@ -0,0 +1,5 @@
def<caret> f(my=None, *, param):
pass
f(param=1)

View File

@@ -0,0 +1,5 @@
def f(my, *, param=1):
pass
f(None, param=1)

View File

@@ -0,0 +1,5 @@
def<caret> f(my=None, *, param=1):
pass
f(param=1)

View File

@@ -0,0 +1,5 @@
def f(my=None, *, param):
pass
f(param=1)

View File

@@ -0,0 +1,5 @@
def<caret> f(my=None, *, param=1):
pass
f(param=1)

View File

@@ -2,4 +2,4 @@ def func(a, b):
pass
func(a=1, b=2)
func(a=1, 2)

View File

@@ -59,7 +59,10 @@ public class PyChangeSignatureTest extends PyTestCase {
}
public void testUpdateDocstring() {
doChangeSignatureTest(null, Arrays.asList(new PyParameterInfo(0, "a", null, false), new PyParameterInfo(1, "d1", "1", true)));
final PyParameterInfo a = new PyParameterInfo(0, "a", null, false);
final PyParameterInfo d1 = new PyParameterInfo(1, "d", "1", true);
d1.setName("d1");
doChangeSignatureTest(null, Arrays.asList(a, d1));
}
public void testFixDocstringRemove() {
@@ -91,6 +94,24 @@ public class PyChangeSignatureTest extends PyTestCase {
new PyParameterInfo(-1, "a", "2", false)), LanguageLevel.PYTHON32);
}
public void testKeywordOnlyParamRemoveDefaultValue() {
doChangeSignatureTest(null, Arrays.asList(new PyParameterInfo(0, "my", "None", false),
new PyParameterInfo(1, "*", null, false),
new PyParameterInfo(2, "param", null, false)), LanguageLevel.PYTHON32);
}
public void testKeywordOnlyParamRemoveDefaultValue1() {
doChangeSignatureTest(null, Arrays.asList(new PyParameterInfo(0, "my", "None", false),
new PyParameterInfo(1, "*", null, false),
new PyParameterInfo(2, "param", "1", true)), LanguageLevel.PYTHON32);
}
public void testKeywordOnlyParamRemoveDefaultValue2() {
doChangeSignatureTest(null, Arrays.asList(new PyParameterInfo(0, "my", "None", true),
new PyParameterInfo(1, "*", null, false),
new PyParameterInfo(2, "param", "1", false)), LanguageLevel.PYTHON32);
}
public void testRenameOverriding() {
doChangeSignatureTest("m1", Arrays.asList(new PyParameterInfo(0, "self", null, false)));
}