PY-12773 Fixed: Misleading warning for attribute assignment when a class has __slots__

Update PyPropertyAccessInspection to check writing to class attribute in case of __slots__ in class

python/src/com/jetbrains/python/inspections/PyPropertyAccessInspection.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2014 JetBrains s.r.o.
+ * Copyright 2000-2016 JetBrains s.r.o.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -22,14 +22,18 @@ import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiElementVisitor;
 import com.intellij.util.containers.HashMap;
 import com.jetbrains.python.PyBundle;
+import com.jetbrains.python.PyNames;
 import com.jetbrains.python.inspections.quickfix.PyCreatePropertyQuickFix;
 import com.jetbrains.python.psi.*;
+import com.jetbrains.python.psi.resolve.PyResolveContext;
 import com.jetbrains.python.psi.types.PyClassType;
 import com.jetbrains.python.psi.types.PyType;
 import com.jetbrains.python.toolbox.Maybe;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
 
+import java.util.List;
+
 /**
  * Checks that properties are accessed correctly.
  * User: dcheryasov
@@ -64,16 +68,17 @@ public class PyPropertyAccessInspection extends PyInspection {
     @Override
     public void visitPyReferenceExpression(PyReferenceExpression node) {
       super.visitPyReferenceExpression(node);
-      checkExpression(node);
+      checkPropertyExpression(node);
     }
 
     @Override
     public void visitPyTargetExpression(PyTargetExpression node) {
       super.visitPyTargetExpression(node);
-      checkExpression(node);
+      checkPropertyExpression(node);
+      checkAttributeExpression(node);
     }
 
-    private void checkExpression(PyQualifiedExpression node) {
+    private void checkPropertyExpression(PyQualifiedExpression node) {
       final PyExpression qualifier = node.getQualifier();
       if (qualifier != null) {
         final PyType type = myTypeEvalContext.getType(qualifier);
@@ -122,5 +127,57 @@ public class PyPropertyAccessInspection extends PyInspection {
       }
     }
 
+    private void checkAttributeExpression(@NotNull PyTargetExpression target) {
+      final String targetName = target.getName();
+      final PyExpression qualifier = target.getQualifier();
+
+      if (targetName == null || qualifier == null) {
+        return;
+      }
+
+      final PyType qualifierType = myTypeEvalContext.getType(qualifier);
+
+      if (qualifierType instanceof PyClassType) {
+        final PyClassType qualifierClassType = (PyClassType)qualifierType;
+
+        if (!qualifierClassType.isDefinition()) {
+          final PyClass qualifierClass = qualifierClassType.getPyClass();
+
+          PyUtil
+            .multiResolveTopPriority(target.getReference(PyResolveContext.noImplicits().withTypeEvalContext(myTypeEvalContext)))
+            .stream()
+            .filter(PyTargetExpression.class::isInstance)
+            .map(declaration -> ((PyTargetExpression)declaration).getContainingClass())
+            .filter(declaringClass -> declaringClass != null && !attributeIsWritable(qualifierClass, declaringClass, target))
+            .findFirst()
+            .ifPresent(
+              cls -> registerProblem(target, String.format("'%s' object attribute '%s' is read-only", qualifierClass.getName(), targetName))
+            );
+        }
+      }
+    }
+
+    private boolean attributeIsWritable(@NotNull PyClass qualifierClass,
+                                        @NotNull PyClass declaringClass,
+                                        @NotNull PyTargetExpression target) {
+      return attributeIsWritableInClass(qualifierClass, declaringClass, target) ||
+             qualifierClass
+               .getAncestorClasses(myTypeEvalContext)
+               .stream()
+               .filter(ancestorClass -> !PyUtil.isObjectClass(ancestorClass))
+               .anyMatch(ancestorClass -> attributeIsWritableInClass(ancestorClass, declaringClass, target));
+    }
+
+    private static boolean attributeIsWritableInClass(@NotNull PyClass cls,
+                                                      @NotNull PyClass declaringClass,
+                                                      @NotNull PyTargetExpression target) {
+      final List<String> ownSlots = cls.getOwnSlots();
+
+      return ownSlots == null ||
+             ownSlots.contains(PyNames.DICT) ||
+             (LanguageLevel.forElement(target).isPy3K() &&
+              cls.equals(declaringClass) &&
+              ownSlots.contains(target.getName()));
+    }
   }
 }

python/testData/inspections/PyPropertyAccessInspection/classAttrAssignmentAndInheritedSlots.py

@@ -0,0 +1,13 @@
+class B(object):
+    __slots__ = ['foo', 'bar']
+
+
+class C(B):
+    attr = 'baz'
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/classAttrAssignmentAndOwnAndInheritedSlots.py

@@ -0,0 +1,13 @@
+class B(object):
+    __slots__ = ['f', 'b']
+
+class C(B):
+    attr = 'baz'
+    __slots__ = ['foo', 'bar']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+<warning descr="'C' object attribute 'attr' is read-only">c.attr</warning> = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/classAttrAssignmentAndOwnAndInheritedWithAttrAndDictSlots.py

@@ -0,0 +1,13 @@
+class B(object):
+    __slots__ = ['attr', 'b', '__dict__']
+
+class C(B):
+    attr = 'baz'
+    __slots__ = ['foo', 'bar']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/classAttrAssignmentAndOwnAndInheritedWithAttrSlots.py

@@ -0,0 +1,13 @@
+class B(object):
+    __slots__ = ['attr', 'b']
+
+class C(B):
+    attr = 'baz'
+    __slots__ = ['foo', 'bar']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+<warning descr="'C' object attribute 'attr' is read-only">c.attr</warning> = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/classAttrAssignmentAndOwnAndInheritedWithDictSlots.py

@@ -0,0 +1,13 @@
+class B(object):
+    __slots__ = ['f', 'b', '__dict__']
+
+class C(B):
+    attr = 'baz'
+    __slots__ = ['foo', 'bar']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/classAttrAssignmentAndOwnSlotsAndEmptyParent.py

@@ -0,0 +1,13 @@
+class B(object):
+    pass
+
+class C(B):
+    attr = 'baz'
+    __slots__ = ['foo', 'bar']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/classAttrAssignmentAndOwnWithAttrAndInheritedSlotsPy2.py

@@ -0,0 +1,13 @@
+class B(object):
+    __slots__ = ['f', 'b']
+
+class C(B):
+    attr = 'baz'
+    __slots__ = ['attr', 'bar']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+<warning descr="'C' object attribute 'attr' is read-only">c.attr</warning> = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/classAttrAssignmentAndOwnWithAttrAndInheritedSlotsPy3.py

@@ -0,0 +1,15 @@
+class B(object):
+    __slots__ = ['f', 'b']
+
+# ValueError: 'attr' in __slots__ conflicts with class variable
+# This is not responsibility of current inspection
+class C(B):
+    attr = 'baz'
+    __slots__ = ['attr', 'bar']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/classAttrAssignmentAndOwnWithDictAndInheritedSlots.py

@@ -0,0 +1,13 @@
+class B(object):
+    __slots__ = ['f', 'b']
+
+class C(B):
+    attr = 'baz'
+    __slots__ = ['foo', 'bar', '__dict__']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/classAttrAssignmentAndOwnWithDictAndInheritedWithAttrSlots.py

@@ -0,0 +1,13 @@
+class B(object):
+    __slots__ = ['attr', 'b']
+
+class C(B):
+    attr = 'baz'
+    __slots__ = ['foo', 'bar', '__dict__']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/classAttrAssignmentAndSlots.py

@@ -0,0 +1,10 @@
+class Foo(object):
+    attr = 'baz'
+    __slots__ = ['foo', 'bar']
+
+Foo.attr = 'spam'
+print(Foo.attr)
+
+foo = Foo()
+<warning descr="'Foo' object attribute 'attr' is read-only">foo.attr</warning> = 'spam'
+print(foo.attr)

python/testData/inspections/PyPropertyAccessInspection/classAttrAssignmentAndSlotsWithAttrPy2.py

@@ -0,0 +1,10 @@
+class Foo(object):
+    attr = 'baz'
+    __slots__ = ['attr', 'bar']
+
+Foo.attr = 'spam'
+print(Foo.attr)
+
+foo = Foo()
+<warning descr="'Foo' object attribute 'attr' is read-only">foo.attr</warning> = 'spam'
+print(foo.attr)

python/testData/inspections/PyPropertyAccessInspection/classAttrAssignmentAndSlotsWithAttrPy3.py

@@ -0,0 +1,12 @@
+# ValueError: 'attr' in __slots__ conflicts with class variable
+# This is not responsibility of current inspection
+class Foo(object):
+    attr = 'baz'
+    __slots__ = ['attr', 'bar']
+
+Foo.attr = 'spam'
+print(Foo.attr)
+
+foo = Foo()
+foo.attr = 'spam'
+print(foo.attr)

python/testData/inspections/PyPropertyAccessInspection/classAttrAssignmentAndSlotsWithDict.py

@@ -0,0 +1,10 @@
+class Foo(object):
+    attr = 'baz'
+    __slots__ = ['foo', 'bar', '__dict__']
+
+Foo.attr = 'spam'
+print(Foo.attr)
+
+foo = Foo()
+foo.attr = 'spam'
+print(foo.attr)

python/testData/inspections/PyPropertyAccessInspection/inheritedClassAttrAssignmentAndInheritedSlots.py

@@ -0,0 +1,14 @@
+class B(object):
+    attr = 'baz'
+    __slots__ = ['foo', 'bar']
+
+
+class C(B):
+    pass
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/inheritedClassAttrAssignmentAndOwnAndInheritedSlots.py

@@ -0,0 +1,13 @@
+class B(object):
+    attr = 'baz'
+    __slots__ = ['f', 'b']
+
+class C(B):
+    __slots__ = ['foo', 'bar']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+<warning descr="'C' object attribute 'attr' is read-only">c.attr</warning> = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/inheritedClassAttrAssignmentAndOwnAndInheritedWithAttrAndDictSlots.py

@@ -0,0 +1,17 @@
+# Py3: OK
+# Py2:
+# ValueError: 'attr' in __slots__ conflicts with class variable
+# This is not responsibility of current inspection
+class B(object):
+    attr = 'baz'
+    __slots__ = ['f', 'attr', '__dict__']
+
+class C(B):
+    __slots__ = ['foo', 'bar']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/inheritedClassAttrAssignmentAndOwnAndInheritedWithAttrSlotsPy2.py

@@ -0,0 +1,13 @@
+class B(object):
+    attr = 'baz'
+    __slots__ = ['attr', 'b']
+
+class C(B):
+    __slots__ = ['foo', 'bar']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+<warning descr="'C' object attribute 'attr' is read-only">c.attr</warning> = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/inheritedClassAttrAssignmentAndOwnAndInheritedWithAttrSlotsPy3.py

@@ -0,0 +1,15 @@
+# ValueError: 'attr' in __slots__ conflicts with class variable
+# This is not responsibility of current inspection
+class B(object):
+    attr = 'baz'
+    __slots__ = ['attr', 'b']
+
+class C(B):
+    __slots__ = ['foo', 'bar']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/inheritedClassAttrAssignmentAndOwnAndInheritedWithDictSlots.py

@@ -0,0 +1,13 @@
+class B(object):
+    attr = 'baz'
+    __slots__ = ['f', 'b', '__dict__']
+
+class C(B):
+    __slots__ = ['foo', 'bar']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/inheritedClassAttrAssignmentAndOwnSlots.py

@@ -0,0 +1,13 @@
+class B(object):
+    attr = 'baz'
+
+
+class C(B):
+    __slots__ = ['foo', 'bar']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/inheritedClassAttrAssignmentAndOwnWithAttrAndDictAndInheritedSlots.py

@@ -0,0 +1,13 @@
+class B(object):
+    attr = 'baz'
+    __slots__ = ['f', 'b']
+
+class C(B):
+    __slots__ = ['attr', 'bar', '__dict__']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/inheritedClassAttrAssignmentAndOwnWithAttrAndInheritedSlots.py

@@ -0,0 +1,13 @@
+class B(object):
+    attr = 'baz'
+    __slots__ = ['f', 'b']
+
+class C(B):
+    __slots__ = ['attr', 'bar']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+<warning descr="'C' object attribute 'attr' is read-only">c.attr</warning> = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/inheritedClassAttrAssignmentAndOwnWithAttrAndInheritedWithDictSlots.py

@@ -0,0 +1,13 @@
+class B(object):
+    attr = 'baz'
+    __slots__ = ['f', 'b', '__dict__']
+
+class C(B):
+    __slots__ = ['attr', 'bar']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/inheritedClassAttrAssignmentAndOwnWithDictAndInheritedSlots.py

@@ -0,0 +1,13 @@
+class B(object):
+    attr = 'baz'
+    __slots__ = ['f', 'b']
+
+class C(B):
+    __slots__ = ['foo', 'bar', '__dict__']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testData/inspections/PyPropertyAccessInspection/inheritedClassAttrAssignmentAndOwnWithDictAndInheritedWithAttrSlots.py

@@ -0,0 +1,17 @@
+# Py2: OK
+# Py3:
+# ValueError: 'attr' in __slots__ conflicts with class variable
+# This is not responsibility of current inspection
+class B(object):
+    attr = 'baz'
+    __slots__ = ['f', 'attr']
+
+class C(B):
+    __slots__ = ['foo', 'bar', '__dict__']
+
+C.attr = 'spam'
+print(C.attr)
+
+c = C()
+c.attr = 'spam'
+print(c.attr)

python/testSrc/com/jetbrains/python/inspections/PyPropertyAccessInspectionTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2013 JetBrains s.r.o.
+ * Copyright 2000-2016 JetBrains s.r.o.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,15 +23,153 @@ import com.jetbrains.python.psi.LanguageLevel;
  */
 public class PyPropertyAccessInspectionTest extends PyTestCase {
   public void testTest() {
-    doTest();
+    doTestPy2();
   }
 
-  public void testOverrideAssignment() {  // PY-2313
-    doTest();
+  // PY-2313
+  public void testOverrideAssignment() {
+    doTestPy2();
   }
 
-  private void doTest() {
-    setLanguageLevel(LanguageLevel.PYTHON26);
+  // PY-12773
+  public void testClassAttrAssignmentAndSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testClassAttrAssignmentAndSlotsWithDict() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testClassAttrAssignmentAndSlotsWithAttrPy2() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testClassAttrAssignmentAndSlotsWithAttrPy3() {
+    doTestPy3();
+  }
+
+  // PY-12773
+  public void testInheritedClassAttrAssignmentAndInheritedSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testClassAttrAssignmentAndInheritedSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testInheritedClassAttrAssignmentAndOwnSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testClassAttrAssignmentAndOwnSlotsAndEmptyParent() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testClassAttrAssignmentAndOwnAndInheritedSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testClassAttrAssignmentAndOwnWithDictAndInheritedSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testClassAttrAssignmentAndOwnWithDictAndInheritedWithAttrSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testClassAttrAssignmentAndOwnAndInheritedWithDictSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testClassAttrAssignmentAndOwnWithAttrAndInheritedSlotsPy2() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testClassAttrAssignmentAndOwnWithAttrAndInheritedSlotsPy3() {
+    doTestPy3();
+  }
+
+  // PY-12773
+  public void testClassAttrAssignmentAndOwnAndInheritedWithAttrSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testClassAttrAssignmentAndOwnAndInheritedWithAttrAndDictSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testInheritedClassAttrAssignmentAndOwnAndInheritedSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testInheritedClassAttrAssignmentAndOwnWithAttrAndInheritedSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testInheritedClassAttrAssignmentAndOwnWithDictAndInheritedSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testInheritedClassAttrAssignmentAndOwnWithAttrAndDictAndInheritedSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testInheritedClassAttrAssignmentAndOwnAndInheritedWithAttrSlotsPy2() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testInheritedClassAttrAssignmentAndOwnAndInheritedWithAttrSlotsPy3() {
+    doTestPy3();
+  }
+
+  // PY-12773
+  public void testInheritedClassAttrAssignmentAndOwnAndInheritedWithDictSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testInheritedClassAttrAssignmentAndOwnAndInheritedWithAttrAndDictSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testInheritedClassAttrAssignmentAndOwnWithAttrAndInheritedWithDictSlots() {
+    doTestPy2();
+  }
+
+  // PY-12773
+  public void testInheritedClassAttrAssignmentAndOwnWithDictAndInheritedWithAttrSlots() {
+    doTestPy2();
+  }
+
+  private void doTestPy2() {
+    runWithLanguageLevel(LanguageLevel.PYTHON26, this::doTestPy);
+  }
+
+  private void doTestPy3() {
+    runWithLanguageLevel(LanguageLevel.PYTHON30, this::doTestPy);
+  }
+
+  private void doTestPy() {
     myFixture.configureByFile("inspections/PyPropertyAccessInspection/" + getTestName(true) + ".py");
     myFixture.enableInspections(PyPropertyAccessInspection.class);
     myFixture.checkHighlighting(true, false, false);