From ae91878f131fb180bc5571bd0d86c7ed36f0e507 Mon Sep 17 00:00:00 2001 From: Semyon Proshev Date: Thu, 22 Mar 2018 18:23:59 +0300 Subject: [PATCH] Improve processing `__slots__` in PyUnresolvedReferencesInspection (PY-29229) Fix false positive when accessed attribute is declared in ancestor `__slots__` --- .../PyUnresolvedReferencesInspection.java | 71 ++++++++++--------- .../src/com/jetbrains/python/psi/PyUtil.java | 7 ++ .../ownSlots.py | 34 +++++++++ .../slotsAndInheritance.py | 48 +++++++++++++ .../slotsAndListedAttrAccess.py | 5 -- .../slotsSubclass.py | 15 ---- .../slotsSuperclass.py | 8 --- .../slotsWithDict.py | 1 + .../PyUnresolvedReferencesInspectionTest.java | 16 ++--- 9 files changed, 132 insertions(+), 73 deletions(-) create mode 100644 python/testData/inspections/PyUnresolvedReferencesInspection/ownSlots.py create mode 100644 python/testData/inspections/PyUnresolvedReferencesInspection/slotsAndInheritance.py delete mode 100644 python/testData/inspections/PyUnresolvedReferencesInspection/slotsAndListedAttrAccess.py delete mode 100644 python/testData/inspections/PyUnresolvedReferencesInspection/slotsSubclass.py delete mode 100644 python/testData/inspections/PyUnresolvedReferencesInspection/slotsSuperclass.py diff --git a/python/src/com/jetbrains/python/inspections/unresolvedReference/PyUnresolvedReferencesInspection.java b/python/src/com/jetbrains/python/inspections/unresolvedReference/PyUnresolvedReferencesInspection.java index c4b41b9ad620..f57689177e7c 100644 --- a/python/src/com/jetbrains/python/inspections/unresolvedReference/PyUnresolvedReferencesInspection.java +++ b/python/src/com/jetbrains/python/inspections/unresolvedReference/PyUnresolvedReferencesInspection.java @@ -166,45 +166,34 @@ public class PyUnresolvedReferencesInspection extends PyInspection { final PyType type = myTypeEvalContext.getType(qualifier); if (type instanceof PyClassType) { final PyClass pyClass = ((PyClassType)type).getPyClass(); - if (pyClass.isNewStyleClass(myTypeEvalContext)) { - if (pyClass.getOwnSlots() == null) { - return; - } - final String attrName = node.getReferencedName(); - if (!canHaveAttribute(pyClass, attrName)) { - for (PyClass ancestor : pyClass.getAncestorClasses(myTypeEvalContext)) { - if (ancestor == null) { - return; - } - if (PyNames.OBJECT.equals(ancestor.getName())) { - break; - } - if (canHaveAttribute(ancestor, attrName)) { - return; - } + final String attrName = node.getReferencedName(); + if (attrName != null && !canHaveAttribute(pyClass, attrName)) { + for (PyClass ancestor : pyClass.getAncestorClasses(myTypeEvalContext)) { + if (ancestor == null) { + return; + } + if (PyUtil.isObjectClass(ancestor)) { + break; + } + if (canHaveAttribute(ancestor, attrName)) { + return; } - final ASTNode nameNode = node.getNameElement(); - final PsiElement e = nameNode != null ? nameNode.getPsi() : node; - registerProblem(e, "'" + pyClass.getName() + "' object has no attribute '" + attrName + "'"); } + final ASTNode nameNode = node.getNameElement(); + final PsiElement e = nameNode != null ? nameNode.getPsi() : node; + registerProblem(e, "'" + pyClass.getName() + "' object has no attribute '" + attrName + "'"); } } } } - private boolean canHaveAttribute(@NotNull PyClass cls, @Nullable String attrName) { - final List slots = cls.getOwnSlots(); + private boolean canHaveAttribute(@NotNull PyClass cls, @NotNull String attrName) { + final List slots = PyUtil.deactivateSlots(cls, cls.getOwnSlots(), myTypeEvalContext); - // Class instance can contain attributes with arbitrary names - if (slots == null || slots.contains(PyNames.DICT)) { - return true; - } - - if (attrName != null && cls.findClassAttribute(attrName, true, myTypeEvalContext) != null) { - return true; - } - - return slots.contains(attrName) || cls.getProperties().containsKey(attrName); + return slots == null || + slots.contains(attrName) || + cls.findClassAttribute(attrName, false, myTypeEvalContext) != null || + cls.findProperty(attrName, false, myTypeEvalContext) != null; } @Override @@ -583,9 +572,7 @@ public class PyUnresolvedReferencesInspection extends PyInspection { ((PyOperatorReference)reference).getReadableOperatorName()); } else { - final List slots = classType.getPyClass().getOwnSlots(); - - if (slots != null && slots.contains(refName)) { + if (isDeclaredInSlots(classType, refName)) { return; } @@ -683,6 +670,22 @@ public class PyUnresolvedReferencesInspection extends PyInspection { return null; } + private boolean isDeclaredInSlots(@NotNull PyType type, @NotNull String attrName) { + if (type instanceof PyClassType) { + final PyClass cls = ((PyClassType)type).getPyClass(); + + return StreamEx + .of(cls) + .append(cls.getAncestorClasses(myTypeEvalContext)) + .nonNull() + .filter(c -> c.isNewStyleClass(myTypeEvalContext)) + .flatCollection(PyClass::getOwnSlots) + .anyMatch(attrName::equals); + } + + return false; + } + private static void addInstallPackageAction(List actions, String packageName, Module module, Sdk sdk) { final List requirements = Collections.singletonList(PyRequirementsKt.pyRequirement(packageName)); final String name = "Install package " + packageName; diff --git a/python/src/com/jetbrains/python/psi/PyUtil.java b/python/src/com/jetbrains/python/psi/PyUtil.java index 470b204d40f9..b4372acb86cb 100644 --- a/python/src/com/jetbrains/python/psi/PyUtil.java +++ b/python/src/com/jetbrains/python/psi/PyUtil.java @@ -1905,6 +1905,13 @@ public class PyUtil { } } + @Nullable + public static List deactivateSlots(@NotNull PyClass cls, @Nullable List slots, @NotNull TypeEvalContext context) { + if (!cls.isNewStyleClass(context)) return null; + if (slots == null || slots.contains(PyNames.DICT)) return null; + return slots; + } + /** * This helper class allows to collect various information about AST nodes composing {@link PyStringLiteralExpression}. */ diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/ownSlots.py b/python/testData/inspections/PyUnresolvedReferencesInspection/ownSlots.py new file mode 100644 index 000000000000..55d4a975bb0c --- /dev/null +++ b/python/testData/inspections/PyUnresolvedReferencesInspection/ownSlots.py @@ -0,0 +1,34 @@ +def access1(): + class B(object): + __slots__ = ['foo'] + + b = B() + print(b.baz) + print(b.foo) + + +def assign1(): + class B(object): + __slots__ = ['foo'] + + b = B() + b.bar = 1 + b.foo = 1 + + +def access2(): + class A: + __slots__ = ['foo'] + + a = A() + print(a.foo) + print(a.bar) + + +def assign2(): + class A: + __slots__ = ['foo'] + + a = A() + a.foo = 1 + a.bar = 1 \ No newline at end of file diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/slotsAndInheritance.py b/python/testData/inspections/PyUnresolvedReferencesInspection/slotsAndInheritance.py new file mode 100644 index 000000000000..f5475d0ea0a6 --- /dev/null +++ b/python/testData/inspections/PyUnresolvedReferencesInspection/slotsAndInheritance.py @@ -0,0 +1,48 @@ +def access1(): + class B(object): + __slots__ = ['foo'] + + class C(B): + pass + + c = C() + print(c.bar) + print(c.foo) + + +def assign1(): + class B(object): + __slots__ = ['foo'] + + class C(B): + pass + + c = C() + c.bar = 1 + c.foo = 1 + + +def access2(): + class A(object): + __slots__ = ['foo'] + + class D(A): + __slots__ = ['bar'] + + d = D() + print(d.foo) + print(d.bar) + print(d.baz) + + +def assign2(): + class A(object): + __slots__ = ['foo'] + + class D(A): + __slots__ = ['bar'] + + d = D() + d.foo = 1 + d.bar = 1 + d.baz = 1 \ No newline at end of file diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/slotsAndListedAttrAccess.py b/python/testData/inspections/PyUnresolvedReferencesInspection/slotsAndListedAttrAccess.py deleted file mode 100644 index bd61153c5cda..000000000000 --- a/python/testData/inspections/PyUnresolvedReferencesInspection/slotsAndListedAttrAccess.py +++ /dev/null @@ -1,5 +0,0 @@ -class C(object): - __slots__ = ['foo'] - -c = C() -c.foo \ No newline at end of file diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/slotsSubclass.py b/python/testData/inspections/PyUnresolvedReferencesInspection/slotsSubclass.py deleted file mode 100644 index dfd3f5e4cc50..000000000000 --- a/python/testData/inspections/PyUnresolvedReferencesInspection/slotsSubclass.py +++ /dev/null @@ -1,15 +0,0 @@ -class A(object): - __slots__ = ['a', 'b'] - def __init__(self): - self.a = None # <- all ok here - self.b = None # <- all ok here - -class C(A): - __slots__ = ['c', 'd'] - - def __init__(self, c): - super(C, self).__init__() - self.c = c - self.d = self.b - if self.c: - self.a = 10 diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/slotsSuperclass.py b/python/testData/inspections/PyUnresolvedReferencesInspection/slotsSuperclass.py deleted file mode 100644 index 9ee0c1d62560..000000000000 --- a/python/testData/inspections/PyUnresolvedReferencesInspection/slotsSuperclass.py +++ /dev/null @@ -1,8 +0,0 @@ -class B(object): - __slots__ = ['foo'] - -class C(B): - pass - -c = C() -c.bar = 1 \ No newline at end of file diff --git a/python/testData/inspections/PyUnresolvedReferencesInspection/slotsWithDict.py b/python/testData/inspections/PyUnresolvedReferencesInspection/slotsWithDict.py index 1b3cfc2d3470..ddfe3e40e97d 100644 --- a/python/testData/inspections/PyUnresolvedReferencesInspection/slotsWithDict.py +++ b/python/testData/inspections/PyUnresolvedReferencesInspection/slotsWithDict.py @@ -2,4 +2,5 @@ class C(object): __slots__ = ['__local', '__name__', '__dict__'] a = C() +print(a.bar) a.foo = 1 #pass \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/inspections/PyUnresolvedReferencesInspectionTest.java b/python/testSrc/com/jetbrains/python/inspections/PyUnresolvedReferencesInspectionTest.java index a936078564ed..de7cd6107ef5 100644 --- a/python/testSrc/com/jetbrains/python/inspections/PyUnresolvedReferencesInspectionTest.java +++ b/python/testSrc/com/jetbrains/python/inspections/PyUnresolvedReferencesInspectionTest.java @@ -41,11 +41,14 @@ public class PyUnresolvedReferencesInspectionTest extends PyInspectionTestCase { doTest(); } - public void testSlotsAndUnlistedAttrAssign() { + // PY-10397 + public void testOwnSlots() { doTest(); } - public void testSlotsSuperclass() { + // PY-5939 + // PY-29229 + public void testSlotsAndInheritance() { doTest(); } @@ -53,20 +56,11 @@ public class PyUnresolvedReferencesInspectionTest extends PyInspectionTestCase { doTest(); } - // PY-10397 - public void testSlotsAndListedAttrAccess() { - doTest(); - } - // PY-18422 public void testSlotsAndClassAttr() { doTest(); } - public void testSlotsSubclass() { // PY-5939 - doTest(); - } - public void testImportExceptImportError() { doTest(); }