From e2da1f9f3dbd9a6462e0549da75b37249467afb8 Mon Sep 17 00:00:00 2001
From: Laura M?dioni <laura.medioni@logilab.fr>
Date: Thu, 29 Oct 2015 09:24:52 +0100
Subject: check for class methods declared without a decorator

related to the issue #675
---
 pylint/checkers/classes.py                         | 39 ++++++++++++++++++++--
 .../test/functional/access_to_protected_members.py |  1 +
 .../functional/access_to_protected_members.txt     |  6 ++--
 pylint/test/functional/no_classmethod_decorator.py | 28 ++++++++++++++++
 .../test/functional/no_classmethod_decorator.txt   |  1 +
 ...func_noerror_classes_protected_member_access.py |  1 +
 pylint/test/input/func_noerror_static_method.py    |  1 +
 pylint/test/messages/func_e0203.txt                |  1 +
 pylint/test/messages/func_first_arg.txt            |  4 +++
 9 files changed, 76 insertions(+), 6 deletions(-)
 create mode 100644 pylint/test/functional/no_classmethod_decorator.py
 create mode 100644 pylint/test/functional/no_classmethod_decorator.txt

(limited to 'pylint')

diff --git a/pylint/checkers/classes.py b/pylint/checkers/classes.py
index 86ccb19..98a13bf 100644
--- a/pylint/checkers/classes.py
+++ b/pylint/checkers/classes.py
@@ -262,7 +262,10 @@ MSGS = {
     'E0241': ('Duplicate bases for class %r',
               'duplicate-bases',
               'Used when a class has duplicate bases.'),
-
+    'R0202': ('Consider using a decorator instead of calling classmethod',
+              'no-classmethod-decorator',
+              'Used when a class method is defined without using the decorator '
+              'syntax.'),
     }
 
 
@@ -613,17 +616,47 @@ a metaclass class method.'}
                     self.add_message('assigning-non-slot',
                                      args=(node.attrname, ), node=node)
 
-    @check_messages('protected-access')
+    @check_messages('protected-access', 'no-classmethod-decorator')
     def visit_assign(self, assign_node):
+        self._check_classmethod_declaration(assign_node)
         node = assign_node.targets[0]
         if not isinstance(node, astroid.AssignAttr):
             return
 
         if self.is_first_attr(node):
             return
-
         self._check_protected_attribute_access(node)
 
+    def _check_classmethod_declaration(self, node):
+        """checks for uses of classmethod() instead of @classmethod decorator
+
+        A message will be emitted only if the assignment is at a class scope
+        and only if the classmethod's argument belongs to the class where it
+        is defined.
+        `node` is an assign node.
+        """
+        if not isinstance(node.value, astroid.Call):
+            return
+        # check the function called is "classmethod"
+        func = node.value.func
+        if not isinstance(func, astroid.Name) or not func.name == 'classmethod':
+            return
+        # assignment must be at a class scope
+        parent_class = node.parent
+        if not isinstance(parent_class, astroid.ClassDef):
+            return
+        # Check if the arg passed to classmethod is a class member
+        classmeth_arg = node.value.args[0]
+        if not isinstance(classmeth_arg, astroid.Name):
+            return
+        method_name = classmeth_arg.name
+        for member in parent_class.get_children():
+            if (isinstance(member, astroid.FunctionDef) and
+                    method_name == member.name):
+                self.add_message('no-classmethod-decorator',
+                                 node=node.targets[0])
+                break
+
     def _check_protected_attribute_access(self, node):
         '''Given an attribute access node (set or get), check if attribute
         access is legitimate. Call _check_first_attr with node before calling
diff --git a/pylint/test/functional/access_to_protected_members.py b/pylint/test/functional/access_to_protected_members.py
index 9c92306..fd96baf 100644
--- a/pylint/test/functional/access_to_protected_members.py
+++ b/pylint/test/functional/access_to_protected_members.py
@@ -1,4 +1,5 @@
 # pylint: disable=too-few-public-methods, W0231, print-statement
+# pylint: disable=no-classmethod-decorator
 """Test external access to protected class members."""
 from __future__ import print_function
 
diff --git a/pylint/test/functional/access_to_protected_members.txt b/pylint/test/functional/access_to_protected_members.txt
index 123c1dd..7ba601b 100644
--- a/pylint/test/functional/access_to_protected_members.txt
+++ b/pylint/test/functional/access_to_protected_members.txt
@@ -1,5 +1,5 @@
-protected-access:18:MyClass.test:Access to a protected member _haha of a client class
-protected-access:40::Access to a protected member _protected of a client class
+protected-access:19:MyClass.test:Access to a protected member _haha of a client class
 protected-access:41::Access to a protected member _protected of a client class
-protected-access:42::Access to a protected member _cls_protected of a client class
+protected-access:42::Access to a protected member _protected of a client class
 protected-access:43::Access to a protected member _cls_protected of a client class
+protected-access:44::Access to a protected member _cls_protected of a client class
diff --git a/pylint/test/functional/no_classmethod_decorator.py b/pylint/test/functional/no_classmethod_decorator.py
new file mode 100644
index 0000000..205594d
--- /dev/null
+++ b/pylint/test/functional/no_classmethod_decorator.py
@@ -0,0 +1,28 @@
+"""Checks classes methods are declared with a decorator if whithin the class
+scope and if classmethod's argument is a member of the class
+"""
+
+# pylint: disable=too-few-public-methods
+
+class MyClass(object):
+    """Some class"""
+    def __init__(self):
+        pass
+
+    def cmethod(cls):
+        """class method-to-be"""
+    cmethod = classmethod(cmethod) # [no-classmethod-decorator]
+
+    @classmethod
+    def my_second_method(cls):
+        """correct class method definition"""
+
+def helloworld():
+    """says hello"""
+    print 'hello world'
+
+MyClass.new_class_method = classmethod(helloworld)
+
+class MyOtherClass(object):
+    """Some other class"""
+    _make = classmethod(tuple.__new__)
diff --git a/pylint/test/functional/no_classmethod_decorator.txt b/pylint/test/functional/no_classmethod_decorator.txt
new file mode 100644
index 0000000..8c1060f
--- /dev/null
+++ b/pylint/test/functional/no_classmethod_decorator.txt
@@ -0,0 +1 @@
+no-classmethod-decorator:14:MyClass:Consider using a decorator instead of calling classmethod
diff --git a/pylint/test/input/func_noerror_classes_protected_member_access.py b/pylint/test/input/func_noerror_classes_protected_member_access.py
index eeff97d..670e3e8 100644
--- a/pylint/test/input/func_noerror_classes_protected_member_access.py
+++ b/pylint/test/input/func_noerror_classes_protected_member_access.py
@@ -3,6 +3,7 @@
 """
 __revision__ = 1
 
+# pylint: disable=no-classmethod-decorator
 class A3123(object):
     """oypuee"""
     _protected = 1
diff --git a/pylint/test/input/func_noerror_static_method.py b/pylint/test/input/func_noerror_static_method.py
index ef21cb9..8a7a0a2 100644
--- a/pylint/test/input/func_noerror_static_method.py
+++ b/pylint/test/input/func_noerror_static_method.py
@@ -3,6 +3,7 @@
 from __future__ import print_function
 __revision__ = ''
 
+#pylint: disable=no-classmethod-decorator
 class MyClass(object):
     """doc
     """
diff --git a/pylint/test/messages/func_e0203.txt b/pylint/test/messages/func_e0203.txt
index c330ffa..96769de 100644
--- a/pylint/test/messages/func_e0203.txt
+++ b/pylint/test/messages/func_e0203.txt
@@ -1 +1,2 @@
 C: 12:Abcd.abcd: Class method abcd should have 'cls' as first argument
+R: 15:Abcd: Consider using a decorator instead of calling classmethod
diff --git a/pylint/test/messages/func_first_arg.txt b/pylint/test/messages/func_first_arg.txt
index 75090dd..ba4efb8 100644
--- a/pylint/test/messages/func_first_arg.txt
+++ b/pylint/test/messages/func_first_arg.txt
@@ -3,3 +3,7 @@ C: 18:Obj.class2: Class method class2 should have 'cls' as first argument
 C: 25:Meta.__new__: Metaclass class method __new__ should have 'mcs' as first argument
 C: 32:Meta.method2: Metaclass method method2 should have 'cls' as first argument
 C: 40:Meta.class2: Metaclass class method class2 should have 'mcs' as first argument
+R: 16:Obj: Consider using a decorator instead of calling classmethod
+R: 20:Obj: Consider using a decorator instead of calling classmethod
+R: 38:Meta: Consider using a decorator instead of calling classmethod
+R: 42:Meta: Consider using a decorator instead of calling classmethod
-- 
cgit v1.2.1


From 171326299f836782edd6167ac15de4a738dd53f8 Mon Sep 17 00:00:00 2001
From: Laura M?dioni <laura.medioni@logilab.fr>
Date: Thu, 29 Oct 2015 11:17:35 +0100
Subject: check for static methods declared without a decorator

closes issue #675
---
 pylint/checkers/classes.py                         | 17 +++++++++----
 .../test/functional/bad_staticmethod_argument.py   |  2 +-
 .../test/functional/no_staticmethod_decorator.py   | 28 ++++++++++++++++++++++
 .../test/functional/no_staticmethod_decorator.txt  |  1 +
 ...func_noerror_classes_protected_member_access.py |  2 +-
 pylint/test/input/func_noerror_static_method.py    |  2 +-
 6 files changed, 44 insertions(+), 8 deletions(-)
 create mode 100644 pylint/test/functional/no_staticmethod_decorator.py
 create mode 100644 pylint/test/functional/no_staticmethod_decorator.txt

(limited to 'pylint')

diff --git a/pylint/checkers/classes.py b/pylint/checkers/classes.py
index 98a13bf..ee3c985 100644
--- a/pylint/checkers/classes.py
+++ b/pylint/checkers/classes.py
@@ -266,6 +266,10 @@ MSGS = {
               'no-classmethod-decorator',
               'Used when a class method is defined without using the decorator '
               'syntax.'),
+    'R0203': ('Consider using a decorator instead of calling staticmethod',
+              'no-staticmethod-decorator',
+              'Used when a static method is defined without using the decorator '
+              'syntax.'),
     }
 
 
@@ -616,7 +620,8 @@ a metaclass class method.'}
                     self.add_message('assigning-non-slot',
                                      args=(node.attrname, ), node=node)
 
-    @check_messages('protected-access', 'no-classmethod-decorator')
+    @check_messages('protected-access', 'no-classmethod-decorator',
+                    'no-staticmethod-decorator')
     def visit_assign(self, assign_node):
         self._check_classmethod_declaration(assign_node)
         node = assign_node.targets[0]
@@ -637,10 +642,13 @@ a metaclass class method.'}
         """
         if not isinstance(node.value, astroid.Call):
             return
-        # check the function called is "classmethod"
+        # check the function called is "classmethod" or "staticmethod"
         func = node.value.func
-        if not isinstance(func, astroid.Name) or not func.name == 'classmethod':
+        if (not isinstance(func, astroid.Name) or
+                func.name not in('classmethod', 'staticmethod')):
             return
+        msg = ('no-classmethod-decorator' if func.name == 'classmethod' else
+               'no-staticmethod-decorator')
         # assignment must be at a class scope
         parent_class = node.parent
         if not isinstance(parent_class, astroid.ClassDef):
@@ -653,8 +661,7 @@ a metaclass class method.'}
         for member in parent_class.get_children():
             if (isinstance(member, astroid.FunctionDef) and
                     method_name == member.name):
-                self.add_message('no-classmethod-decorator',
-                                 node=node.targets[0])
+                self.add_message(msg, node=node.targets[0])
                 break
 
     def _check_protected_attribute_access(self, node):
diff --git a/pylint/test/functional/bad_staticmethod_argument.py b/pylint/test/functional/bad_staticmethod_argument.py
index 0ff5d9b..a71a40e 100644
--- a/pylint/test/functional/bad_staticmethod_argument.py
+++ b/pylint/test/functional/bad_staticmethod_argument.py
@@ -1,4 +1,4 @@
-# pylint: disable=missing-docstring
+# pylint: disable=missing-docstring, no-staticmethod-decorator
 
 class Abcd(object):
 
diff --git a/pylint/test/functional/no_staticmethod_decorator.py b/pylint/test/functional/no_staticmethod_decorator.py
new file mode 100644
index 0000000..636b2a5
--- /dev/null
+++ b/pylint/test/functional/no_staticmethod_decorator.py
@@ -0,0 +1,28 @@
+"""Checks static methods are declared with a decorator if whithin the class
+scope and if static method's argument is a member of the class
+"""
+
+# pylint: disable=too-few-public-methods
+
+class MyClass(object):
+    """Some class"""
+    def __init__(self):
+        pass
+
+    def smethod():
+        """static method-to-be"""
+    smethod = staticmethod(smethod) # [no-staticmethod-decorator]
+
+    @staticmethod
+    def my_second_method():
+        """correct static method definition"""
+
+def helloworld():
+    """says hello"""
+    print 'hello world'
+
+MyClass.new_static_method = staticmethod(helloworld)
+
+class MyOtherClass(object):
+    """Some other class"""
+    _make = staticmethod(tuple.__new__)
diff --git a/pylint/test/functional/no_staticmethod_decorator.txt b/pylint/test/functional/no_staticmethod_decorator.txt
new file mode 100644
index 0000000..b8d23ae
--- /dev/null
+++ b/pylint/test/functional/no_staticmethod_decorator.txt
@@ -0,0 +1 @@
+no-staticmethod-decorator:14:MyClass:Consider using a decorator instead of calling staticmethod
diff --git a/pylint/test/input/func_noerror_classes_protected_member_access.py b/pylint/test/input/func_noerror_classes_protected_member_access.py
index 670e3e8..2ffd9d1 100644
--- a/pylint/test/input/func_noerror_classes_protected_member_access.py
+++ b/pylint/test/input/func_noerror_classes_protected_member_access.py
@@ -3,7 +3,7 @@
 """
 __revision__ = 1
 
-# pylint: disable=no-classmethod-decorator
+# pylint: disable=no-classmethod-decorator, no-staticmethod-decorator
 class A3123(object):
     """oypuee"""
     _protected = 1
diff --git a/pylint/test/input/func_noerror_static_method.py b/pylint/test/input/func_noerror_static_method.py
index 8a7a0a2..7457f45 100644
--- a/pylint/test/input/func_noerror_static_method.py
+++ b/pylint/test/input/func_noerror_static_method.py
@@ -3,7 +3,7 @@
 from __future__ import print_function
 __revision__ = ''
 
-#pylint: disable=no-classmethod-decorator
+#pylint: disable=no-classmethod-decorator, no-staticmethod-decorator
 class MyClass(object):
     """doc
     """
-- 
cgit v1.2.1


From dd419712a064c2ec25839492e8c3b2acccbd76a3 Mon Sep 17 00:00:00 2001
From: Laura M?dioni <laura.medioni@logilab.fr>
Date: Thu, 29 Oct 2015 14:43:10 +0100
Subject: improve style and fix typos regarding no_class/staticmethod_decorator

---
 pylint/checkers/classes.py                          | 5 +++--
 pylint/test/functional/no_classmethod_decorator.py  | 2 +-
 pylint/test/functional/no_staticmethod_decorator.py | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

(limited to 'pylint')

diff --git a/pylint/checkers/classes.py b/pylint/checkers/classes.py
index ee3c985..d12f45d 100644
--- a/pylint/checkers/classes.py
+++ b/pylint/checkers/classes.py
@@ -633,8 +633,9 @@ a metaclass class method.'}
         self._check_protected_attribute_access(node)
 
     def _check_classmethod_declaration(self, node):
-        """checks for uses of classmethod() instead of @classmethod decorator
+        """Checks for uses of classmethod() or staticmethod()
 
+        When a @classmethod or @staticmethod decorator should be used instead.
         A message will be emitted only if the assignment is at a class scope
         and only if the classmethod's argument belongs to the class where it
         is defined.
@@ -645,7 +646,7 @@ a metaclass class method.'}
         # check the function called is "classmethod" or "staticmethod"
         func = node.value.func
         if (not isinstance(func, astroid.Name) or
-                func.name not in('classmethod', 'staticmethod')):
+                func.name not in ('classmethod', 'staticmethod')):
             return
         msg = ('no-classmethod-decorator' if func.name == 'classmethod' else
                'no-staticmethod-decorator')
diff --git a/pylint/test/functional/no_classmethod_decorator.py b/pylint/test/functional/no_classmethod_decorator.py
index 205594d..f44dca5 100644
--- a/pylint/test/functional/no_classmethod_decorator.py
+++ b/pylint/test/functional/no_classmethod_decorator.py
@@ -1,4 +1,4 @@
-"""Checks classes methods are declared with a decorator if whithin the class
+"""Checks class methods are declared with a decorator if within the class
 scope and if classmethod's argument is a member of the class
 """
 
diff --git a/pylint/test/functional/no_staticmethod_decorator.py b/pylint/test/functional/no_staticmethod_decorator.py
index 636b2a5..9e26454 100644
--- a/pylint/test/functional/no_staticmethod_decorator.py
+++ b/pylint/test/functional/no_staticmethod_decorator.py
@@ -1,4 +1,4 @@
-"""Checks static methods are declared with a decorator if whithin the class
+"""Checks static methods are declared with a decorator if within the class
 scope and if static method's argument is a member of the class
 """
 
-- 
cgit v1.2.1


From ac584cb0f525d5ba86e06925077eb82d270b8a39 Mon Sep 17 00:00:00 2001
From: Laura M?dioni <laura.medioni@logilab.fr>
Date: Thu, 29 Oct 2015 15:30:59 +0100
Subject: no-static/class-method: enhance the tests and fix the code
 accordingly

---
 pylint/checkers/classes.py                           |  7 +++----
 pylint/test/functional/no_classmethod_decorator.py   | 11 +++++++++--
 pylint/test/functional/no_classmethod_decorator.txt  |  2 ++
 pylint/test/functional/no_staticmethod_decorator.py  |  9 ++++++++-
 pylint/test/functional/no_staticmethod_decorator.txt |  2 ++
 5 files changed, 24 insertions(+), 7 deletions(-)

(limited to 'pylint')

diff --git a/pylint/checkers/classes.py b/pylint/checkers/classes.py
index d12f45d..014dd4a 100644
--- a/pylint/checkers/classes.py
+++ b/pylint/checkers/classes.py
@@ -651,7 +651,7 @@ a metaclass class method.'}
         msg = ('no-classmethod-decorator' if func.name == 'classmethod' else
                'no-staticmethod-decorator')
         # assignment must be at a class scope
-        parent_class = node.parent
+        parent_class = node.scope()
         if not isinstance(parent_class, astroid.ClassDef):
             return
         # Check if the arg passed to classmethod is a class member
@@ -659,9 +659,8 @@ a metaclass class method.'}
         if not isinstance(classmeth_arg, astroid.Name):
             return
         method_name = classmeth_arg.name
-        for member in parent_class.get_children():
-            if (isinstance(member, astroid.FunctionDef) and
-                    method_name == member.name):
+        for member in parent_class.mymethods():
+            if method_name == member.name:
                 self.add_message(msg, node=node.targets[0])
                 break
 
diff --git a/pylint/test/functional/no_classmethod_decorator.py b/pylint/test/functional/no_classmethod_decorator.py
index f44dca5..2f91fde 100644
--- a/pylint/test/functional/no_classmethod_decorator.py
+++ b/pylint/test/functional/no_classmethod_decorator.py
@@ -2,7 +2,7 @@
 scope and if classmethod's argument is a member of the class
 """
 
-# pylint: disable=too-few-public-methods
+# pylint: disable=too-few-public-methods, using-constant-test, no-self-argument
 
 class MyClass(object):
     """Some class"""
@@ -11,12 +11,19 @@ class MyClass(object):
 
     def cmethod(cls):
         """class method-to-be"""
-    cmethod = classmethod(cmethod) # [no-classmethod-decorator]
+    cmethod = classmethod(cmethod)  # [no-classmethod-decorator]
+
+    if True:
+        cmethod = classmethod(cmethod)  # [no-classmethod-decorator]
 
     @classmethod
     def my_second_method(cls):
         """correct class method definition"""
 
+    def other_method(cls):
+        """some method"""
+    cmethod2 = classmethod(other_method)  # [no-classmethod-decorator]
+
 def helloworld():
     """says hello"""
     print 'hello world'
diff --git a/pylint/test/functional/no_classmethod_decorator.txt b/pylint/test/functional/no_classmethod_decorator.txt
index 8c1060f..ba51f0b 100644
--- a/pylint/test/functional/no_classmethod_decorator.txt
+++ b/pylint/test/functional/no_classmethod_decorator.txt
@@ -1 +1,3 @@
 no-classmethod-decorator:14:MyClass:Consider using a decorator instead of calling classmethod
+no-classmethod-decorator:17:MyClass:Consider using a decorator instead of calling classmethod
+no-classmethod-decorator:25:MyClass:Consider using a decorator instead of calling classmethod
diff --git a/pylint/test/functional/no_staticmethod_decorator.py b/pylint/test/functional/no_staticmethod_decorator.py
index 9e26454..a64cd7c 100644
--- a/pylint/test/functional/no_staticmethod_decorator.py
+++ b/pylint/test/functional/no_staticmethod_decorator.py
@@ -2,7 +2,7 @@
 scope and if static method's argument is a member of the class
 """
 
-# pylint: disable=too-few-public-methods
+# pylint: disable=too-few-public-methods, using-constant-test, no-method-argument
 
 class MyClass(object):
     """Some class"""
@@ -13,10 +13,17 @@ class MyClass(object):
         """static method-to-be"""
     smethod = staticmethod(smethod) # [no-staticmethod-decorator]
 
+    if True:
+        smethod = staticmethod(smethod)  # [no-staticmethod-decorator]
+
     @staticmethod
     def my_second_method():
         """correct static method definition"""
 
+    def other_method():
+        """some method"""
+    smethod2 = staticmethod(other_method)  # [no-staticmethod-decorator]
+
 def helloworld():
     """says hello"""
     print 'hello world'
diff --git a/pylint/test/functional/no_staticmethod_decorator.txt b/pylint/test/functional/no_staticmethod_decorator.txt
index b8d23ae..c0aea0e 100644
--- a/pylint/test/functional/no_staticmethod_decorator.txt
+++ b/pylint/test/functional/no_staticmethod_decorator.txt
@@ -1 +1,3 @@
 no-staticmethod-decorator:14:MyClass:Consider using a decorator instead of calling staticmethod
+no-staticmethod-decorator:17:MyClass:Consider using a decorator instead of calling staticmethod
+no-staticmethod-decorator:25:MyClass:Consider using a decorator instead of calling staticmethod
-- 
cgit v1.2.1