From 4aeb385d9489f6a75400395a2f2bb5daebdd2dc6 Mon Sep 17 00:00:00 2001 From: Anton Bragin Date: Tue, 20 Mar 2018 19:16:38 +0300 Subject: [PATCH] PY-25263 Attribute initialization via property set implemented --- .../PyAttributeOutsideInitInspection.java | 78 ++++++++++++++----- .../property.py | 11 +++ .../propertyAnnotation.py | 11 +++ .../propertyNotSetInInit.py | 11 +++ .../PyAttributeOutsideInitInspectionTest.java | 15 ++++ 5 files changed, 107 insertions(+), 19 deletions(-) create mode 100644 python/testData/inspections/PyAttributeOutsideInitInspection/property.py create mode 100644 python/testData/inspections/PyAttributeOutsideInitInspection/propertyAnnotation.py create mode 100644 python/testData/inspections/PyAttributeOutsideInitInspection/propertyNotSetInInit.py diff --git a/python/src/com/jetbrains/python/inspections/PyAttributeOutsideInitInspection.java b/python/src/com/jetbrains/python/inspections/PyAttributeOutsideInitInspection.java index 83b9c14a3098..d08be9ae2fd4 100644 --- a/python/src/com/jetbrains/python/inspections/PyAttributeOutsideInitInspection.java +++ b/python/src/com/jetbrains/python/inspections/PyAttributeOutsideInitInspection.java @@ -21,6 +21,7 @@ import com.intellij.psi.PsiElementVisitor; import com.intellij.util.ThreeState; import com.jetbrains.python.PyBundle; import com.jetbrains.python.PyNames; +import com.jetbrains.python.codeInsight.controlflow.ControlFlowCache; import com.jetbrains.python.inspections.quickfix.PyMoveAttributeToInitQuickFix; import com.jetbrains.python.psi.Property; import com.jetbrains.python.psi.PyClass; @@ -29,10 +30,12 @@ import com.jetbrains.python.psi.PyTargetExpression; import com.jetbrains.python.psi.impl.PyClassImpl; import com.jetbrains.python.psi.types.TypeEvalContext; import com.jetbrains.python.testing.PythonUnitTestUtil; +import one.util.streamex.StreamEx; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -40,8 +43,7 @@ import java.util.Map; /** * User: ktisha * - * Inspection to detect situations, where instance attribute - * defined outside __init__ function + * Inspection to detect situations, where instance attribute is defined outside __init__ function. */ public class PyAttributeOutsideInitInspection extends PyInspection { @Nls @@ -74,46 +76,84 @@ public class PyAttributeOutsideInitInspection extends PyInspection { if (!isApplicable(containingClass, myTypeEvalContext)) { return; } - - final PyFunction.Modifier modifier = node.getModifier(); - if (modifier != null) return; - final List classAttributes = containingClass.getClassAttributes(); - - Map attributesInInit = new HashMap<>(); - for (PyTargetExpression classAttr : classAttributes) { - attributesInInit.put(classAttr.getName(), classAttr); + if (node.getModifier() != null) { + return; } + final List classAttributes = containingClass.getClassAttributes(); + final Map properties = containingClass.getProperties(); + final Map attributesInInit = new HashMap<>(); + + StreamEx.of(classAttributes) + .filter(attribute -> !properties.containsKey(attribute.getName())) + .forEach(attribute -> attributesInInit.put(attribute.getName(), attribute)); + final PyFunction initMethod = containingClass.findMethodByName(PyNames.INIT, false, null); if (initMethod != null) { PyClassImpl.collectInstanceAttributes(initMethod, attributesInInit); } for (PyClass superClass : containingClass.getAncestorClasses(myTypeEvalContext)) { final PyFunction superInit = superClass.findMethodByName(PyNames.INIT, false, null); - if (superInit != null) + if (superInit != null) { PyClassImpl.collectInstanceAttributes(superInit, attributesInInit); + } for (PyTargetExpression classAttr : superClass.getClassAttributes()) { attributesInInit.put(classAttr.getName(), classAttr); } } - Map attributes = new HashMap<>(); + final Map attributes = new HashMap<>(); PyClassImpl.collectInstanceAttributes(node, attributes); - for (Map.Entry attribute : attributes.entrySet()) { - String attributeName = attribute.getKey(); + for (PyTargetExpression attribute : attributes.values()) { + final String attributeName = attribute.getName(); if (attributeName == null) continue; - final Property property = containingClass.findProperty(attributeName, true, null); - if (!attributesInInit.containsKey(attributeName) && property == null) { - registerProblem(attribute.getValue(), PyBundle.message("INSP.attribute.$0.outside.init", attributeName), + if (!attributesInInit.containsKey(attributeName) && + !isDefinedByProperty(attribute, properties.values(), attributesInInit)) { + registerProblem(attribute, PyBundle.message("INSP.attribute.$0.outside.init", attributeName), new PyMoveAttributeToInitQuickFix()); } } } } - private static boolean isApplicable(@NotNull PyClass containingClass, @NotNull TypeEvalContext context) { - return !PythonUnitTestUtil.isTestClass(containingClass, ThreeState.UNSURE, context) && !containingClass.isSubclass("django.db.models.base.Model", context); + private static boolean isDefinedByProperty(@NotNull PyTargetExpression attribute, + @NotNull Collection properties, + @NotNull Map attributesInInit) { + return StreamEx.of(properties) + .filter(it -> isSetBy(attribute, it)) + .anyMatch(it -> attributesInInit.containsKey(it.getName())); } + + private static boolean isApplicable(@NotNull PyClass containingClass, @NotNull TypeEvalContext context) { + return !PythonUnitTestUtil.isTestClass(containingClass, ThreeState.UNSURE, context) && + !containingClass.isSubclass("django.db.models.base.Model", context); + } + + @Nullable + private static Collection getSetterTargetExpressions(@NotNull Property property) { + if (!property.getSetter().isDefined() || property.getSetter().value() == null) { + return null; + } + final PyFunction setter = property.getSetter().value().asMethod(); + if (setter == null) { + return null; + } + return ControlFlowCache.getScope(setter).getTargetExpressions(); + } + + /** + * Check whether the {@code property} sets the {@code attribute} provided. + */ + private static boolean isSetBy(@NotNull PyTargetExpression attribute, @NotNull Property property) { + final Collection propertyTargetExpressions = getSetterTargetExpressions(property); + return propertyTargetExpressions != null && + attribute.getName() != null && + StreamEx.of(propertyTargetExpressions) + .map(targetExpression -> targetExpression.getName()) + .nonNull() + .anyMatch(name -> name.equals(attribute.getName())); + } + } diff --git a/python/testData/inspections/PyAttributeOutsideInitInspection/property.py b/python/testData/inspections/PyAttributeOutsideInitInspection/property.py new file mode 100644 index 000000000000..29a69483780c --- /dev/null +++ b/python/testData/inspections/PyAttributeOutsideInitInspection/property.py @@ -0,0 +1,11 @@ +class C(object): + def __init__(self, value): + self.x = value + + def getx(self): + return self._x + + def setx(self, value): + self._x = value # False positive for self._x + + x = property(getx, setx, doc="The 'x' property.") diff --git a/python/testData/inspections/PyAttributeOutsideInitInspection/propertyAnnotation.py b/python/testData/inspections/PyAttributeOutsideInitInspection/propertyAnnotation.py new file mode 100644 index 000000000000..6e49c86664c7 --- /dev/null +++ b/python/testData/inspections/PyAttributeOutsideInitInspection/propertyAnnotation.py @@ -0,0 +1,11 @@ +class C(object): + def __init__(self): + self.x = None + + @property + def x(self): + return self._x + + @x.setter + def x(self, value): + self._x = value # False positive for self._x diff --git a/python/testData/inspections/PyAttributeOutsideInitInspection/propertyNotSetInInit.py b/python/testData/inspections/PyAttributeOutsideInitInspection/propertyNotSetInInit.py new file mode 100644 index 000000000000..298b5a4cf5ef --- /dev/null +++ b/python/testData/inspections/PyAttributeOutsideInitInspection/propertyNotSetInInit.py @@ -0,0 +1,11 @@ +class C(object): + def __init__(self, value): + pass + + def getx(self): + return self._x + + def setx(self, value): + self._x = value + + x = property(getx, setx, doc="The 'x' property.") diff --git a/python/testSrc/com/jetbrains/python/inspections/PyAttributeOutsideInitInspectionTest.java b/python/testSrc/com/jetbrains/python/inspections/PyAttributeOutsideInitInspectionTest.java index be0ec563e576..37a651c6f711 100644 --- a/python/testSrc/com/jetbrains/python/inspections/PyAttributeOutsideInitInspectionTest.java +++ b/python/testSrc/com/jetbrains/python/inspections/PyAttributeOutsideInitInspectionTest.java @@ -70,6 +70,21 @@ public class PyAttributeOutsideInitInspectionTest extends PyInspectionTestCase { doTest(); } + // PY-25263 + public void testProperty() { + doTest(); + } + + // PY-25263 + public void testPropertyAnnotation() { + doTest(); + } + + // PY-25263 + public void testPropertyNotSetInInit() { + doTest(); + } + @NotNull @Override protected Class getInspectionClass() {