Fix line counting for missing-docstring check in combination with docstring-min-length (#1672)

Author: ttenhoeve-aa

ChangeLog

@@ -139,6 +139,9 @@ What's New in Pylint 1.8?
       mixing ``Args`` and ``Keyword Args`` in Google docstring.
       Close #1409
 
+     * Fix ``missing-docstring`` false negatives when modules, classes, or methods
+     consist of compound statements that exceed the ``docstring-min-length``
+
     * Fix ``useless-else-on-loop`` false positives when break statements are
       deeply nested inside loop.
       Close #1661

pylint/checkers/base.py

@@ -35,6 +35,7 @@
 from pylint import interfaces
 from pylint.checkers import utils
 from pylint import reporters
+from pylint.checkers.utils import get_node_last_lineno
 from pylint.reporters.ureports import nodes as reporter_nodes
 import pylint.utils as lint_utils
 
@@ -1604,10 +1605,7 @@ def _check_docstring(self, node_type, node, report_missing=True,
         if docstring is None:
             if not report_missing:
                 return
-            if node.body:
-                lines = node.body[-1].lineno - node.body[0].lineno + 1
-            else:
-                lines = 0
+            lines = get_node_last_lineno(node) - node.lineno
 
             if node_type == 'module' and not lines:
                 # If the module has no body, there's no reason

pylint/checkers/utils.py

@@ -878,3 +878,26 @@ def is_registered_in_singledispatch_function(node):
             return decorated_with(func_def, singledispatch_qnames)
 
     return False
+
+
+def get_node_last_lineno(node):
+    """
+    Get the last lineno of the given node. For a simple statement this will just be node.lineno,
+    but for a node that has child statements (e.g. a method) this will be the lineno of the last
+    child statement recursively.
+    """
+    # 'finalbody' is always the last clause in a try statement, if present
+    if getattr(node, 'finalbody', False):
+        return get_node_last_lineno(node.finalbody[-1])
+    # For if, while, and for statements 'orelse' is always the last clause.
+    # For try statements 'orelse' is the last in the absence of a 'finalbody'
+    if getattr(node, 'orelse', False):
+        return get_node_last_lineno(node.orelse[-1])
+    # try statements have the 'handlers' last if there is no 'orelse' or 'finalbody'
+    if getattr(node, 'handlers', False):
+        return get_node_last_lineno(node.handlers[-1])
+    # All compound statements have a 'body'
+    if getattr(node, 'body', False):
+        return get_node_last_lineno(node.body[-1])
+    # Not a compound statement
+    return node.lineno

pylint/test/unittest_checker_base.py

@@ -54,6 +54,30 @@ def func(tion):
         with self.assertNoMessages():
             self.checker.visit_functiondef(func)
 
+    @set_config(docstring_min_length=2)
+    def test_long_function_no_docstring(self):
+        func = astroid.extract_node("""
+        def func(tion):
+            pass
+            pass
+           """)
+        message = Message('missing-docstring', node=func, args=('function',))
+        with self.assertAddsMessages(message):
+            self.checker.visit_functiondef(func)
+
+    @set_config(docstring_min_length=2)
+    def test_long_function_nested_statements_no_docstring(self):
+        func = astroid.extract_node("""
+        def func(tion):
+            try:
+                pass
+            except:
+                pass
+           """)
+        message = Message('missing-docstring', node=func, args=('function',))
+        with self.assertAddsMessages(message):
+            self.checker.visit_functiondef(func)
+
     @set_config(docstring_min_length=2)
     def test_function_no_docstring_by_name(self):
         func = astroid.extract_node("""

pylint/test/unittest_utils.py

@@ -14,7 +14,7 @@
 import astroid
 
 from pylint import utils
-from pylint.checkers.utils import check_messages
+from pylint.checkers.utils import check_messages, get_node_last_lineno
 from pylint.exceptions import InvalidMessageError
 import pytest
 
@@ -207,3 +207,177 @@ def test_decoding_stream_known_encoding():
     binary_io = io.BytesIO(u'€'.encode('cp1252'))
     stream = utils.decoding_stream(binary_io, 'cp1252')
     assert stream.read() == u'€'
+
+class TestGetNodeLastLineno:
+
+    def test_get_node_last_lineno_simple(self):
+        node = astroid.extract_node("""
+            pass
+        """)
+        assert get_node_last_lineno(node) == 2
+
+
+    def test_get_node_last_lineno_if_simple(self):
+        node = astroid.extract_node("""
+            if True:
+                print(1)
+                pass
+            """)
+        assert get_node_last_lineno(node) == 4
+
+
+    def test_get_node_last_lineno_if_elseif_else(self):
+        node = astroid.extract_node("""
+            if True:
+                print(1)
+            elif False:
+                print(2)
+            else:
+                print(3)
+            """)
+        assert get_node_last_lineno(node) == 7
+
+
+    def test_get_node_last_lineno_while(self):
+        node = astroid.extract_node("""
+            while True:
+                print(1)
+            """)
+        assert get_node_last_lineno(node) == 3
+
+
+    def test_get_node_last_lineno_while_else(self):
+        node = astroid.extract_node("""
+            while True:
+                print(1)
+            else:
+                print(2)
+            """)
+        assert get_node_last_lineno(node) == 5
+
+
+    def test_get_node_last_lineno_for(self):
+        node = astroid.extract_node("""
+            for x in range(0, 5):
+                print(1)
+            """)
+        assert get_node_last_lineno(node) == 3
+
+
+    def test_get_node_last_lineno_for_else(self):
+        node = astroid.extract_node("""
+            for x in range(0, 5):
+                print(1)
+            else:
+                print(2)
+            """)
+        assert get_node_last_lineno(node) == 5
+
+
+    def test_get_node_last_lineno_try(self):
+        node = astroid.extract_node("""
+            try:
+                print(1)
+            except ValueError:
+                print(2)
+            except Exception:
+                print(3)
+            """)
+        assert get_node_last_lineno(node) == 7
+
+
+    def test_get_node_last_lineno_try_except_else(self):
+        node = astroid.extract_node("""
+            try:
+                print(1)
+            except Exception:
+                print(2)
+                print(3)
+            else:
+                print(4)
+            """)
+        assert get_node_last_lineno(node) == 8
+
+
+    def test_get_node_last_lineno_try_except_finally(self):
+        node = astroid.extract_node("""
+            try:
+                print(1)
+            except Exception:
+                print(2)
+            finally:
+                print(4)
+            """)
+        assert get_node_last_lineno(node) == 7
+
+
+    def test_get_node_last_lineno_try_except_else_finally(self):
+        node = astroid.extract_node("""
+            try:
+                print(1)
+            except Exception:
+                print(2)
+            else:
+                print(3)
+            finally:
+                print(4)
+            """)
+        assert get_node_last_lineno(node) == 9
+
+
+    def test_get_node_last_lineno_with(self):
+        node = astroid.extract_node("""
+            with x as y:
+                print(1)
+                pass
+            """)
+        assert get_node_last_lineno(node) == 4
+
+
+    def test_get_node_last_lineno_method(self):
+        node = astroid.extract_node("""
+            def x(a, b):
+                print(a, b)
+                pass
+            """)
+        assert get_node_last_lineno(node) == 4
+
+
+    def test_get_node_last_lineno_decorator(self):
+        node = astroid.extract_node("""
+            @decor()
+            def x(a, b):
+                print(a, b)
+                pass
+            """)
+        assert get_node_last_lineno(node) == 5
+
+    def test_get_node_last_lineno_class(self):
+        node = astroid.extract_node("""
+            class C(object):
+                CONST = True
+
+                def x(self, b):
+                    print(b)
+
+                def y(self):
+                    pass
+                    pass
+            """)
+        assert get_node_last_lineno(node) == 10
+
+
+    def test_get_node_last_lineno_combined(self):
+        node = astroid.extract_node("""
+            class C(object):
+                CONST = True
+
+                def y(self):
+                    try:
+                        pass
+                    except:
+                        pass
+                    finally:
+                        pass
+            """)
+        assert get_node_last_lineno(node) == 11