From 3b99571638d14ff20f4926b94a5acfa43fdf217f Mon Sep 17 00:00:00 2001 From: "andrey.matveev" Date: Wed, 11 Nov 2020 17:13:42 +0700 Subject: [PATCH] PY-28782 Fix remove assignment expression target inspection for multiple targets (cherry picked from commit 2a8862d459955a23def7941229007e26d076c54e) IJ-CR-4054 GitOrigin-RevId: 0068c0195a89e2272e9c632acc5494ea7ff0b120 --- ...moveAssignmentStatementTargetQuickFix.java | 21 ++++++--- .../PyUnusedLocalInspectionVisitor.java | 11 +++-- ...veChainedAssignmentStatementFirstTarget.py | 3 ++ ...nedAssignmentStatementFirstTarget_after.py | 3 ++ ...eChainedAssignmentStatementSecondTarget.py | 3 ++ ...edAssignmentStatementSecondTarget_after.py | 3 ++ ...AssignmentStatementUnpackingFirstTarget.py | 3 ++ ...mentStatementUnpackingFirstTarget_after.py | 3 ++ ...ssignmentStatementUnpackingSecondTarget.py | 3 ++ ...entStatementUnpackingSecondTarget_after.py | 3 ++ .../PyRemoveUnusedLocalQuickFixTest.java | 43 +++++++++++++++++++ 11 files changed, 90 insertions(+), 9 deletions(-) create mode 100644 python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementFirstTarget.py create mode 100644 python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementFirstTarget_after.py create mode 100644 python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementSecondTarget.py create mode 100644 python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementSecondTarget_after.py create mode 100644 python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingFirstTarget.py create mode 100644 python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingFirstTarget_after.py create mode 100644 python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingSecondTarget.py create mode 100644 python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingSecondTarget_after.py diff --git a/python/python-psi-impl/src/com/jetbrains/python/inspections/quickfix/PyRemoveAssignmentStatementTargetQuickFix.java b/python/python-psi-impl/src/com/jetbrains/python/inspections/quickfix/PyRemoveAssignmentStatementTargetQuickFix.java index 24d7b6496777..51f1a1f412c6 100644 --- a/python/python-psi-impl/src/com/jetbrains/python/inspections/quickfix/PyRemoveAssignmentStatementTargetQuickFix.java +++ b/python/python-psi-impl/src/com/jetbrains/python/inspections/quickfix/PyRemoveAssignmentStatementTargetQuickFix.java @@ -22,6 +22,7 @@ import com.intellij.openapi.project.Project; import com.intellij.psi.PsiElement; import com.intellij.psi.util.PsiTreeUtil; import com.jetbrains.python.PyPsiBundle; +import com.jetbrains.python.PyTokenTypes; import com.jetbrains.python.psi.*; import org.jetbrains.annotations.NotNull; @@ -37,11 +38,19 @@ public class PyRemoveAssignmentStatementTargetQuickFix implements LocalQuickFix, final PsiElement element = descriptor.getPsiElement(); final PyAssignmentStatement assignmentStatement = PsiTreeUtil.getParentOfType(element, PyAssignmentStatement.class); if (assignmentStatement == null) return; - final PyExpression expression = assignmentStatement.getAssignedValue(); - if (expression == null) return; - final PyElementGenerator elementGenerator = PyElementGenerator.getInstance(project); - PyExpressionStatement statement = elementGenerator.createFromText(LanguageLevel.forElement(expression), PyExpressionStatement.class, - expression.getText()); - assignmentStatement.replace(statement); + if (assignmentStatement.getRawTargets().length == 1) { + final PyExpression expression = assignmentStatement.getAssignedValue(); + if (expression == null) return; + final PyElementGenerator elementGenerator = PyElementGenerator.getInstance(project); + PyExpressionStatement statement = elementGenerator.createFromText(LanguageLevel.forElement(expression), PyExpressionStatement.class, + expression.getText()); + assignmentStatement.replace(statement); + } + else { + PsiElement possibleNextEq = PsiTreeUtil.nextVisibleLeaf(element); + if (possibleNextEq == null) return; + assert possibleNextEq.getNode().getElementType() == PyTokenTypes.EQ; + element.getParent().deleteChildRange(element, possibleNextEq); + } } } diff --git a/python/python-psi-impl/src/com/jetbrains/python/inspections/unusedLocal/PyUnusedLocalInspectionVisitor.java b/python/python-psi-impl/src/com/jetbrains/python/inspections/unusedLocal/PyUnusedLocalInspectionVisitor.java index 43033b5d7937..b0e5c68a2178 100644 --- a/python/python-psi-impl/src/com/jetbrains/python/inspections/unusedLocal/PyUnusedLocalInspectionVisitor.java +++ b/python/python-psi-impl/src/com/jetbrains/python/inspections/unusedLocal/PyUnusedLocalInspectionVisitor.java @@ -12,6 +12,7 @@ import com.intellij.openapi.util.TextRange; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiFile; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.ArrayUtil; import com.intellij.util.containers.ContainerUtil; import com.jetbrains.python.PyNames; import com.jetbrains.python.PyPsiBundle; @@ -422,14 +423,18 @@ public final class PyUnusedLocalInspectionVisitor extends PyInspectionVisitor { continue; } - // TODO: consider assignmentStatement.getRawTargets().length > 1 in PY-28782 final PyAssignmentStatement assignmentStatement = PsiTreeUtil.getParentOfType(element, PyAssignmentStatement.class); - if (assignmentStatement != null && assignmentStatement.getRawTargets().length == 1 && - PsiTreeUtil.isAncestor(assignmentStatement.getLeftHandSideExpression(), element, false)) { + if (assignmentStatement != null && !PsiTreeUtil.isAncestor(assignmentStatement.getAssignedValue(), element, false)) { if (assignmentStatement.getLeftHandSideExpression() == element) { + // Single assignment target (unused = value) registerWarning(element, warningMsg, new PyRemoveAssignmentStatementTargetQuickFix(), new PyRemoveStatementQuickFix()); } + else if (ArrayUtil.contains(element, assignmentStatement.getRawTargets())) { + // Chained assignment target (used = unused = value) + registerWarning(element, warningMsg, new PyRemoveAssignmentStatementTargetQuickFix()); + } else { + // Unpacking (used, unused = value) registerWarning(element, warningMsg, new ReplaceWithWildCard()); } continue; diff --git a/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementFirstTarget.py b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementFirstTarget.py new file mode 100644 index 000000000000..fb5dad2e6972 --- /dev/null +++ b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementFirstTarget.py @@ -0,0 +1,3 @@ +def f(): + a = b = 0 + return b \ No newline at end of file diff --git a/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementFirstTarget_after.py b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementFirstTarget_after.py new file mode 100644 index 000000000000..b32a64ff545e --- /dev/null +++ b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementFirstTarget_after.py @@ -0,0 +1,3 @@ +def f(): + b = 0 + return b \ No newline at end of file diff --git a/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementSecondTarget.py b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementSecondTarget.py new file mode 100644 index 000000000000..26abfe1ef319 --- /dev/null +++ b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementSecondTarget.py @@ -0,0 +1,3 @@ +def f(): + a = b = 0 + return a \ No newline at end of file diff --git a/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementSecondTarget_after.py b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementSecondTarget_after.py new file mode 100644 index 000000000000..935aa2751299 --- /dev/null +++ b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementSecondTarget_after.py @@ -0,0 +1,3 @@ +def f(): + a = 0 + return a \ No newline at end of file diff --git a/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingFirstTarget.py b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingFirstTarget.py new file mode 100644 index 000000000000..4044a411c222 --- /dev/null +++ b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingFirstTarget.py @@ -0,0 +1,3 @@ +def f(): + a = unused, b = 42, 42 + return a, b \ No newline at end of file diff --git a/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingFirstTarget_after.py b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingFirstTarget_after.py new file mode 100644 index 000000000000..83c497f01173 --- /dev/null +++ b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingFirstTarget_after.py @@ -0,0 +1,3 @@ +def f(): + a = _, b = 42, 42 + return a, b \ No newline at end of file diff --git a/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingSecondTarget.py b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingSecondTarget.py new file mode 100644 index 000000000000..416d39562224 --- /dev/null +++ b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingSecondTarget.py @@ -0,0 +1,3 @@ +def f(): + a = b, unused = 42, 42 + return a, b \ No newline at end of file diff --git a/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingSecondTarget_after.py b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingSecondTarget_after.py new file mode 100644 index 000000000000..0dc1e6047bf5 --- /dev/null +++ b/python/testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/removeChainedAssignmentStatementUnpackingSecondTarget_after.py @@ -0,0 +1,3 @@ +def f(): + a = b, _ = 42, 42 + return a, b \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/quickFixes/PyRemoveUnusedLocalQuickFixTest.java b/python/testSrc/com/jetbrains/python/quickFixes/PyRemoveUnusedLocalQuickFixTest.java index ed187db02826..e930769a7a20 100644 --- a/python/testSrc/com/jetbrains/python/quickFixes/PyRemoveUnusedLocalQuickFixTest.java +++ b/python/testSrc/com/jetbrains/python/quickFixes/PyRemoveUnusedLocalQuickFixTest.java @@ -15,11 +15,13 @@ */ package com.jetbrains.python.quickFixes; +import com.intellij.codeInsight.intention.IntentionAction; import com.intellij.testFramework.TestDataPath; import com.jetbrains.python.PyPsiBundle; import com.jetbrains.python.PyQuickFixTestCase; import com.jetbrains.python.inspections.unusedLocal.PyUnusedLocalInspection; import com.jetbrains.python.psi.LanguageLevel; +import org.jetbrains.annotations.NotNull; @TestDataPath("$CONTENT_ROOT/../testData/quickFixes/PyRemoveUnusedLocalQuickFixTest/") public class PyRemoveUnusedLocalQuickFixTest extends PyQuickFixTestCase { @@ -66,6 +68,47 @@ public class PyRemoveUnusedLocalQuickFixTest extends PyQuickFixTestCase { }); } + // PY-28782 + public void testRemoveChainedAssignmentStatementFirstTarget() { + runWithLanguageLevel(LanguageLevel.getLatest(), () -> { + doQuickFixTest(PyUnusedLocalInspection.class, PyPsiBundle.message("QFIX.NAME.remove.target.expr")); + }); + } + + // PY-28782 + public void testRemoveChainedAssignmentStatementSecondTarget() { + runWithLanguageLevel(LanguageLevel.getLatest(), () -> { + doQuickFixTest(PyUnusedLocalInspection.class, PyPsiBundle.message("QFIX.NAME.remove.target.expr")); + }); + } + + // PY-28782 + public void testRemoveChainedAssignmentStatementUnpackingFirstTarget() { + runWithLanguageLevel(LanguageLevel.getLatest(), () -> { + doTestNotIgnoreTupleUnpacking(PyPsiBundle.message("INSP.unused.locals.replace.with.wildcard")); + }); + } + + // PY-28782 + public void testRemoveChainedAssignmentStatementUnpackingSecondTarget() { + runWithLanguageLevel(LanguageLevel.getLatest(), () -> { + doTestNotIgnoreTupleUnpacking(PyPsiBundle.message("INSP.unused.locals.replace.with.wildcard")); + }); + } + + private void doTestNotIgnoreTupleUnpacking(@NotNull String hint) { + final String testFileName = getTestName(true); + final PyUnusedLocalInspection inspection = new PyUnusedLocalInspection(); + inspection.ignoreTupleUnpacking = false; + myFixture.configureByFile(testFileName + ".py"); + myFixture.enableInspections(inspection); + myFixture.checkHighlighting(true, false, false); + final IntentionAction intentionAction = myFixture.findSingleIntention(hint); + assertNotNull(intentionAction); + myFixture.launchAction(intentionAction); + myFixture.checkResultByFile(testFileName + "_after.py", true); + } + // PY-32037 public void testGeneratorIterator() { doQuickFixTest(PyUnusedLocalInspection.class, PyPsiBundle.message("INSP.unused.locals.replace.with.wildcard"));