From 87833e170bc8beb952c03fb6fe73cb598b88140e Mon Sep 17 00:00:00 2001 From: Oleg Shpynov Date: Fri, 16 Jul 2010 14:06:20 +0400 Subject: [PATCH] PY-959 Inspection to detect unused inner lambdas/functions --- .../com/jetbrains/python/PyBundle.properties | 5 +- .../controlflow/PyControlFlowUtil.java | 22 ++-- ...tion.java => PyUnusedLocalInspection.java} | 8 +- ...va => PyUnusedLocalInspectionVisitor.java} | 107 +++++++++++------- .../PythonInspectionToolProvider.java | 2 +- .../expected.xml | 13 +++ .../src/test.py | 15 +++ .../expected.xml | 4 + .../python/PySuppressInspectionsTest.java | 4 +- .../python/PythonInspectionsTest.java | 9 +- 10 files changed, 125 insertions(+), 64 deletions(-) rename python/src/com/jetbrains/python/inspections/{PyUnusedLocalVariableInspection.java => PyUnusedLocalInspection.java} (72%) rename python/src/com/jetbrains/python/inspections/{PyUnusedLocalVariableInspectionVisitor.java => PyUnusedLocalInspectionVisitor.java} (71%) create mode 100644 python/testData/inspections/PyUnusedLocalFunctionInspection/expected.xml create mode 100644 python/testData/inspections/PyUnusedLocalFunctionInspection/src/test.py diff --git a/python/src/com/jetbrains/python/PyBundle.properties b/python/src/com/jetbrains/python/PyBundle.properties index 0f150a2f0512..e8dcbb18d0d3 100644 --- a/python/src/com/jetbrains/python/PyBundle.properties +++ b/python/src/com/jetbrains/python/PyBundle.properties @@ -173,11 +173,12 @@ INSP.init.incompatible.to.new=Signature is not compatible to __new__ # PyTrailingSemicolonInspection INSP.NAME.trailing.semicolon=Trailing semicolon in statement -# PyUnusedLocalVariableInspection -INSP.NAME.unused=Unused local variable +# PyUnusedLocalInspection +INSP.NAME.unused=Unused local INSP.unused.locals.parameter.isnot.used=Parameter ''{0}'' value is not used INSP.unused.locals.local.variable.isnot.used=Local variable ''{0}'' value is not used INSP.unused.locals.replace.with.wildcard=Replace with _ +INSP.unused.locals.local.function.isnot.used=Local function ''{0}'' is not used # PyUnboundLocalVariableInspection INSP.NAME.unbound=Unbound local variable diff --git a/python/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowUtil.java b/python/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowUtil.java index de80650a0933..8a6ac9888cfc 100644 --- a/python/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowUtil.java +++ b/python/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowUtil.java @@ -9,10 +9,9 @@ import org.jetbrains.annotations.NotNull; * @author oleg */ public class PyControlFlowUtil { - public static void iterateWriteAccessFor(@NotNull final String variableName, - final int statInstruction, - @NotNull final Instruction[] instructions, - @NotNull final Function closure) { + public static void iteratePrev(final int statInstruction, + @NotNull final Instruction[] instructions, + @NotNull final Function closure) { final ControlFlowUtil.Stack stack = new ControlFlowUtil.Stack(instructions.length); final boolean[] visited = new boolean[instructions.length]; @@ -24,16 +23,11 @@ public class PyControlFlowUtil { } visited[num] = true; final Instruction instr = instructions[num]; - if (instr instanceof ReadWriteInstruction) { - final ReadWriteInstruction rwInstr = (ReadWriteInstruction)instr; - if (variableName.equals(rwInstr.getName()) && rwInstr.getAccess().isWriteAccess()){ - final Operation nextOperation = closure.fun(rwInstr); - if (nextOperation == Operation.CONTINUE) { - continue; - } else if (nextOperation == Operation.BREAK) { - break; - } - } + final Operation nextOperation = closure.fun(instr); + if (nextOperation == Operation.CONTINUE) { + continue; + } else if (nextOperation == Operation.BREAK) { + break; } for (Instruction pred : instr.allPred()) { stack.push(pred.num()); diff --git a/python/src/com/jetbrains/python/inspections/PyUnusedLocalVariableInspection.java b/python/src/com/jetbrains/python/inspections/PyUnusedLocalInspection.java similarity index 72% rename from python/src/com/jetbrains/python/inspections/PyUnusedLocalVariableInspection.java rename to python/src/com/jetbrains/python/inspections/PyUnusedLocalInspection.java index 72e8d571b45d..70b7604f09e6 100644 --- a/python/src/com/jetbrains/python/inspections/PyUnusedLocalVariableInspection.java +++ b/python/src/com/jetbrains/python/inspections/PyUnusedLocalInspection.java @@ -13,8 +13,8 @@ import javax.swing.*; /** * @author oleg */ -public class PyUnusedLocalVariableInspection extends PyInspection { - private final ThreadLocal myLastVisitor = new ThreadLocal(); +public class PyUnusedLocalInspection extends PyInspection { + private final ThreadLocal myLastVisitor = new ThreadLocal(); public boolean ignoreTupleUnpacking = true; @@ -26,14 +26,14 @@ public class PyUnusedLocalVariableInspection extends PyInspection { @NotNull public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { - final PyUnusedLocalVariableInspectionVisitor visitor = new PyUnusedLocalVariableInspectionVisitor(holder, ignoreTupleUnpacking); + final PyUnusedLocalInspectionVisitor visitor = new PyUnusedLocalInspectionVisitor(holder, ignoreTupleUnpacking); myLastVisitor.set(visitor); return visitor; } @Override public void inspectionFinished(LocalInspectionToolSession session) { - final PyUnusedLocalVariableInspectionVisitor visitor = myLastVisitor.get(); + final PyUnusedLocalInspectionVisitor visitor = myLastVisitor.get(); assert visitor != null; visitor.registerProblems(); myLastVisitor.remove(); diff --git a/python/src/com/jetbrains/python/inspections/PyUnusedLocalVariableInspectionVisitor.java b/python/src/com/jetbrains/python/inspections/PyUnusedLocalInspectionVisitor.java similarity index 71% rename from python/src/com/jetbrains/python/inspections/PyUnusedLocalVariableInspectionVisitor.java rename to python/src/com/jetbrains/python/inspections/PyUnusedLocalInspectionVisitor.java index 3008b4a1410e..15ea0fbc50ab 100644 --- a/python/src/com/jetbrains/python/inspections/PyUnusedLocalVariableInspectionVisitor.java +++ b/python/src/com/jetbrains/python/inspections/PyUnusedLocalInspectionVisitor.java @@ -36,12 +36,12 @@ import java.util.Set; /** * @author oleg */ -class PyUnusedLocalVariableInspectionVisitor extends PyInspectionVisitor { +class PyUnusedLocalInspectionVisitor extends PyInspectionVisitor { private final boolean myIgnoreTupleUnpacking; private final HashSet myUnusedElements; private final HashSet myUsedElements; - public PyUnusedLocalVariableInspectionVisitor(final ProblemsHolder holder, boolean ignoreTupleUnpacking) { + public PyUnusedLocalInspectionVisitor(final ProblemsHolder holder, boolean ignoreTupleUnpacking) { super(holder); myIgnoreTupleUnpacking = ignoreTupleUnpacking; myUnusedElements = new HashSet(); @@ -81,14 +81,18 @@ class PyUnusedLocalVariableInspectionVisitor extends PyInspectionVisitor { // Iteration over write accesses for (int i = 0; i < instructions.length; i++) { final Instruction instruction = instructions[i]; - if (instruction instanceof ReadWriteInstruction) { + final PsiElement element = instruction.getElement(); + if (element instanceof PyFunction && owner instanceof PyFunction){ + if (!myUsedElements.contains(element)){ + myUnusedElements.add(element); + } + } + else if (instruction instanceof ReadWriteInstruction) { final String name = ((ReadWriteInstruction)instruction).getName(); // Ignore empty, wildcards or global names if (name == null || "_".equals(name) || scope.isGlobal(name)) { continue; } - final PsiElement element = instruction.getElement(); - // Ignore elements out of scope if (element == null || !PsiTreeUtil.isAncestor(node, element, false)){ continue; @@ -151,20 +155,36 @@ class PyUnusedLocalVariableInspectionVisitor extends PyInspectionVisitor { } PyControlFlowUtil - .iterateWriteAccessFor(name, number, instructions, new Function() { - public PyControlFlowUtil.Operation fun(final ReadWriteInstruction rwInstr) { - final PsiElement instrElement = rwInstr.getElement(); - // Ignore elements out of scope - if (instrElement == null || !PsiTreeUtil.isAncestor(node, instrElement, false)){ + .iteratePrev(number, instructions, new Function() { + public PyControlFlowUtil.Operation fun(final Instruction inst) { + final PsiElement element = inst.getElement(); + // Mark function as used + if (element instanceof PyFunction){ + if (name.equals(((PyFunction)element).getName())){ + myUsedElements.add(element); + myUnusedElements.remove(element); + return PyControlFlowUtil.Operation.CONTINUE; + } + } + // Mark write access as used + else if (inst instanceof ReadWriteInstruction) { + final ReadWriteInstruction rwInstruction = (ReadWriteInstruction)inst; + if (!name.equals(rwInstruction.getName()) || !rwInstruction.getAccess().isWriteAccess()) { + return PyControlFlowUtil.Operation.NEXT; + } + // Ignore elements out of scope + if (element == null || !PsiTreeUtil.isAncestor(node, element, false)) { + return PyControlFlowUtil.Operation.CONTINUE; + } + myUsedElements.add(element); + myUnusedElements.remove(element); + // In case when assignment is inside try part we should move further + if (PsiTreeUtil.getParentOfType(element, PyTryPart.class) != null) { + return PyControlFlowUtil.Operation.NEXT; + } return PyControlFlowUtil.Operation.CONTINUE; } - myUsedElements.add(instrElement); - myUnusedElements.remove(instrElement); - // In case when assignment is inside try part we should move further - if (PsiTreeUtil.getParentOfType(instrElement, PyTryPart.class) != null){ - return PyControlFlowUtil.Operation.NEXT; - } - return PyControlFlowUtil.Operation.CONTINUE; + return PyControlFlowUtil.Operation.NEXT; } }); } @@ -172,7 +192,7 @@ class PyUnusedLocalVariableInspectionVisitor extends PyInspectionVisitor { } } - private static boolean callsLocals(ScopeOwner owner) { + private static boolean callsLocals(final ScopeOwner owner) { try { owner.acceptChildren(new PyRecursiveElementVisitor(){ @Override @@ -201,31 +221,40 @@ class PyUnusedLocalVariableInspectionVisitor extends PyInspectionVisitor { void registerProblems() { // Register problems for (PsiElement element : myUnusedElements) { - final String name = element.getText(); - if (element instanceof PyNamedParameter || element.getParent() instanceof PyNamedParameter) { - // Ignore unused self parameters as obligatory - if ("self".equals(name) && PyPsiUtils.isMethodContext(element)) { - continue; - } - // cls for @classmethod decorated methods - if ("cls".equals(name)) { - final Set flagSet = PyUtil.detectDecorationsAndWrappersOf(PsiTreeUtil.getParentOfType(element, PyFunction.class)); - if (flagSet.contains(PyFunction.Flag.CLASSMETHOD)) { + // Local function + if (element instanceof PyFunction){ + registerWarning(((PyFunction)element).getNameIdentifier(), + PyBundle.message("INSP.unused.locals.local.function.isnot.used", + ((PyFunction)element).getName())); + } + // Local variable or parameter + else { + final String name = element.getText(); + if (element instanceof PyNamedParameter || element.getParent() instanceof PyNamedParameter) { + // Ignore unused self parameters as obligatory + if ("self".equals(name) && PyPsiUtils.isMethodContext(element)) { continue; } - } - registerWarning(element, PyBundle.message("INSP.unused.locals.parameter.isnot.used", name)); - } - else { - if (myIgnoreTupleUnpacking && isTupleUnpacking(element)) { - continue; - } - if (PyForStatementNavigator.getPyForStatementByIterable(element) != null) { - registerProblem(element, PyBundle.message("INSP.unused.locals.local.variable.isnot.used", name), - ProblemHighlightType.LIKE_UNUSED_SYMBOL, null, new ReplaceWithWildCard()); + // cls for @classmethod decorated methods + if ("cls".equals(name)) { + final Set flagSet = PyUtil.detectDecorationsAndWrappersOf(PsiTreeUtil.getParentOfType(element, PyFunction.class)); + if (flagSet.contains(PyFunction.Flag.CLASSMETHOD)) { + continue; + } + } + registerWarning(element, PyBundle.message("INSP.unused.locals.parameter.isnot.used", name)); } else { - registerWarning(element, PyBundle.message("INSP.unused.locals.local.variable.isnot.used", name)); + if (myIgnoreTupleUnpacking && isTupleUnpacking(element)) { + continue; + } + if (PyForStatementNavigator.getPyForStatementByIterable(element) != null) { + registerProblem(element, PyBundle.message("INSP.unused.locals.local.variable.isnot.used", name), + ProblemHighlightType.LIKE_UNUSED_SYMBOL, null, new ReplaceWithWildCard()); + } + else { + registerWarning(element, PyBundle.message("INSP.unused.locals.local.variable.isnot.used", name)); + } } } } diff --git a/python/src/com/jetbrains/python/inspections/PythonInspectionToolProvider.java b/python/src/com/jetbrains/python/inspections/PythonInspectionToolProvider.java index 624c7c677654..933453cc9f26 100644 --- a/python/src/com/jetbrains/python/inspections/PythonInspectionToolProvider.java +++ b/python/src/com/jetbrains/python/inspections/PythonInspectionToolProvider.java @@ -21,7 +21,7 @@ public class PythonInspectionToolProvider implements InspectionToolProvider { PyInitNewSignatureInspection.class, PyTrailingSemicolonInspection.class, PyReturnFromInitInspection.class, - PyUnusedLocalVariableInspection.class, + PyUnusedLocalInspection.class, PyDeprecatedModulesInspection.class, PyDictCreationInspection.class, PyExceptClausesOrderInspection.class, diff --git a/python/testData/inspections/PyUnusedLocalFunctionInspection/expected.xml b/python/testData/inspections/PyUnusedLocalFunctionInspection/expected.xml new file mode 100644 index 000000000000..4b14ee1b1008 --- /dev/null +++ b/python/testData/inspections/PyUnusedLocalFunctionInspection/expected.xml @@ -0,0 +1,13 @@ + + + + test.py + 2 + "Local function 'bar' is not used" + + + test.py + 13 + "Local function 'bar1' is not used" + + \ No newline at end of file diff --git a/python/testData/inspections/PyUnusedLocalFunctionInspection/src/test.py b/python/testData/inspections/PyUnusedLocalFunctionInspection/src/test.py new file mode 100644 index 000000000000..dad9757e08af --- /dev/null +++ b/python/testData/inspections/PyUnusedLocalFunctionInspection/src/test.py @@ -0,0 +1,15 @@ +def foo(): + def bar(): pass #fail + + +def baz(cond): + def bzz(): pass #pass + if cond: + return bzz + else: + return None + +def bar(): + def bar1():pass #fail + def bar1():pass #pass + return bar1 \ No newline at end of file diff --git a/python/testData/inspections/PyUnusedLocalVariableInspection/expected.xml b/python/testData/inspections/PyUnusedLocalVariableInspection/expected.xml index 83631d439630..eff88d7d473b 100644 --- a/python/testData/inspections/PyUnusedLocalVariableInspection/expected.xml +++ b/python/testData/inspections/PyUnusedLocalVariableInspection/expected.xml @@ -36,6 +36,10 @@ "Local variable 'i' value is not used" + test.py + 51 + "Local function 'test' is not used" + test.py 83 "Parameter 'x' value is not used" diff --git a/python/testSrc/com/jetbrains/python/PySuppressInspectionsTest.java b/python/testSrc/com/jetbrains/python/PySuppressInspectionsTest.java index 7deb9980f0cd..e28edce2079e 100644 --- a/python/testSrc/com/jetbrains/python/PySuppressInspectionsTest.java +++ b/python/testSrc/com/jetbrains/python/PySuppressInspectionsTest.java @@ -4,7 +4,7 @@ import com.intellij.codeInsight.intention.IntentionAction; import com.jetbrains.python.fixtures.PyLightFixtureTestCase; import com.jetbrains.python.inspections.PyInspection; import com.jetbrains.python.inspections.PyUnresolvedReferencesInspection; -import com.jetbrains.python.inspections.PyUnusedLocalVariableInspection; +import com.jetbrains.python.inspections.PyUnusedLocalInspection; /** * @author yole @@ -23,7 +23,7 @@ public class PySuppressInspectionsTest extends PyLightFixtureTestCase { } public void testSuppressedUnusedLocal() throws Exception { - doTestHighlighting(PyUnusedLocalVariableInspection.class); + doTestHighlighting(PyUnusedLocalInspection.class); } private void doTestHighlighting(final Class inspectionClass) throws Exception { diff --git a/python/testSrc/com/jetbrains/python/PythonInspectionsTest.java b/python/testSrc/com/jetbrains/python/PythonInspectionsTest.java index f9364b7e308d..a40f8949a72d 100644 --- a/python/testSrc/com/jetbrains/python/PythonInspectionsTest.java +++ b/python/testSrc/com/jetbrains/python/PythonInspectionsTest.java @@ -87,13 +87,18 @@ public class PythonInspectionsTest extends PyLightFixtureTestCase { } public void testPyUnusedLocalVariableInspection() { - PyUnusedLocalVariableInspection inspection = new PyUnusedLocalVariableInspection(); + PyUnusedLocalInspection inspection = new PyUnusedLocalInspection(); inspection.ignoreTupleUnpacking = false; doTest(getTestName(false), inspection); } public void testPyUnusedVariableTupleUnpacking() { - doHighlightingTest(PyUnusedLocalVariableInspection.class); + doHighlightingTest(PyUnusedLocalInspection.class); + } + + public void testPyUnusedLocalFunctionInspection() { + PyUnusedLocalInspection inspection = new PyUnusedLocalInspection(); + doTest(getTestName(false), inspection); } public void testPyDictCreationInspection() throws Throwable {