From 4fafd9f0588d0a2b2e046c0f8e72e1b0e4599da3 Mon Sep 17 00:00:00 2001 From: Valentina Kiryushkina Date: Fri, 25 Mar 2016 17:54:47 +0300 Subject: [PATCH] Fixes according to review IDEA-CR-9573 * Fix problems found in review and add tests * Fix problems with positional substitution resolving, where arguments consist of positional and packed arguments and add tests * Remove doc string check --- .../PySubstitutionChunkReference.java | 277 ++++++++++-------- ...ythonFormattedStringReferenceProvider.java | 2 - .../PyUnresolvedReferencesInspection.java | 6 +- ...gDictLiteralArgumentWithNumericExprKeys.py | 2 +- .../formatStringPackedDict.py | 1 - .../formatStringPackedDictCall.py | 1 - ...StringPositionalSubstitutionWithDictArg.py | 1 + .../formatStringWithDictArgWithCallExprKey.py | 3 + ...StringWithDictLiteralExprInsideDictCall.py | 1 + .../formatStringWithEmptyDictArg.py | 1 + .../formatStringWithPackedAndNonPackedArgs.py | 4 + ...ordArgumentWithReferenceKeyDictArgument.py | 2 + .../percentStringWithCallArgument.py | 3 + .../resolve/FormatStringWithBinExprAsArg.py | 1 - .../FormatStringWithPackedAndNonPackedArgs.py | 1 + .../PercentStringWithOneStringArgument.py | 2 +- .../com/jetbrains/python/PyResolveTest.java | 18 +- .../PyUnresolvedReferencesInspectionTest.java | 48 ++- 18 files changed, 220 insertions(+), 154 deletions(-) delete mode 100644 python/testData/inspections/PyUnresolvedReferencesInspection/formatStringPackedDict.py delete mode 100644 python/testData/inspections/PyUnresolvedReferencesInspection/formatStringPackedDictCall.py create mode 100644 python/testData/inspections/PyUnresolvedReferencesInspection/formatStringPositionalSubstitutionWithDictArg.py create mode 100644 python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithDictArgWithCallExprKey.py create mode 100644 python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithDictLiteralExprInsideDictCall.py create mode 100644 python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithEmptyDictArg.py create mode 100644 python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithPackedAndNonPackedArgs.py create mode 100644 python/testData/inspections/PyUnresolvedReferencesInspection/percentStringKeywordArgumentWithReferenceKeyDictArgument.py create mode 100644 python/testData/inspections/PyUnresolvedReferencesInspection/percentStringWithCallArgument.py delete mode 100644 python/testData/resolve/FormatStringWithBinExprAsArg.py create mode 100644 python/testData/resolve/FormatStringWithPackedAndNonPackedArgs.py diff --git a/python/src/com/jetbrains/python/codeInsight/PySubstitutionChunkReference.java b/python/src/com/jetbrains/python/codeInsight/PySubstitutionChunkReference.java index 71e96f63be14..866df0fe1845 100644 --- a/python/src/com/jetbrains/python/codeInsight/PySubstitutionChunkReference.java +++ b/python/src/com/jetbrains/python/codeInsight/PySubstitutionChunkReference.java @@ -15,7 +15,9 @@ */ package com.jetbrains.python.codeInsight; +import com.google.common.collect.Iterables; import com.intellij.lang.annotation.HighlightSeverity; +import com.intellij.openapi.util.Ref; import com.intellij.openapi.util.TextRange; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiReferenceBase; @@ -28,15 +30,18 @@ import com.jetbrains.python.psi.types.TypeEvalContext; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + public class PySubstitutionChunkReference extends PsiReferenceBase implements PsiReferenceEx{ private final int myPosition; - private final PyStringFormatParser.SubstitutionChunk myChunk; + @NotNull private final PyStringFormatParser.SubstitutionChunk myChunk; private final boolean myIsPercent; - private boolean myIgnoreUnresolved = true; public PySubstitutionChunkReference(@NotNull final PyStringLiteralExpression element, @NotNull final PyStringFormatParser.SubstitutionChunk chunk, final int position, boolean isPercent) { - super(element, getKeyWordRange(element, chunk)); + super(element, getKeywordRange(element, chunk)); myChunk = chunk; myPosition = position; myIsPercent = isPercent; @@ -54,7 +59,8 @@ public class PySubstitutionChunkReference extends PsiReferenceBase keywordStarArgs = getStarArguments(argumentList, true); + boolean notSureAboutStarArgs = false; + for (PyStarArgument arg : keywordStarArgs) { + final Ref resolvedRef = resolveKeywordStarExpression(arg); + if (resolvedRef != null) { + final PsiElement resolved = resolvedRef.get(); + if (resolved != null) { + return resolved; + } + } + else { + notSureAboutStarArgs = true; + } + } + return notSureAboutStarArgs ? Iterables.getFirst(keywordStarArgs, null) : null; + } + } + + @NotNull + private static List getStarArguments(@NotNull PyArgumentList argumentList, boolean isKeyword) { + return Arrays.asList(argumentList.getArguments()).stream() + .map(expression -> PyUtil.as(expression, PyStarArgument.class)) + .filter(argument -> argument != null && argument.isKeyword() == isKeyword).collect(Collectors.toList()); } @Nullable private PsiElement resolvePercentString() { - PsiElement result = null; - final PyBinaryExpression binaryExpression = PsiTreeUtil.getParentOfType(getElement(), PyBinaryExpression.class); if (binaryExpression != null) { final PyExpression rightExpression = binaryExpression.getRightExpression(); - + if (rightExpression == null) { + return null; + } boolean isKeyWordSubstitution = myChunk.getMappingKey() != null; - result = isKeyWordSubstitution? resolveKeyword(rightExpression) : resolvePositional(rightExpression); + return isKeyWordSubstitution ? resolveKeywordPercent(rightExpression) : resolvePositionalPercent(rightExpression); } - return result; - } - - @Nullable - private PsiElement resolveKeyword(PyExpression pyExpression) { - PyExpression expression = pyExpression; - if (pyExpression instanceof PyParenthesizedExpression) { - expression = PyPsiUtils.flattenParens(pyExpression); - } - - if (expression instanceof PyDictLiteralExpression) { - return resolveDictLiteralExpression((PyDictLiteralExpression)expression); - } - else if (expression instanceof PyCallExpression) { - return resolveCallExpressionForKeywordSubstitution((PyCallExpression)expression); - } - return null; } @Nullable - private PsiElement resolvePositional(PyExpression expression) { - PyExpression containedExpression = expression; - if (expression instanceof PyParenthesizedExpression) { - containedExpression = PyPsiUtils.flattenParens(expression); + private PsiElement resolveKeywordPercent(@NotNull PyExpression expression) { + final PyExpression containedExpr = PyPsiUtils.flattenParens(expression); + if (containedExpr instanceof PyDictLiteralExpression) { + final Ref resolvedRef = resolveDictLiteralExpression((PyDictLiteralExpression)containedExpr); + return resolvedRef != null ? resolvedRef.get() : containedExpr; } - PsiElement result = null; - + else if (containedExpr instanceof PyLiteralExpression) { + return null; + } + else if (containedExpr instanceof PyCallExpression) { + return resolveDictCall((PyCallExpression)containedExpr); + } + return containedExpr; + } + + @Nullable + private PsiElement resolvePositionalPercent(@NotNull PyExpression expression) { + final PyExpression containedExpression = PyPsiUtils.flattenParens(expression); if (containedExpression instanceof PyTupleExpression) { - myIgnoreUnresolved = false; - final PyExpression[] elements = ((PySequenceExpression)containedExpression).getElements(); - if (elements.length > myPosition) { - result = elements[myPosition]; - } + final PyExpression[] elements = ((PyTupleExpression)containedExpression).getElements(); + return myPosition < elements.length ? elements[myPosition] : null; } else if (containedExpression instanceof PyBinaryExpression && ((PyBinaryExpression)containedExpression).isOperator("+")) { - result = resolveNotNestedBinaryExpression((PyBinaryExpression)containedExpression); + return resolveNotNestedBinaryExpression((PyBinaryExpression)containedExpression); } - else if (myPosition == 0) { - result = containedExpression; + else if (containedExpression instanceof PyCallExpression) { + final PyExpression callee = ((PyCallExpression)containedExpression).getCallee(); + if (callee != null && "dict".equals(callee.getName()) && myPosition != 0) { + return null; + } } - else { - myIgnoreUnresolved = false; + else if (myPosition != 0 && PsiTreeUtil.instanceOf(containedExpression, PyLiteralExpression.class)) { + return null; } - return result; + return containedExpression; } @Nullable @@ -188,7 +221,7 @@ public class PySubstitutionChunkReference extends PsiReferenceBase resolveKeywordStarExpression(@NotNull PyStarArgument starArgument) { + final PyDictLiteralExpression dictExpr = PsiTreeUtil.getChildOfType(starArgument, PyDictLiteralExpression.class); + return dictExpr != null ? resolveDictLiteralExpression(dictExpr) : null; + } + + @Nullable + private PsiElement resolvePositionalStarExpression(@NotNull PyStarArgument starArgument, int argumentPosition) { + final PyExpression expr = PsiTreeUtil.getChildOfAnyType(starArgument, PyListLiteralExpression.class, PyParenthesizedExpression.class); + if (expr == null) { + return starArgument; } - return null; + final int position = (myChunk.getPosition() != null ? myChunk.getPosition() : myPosition) - argumentPosition; + final PyExpression[] elements; + if (expr instanceof PyListLiteralExpression) { + elements = ((PyListLiteralExpression)expr).getElements(); + } + else if (expr instanceof PyParenthesizedExpression) { + final PyExpression expression = PyPsiUtils.flattenParens(expr); + final PyTupleExpression tupleExpr = PyUtil.as(expression, PyTupleExpression.class); + if (tupleExpr == null) { + return starArgument; + } + elements = tupleExpr.getElements(); + } + else { + elements = null; + } + return elements != null && position < elements.length ? elements[position] : null; } @Nullable - private PsiElement resolveDictLiteralExpression(PyDictLiteralExpression expression) { + private Ref resolveDictLiteralExpression(PyDictLiteralExpression expression) { final PyKeyValueExpression[] keyValueExpressions = expression.getElements(); - for (PyKeyValueExpression keyValueExpression: keyValueExpressions) { + if (keyValueExpressions.length == 0) { + return Ref.create(); + } + boolean allKeysForSure = true; + for (PyKeyValueExpression keyValueExpression : keyValueExpressions) { PyExpression keyExpression = keyValueExpression.getKey(); if (keyExpression instanceof PyStringLiteralExpression) { - myIgnoreUnresolved = false; final PyStringLiteralExpression key = (PyStringLiteralExpression)keyExpression; if (key.getStringValue().equals(myChunk.getMappingKey())) { - return key; + return Ref.create(key); } } + else if (!(keyExpression instanceof PyLiteralExpression)) { + allKeysForSure = false; + } } - return null; + return allKeysForSure ? Ref.create() : null; } @Nullable - private PyExpression resolveCallExpressionForKeywordSubstitution(PyCallExpression pyExpression) { - PyExpression callee = pyExpression.getCallee(); + private PsiElement resolveDictCall(@NotNull PyCallExpression expression) { + final PyExpression callee = expression.getCallee(); if (callee != null) { - String name = callee.getName(); - if ("dict".equals(name)) { - myIgnoreUnresolved = false; - PyArgumentList list = pyExpression.getArgumentList(); - if (list != null) { - return list.getKeywordArgument(myChunk.getMappingKey()); + final String name = callee.getName(); + if ("dict".equals(name)) { + for (PyExpression arg : expression.getArguments()) { + if (!(arg instanceof PyKeywordArgument)) { + return expression; } } + final PyArgumentList argumentList = expression.getArgumentList(); + if (argumentList != null) { + return argumentList.getKeywordArgument(myChunk.getMappingKey()); + } + } } - return null; + return expression; } @NotNull @@ -271,8 +308,4 @@ public class PySubstitutionChunkReference extends PsiReferenceBase 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, isPercent); } - } return result; } } diff --git a/python/src/com/jetbrains/python/inspections/unresolvedReference/PyUnresolvedReferencesInspection.java b/python/src/com/jetbrains/python/inspections/unresolvedReference/PyUnresolvedReferencesInspection.java index e5cecb856ed0..dff036a47f82 100644 --- a/python/src/com/jetbrains/python/inspections/unresolvedReference/PyUnresolvedReferencesInspection.java +++ b/python/src/com/jetbrains/python/inspections/unresolvedReference/PyUnresolvedReferencesInspection.java @@ -42,7 +42,6 @@ import com.jetbrains.python.PyCustomType; import com.jetbrains.python.PyNames; import com.jetbrains.python.codeInsight.PyCodeInsightSettings; import com.jetbrains.python.codeInsight.PyCustomMember; -import com.jetbrains.python.codeInsight.PySubstitutionChunkReference; import com.jetbrains.python.codeInsight.PyFunctionTypeCommentReferenceContributor; import com.jetbrains.python.codeInsight.controlflow.ScopeOwner; import com.jetbrains.python.codeInsight.dataflow.scope.ScopeUtil; @@ -658,10 +657,7 @@ public class PyUnresolvedReferencesInspection extends PyInspection { } } } - - if (reference instanceof PySubstitutionChunkReference && ((PySubstitutionChunkReference)reference).ignoreUnresolved()) { - return; - } + registerProblem(node, description, hl_type, null, rangeInElement, actions.toArray(new LocalQuickFix[actions.size()])); } diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringDictLiteralArgumentWithNumericExprKeys.py b/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringDictLiteralArgumentWithNumericExprKeys.py index 202ddfa5ad5c..cb6df8b5221c 100644 --- a/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringDictLiteralArgumentWithNumericExprKeys.py +++ b/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringDictLiteralArgumentWithNumericExprKeys.py @@ -1 +1 @@ -print ("first is %(fst)s" % {1: "3"}) \ No newline at end of file +print ("first is %(fst)s" % {1: "3"}) \ No newline at end of file diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringPackedDict.py b/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringPackedDict.py deleted file mode 100644 index 6248dd51403d..000000000000 --- a/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringPackedDict.py +++ /dev/null @@ -1 +0,0 @@ -'{foo}'.format(**{"boo": 1}) \ No newline at end of file diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringPackedDictCall.py b/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringPackedDictCall.py deleted file mode 100644 index dfbb03d2a4f1..000000000000 --- a/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringPackedDictCall.py +++ /dev/null @@ -1 +0,0 @@ -'{foo}'.format(**dict(t=1)) \ No newline at end of file diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringPositionalSubstitutionWithDictArg.py b/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringPositionalSubstitutionWithDictArg.py new file mode 100644 index 000000000000..890855209fa8 --- /dev/null +++ b/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringPositionalSubstitutionWithDictArg.py @@ -0,0 +1 @@ +print('{}'.format(foo='foo')) \ No newline at end of file diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithDictArgWithCallExprKey.py b/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithDictArgWithCallExprKey.py new file mode 100644 index 000000000000..d2f29b8eff4a --- /dev/null +++ b/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithDictArgWithCallExprKey.py @@ -0,0 +1,3 @@ +def foo(): + return 'foo' +print("{foo}".format(**{'bar': 10, foo(): 20})) \ No newline at end of file diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithDictLiteralExprInsideDictCall.py b/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithDictLiteralExprInsideDictCall.py new file mode 100644 index 000000000000..9555a24b4565 --- /dev/null +++ b/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithDictLiteralExprInsideDictCall.py @@ -0,0 +1 @@ +print("{foo}".format(**dict({'foo': 'bar'}))) \ No newline at end of file diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithEmptyDictArg.py b/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithEmptyDictArg.py new file mode 100644 index 000000000000..5bd7625b2c71 --- /dev/null +++ b/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithEmptyDictArg.py @@ -0,0 +1 @@ +print("{foo}".format(**{})) \ No newline at end of file diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithPackedAndNonPackedArgs.py b/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithPackedAndNonPackedArgs.py new file mode 100644 index 000000000000..9f57c3e29b14 --- /dev/null +++ b/python/testData/inspections/PyUnresolvedReferencesInspection/formatStringWithPackedAndNonPackedArgs.py @@ -0,0 +1,4 @@ +def foo(): + return {"foo":"bar"} + +print("pos: {} {} {}".format(1, *[2, 3])) \ No newline at end of file diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/percentStringKeywordArgumentWithReferenceKeyDictArgument.py b/python/testData/inspections/PyUnresolvedReferencesInspection/percentStringKeywordArgumentWithReferenceKeyDictArgument.py new file mode 100644 index 000000000000..cb9585bbcc1e --- /dev/null +++ b/python/testData/inspections/PyUnresolvedReferencesInspection/percentStringKeywordArgumentWithReferenceKeyDictArgument.py @@ -0,0 +1,2 @@ +snd = "snd" +print "%(f)s %(snd)s" % { "f": 2, 1: 1, snd: 2} \ No newline at end of file diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/percentStringWithCallArgument.py b/python/testData/inspections/PyUnresolvedReferencesInspection/percentStringWithCallArgument.py new file mode 100644 index 000000000000..9cab28de4105 --- /dev/null +++ b/python/testData/inspections/PyUnresolvedReferencesInspection/percentStringWithCallArgument.py @@ -0,0 +1,3 @@ +def f(): + return 'foo', 'bar' +print('%s %s' % f()) \ No newline at end of file diff --git a/python/testData/resolve/FormatStringWithBinExprAsArg.py b/python/testData/resolve/FormatStringWithBinExprAsArg.py deleted file mode 100644 index 852e645a73b8..000000000000 --- a/python/testData/resolve/FormatStringWithBinExprAsArg.py +++ /dev/null @@ -1 +0,0 @@ -v = "first is {}, second is {}".format(("fst", ) + ("snd", )) \ No newline at end of file diff --git a/python/testData/resolve/FormatStringWithPackedAndNonPackedArgs.py b/python/testData/resolve/FormatStringWithPackedAndNonPackedArgs.py new file mode 100644 index 000000000000..900061b699ea --- /dev/null +++ b/python/testData/resolve/FormatStringWithPackedAndNonPackedArgs.py @@ -0,0 +1 @@ +print("pos: {} {} {}".format(1, *[2, 3])) \ No newline at end of file diff --git a/python/testData/resolve/PercentStringWithOneStringArgument.py b/python/testData/resolve/PercentStringWithOneStringArgument.py index 9ccdf86b06e9..d38113d248ea 100644 --- a/python/testData/resolve/PercentStringWithOneStringArgument.py +++ b/python/testData/resolve/PercentStringWithOneStringArgument.py @@ -1 +1 @@ -v = "%s" % "hello" \ No newline at end of file +v = "%s" % "hello" \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/PyResolveTest.java b/python/testSrc/com/jetbrains/python/PyResolveTest.java index a24620e2bf54..f27f7e4dbf88 100644 --- a/python/testSrc/com/jetbrains/python/PyResolveTest.java +++ b/python/testSrc/com/jetbrains/python/PyResolveTest.java @@ -659,17 +659,10 @@ public class PyResolveTest extends PyResolveTestCase { 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); + assertInstanceOf(target, PyStarArgument.class); } @@ -726,7 +719,7 @@ public class PyResolveTest extends PyResolveTestCase { //PY-2748 public void testFormatStringPackedDictCall() { PsiElement target = resolve(); - assertEquals("fo", ((PyStringLiteralExpression)((PyKeywordArgument)target).getValueExpression()).getStringValue()); + assertInstanceOf(target, PyStarArgument.class); } //PY-2748 @@ -748,6 +741,13 @@ public class PyResolveTest extends PyResolveTestCase { assertEquals("dict()", target.getText()); } + // PY-2748 + public void testFormatStringWithPackedAndNonPackedArgs() { + PsiElement target = resolve(); + assertInstanceOf(target, PyNumericLiteralExpression.class); + assertEquals("2", target.getText()); + } + public void testGlobalNotDefinedAtTopLevel() { assertResolvesTo(PyTargetExpression.class, "foo"); diff --git a/python/testSrc/com/jetbrains/python/inspections/PyUnresolvedReferencesInspectionTest.java b/python/testSrc/com/jetbrains/python/inspections/PyUnresolvedReferencesInspectionTest.java index b50f92713753..6648e75e2a99 100644 --- a/python/testSrc/com/jetbrains/python/inspections/PyUnresolvedReferencesInspectionTest.java +++ b/python/testSrc/com/jetbrains/python/inspections/PyUnresolvedReferencesInspectionTest.java @@ -546,17 +546,7 @@ public class PyUnresolvedReferencesInspectionTest extends PyInspectionTestCase { public void testPropertyNotListedInSlots() { doTest(); } - - // PY-2748 - public void testFormatStringPackedDictCall() { - doTest(); - } - - // PY-2748 - public void testFormatStringPackedDict() { - doTest(); - } - + // PY-2748 public void testFormatStringPositional() { doTest(); @@ -672,6 +662,42 @@ public class PyUnresolvedReferencesInspectionTest extends PyInspectionTestCase { doTest(); } + // PY-18115 + public void testFormatStringPositionalSubstitutionWithDictArg() { + doTest(); + } + + // PY-18115 + public void testPercentStringWithCallArgument() { + doTest(); + } + + // PY-18115 + public void testFormatStringWithEmptyDictArg() { + doTest(); + } + + // PY-18115 + public void testFormatStringWithDictLiteralExprInsideDictCall() { + doTest(); + } + + // PY-18115 + public void testFormatStringWithDictArgWithCallExprKey() { + doTest(); + } + + // PY-18115 + public void testFormatStringWithPackedAndNonPackedArgs() { + doTest(); + } + + + // PY-18950 + public void testPercentStringKeywordArgumentWithReferenceKeyDictArgument() { + doTest(); + } + // PY-18254 public void testVarargsAnnotatedWithFunctionComment() { doTest();