From e25a3c6e18b1167b3be3b2bb165edc883f68fcd5 Mon Sep 17 00:00:00 2001 From: Semyon Proshev Date: Fri, 4 Aug 2017 16:23:04 +0300 Subject: [PATCH] PY-22716 Fixed: False positive __slots__ inspection in subclasses of classes Update PyDunderSlotsInspection to correctly infer read-only attribute status in Py2 and Py3+. --- .../inspections/PyDunderSlotsInspection.kt | 90 +++++++++++-------- .../writeToInheritedSlot.py | 10 +++ .../PyDunderSlotsInspectionTest.java | 6 ++ 3 files changed, 70 insertions(+), 36 deletions(-) create mode 100644 python/testData/inspections/PyDunderSlotsInspection/writeToInheritedSlot.py diff --git a/python/src/com/jetbrains/python/inspections/PyDunderSlotsInspection.kt b/python/src/com/jetbrains/python/inspections/PyDunderSlotsInspection.kt index 0bffce77a8d2..7f9bff3b2c87 100644 --- a/python/src/com/jetbrains/python/inspections/PyDunderSlotsInspection.kt +++ b/python/src/com/jetbrains/python/inspections/PyDunderSlotsInspection.kt @@ -1,5 +1,5 @@ /* - * Copyright 2000-2016 JetBrains s.r.o. + * Copyright 2000-2017 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. @@ -15,13 +15,13 @@ */ package com.jetbrains.python.inspections +import com.google.common.collect.Iterables import com.intellij.codeInspection.LocalInspectionToolSession import com.intellij.codeInspection.ProblemsHolder import com.intellij.psi.PsiElementVisitor import com.jetbrains.python.PyNames import com.jetbrains.python.psi.* import com.jetbrains.python.psi.impl.PyPsiUtils -import com.jetbrains.python.psi.resolve.PyResolveContext import com.jetbrains.python.psi.types.PyClassType class PyDunderSlotsInspection : PyInspection() { @@ -40,12 +40,12 @@ class PyDunderSlotsInspection : PyInspection() { when (slots) { is PySequenceExpression -> slots - .elements - .asSequence() - .filterIsInstance() - .forEach { - processSlot(node, it) - } + .elements + .asSequence() + .filterIsInstance() + .forEach { + processSlot(node, it) + } is PyStringLiteralExpression -> processSlot(node, slots) } } @@ -60,7 +60,7 @@ class PyDunderSlotsInspection : PyInspection() { } private fun findSlotsValue(pyClass: PyClass): PyExpression? { - val target = pyClass.findClassAttribute(PyNames.SLOTS, false, myTypeEvalContext) as? PyTargetExpression + val target = pyClass.findClassAttribute(PyNames.SLOTS, false, myTypeEvalContext) val value = target?.findAssignedValue() return PyPsiUtils.flattenParens(value) @@ -84,45 +84,63 @@ class PyDunderSlotsInspection : PyInspection() { val qualifierType = myTypeEvalContext.getType(qualifier) if (qualifierType is PyClassType && !qualifierType.isDefinition) { - val reference = target.getReference(PyResolveContext.noImplicits().withTypeEvalContext(myTypeEvalContext)) val qualifierClass = qualifierType.pyClass - val classWithReadOnlyAttr = PyUtil - .multiResolveTopPriority(reference) - .asSequence() - .filterIsInstance() - .map { declaration -> declaration.containingClass } - .filterNotNull() - .find { declaringClass -> !attributeIsWritable(qualifierClass, declaringClass, targetName) } - - if (classWithReadOnlyAttr != null) { + if (!attributeIsWritable(qualifierClass, targetName)) { registerProblem(target, "'${qualifierClass.name}' object attribute '$targetName' is read-only") } } } - private fun attributeIsWritable(qualifierClass: PyClass, declaringClass: PyClass, targetName: String): Boolean { - return attributeIsWritableInClass(qualifierClass, declaringClass, targetName) || - qualifierClass - .getAncestorClasses(myTypeEvalContext) - .asSequence() - .filter { ancestorClass -> !PyUtil.isObjectClass(ancestorClass) } - .any { ancestorClass -> attributeIsWritableInClass(ancestorClass, declaringClass, targetName) } + private fun attributeIsWritable(cls: PyClass, name: String): Boolean { + /* + The only difference between Py2 and Py3+ is that the following case is not highlighted in Py3+: + + class A: + attr = "attr" + __slots__ = ("attr") + + A().attr + + Py3+ raises ValueError about conflict between __slots__ and class variable. + This case is handled above by com.jetbrains.python.inspections.PyDunderSlotsInspection.Visitor.processSlot method. + */ + if (LanguageLevel.forElement(cls).isOlderThan(LanguageLevel.PYTHON30)) { + return attributeIsWritableInPy2(cls, name) + } + else { + return attributeIsWritableInPy3(cls, name) + } } - private fun attributeIsWritableInClass(cls: PyClass, declaringClass: PyClass, targetName: String): Boolean { - val ownSlots = cls.ownSlots + private fun attributeIsWritableInPy2(cls: PyClass, name: String): Boolean { + val slots = cls.getSlots(myTypeEvalContext) + return slots == null || + slots.contains(PyNames.DICT) || + slots.contains(name) && cls.findClassAttribute(name, true, myTypeEvalContext) == null + } - if (ownSlots == null || ownSlots.contains(PyNames.DICT)) { - return true + private fun attributeIsWritableInPy3(cls: PyClass, name: String): Boolean { + var classAttrIsFound = false + var slotIsFound = false + + for (c in Iterables.concat(listOf(cls), cls.getAncestorClasses(myTypeEvalContext))) { + if (PyUtil.isObjectClass(c)) continue + + val ownSlots = c.ownSlots + + if (ownSlots == null || ownSlots.contains(PyNames.DICT)) return true + + if (!classAttrIsFound) { + classAttrIsFound = c.findClassAttribute(name, false, myTypeEvalContext) != null + if (ownSlots.contains(name)) { + if (classAttrIsFound) return true + slotIsFound = true + } + } } - if (!cls.equals(declaringClass) || !ownSlots.contains(targetName)) { - return false - } - - return LanguageLevel.forElement(declaringClass).isAtLeast(LanguageLevel.PYTHON30) || - declaringClass.findClassAttribute(targetName, false, myTypeEvalContext) == null + return slotIsFound && !classAttrIsFound } } } diff --git a/python/testData/inspections/PyDunderSlotsInspection/writeToInheritedSlot.py b/python/testData/inspections/PyDunderSlotsInspection/writeToInheritedSlot.py new file mode 100644 index 000000000000..ad02eba4c8b1 --- /dev/null +++ b/python/testData/inspections/PyDunderSlotsInspection/writeToInheritedSlot.py @@ -0,0 +1,10 @@ +class A(object): + __slots__ = ("a", "b", "c") + + +class B(A): + __slots__ = ("d", "e") + + def __init__(self): + self.a = 2 + self.d = 2 \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/inspections/PyDunderSlotsInspectionTest.java b/python/testSrc/com/jetbrains/python/inspections/PyDunderSlotsInspectionTest.java index 411de65a249d..806d09f2c11b 100644 --- a/python/testSrc/com/jetbrains/python/inspections/PyDunderSlotsInspectionTest.java +++ b/python/testSrc/com/jetbrains/python/inspections/PyDunderSlotsInspectionTest.java @@ -234,6 +234,12 @@ public class PyDunderSlotsInspectionTest extends PyTestCase { doTestPy3(); } + // PY-22716 + public void testWriteToInheritedSlot() { + doTestPy2(); + doTestPy3(); + } + private void doTestPy2() { runWithLanguageLevel(LanguageLevel.PYTHON26, this::doTestPy); }