diff --git a/python/psi-api/src/com/jetbrains/python/psi/PySubscriptionExpression.java b/python/psi-api/src/com/jetbrains/python/psi/PySubscriptionExpression.java
index 0a122243805a..dc3f28d8ea38 100644
--- a/python/psi-api/src/com/jetbrains/python/psi/PySubscriptionExpression.java
+++ b/python/psi-api/src/com/jetbrains/python/psi/PySubscriptionExpression.java
@@ -15,12 +15,21 @@
*/
package com.jetbrains.python.psi;
+import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
/**
* @author yole
*/
public interface PySubscriptionExpression extends PyQualifiedExpression, PyReferenceOwner {
+
+ /**
+ * @return For spam[x][y][n] will return spam regardless number of its dimensions
+ */
+ @NotNull
+ PyExpression getRootOperand();
+
+ @NotNull
PyExpression getOperand();
@Nullable
diff --git a/python/resources/inspectionDescriptions/PyAssignmentToLoopOrWithParameterInspection.html b/python/resources/inspectionDescriptions/PyAssignmentToLoopOrWithParameterInspection.html
index 08a234cec60f..4fc3dcd20164 100644
--- a/python/resources/inspectionDescriptions/PyAssignmentToLoopOrWithParameterInspection.html
+++ b/python/resources/inspectionDescriptions/PyAssignmentToLoopOrWithParameterInspection.html
@@ -1,7 +1,7 @@
- This inspection checks for cases when loop variable is redeclared inside of loop:
+ Checks for cases when you rewrite loop variable with inner loop
for i in xrange(5):
@@ -15,7 +15,7 @@
with open("file") as f:
f.read()
- f = open("another file")
+ with open("file") as f:
diff --git a/python/src/com/jetbrains/python/inspections/PyAssignmentToLoopOrWithParameterInspection.java b/python/src/com/jetbrains/python/inspections/PyAssignmentToLoopOrWithParameterInspection.java
index 6c29fc330315..f2aa2018cfd7 100644
--- a/python/src/com/jetbrains/python/inspections/PyAssignmentToLoopOrWithParameterInspection.java
+++ b/python/src/com/jetbrains/python/inspections/PyAssignmentToLoopOrWithParameterInspection.java
@@ -17,31 +17,23 @@ package com.jetbrains.python.inspections;
import com.intellij.codeInspection.LocalInspectionToolSession;
import com.intellij.codeInspection.ProblemsHolder;
+import com.intellij.openapi.util.Condition;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiElementVisitor;
+import com.intellij.psi.PsiReference;
import com.intellij.psi.util.PsiTreeUtil;
import com.jetbrains.python.PyBundle;
-import com.jetbrains.python.psi.PyForPart;
-import com.jetbrains.python.psi.PyTargetExpression;
-import com.jetbrains.python.psi.PyUtil;
-import com.jetbrains.python.psi.PyWithStatement;
+import com.jetbrains.python.codeInsight.controlflow.ScopeOwner;
+import com.jetbrains.python.psi.*;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
//TODO: Try to share logic with AssignmentToForLoopParameterInspection
/**
- * Checks for cases like
- *
- * for i in range(1, 10):
- * i = "new value"
- *
- * and
- *
- * with open("file") as f:
- * f.read()
- * f = open("another file")
- *
+ * Checks for cases when you rewrite loop variable with inner loop.
+ * It finds all with and for statements, takes variables declared by them and ensures none of parent
+ * with or for declares variable with the same name
*
* @author link
*/
@@ -70,24 +62,97 @@ public class PyAssignmentToLoopOrWithParameterInspection extends PyInspection {
}
@Override
- public void visitPyTargetExpression(PyTargetExpression node) {
- PsiElement variableDeclaration = node.getReference().resolve();
- if (variableDeclaration == null) {
- return;
- }
- if (!PyUtil.inSameFile(node, variableDeclaration)) {
- return;
- }
- PsiElement variableFirstTimeDeclaration = variableDeclaration.getParent();
+ public void visitPyWithStatement(PyWithStatement node) {
+ checkNotReDeclaringUpperLoopOrStatement(node);
+ }
- if (variableFirstTimeDeclaration.equals(node.getParent())) {
- return; //We are checking first time declaration
- }
+ @Override
+ public void visitPyForStatement(PyForStatement node) {
+ checkNotReDeclaringUpperLoopOrStatement(node);
+ }
- //Check if variable declared in "for" or "with" statement
- if (PsiTreeUtil.getNonStrictParentOfType(variableFirstTimeDeclaration, PyForPart.class, PyWithStatement.class) != null) {
- registerProblem(node, MESSAGE);
+ /**
+ * Finds first parent of specific type (See {@link #isRequiredStatement(com.intellij.psi.PsiElement)})
+ * that declares one of names, declared in this statement
+ */
+ private void checkNotReDeclaringUpperLoopOrStatement(NameDefiner statement) {
+ for (PsiElement declaredVar : statement.iterateNames()) {
+ Filter filter = new Filter(handleSubscriptionsAndResolveSafely(declaredVar));
+ PsiElement firstParent = PsiTreeUtil.findFirstParent(statement, true, filter);
+ if (firstParent != null && isRequiredStatement(firstParent)) {
+ registerProblem(declaredVar, MESSAGE);
+ }
}
}
}
+
+ /**
+ * Filters list of parents trying to find parent that declares var that refers to {@link #node}
+ * Returns {@link com.jetbrains.python.codeInsight.controlflow.ScopeOwner} if nothing found.
+ * Returns parent otherwise.
+ */
+ private static class Filter implements Condition {
+ private final PsiElement node;
+
+ private Filter(PsiElement node) {
+ this.node = node;
+ }
+
+ @Override
+ public boolean value(PsiElement psiElement) {
+ if (psiElement instanceof ScopeOwner) {
+ return true; //Do not go any further
+ }
+ if (!(isRequiredStatement(psiElement))) {
+ return false; //Parent has wrong type, skip
+ }
+ Iterable varsDeclaredInStatement = ((NameDefiner)psiElement).iterateNames();
+ for (PsiElement varDeclaredInStatement : varsDeclaredInStatement) {
+ //For each variable, declared by this parent take first declaration and open subscription list if any
+ PsiReference reference = handleSubscriptionsAndResolveSafely(varDeclaredInStatement).getReference();
+ if (reference != null && reference.isReferenceTo(node)) {
+ return true; //One of variables declared by this parent refers to node
+ }
+ }
+ return false;
+ }
+
+ }
+
+ /**
+ * Opens subscription list (i[n][q][f] --> i) and resolves ref recursively to the topmost element,
+ * but not further than file borders (to prevent Stub to AST conversion)
+ *
+ * @param element element to open and resolve
+ * @return opened and resolved element
+ */
+ private static PsiElement handleSubscriptionsAndResolveSafely(PsiElement element) {
+ assert element != null;
+ if (element instanceof PySubscriptionExpression) {
+ element = ((PySubscriptionExpression)element).getRootOperand();
+ }
+ while (true) {
+ PsiReference reference = element.getReference();
+ if (reference == null) {
+ break;
+ }
+ PsiElement resolve = reference.resolve();
+ if (resolve == null || resolve.equals(element) || !PyUtil.inSameFile(resolve, element)) {
+ break;
+ }
+ element = resolve;
+ }
+ return element;
+ }
+
+ /**
+ * Checks if element is statement this inspection should work with
+ *
+ * @param element to check
+ * @return true if inspection should work with this element
+ */
+ private static boolean isRequiredStatement(PsiElement element) {
+ assert element != null;
+ return element instanceof PyWithStatement || element instanceof PyForStatement;
+ }
}
diff --git a/python/src/com/jetbrains/python/psi/impl/PySubscriptionExpressionImpl.java b/python/src/com/jetbrains/python/psi/impl/PySubscriptionExpressionImpl.java
index c19e9ad554d7..bfa1f6dfe4b9 100644
--- a/python/src/com/jetbrains/python/psi/impl/PySubscriptionExpressionImpl.java
+++ b/python/src/com/jetbrains/python/psi/impl/PySubscriptionExpressionImpl.java
@@ -37,10 +37,21 @@ public class PySubscriptionExpressionImpl extends PyElementImpl implements PySub
super(astNode);
}
+ @NotNull
public PyExpression getOperand() {
return childToPsiNotNull(PythonDialectsTokenSetProvider.INSTANCE.getExpressionTokens(), 0);
}
+ @NotNull
+ @Override
+ public PyExpression getRootOperand() {
+ PyExpression operand = getOperand();
+ while (operand instanceof PySubscriptionExpression) {
+ operand = ((PySubscriptionExpression)operand).getOperand();
+ }
+ return operand;
+ }
+
@Nullable
public PyExpression getIndexExpression() {
return childToPsi(PythonDialectsTokenSetProvider.INSTANCE.getExpressionTokens(), 1);
@@ -74,7 +85,7 @@ public class PySubscriptionExpressionImpl extends PyElementImpl implements PySub
res = ((PySubscriptableType)type).getElementType(indexExpression, context);
}
else if (type instanceof PyCollectionType) {
- res = ((PyCollectionType) type).getElementType(context);
+ res = ((PyCollectionType)type).getElementType(context);
}
}
}
diff --git a/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/bad.py b/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/bad.py
new file mode 100644
index 000000000000..b7346bebda61
--- /dev/null
+++ b/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/bad.py
@@ -0,0 +1,62 @@
+i = []
+for i[0] in xrange(5):
+ for i[0] in xrange(20, 25):
+ print("Inner", i)
+ for i in xrange(20, 25):
+ pass
+ print("Outer", i)
+
+for i in xrange(5):
+ for i in xrange(20, 25):
+ print("Inner", i)
+ print("Outer", i)
+
+for i in xrange(5):
+ i = []
+ for i[0] in xrange(20, 25):
+ print("Inner", i)
+ print("Outer", i)
+
+i = [0]
+for i[0] in xrange(5):
+ for i[0] in xrange(20, 25):
+ print("Inner", i)
+ print("Outer", i)
+
+i = [[]]
+for i[0] in xrange(5):
+ for i in xrange(20, 25):
+ print("Inner", i)
+ print("Outer", i)
+
+with open("a") as f:
+ spam(f)
+ f.eggs()
+ with open("b") as f: #
+ pass
+
+with open("a") as z, open("A") as f:
+ spam(f)
+ f.eggs()
+ for (a,b,c,d,(e,f)) in []:
+ pass
+
+
+with open("a") as f:
+ spam(f)
+ f.eggs()
+ for z in []:
+ with open("b") as q:
+ with open("a") as f: #
+ pass
+
+
+class Foo(object):
+ def __init__(self):
+ super(Foo, self).__init__()
+ self.data = "ddd"
+
+ def foo(self):
+ for self.data in [1,2,3]:
+ for self.data in [1,2,3]:
+ pass
\ No newline at end of file
diff --git a/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/good.py b/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/good.py
index 3ea1ee1b26c5..f324656ca216 100644
--- a/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/good.py
+++ b/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/good.py
@@ -1,3 +1,8 @@
+from spam import eggs
+
+for eggs in (1, 12):
+ eggs = 12
+
for a in (1, 12):
for b in (2, 24):
for (c, d) in {"C": "D"}.items():
@@ -6,4 +11,79 @@ for a in (1, 12):
i = 12
print(i)
(z, x) = (i, 12)
-print(z)
\ No newline at end of file
+print(z)
+
+for root in settings.STATICFILES_DIRS:
+ if isinstance(root, (list, tuple)):
+ prefix, root = root
+
+
+for field, model in self.model._meta.get_concrete_fields_with_model():
+ if model is None:
+ model = self.model
+
+with open('a', 'w') as a, open('b', 'w') as b:
+ do_something()
+
+
+for f in [1,2,3]:
+ f = f + 1
+
+for f in [1,2,3]:
+ f = spam(f)
+
+for f in [1,2,3]:
+ f = eggs(lambda x: x + f)
+
+q = []
+for q[0] in [1,2,3]:
+ q[0] = eggs(q)
+
+q = []
+for q[0] in [1,2,3]:
+ q[0] = eggs(q)
+
+for f in [1,2,3]:
+ f = eggs(lambda x: x, f)
+
+for a in [1,2]:
+ pass
+
+for a in [1,2]:
+ pass
+
+b = 12
+for b in [1,2]:
+ pass
+
+for item in range(5):
+ want_to_import = False
+ print want_to_import
+ want_to_import = 2 #No error should be here
+ if True:
+ pass
+
+for ((a, b), (c, d)) in {(1, 2): (3, 4)}.items():
+ print b
+
+x = [1]
+for x[0] in range(1,2):
+ print i
+
+for x[i] in range(1,2):
+ print i
+
+x = [[1]]
+for x[0][0] in range(1,2):
+ x[0][1] = 1
+
+class Foo(object):
+ def __init__(self):
+ super(Foo, self).__init__()
+ self.data = "ddd"
+
+ def foo(self):
+ data, self.data = self.data
+ for data in [1,2,3]:
+ for self.data in [1,2,3]:
+ pass
diff --git a/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/simpleReassignment.py b/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/simpleReassignment.py
deleted file mode 100644
index bff004fdc4f5..000000000000
--- a/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/simpleReassignment.py
+++ /dev/null
@@ -1,3 +0,0 @@
-for i in range(1, 2):
- print(i)
- i = 12
\ No newline at end of file
diff --git a/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/tupleAssignment.py b/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/tupleAssignment.py
deleted file mode 100644
index 0606522ae69c..000000000000
--- a/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/tupleAssignment.py
+++ /dev/null
@@ -1,3 +0,0 @@
-for i in [1, 2, 3]:
- print(i)
- (i, f) = (1, 2)
\ No newline at end of file
diff --git a/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/tupleDeclaration.py b/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/tupleDeclaration.py
deleted file mode 100644
index 1469eb81a36d..000000000000
--- a/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/tupleDeclaration.py
+++ /dev/null
@@ -1,3 +0,0 @@
-for (k, v) in {"K": "V"}.items():
- print(k)
- k = "12"
\ No newline at end of file
diff --git a/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/twoLoops.py b/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/twoLoops.py
deleted file mode 100644
index d26a791db081..000000000000
--- a/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/twoLoops.py
+++ /dev/null
@@ -1,4 +0,0 @@
-for i in range(5):
- for i in range(20, 25):
- print("Inner", i)
- print("Outer", i)
\ No newline at end of file
diff --git a/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/withStatement.py b/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/withStatement.py
deleted file mode 100644
index e642026ba896..000000000000
--- a/python/testData/inspections/PyAssignmentToLoopOrWithParameterInspection/withStatement.py
+++ /dev/null
@@ -1,3 +0,0 @@
-with open("file") as f:
- f.read()
- f = open("another file")
\ No newline at end of file
diff --git a/python/testSrc/com/jetbrains/python/inspections/PyAssignmentToLoopOrWithParameterInspectionTest.java b/python/testSrc/com/jetbrains/python/inspections/PyAssignmentToLoopOrWithParameterInspectionTest.java
index 621e2a10fb77..519d2075ad32 100644
--- a/python/testSrc/com/jetbrains/python/inspections/PyAssignmentToLoopOrWithParameterInspectionTest.java
+++ b/python/testSrc/com/jetbrains/python/inspections/PyAssignmentToLoopOrWithParameterInspectionTest.java
@@ -27,26 +27,10 @@ public class PyAssignmentToLoopOrWithParameterInspectionTest extends PyInspectio
doTest();
}
- public void testSimpleReassignment() {
+ public void testBad() {
doTest();
}
- public void testTupleAssignment() {
- doTest();
- }
-
- public void testTupleDeclaration() {
- doTest();
- }
-
- public void testTwoLoops() {
- doTest();
- }
- public void testWithStatement() {
- doTest();
- }
-
-
@NotNull
@Override
protected Class getInspectionClass() {