From 4e40d02ee66b67fd6c87bbf1dd65ea5674db54d1 Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Fri, 30 Sep 2016 18:08:58 +0300 Subject: [PATCH] PY-20805 PY-8219 Detect usages of local variables inside f-strings Additionally, as a positive side-effect, "Unused local" inspection no longer warns about function parameters that appear in doctests now. --- .../PyUnusedLocalInspectionVisitor.java | 118 ++++++++++++------ .../UnusedLocalDoctestReference/test.py | 13 ++ .../UnusedLocalFStringReferences/test.py | 37 ++++++ .../python/PythonInspectionsTest.java | 10 ++ 4 files changed, 142 insertions(+), 36 deletions(-) create mode 100644 python/testData/inspections/UnusedLocalDoctestReference/test.py create mode 100644 python/testData/inspections/UnusedLocalFStringReferences/test.py diff --git a/python/src/com/jetbrains/python/inspections/PyUnusedLocalInspectionVisitor.java b/python/src/com/jetbrains/python/inspections/PyUnusedLocalInspectionVisitor.java index 0ac9e3b6e3e6..31e1f17de58d 100644 --- a/python/src/com/jetbrains/python/inspections/PyUnusedLocalInspectionVisitor.java +++ b/python/src/com/jetbrains/python/inspections/PyUnusedLocalInspectionVisitor.java @@ -19,11 +19,15 @@ import com.intellij.codeInsight.FileModificationService; import com.intellij.codeInsight.controlflow.ControlFlowUtil; import com.intellij.codeInsight.controlflow.Instruction; import com.intellij.codeInspection.*; +import com.intellij.lang.injection.InjectedLanguageManager; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.command.CommandProcessor; import com.intellij.openapi.extensions.Extensions; import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.Pair; +import com.intellij.openapi.util.TextRange; import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiFile; import com.intellij.psi.util.PsiTreeUtil; import com.jetbrains.python.PyBundle; import com.jetbrains.python.PyNames; @@ -44,9 +48,12 @@ import com.jetbrains.python.psi.resolve.PyResolveContext; import com.jetbrains.python.psi.search.PyOverridingMethodsSearch; import com.jetbrains.python.psi.search.PySuperMethodsSearch; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.*; +import static com.jetbrains.python.psi.PyUtil.as; + /** * @author oleg */ @@ -95,6 +102,38 @@ public class PyUnusedLocalInspectionVisitor extends PyInspectionVisitor { collectUsedReads(owner); } + @Override + public void visitPyStringLiteralExpression(PyStringLiteralExpression pyString) { + final ScopeOwner owner = ScopeUtil.getScopeOwner(pyString); + if (owner != null && !(owner instanceof PsiFile)) { + final PyStatement instrAnchor = PsiTreeUtil.getParentOfType(pyString, PyStatement.class); + if (instrAnchor == null) return; + final Instruction[] instructions = ControlFlowCache.getControlFlow(owner).getInstructions(); + final int startInstruction = ControlFlowUtil.findInstructionNumberByElement(instructions, instrAnchor); + if (startInstruction < 0) return; + final Project project = pyString.getProject(); + final List> pairs = InjectedLanguageManager.getInstance(project).getInjectedPsiFiles(pyString); + if (pairs != null) { + for (Pair pair : pairs) { + pair.getFirst().accept(new PyRecursiveElementVisitor() { + @Override + public void visitPyReferenceExpression(PyReferenceExpression expr) { + final PyExpression qualifier = expr.getQualifier(); + if (qualifier != null) { + qualifier.accept(this); + return; + } + final String name = expr.getName(); + if (name != null) { + analyzeReadsInScope(name, owner, instructions, startInstruction, pyString); + } + } + }); + } + } + } + } + private void collectAllWrites(ScopeOwner owner) { final Instruction[] instructions = ControlFlowCache.getControlFlow(owner).getInstructions(); for (Instruction instruction : instructions) { @@ -180,46 +219,53 @@ public class PyUnusedLocalInspectionVisitor extends PyInspectionVisitor { else { startInstruction = i; } - // Check if the element is declared out of scope, mark all out of scope write accesses as used - if (element instanceof PyReferenceExpression) { - final PyReferenceExpression ref = (PyReferenceExpression)element; - final ScopeOwner declOwner = ScopeUtil.getDeclarationScopeOwner(ref, name); - if (declOwner != null && declOwner != owner) { - Collection writeElements = ScopeUtil.getReadWriteElements(name, declOwner, false, true); - for (PsiElement e : writeElements) { - myUsedElements.add(e); - myUnusedElements.remove(e); - } - } - } - ControlFlowUtil.iteratePrev(startInstruction, instructions, inst -> { - final PsiElement element1 = inst.getElement(); - // Mark function as used - if (element1 instanceof PyFunction) { - if (name.equals(((PyFunction)element1).getName())){ - myUsedElements.add(element1); - myUnusedElements.remove(element1); - return ControlFlowUtil.Operation.CONTINUE; - } - } - // Mark write access as used - else if (inst instanceof ReadWriteInstruction) { - final ReadWriteInstruction rwInstruction = (ReadWriteInstruction)inst; - if (rwInstruction.getAccess().isWriteAccess() && name.equals(rwInstruction.getName())) { - // For elements in scope - if (element1 != null && PsiTreeUtil.isAncestor(owner, element1, false)) { - myUsedElements.add(element1); - myUnusedElements.remove(element1); - } - return ControlFlowUtil.Operation.CONTINUE; - } - } - return ControlFlowUtil.Operation.NEXT; - }); + analyzeReadsInScope(name, owner, instructions, startInstruction, as(element, PyReferenceExpression.class)); } } } + private void analyzeReadsInScope(@NotNull String name, + @NotNull ScopeOwner owner, + @NotNull Instruction[] instructions, + int startInstruction, + @Nullable PsiElement scopeAnchor) { + // Check if the element is declared out of scope, mark all out of scope write accesses as used + if (scopeAnchor != null) { + final ScopeOwner declOwner = ScopeUtil.getDeclarationScopeOwner(scopeAnchor, name); + if (declOwner != null && declOwner != owner) { + final Collection writeElements = ScopeUtil.getReadWriteElements(name, declOwner, false, true); + for (PsiElement e : writeElements) { + myUsedElements.add(e); + myUnusedElements.remove(e); + } + } + } + ControlFlowUtil.iteratePrev(startInstruction, instructions, inst -> { + final PsiElement instElement = inst.getElement(); + // Mark function as used + if (instElement instanceof PyFunction) { + if (name.equals(((PyFunction)instElement).getName())){ + myUsedElements.add(instElement); + myUnusedElements.remove(instElement); + return ControlFlowUtil.Operation.CONTINUE; + } + } + // Mark write access as used + else if (inst instanceof ReadWriteInstruction) { + final ReadWriteInstruction rwInstruction = (ReadWriteInstruction)inst; + if (rwInstruction.getAccess().isWriteAccess() && name.equals(rwInstruction.getName())) { + // For elements in scope + if (instElement != null && PsiTreeUtil.isAncestor(owner, instElement, false)) { + myUsedElements.add(instElement); + myUnusedElements.remove(instElement); + } + return ControlFlowUtil.Operation.CONTINUE; + } + } + return ControlFlowUtil.Operation.NEXT; + }); + } + static class DontPerformException extends RuntimeException {} private static boolean callsLocals(final ScopeOwner owner) { diff --git a/python/testData/inspections/UnusedLocalDoctestReference/test.py b/python/testData/inspections/UnusedLocalDoctestReference/test.py new file mode 100644 index 000000000000..aa264436e80f --- /dev/null +++ b/python/testData/inspections/UnusedLocalDoctestReference/test.py @@ -0,0 +1,13 @@ +def f(x, y): + """ + >>> print(x) + """ + + +def bar(): + def tokenize(): + """ + >>> tokenize() + ['foo', Escaped('='), 'bar', Escaped('\\'), 'baz'] + + """ \ No newline at end of file diff --git a/python/testData/inspections/UnusedLocalFStringReferences/test.py b/python/testData/inspections/UnusedLocalFStringReferences/test.py new file mode 100644 index 000000000000..6b6234dd1b32 --- /dev/null +++ b/python/testData/inspections/UnusedLocalFStringReferences/test.py @@ -0,0 +1,37 @@ +def simple(x): + y = 1 + print(f"{x}{y}") + + +def annotations(x, y): + def g(x: f'{x}') -> f'{y}': + return x + + g() + + +def default_value(x): + def g(x=f'{x}'): + return x + + g() + + +def super_classes(x): + class C(f'{x}'): + pass + + C() + + +def qualified_names(): + foo = 42 + bar = undefined() + print(f'{bar.foo}') + + +def nested(x): + def g(): + print(f'{x}') + + g() diff --git a/python/testSrc/com/jetbrains/python/PythonInspectionsTest.java b/python/testSrc/com/jetbrains/python/PythonInspectionsTest.java index 76130c106c11..646b22ff1213 100644 --- a/python/testSrc/com/jetbrains/python/PythonInspectionsTest.java +++ b/python/testSrc/com/jetbrains/python/PythonInspectionsTest.java @@ -113,6 +113,16 @@ public class PythonInspectionsTest extends PyTestCase { doHighlightingTest(PyUnusedLocalInspection.class, LanguageLevel.PYTHON34); } + // PY-20805 + public void testUnusedLocalFStringReferences() { + doHighlightingTest(PyUnusedLocalInspection.class, LanguageLevel.PYTHON36); + } + + // PY-8219 + public void testUnusedLocalDoctestReference() { + doHighlightingTest(PyUnusedLocalInspection.class); + } + public void testPyDictCreationInspection() { doHighlightingTest(PyDictCreationInspection.class, LanguageLevel.PYTHON26); }