Merge pull request #5198 from nozzy123nozzy/bugfix_of_property

Fix exception of pyreverse in handling property function within a class.

18 files changed, 79 insertions, 1 deletions

diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt
index 28b2e332c..8733bb995 100644
--- a/CONTRIBUTORS.txt
+++ b/CONTRIBUTORS.txt
@@ -564,3 +564,5 @@ contributors:
 * Arianna Yang: contributor
 
 * Mike Fiedler (miketheman): contributor
+
+* Takahide Nojima: contributor
diff --git a/ChangeLog b/ChangeLog
index 4e292305f..c9c0880ef 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -11,6 +11,8 @@ Release date: TBA
 ..
   Put new features here and also in 'doc/whatsnew/2.12.rst'
 
+* Fix exception when pyreverse parses ``property function`` of a class.
+
 * Add an optional extension ``consider-using-any-or-all`` : Emitted when a ``for`` loop only
   produces a boolean and could be replaced by ``any`` or ``all`` using a generator. Also suggests
   a suitable any or all statement.
diff --git a/doc/whatsnew/2.12.rst b/doc/whatsnew/2.12.rst
index 6c71ae159..e4bb8ac47 100644
--- a/doc/whatsnew/2.12.rst
+++ b/doc/whatsnew/2.12.rst
@@ -64,6 +64,8 @@ Extensions
 Other Changes
 =============
 
+* Fix exception when pyreverse parses ``property function`` of a class.
+
 * Improve and flatten ``unused-wildcard-import`` message
 
   Closes #3859
diff --git a/pylint/pyreverse/diagrams.py b/pylint/pyreverse/diagrams.py
index 5c82df1e6..08a00a8f1 100644
--- a/pylint/pyreverse/diagrams.py
+++ b/pylint/pyreverse/diagrams.py
@@ -121,6 +121,7 @@ class ClassDiagram(Figure, FilterMixIn):
             m
             for m in node.values()
             if isinstance(m, nodes.FunctionDef)
+            and not isinstance(m, astroid.objects.Property)
             and not decorated_with_property(m)
             and self.show_attr(m.name)
         ]
diff --git a/tests/data/property_pattern.py b/tests/data/property_pattern.py
new file mode 100644
index 000000000..2828922f0
--- /dev/null
+++ b/tests/data/property_pattern.py
@@ -0,0 +1,16 @@
+""" docstring for file property_pattern.py """
+class PropertyPatterns:
+    prop1 = property(lambda self: self._prop1*2, None, None, "property usage 1")
+
+    @property
+    def prop2(self):
+        """property usage 2"""
+        return self._prop2
+
+    @prop2.setter
+    def prop2(self, value):
+        self._prop2 = value * 2
+
+    def __init__(self):
+        self._prop1=1
+        self._prop2=2
diff --git a/tests/pyreverse/data/classes_No_Name.dot b/tests/pyreverse/data/classes_No_Name.dot
index cbe3d155d..b2c555108 100644
--- a/tests/pyreverse/data/classes_No_Name.dot
+++ b/tests/pyreverse/data/classes_No_Name.dot
@@ -7,6 +7,7 @@ charset="utf-8"
 "data.suppliermodule_test.DoNothing2" [color="black", fontcolor="black", label="{DoNothing2|\l|}", shape="record", style="solid"];
 "data.suppliermodule_test.DoSomething" [color="black", fontcolor="black", label="{DoSomething|my_int : Optional[int]\lmy_int_2 : Optional[int]\lmy_string : str\l|do_it(new_int: int): int\l}", shape="record", style="solid"];
 "data.suppliermodule_test.Interface" [color="black", fontcolor="black", label="{Interface|\l|get_value()\lset_value(value)\l}", shape="record", style="solid"];
+"data.property_pattern.PropertyPatterns" [color="black", fontcolor="black", label="{PropertyPatterns|prop1\lprop2\l|}", shape="record", style="solid"];
 "data.clientmodule_test.Specialization" [color="black", fontcolor="black", label="{Specialization|TYPE : str\lrelation\lrelation2\ltop : str\l|}", shape="record", style="solid"];
 "data.clientmodule_test.Specialization" -> "data.clientmodule_test.Ancestor" [arrowhead="empty", arrowtail="none"];
 "data.clientmodule_test.Ancestor" -> "data.suppliermodule_test.Interface" [arrowhead="empty", arrowtail="node", style="dashed"];
diff --git a/tests/pyreverse/data/classes_No_Name.puml b/tests/pyreverse/data/classes_No_Name.puml
index 35bda65c1..1d9e5f37d 100644
--- a/tests/pyreverse/data/classes_No_Name.puml
+++ b/tests/pyreverse/data/classes_No_Name.puml
@@ -22,6 +22,10 @@ class "Interface" as data.suppliermodule_test.Interface {
   get_value()
   set_value(value)
 }
+class "PropertyPatterns" as data.property_pattern.PropertyPatterns {
+  prop1
+  prop2
+}
 class "Specialization" as data.clientmodule_test.Specialization {
   TYPE : str
   relation
diff --git a/tests/pyreverse/data/classes_No_Name.vcg b/tests/pyreverse/data/classes_No_Name.vcg
index 748c432f2..bc542b64b 100644
--- a/tests/pyreverse/data/classes_No_Name.vcg
+++ b/tests/pyreverse/data/classes_No_Name.vcg
@@ -22,6 +22,9 @@ graph:{
   node: {title:"data.suppliermodule_test.Interface"  label:"\fbInterface\fn\n\f___________\n\f10get_value()\n\f10set_value()"
   shape:box
 }
+  node: {title:"data.property_pattern.PropertyPatterns"  label:"\fbPropertyPatterns\fn\n\f__________________\n\f08prop1\n\f08prop2\n\f__________________"
+  shape:box
+}
   node: {title:"data.clientmodule_test.Specialization"  label:"\fbSpecialization\fn\n\f________________\n\f08TYPE : str\n\f08relation\n\f08relation2\n\f08top : str\n\f________________"
   shape:box
 }
diff --git a/tests/pyreverse/data/classes_colorized.dot b/tests/pyreverse/data/classes_colorized.dot
index ff96df77c..0ed328566 100644
--- a/tests/pyreverse/data/classes_colorized.dot
+++ b/tests/pyreverse/data/classes_colorized.dot
@@ -7,6 +7,7 @@ charset="utf-8"
 "data.suppliermodule_test.DoNothing2" [color="aliceblue", fontcolor="black", label="{DoNothing2|\l|}", shape="record", style="filled"];
 "data.suppliermodule_test.DoSomething" [color="aliceblue", fontcolor="black", label="{DoSomething|my_int : Optional[int]\lmy_int_2 : Optional[int]\lmy_string : str\l|do_it(new_int: int): int\l}", shape="record", style="filled"];
 "data.suppliermodule_test.Interface" [color="aliceblue", fontcolor="black", label="{Interface|\l|get_value()\lset_value(value)\l}", shape="record", style="filled"];
+"data.property_pattern.PropertyPatterns" [color="aliceblue", fontcolor="black", label="{PropertyPatterns|prop1\lprop2\l|}", shape="record", style="filled"];
 "data.clientmodule_test.Specialization" [color="aliceblue", fontcolor="black", label="{Specialization|TYPE : str\lrelation\lrelation2\ltop : str\l|}", shape="record", style="filled"];
 "data.clientmodule_test.Specialization" -> "data.clientmodule_test.Ancestor" [arrowhead="empty", arrowtail="none"];
 "data.clientmodule_test.Ancestor" -> "data.suppliermodule_test.Interface" [arrowhead="empty", arrowtail="node", style="dashed"];
diff --git a/tests/pyreverse/data/classes_colorized.puml b/tests/pyreverse/data/classes_colorized.puml
index 25c0f34e9..611a669db 100644
--- a/tests/pyreverse/data/classes_colorized.puml
+++ b/tests/pyreverse/data/classes_colorized.puml
@@ -22,6 +22,10 @@ class "Interface" as data.suppliermodule_test.Interface #aliceblue {
   get_value()
   set_value(value)
 }
+class "PropertyPatterns" as data.property_pattern.PropertyPatterns #aliceblue {
+  prop1
+  prop2
+}
 class "Specialization" as data.clientmodule_test.Specialization #aliceblue {
   TYPE : str
   relation
diff --git a/tests/pyreverse/data/packages_No_Name.dot b/tests/pyreverse/data/packages_No_Name.dot
index 7b145dc91..461c8f9b4 100644
--- a/tests/pyreverse/data/packages_No_Name.dot
+++ b/tests/pyreverse/data/packages_No_Name.dot
@@ -3,6 +3,7 @@ rankdir=BT
 charset="utf-8"
 "data" [color="black", label="data", shape="box", style="solid"];
 "data.clientmodule_test" [color="black", label="data.clientmodule_test", shape="box", style="solid"];
+"data.property_pattern" [color="black", label="data.property_pattern", shape="box", style="solid"];
 "data.suppliermodule_test" [color="black", label="data.suppliermodule_test", shape="box", style="solid"];
 "data.clientmodule_test" -> "data.suppliermodule_test" [arrowhead="open", arrowtail="none"];
 }
diff --git a/tests/pyreverse/data/packages_No_Name.puml b/tests/pyreverse/data/packages_No_Name.puml
index 7089df0ea..4037b91bd 100644
--- a/tests/pyreverse/data/packages_No_Name.puml
+++ b/tests/pyreverse/data/packages_No_Name.puml
@@ -6,6 +6,9 @@ package "data" as data {
 package "data.clientmodule_test" as data.clientmodule_test {
 
 }
+package "data.property_pattern" as data.property_pattern {
+
+}
 package "data.suppliermodule_test" as data.suppliermodule_test {
 
 }
diff --git a/tests/pyreverse/data/packages_No_Name.vcg b/tests/pyreverse/data/packages_No_Name.vcg
index 03f4e2481..f9f4e4cec 100644
--- a/tests/pyreverse/data/packages_No_Name.vcg
+++ b/tests/pyreverse/data/packages_No_Name.vcg
@@ -10,6 +10,9 @@ graph:{
   node: {title:"data.clientmodule_test"  label:"\fbdata.clientmodule_test\fn"
   shape:box
 }
+  node: {title:"data.property_pattern"  label:"\fbdata.property_pattern\fn"
+  shape:box
+}
   node: {title:"data.suppliermodule_test"  label:"\fbdata.suppliermodule_test\fn"
   shape:box
 }
diff --git a/tests/pyreverse/data/packages_colorized.dot b/tests/pyreverse/data/packages_colorized.dot
index 917fa86f5..1a95d4c97 100644
--- a/tests/pyreverse/data/packages_colorized.dot
+++ b/tests/pyreverse/data/packages_colorized.dot
@@ -3,6 +3,7 @@ rankdir=BT
 charset="utf-8"
 "data" [color="aliceblue", label="data", shape="box", style="filled"];
 "data.clientmodule_test" [color="aliceblue", label="data.clientmodule_test", shape="box", style="filled"];
+"data.property_pattern" [color="aliceblue", label="data.property_pattern", shape="box", style="filled"];
 "data.suppliermodule_test" [color="aliceblue", label="data.suppliermodule_test", shape="box", style="filled"];
 "data.clientmodule_test" -> "data.suppliermodule_test" [arrowhead="open", arrowtail="none"];
 }
diff --git a/tests/pyreverse/data/packages_colorized.puml b/tests/pyreverse/data/packages_colorized.puml
index 8d489e7aa..353ae8c47 100644
--- a/tests/pyreverse/data/packages_colorized.puml
+++ b/tests/pyreverse/data/packages_colorized.puml
@@ -6,6 +6,9 @@ package "data" as data #aliceblue {
 package "data.clientmodule_test" as data.clientmodule_test #aliceblue {
 
 }
+package "data.property_pattern" as data.property_pattern #aliceblue {
+
+}
 package "data.suppliermodule_test" as data.suppliermodule_test #aliceblue {
 
 }
diff --git a/tests/pyreverse/test_diadefs.py b/tests/pyreverse/test_diadefs.py
index ec7b48614..5c5ca13e1 100644
--- a/tests/pyreverse/test_diadefs.py
+++ b/tests/pyreverse/test_diadefs.py
@@ -148,6 +148,7 @@ def test_known_values1(HANDLER: DiadefsHandler, PROJECT: Project) -> None:
     assert modules == [
         (True, "data"),
         (True, "data.clientmodule_test"),
+        (True, "data.property_pattern"),
         (True, "data.suppliermodule_test"),
     ]
     cd = dd[1]
@@ -160,6 +161,7 @@ def test_known_values1(HANDLER: DiadefsHandler, PROJECT: Project) -> None:
         (True, "DoNothing2"),
         (True, "DoSomething"),
         (True, "Interface"),
+        (True, "PropertyPatterns"),
         (True, "Specialization"),
     ]
 
diff --git a/tests/pyreverse/test_diagrams.py b/tests/pyreverse/test_diagrams.py
new file mode 100644
index 000000000..a2da6e27d
--- /dev/null
+++ b/tests/pyreverse/test_diagrams.py
@@ -0,0 +1,24 @@
+# Copyright (c) 2021 Takahide Nojima <nozzy123nozzy@gmail.com>
+#
+# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE
+
+"""Unit test for the diagrams modules"""
+# pylint: disable=redefined-outer-name
+from typing import Callable
+
+from pylint.pyreverse.diadefslib import DefaultDiadefGenerator, DiadefsHandler
+from pylint.pyreverse.inspector import Linker
+from pylint.testutils.pyreverse import PyreverseConfig
+
+
+def test_property_handling(
+    default_config: PyreverseConfig, get_project: Callable
+) -> None:
+    project = get_project("data.property_pattern")
+    class_diagram = DefaultDiadefGenerator(
+        Linker(project), DiadefsHandler(default_config)
+    ).visit(project)[0]
+    obj = class_diagram.classe("PropertyPatterns")
+    assert len(class_diagram.get_methods(obj.node)) == 0
+    assert class_diagram.get_attrs(obj.node) == ["prop1", "prop2"]
diff --git a/tests/pyreverse/test_inspector.py b/tests/pyreverse/test_inspector.py
index 8be038396..0e06bb48a 100644
--- a/tests/pyreverse/test_inspector.py
+++ b/tests/pyreverse/test_inspector.py
@@ -132,5 +132,10 @@ def test_from_directory(project: Project) -> None:
 
 
 def test_project_node(project: Project) -> None:
-    expected = ["data", "data.clientmodule_test", "data.suppliermodule_test"]
+    expected = [
+        "data",
+        "data.clientmodule_test",
+        "data.property_pattern",
+        "data.suppliermodule_test",
+    ]
     assert sorted(project.keys()) == expected