For #PY-2748 Fix false positives, rename issues, add tests and minor fixes according to review

* Don't process renaming string literal expression if it's PyKeyValueExpression: it corrupts user's code by renaming only key-value expression and leaving substitution key the same
* Pass isPercentString as constructor parameter, use PyPsiUtil.flattenParens(), rename method(process...-> resolve...)
* Fix false positive with function call and add tests
* In percent string resolve to all literal expressions, not only string literal
* In RenamePyLiteralExpressionProcessor always throw IncorrectOperationException
This commit is contained in:
Valentina Kiryushkina
2016-03-09 13:33:27 +03:00
parent d226e3bf44
commit baee11599d
22 changed files with 205 additions and 61 deletions
@@ -23,6 +23,7 @@ import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.ArrayUtil;
import com.jetbrains.python.inspections.PyStringFormatParser;
import com.jetbrains.python.psi.*;
import com.jetbrains.python.psi.impl.PyPsiUtils;
import com.jetbrains.python.psi.types.TypeEvalContext;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -30,13 +31,15 @@ import org.jetbrains.annotations.Nullable;
public class PySubstitutionChunkReference extends PsiReferenceBase<PyStringLiteralExpression> implements PsiReferenceEx{
private final int myPosition;
private final PyStringFormatParser.SubstitutionChunk myChunk;
private final boolean myIsPercent;
private boolean myIgnoreUnresolved = false;
public PySubstitutionChunkReference(@NotNull final PyStringLiteralExpression element,
@NotNull final PyStringFormatParser.SubstitutionChunk chunk, final int position) {
@NotNull final PyStringFormatParser.SubstitutionChunk chunk, final int position, boolean isPercent) {
super(element, getKeyWordRange(element, chunk));
myChunk = chunk;
myPosition = position;
myIsPercent = isPercent;
}
@Nullable
@@ -64,8 +67,7 @@ public class PySubstitutionChunkReference extends PsiReferenceBase<PyStringLiter
@Nullable
@Override
public PsiElement resolve() {
boolean isPercentString = myElement.getParent() instanceof PyBinaryExpression;
if (isPercentString) {
if (myIsPercent) {
return resolvePercentString();
}
else {
@@ -91,7 +93,7 @@ public class PySubstitutionChunkReference extends PsiReferenceBase<PyStringLiter
if (position < arguments.length) return arguments[position];
if (arguments[0] instanceof PyBinaryExpression && ((PyBinaryExpression)arguments[0]).isOperator("+")) {
return processNotNestedBinaryExpression((PyBinaryExpression)arguments[0]);
return resolveNotNestedBinaryExpression((PyBinaryExpression)arguments[0]);
}
}
}
@@ -117,7 +119,7 @@ public class PySubstitutionChunkReference extends PsiReferenceBase<PyStringLiter
private PsiElement resolveKeyword(PyExpression pyExpression) {
PyExpression expression = pyExpression;
if (pyExpression instanceof PyParenthesizedExpression) {
expression = getContainedExpression((PyParenthesizedExpression)pyExpression);
expression = PyPsiUtils.flattenParens(pyExpression);
}
myIgnoreUnresolved = expression instanceof PyReferenceExpression;
@@ -130,6 +132,9 @@ public class PySubstitutionChunkReference extends PsiReferenceBase<PyStringLiter
}
}
}
else if (expression instanceof PyCallExpression) {
return resolveCallExpression((PyCallExpression)expression);
}
return null;
}
@@ -137,34 +142,37 @@ public class PySubstitutionChunkReference extends PsiReferenceBase<PyStringLiter
@Nullable
private PsiElement resolvePositional(PyExpression expression) {
PsiElement result = null;
PyExpression containedExpression = expression;
if (expression instanceof PyParenthesizedExpression) {
final PyExpression containedExpression = getContainedExpression((PyParenthesizedExpression)expression);
if (containedExpression instanceof PyTupleExpression) {
final PyExpression[] elements = ((PySequenceExpression)containedExpression).getElements();
if (elements.length > myPosition) {
result = elements[myPosition];
}
}
else if (containedExpression instanceof PyBinaryExpression && ((PyBinaryExpression)containedExpression).isOperator("+")) {
result = processNotNestedBinaryExpression((PyBinaryExpression)containedExpression);
}
else if (containedExpression instanceof PyReferenceExpression) {
myIgnoreUnresolved = true;
containedExpression = PyPsiUtils.flattenParens(expression);
}
if (containedExpression instanceof PyTupleExpression) {
final PyExpression[] elements = ((PySequenceExpression)containedExpression).getElements();
if (elements.length > myPosition) {
result = elements[myPosition];
}
}
else if (expression instanceof PyReferenceExpression) {
else if (containedExpression instanceof PyBinaryExpression && ((PyBinaryExpression)containedExpression).isOperator("+")) {
result = resolveNotNestedBinaryExpression((PyBinaryExpression)containedExpression);
}
else if (containedExpression instanceof PyLiteralExpression && myPosition == 0) {
return expression;
}
else if (containedExpression instanceof PyCallExpression) {
return resolveCallExpression((PyCallExpression)expression);
}
else if (containedExpression instanceof PyReferenceExpression) {
myIgnoreUnresolved = true;
}
return result;
}
@Nullable
private PsiElement processNotNestedBinaryExpression(PyBinaryExpression containedExpression) {
private PsiElement resolveNotNestedBinaryExpression(PyBinaryExpression containedExpression) {
PyExpression left = containedExpression.getLeftExpression();
PyExpression right = containedExpression.getRightExpression();
if (left instanceof PyParenthesizedExpression) {
PyExpression leftTuple = getContainedExpression((PyParenthesizedExpression)left);
PyExpression leftTuple = PyPsiUtils.flattenParens(left);
if (leftTuple instanceof PyTupleExpression) {
PyExpression[] leftTupleElements = ((PyTupleExpression)leftTuple).getElements();
int leftTupleLength = leftTupleElements.length;
@@ -172,7 +180,7 @@ public class PySubstitutionChunkReference extends PsiReferenceBase<PyStringLiter
return leftTupleElements[myPosition];
}
if (right instanceof PyParenthesizedExpression) {
PyExpression rightTuple = ((PyParenthesizedExpression)right).getContainedExpression();
PyExpression rightTuple = PyPsiUtils.flattenParens(right);
if (rightTuple instanceof PyTupleExpression) {
PyExpression[] rightTupleElements = ((PyTupleExpression)rightTuple).getElements();
int rightLength = rightTupleElements.length;
@@ -185,15 +193,6 @@ public class PySubstitutionChunkReference extends PsiReferenceBase<PyStringLiter
return null;
}
@Nullable
private static PyExpression getContainedExpression(@NotNull final PyParenthesizedExpression parenthesizedExpression) {
PyExpression containedExpression = parenthesizedExpression.getContainedExpression();
while (containedExpression instanceof PyParenthesizedExpression) {
containedExpression = ((PyParenthesizedExpression)containedExpression).getContainedExpression();
}
return containedExpression;
}
@Nullable
private static PyArgumentList getArgumentList(final PsiElement original) {
final PsiElement pyReferenceExpression = PsiTreeUtil.getParentOfType(original, PyReferenceExpression.class);
@@ -218,6 +217,10 @@ public class PySubstitutionChunkReference extends PsiReferenceBase<PyStringLiter
return key;
}
}
else {
myIgnoreUnresolved = true;
break;
}
}
}
else {
@@ -227,7 +230,7 @@ public class PySubstitutionChunkReference extends PsiReferenceBase<PyStringLiter
elements = ((PyListLiteralExpression)pyExpression).getElements();
}
else if (pyExpression instanceof PyParenthesizedExpression) {
PyExpression expression = getContainedExpression((PyParenthesizedExpression)pyExpression);
PyExpression expression = PyPsiUtils.flattenParens(pyExpression);
if (expression instanceof PyTupleExpression) {
elements = ((PyTupleExpression)expression).getElements();
}
@@ -238,12 +241,35 @@ public class PySubstitutionChunkReference extends PsiReferenceBase<PyStringLiter
}
}
}
else if ((pyExpression = PsiTreeUtil.getChildOfType(args[0], PyCallExpression.class)) != null) {
return resolveCallExpression((PyCallExpression)pyExpression);
}
else if (PsiTreeUtil.getChildOfType(args[0], PyReferenceExpression.class) != null) {
myIgnoreUnresolved = true;
}
}
return null;
}
@Nullable
private PyExpression resolveCallExpression(PyCallExpression pyExpression) {
PyExpression callee = pyExpression.getCallee();
if (callee != null) {
String name = callee.getName();
if ("dict".equals(name)) {
PyArgumentList list = pyExpression.getArgumentList();
if (list != null) {
return list.getKeywordArgument(myChunk.getMappingKey());
}
}
else {
myIgnoreUnresolved = true;
}
}
return null;
}
@NotNull
@Override
public Object[] getVariants() {
@@ -39,23 +39,24 @@ public class PythonFormattedStringReferenceProvider extends PsiReferenceProvider
private static PsiReference[] getReferencesFromFormatString(@NotNull final PyStringLiteralExpression element) {
final List<PyStringFormatParser.SubstitutionChunk> chunks = PyStringFormatParser.filterSubstitutions(
PyStringFormatParser.parseNewStyleFormat(element.getStringValue()));
return getReferencesFromChunks(element, chunks);
return getReferencesFromChunks(element, chunks, false);
}
private static PsiReference[] getReferencesFromPercentString(@NotNull final PyStringLiteralExpression element) {
final List<PyStringFormatParser.SubstitutionChunk>
chunks = PyStringFormatParser.filterSubstitutions(PyStringFormatParser.parsePercentFormat(element.getStringValue()));
return getReferencesFromChunks(element, chunks);
return getReferencesFromChunks(element, chunks, true);
}
@NotNull
private static PsiReference[] getReferencesFromChunks(@NotNull final PyStringLiteralExpression element,
@NotNull final List<PyStringFormatParser.SubstitutionChunk> chunks) {
@NotNull final List<PyStringFormatParser.SubstitutionChunk> chunks,
boolean isPercent) {
final PsiReference[] result = new PsiReference[chunks.size()];
if (!element.isDocString()) {
for (int i = 0; i < chunks.size(); i++) {
final PyStringFormatParser.SubstitutionChunk chunk = chunks.get(i);
result[i] = new PySubstitutionChunkReference(element, chunk, i);
result[i] = new PySubstitutionChunkReference(element, chunk, i, isPercent);
}
}
return result;
@@ -17,18 +17,16 @@ package com.jetbrains.python.refactoring.rename;
import com.intellij.openapi.editor.Editor;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiReference;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.refactoring.listeners.RefactoringElementListener;
import com.intellij.usageView.UsageInfo;
import com.intellij.util.IncorrectOperationException;
import com.jetbrains.python.codeInsight.PyCodeInsightSettings;
import com.jetbrains.python.psi.*;
import com.jetbrains.python.psi.PyLiteralExpression;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
public class RenamePyLiteralExpressionProcessor extends RenamePyElementProcessor {
private static final Class[] UNSUPPORTED = {PyNumericLiteralExpression.class, PyNoneLiteralExpression.class, PyBoolLiteralExpression.class};
@Override
public boolean canProcessElement(@NotNull PsiElement element) {
return PsiTreeUtil.instanceOf(element, PyLiteralExpression.class);
@@ -37,13 +35,7 @@ public class RenamePyLiteralExpressionProcessor extends RenamePyElementProcessor
@Override
public void renameElement(PsiElement element, String newName, UsageInfo[] usages, @Nullable RefactoringElementListener listener)
throws IncorrectOperationException {
if (PsiTreeUtil.instanceOf(element, UNSUPPORTED)) throw new IncorrectOperationException();
((PyStringLiteralExpression)element).updateText("\"" + newName + "\"");
for (UsageInfo usageInfo: usages) {
PsiReference reference = usageInfo.getReference();
if (reference == null) return;
reference.handleElementRename("\"" + newName + "\"");
}
throw new IncorrectOperationException();
}
@Override
@@ -0,0 +1 @@
'{<warning descr="Unresolved reference 'foo'">foo</warning>}'.format(boo=1)
@@ -0,0 +1 @@
'{<warning descr="Unresolved reference 'foo'">foo</warning>}'.format(**{"boo": 1})
@@ -0,0 +1 @@
'{<warning descr="Unresolved reference 'foo'">foo</warning>}'.format(**dict(t=1))
@@ -0,0 +1,5 @@
def f():
return dict(foo=0)
'{foo}'.format(**f())
@@ -0,0 +1,2 @@
ref = {"fst": 1, "snd": 2}
print "first is {fst}, second is {snd}".format(**ref)
@@ -0,0 +1 @@
v = '<warning descr="Unresolved reference '{}'">{}</warning>'.format()
@@ -0,0 +1,4 @@
def f():
return [1]
"%s" % f()
@@ -0,0 +1 @@
v = "first is %(<warning descr="Unresolved reference 'fst'">fst</warning>)s" % {"snd": 2}
@@ -0,0 +1,2 @@
d = {"fst": 1, "snd": 2}
print "first is %(fst)s, second is %(snd)s" % d
@@ -0,0 +1 @@
"I want to rename this{to_be_r<caret>enamed}".format(**{"to_be_renamed": "value"})
@@ -0,0 +1 @@
print "first is {<caret>}, second is {}".format(1, 2)
@@ -1 +0,0 @@
print "first is {<ref>fst}, second is {snd}".format(**{"fst": "f", "snd": "s"})
@@ -0,0 +1 @@
'{<ref>foo}'.format(**dict(foo="fo"))
@@ -0,0 +1 @@
"first is %(<ref>fst)s" % dict(fst="hello")
@@ -0,0 +1 @@
v = "<ref>%s" % "hello"
@@ -611,70 +611,75 @@ public class PyResolveTest extends PyResolveTestCase {
assertResolvesTo(PyFunction.class, "__rmatmul__");
}
//PY-2478
//PY-2748
public void testFormatStringKWArgs() {
PsiElement target = resolve();
assertTrue(target instanceof PyKeywordArgument);
assertEquals("fst", ((PyKeywordArgument)target).getKeyword());
}
//PY-2478
//PY-2748
public void testFormatPositionalArgs() {
PsiElement target = resolve();
assertTrue(target instanceof PyReferenceExpression);
assertEquals("string", target.getText());
}
//PY-2478
//PY-2748
public void testFormatArgsAndKWargs() {
PsiElement target = resolve();
assertTrue(target instanceof PyStringLiteralExpression);
}
//PY-2478
//PY-2748
public void testFormatArgsAndKWargs1() {
PsiElement target = resolve();
assertTrue(target instanceof PyKeywordArgument);
assertEquals("kwd", ((PyKeywordArgument)target).getKeyword());
}
//PY-2748
public void testFormatStringWithPackedDictAsArgument() {
PsiElement target = resolve();
assertTrue(target instanceof PyStringLiteralExpression);
assertEquals("\"fst\"", target.getText());
}
//PY-2748
public void testFormatStringWithPackedListAsArgument() {
PsiElement target = resolve();
assertTrue(target instanceof PyNumericLiteralExpression);
assertEquals("1", target.getText());
}
//PY-2748
public void testFormatStringWithPackedTupleAsArgument() {
PsiElement target = resolve();
assertTrue(target instanceof PyStringLiteralExpression);
assertEquals("\"snd\"", target.getText());
}
//PY-2748
public void testFormatStringWithBinExprAsArg() {
PsiElement target = resolve();
assertTrue(target instanceof PyStringLiteralExpression);
assertEquals("\"snd\"", target.getText());
}
//PY-2748
public void testFormatStringWithRefAsArgument() {
PsiElement target = resolve();
assertEquals(null, target);
}
//PY-2478
//PY-2748
public void testPercentPositionalArgs() {
PsiElement target = resolve();
assertTrue(target instanceof PyStringLiteralExpression);
}
//PY-2478
//PY-2748
public void testPercentKeyWordArgs() {
PsiElement target = resolve();
assertTrue(target instanceof PyStringLiteralExpression);
@@ -692,25 +697,44 @@ public class PyResolveTest extends PyResolveTestCase {
assertEquals("snd", ((PyStringLiteralExpression)target).getStringValue());
}
//PY-2478
//PY-2748
public void testPercentStringBinaryStatementArg() {
PsiElement target = resolve();
assertTrue(target instanceof PyStringLiteralExpression);
assertEquals("1", ((PyStringLiteralExpression)target).getStringValue());
}
//PY-2478
//PY-2748
public void testPercentStringArgWithRedundantParentheses() {
PsiElement target = resolve();
assertTrue(target instanceof PyStringLiteralExpression);
assertEquals("1", ((PyStringLiteralExpression)target).getStringValue());
}
//PY-2748
public void testPercentStringWithRefAsArgument() {
PsiElement target = resolve();
assertEquals(null, target);
}
//PY-2748
public void testPercentStringWithOneStringArgument() {
PsiElement target = resolve();
assertEquals("hello", ((PyStringLiteralExpression)target).getStringValue());
}
//PY-2748
public void testFormatStringPackedDictCall() {
PsiElement target = resolve();
assertEquals("fo", ((PyStringLiteralExpression)((PyKeywordArgument)target).getValueExpression()).getStringValue());
}
//PY-2748
public void testPercentStringDictCall() {
PsiElement target = resolve();
assertEquals("hello", ((PyStringLiteralExpression)((PyKeywordArgument)target).getValueExpression()).getStringValue());
}
public void testGlobalNotDefinedAtTopLevel() {
assertResolvesTo(PyTargetExpression.class, "foo");
@@ -537,6 +537,56 @@ public class PyUnresolvedReferencesInspectionTest extends PyInspectionTestCase {
doTest();
}
// PY-2748
public void testFormatStringPackedDictCall() {
doTest();
}
// PY-2748
public void testFormatStringPackedDict() {
doTest();
}
// PY-2748
public void testFormatStringPositional() {
doTest();
}
// PY-2748
public void testFormatStringKeyword() {
doTest();
}
// PY-2748
public void testPercentStringPositional() {
doTest();
}
// PY-2748
public void testPercentStringKeyword() {
doTest();
}
// PY-2748
public void testFormatStringPackedFunctionCall() {
doTest();
}
// PY-2748
public void testPercentStringFunctionCall() {
doTest();
}
// PY-2748
public void testFormatStringPackedReference() {
doTest();
}
// PY-2748
public void testPercentStringReference() {
doTest();
}
// PY-18254
public void testVarargsAnnotatedWithFunctionComment() {
doTest();
@@ -236,11 +236,39 @@ public class PyRenameTest extends PyTestCase {
renameWithDocStringFormat(DocStringFormat.NUMPY, "bar");
}
//PY-2478
//PY-2748
public void testFormatStringKeyword() {
doTest("renamed");
}
//PY-2748
public void testFormatStringDictLiteral() {
myFixture.configureByFile(RENAME_DATA_PATH + getTestName(true) + ".py");
try {
myFixture.renameElementAtCaret("renamed");
}
catch (RuntimeException e) {
if ("com.intellij.util.IncorrectOperationException".equals(e.getMessage())) {
return;
}
}
fail();
}
//PY-2748
public void testFormatStringNumericLiteralExpression() {
myFixture.configureByFile(RENAME_DATA_PATH + getTestName(true) + ".py");
try {
myFixture.renameElementAtCaret("renamed");
}
catch (RuntimeException e) {
if ("com.intellij.util.IncorrectOperationException".equals(e.getMessage())) {
return;
}
}
fail();
}
private void renameWithDocStringFormat(DocStringFormat format, final String newName) {
runWithDocStringFormat(format, new Runnable() {
public void run() {