Merge pull request #165 from Nurdok/D403

Added D403: First word of first line should be properly capitalized

Author: Nurdok

docs/release_notes.rst

@@ -13,11 +13,16 @@ New Features
   classes are considered public if their names are not prepended with an
   underscore and if their parent class is public, recursively (#13, #146).
 
+* Added the D403 error code - "First word of the first line should be
+  properly capitalized". This new error is turned on by default (#164, #165).
 
 Bug Fixes
 
-* Fixed an issue where a `NameError` was raised when parsing complex defintions
-  of `__all__` (#142, #143).
+* Fixed an issue where a ``NameError`` was raised when parsing complex defintions
+  of ``__all__`` (#142, #143).
+
+* Fixed a bug where D202 was falsely reported when a function with just a
+  docstring and no content was followed by a comment (#165).
 
 
 0.7.0 - October 9th, 2015
@@ -26,21 +31,21 @@ Bug Fixes
 New Features
 
 * Added the D104 error code - "Missing docstring in public package". This new
-  error is turned on by default. Missing docstring in `__init__.py` files which
+  error is turned on by default. Missing docstring in ``__init__.py`` files which
   previously resulted in D100 errors ("Missing docstring in public module")
   will now result in D104 (#105, #127).
 
 * Added the D105 error code - "Missing docstring in magic method'. This new
   error is turned on by default. Missing docstrings in magic method which
   previously resulted in D102 error ("Missing docstring in public method")
   will now result in D105. Note that exceptions to this rule are variadic
-  magic methods - specifically `__init__`, `__call__` and `__new__`, which
+  magic methods - specifically ``__init__``, ``__call__`` and ``__new__``, which
   will be considered non-magic and missing docstrings in them will result
   in D102 (#60, #139).
 
 * Support the option to exclude all error codes. Running pep257 with
-  `--select=` (or `select=` in the configuration file) will exclude all errors
-  which could then be added one by one using `add-select`. Useful for projects
+  ``--select=`` (or ``select=`` in the configuration file) will exclude all errors
+  which could then be added one by one using ``add-select``. Useful for projects
   new to pep257 (#132, #135).
 
 * Added check D211: No blank lines allowed before class docstring. This change

src/pep257.py

@@ -15,6 +15,7 @@
 
 import os
 import sys
+import ast
 import copy
 import logging
 import tokenize as tk
@@ -118,7 +119,6 @@ class Definition(Value):
     module = property(lambda self: self.parent.module)
     all = property(lambda self: self.module.all)
     _slice = property(lambda self: slice(self.start - 1, self.end))
-    source = property(lambda self: ''.join(self._source[self._slice]))
     is_class = False
 
     def __iter__(self):
@@ -128,6 +128,17 @@ def __iter__(self):
     def _publicity(self):
         return {True: 'public', False: 'private'}[self.is_public]
 
+    @property
+    def source(self):
+        """Return the source code for the definition."""
+        full_src = self._source[self._slice]
+
+        def is_empty_or_comment(line):
+            return line.strip() == '' or line.strip().startswith('#')
+
+        filtered_src = dropwhile(is_empty_or_comment, reversed(full_src))
+        return ''.join(reversed(list(filtered_src)))
+
     def __str__(self):
         return 'in %s %s `%s`' % (self._publicity, self._human, self.name)
 
@@ -429,7 +440,7 @@ def parse_module(self):
         return module
 
     def parse_definition(self, class_):
-        """Parse a defintion and return its value in a `class_` object."""
+        """Parse a definition and return its value in a `class_` object."""
         start = self.line
         self.consume(tk.NAME)
         name = self.current.value
@@ -456,9 +467,9 @@ def parse_definition(self, class_):
             docstring = self.parse_docstring()
             decorators = self._accumulated_decorators
             self._accumulated_decorators = []
-            log.debug("parsing nested defintions.")
+            log.debug("parsing nested definitions.")
             children = list(self.parse_definitions(class_))
-            log.debug("finished parsing nested defintions for '%s'", name)
+            log.debug("finished parsing nested definitions for '%s'", name)
             end = self.line - 1
         else:  # one-liner definition
             docstring = self.parse_docstring()
@@ -682,6 +693,8 @@ def to_rst(cls):
                          '%r, not %r')
 D402 = D4xx.create_error('D402', 'First line should not be the function\'s '
                                  '"signature"')
+D403 = D4xx.create_error('D403', 'First word of the first line should be '
+                                 'properly capitalized', '%r, not %r')
 
 
 class AttrDict(dict):
@@ -1361,7 +1374,7 @@ def check_docstring_missing(self, definition, docstring):
 
         """
         if (not docstring and definition.is_public or
-                docstring and is_blank(eval(docstring))):
+                docstring and is_blank(ast.literal_eval(docstring))):
             codes = {Module: D100, Class: D101, NestedClass: D101,
                      Method: (lambda: D105() if is_magic(definition.name)
                               else D102()),
@@ -1377,7 +1390,7 @@ def check_one_liners(self, definition, docstring):
 
         """
         if docstring:
-            lines = eval(docstring).split('\n')
+            lines = ast.literal_eval(docstring).split('\n')
             if len(lines) > 1:
                 non_empty_lines = sum(1 for l in lines if not is_blank(l))
                 if non_empty_lines == 1:
@@ -1390,7 +1403,6 @@ def check_no_blank_before(self, function, docstring):  # def
         There's no blank line either before or after the docstring.
 
         """
-        # NOTE: This does not take comments into account.
         # NOTE: This does not take into account functions with groups of code.
         if docstring:
             before, _, after = function.source.partition(docstring)
@@ -1448,7 +1460,7 @@ def check_blank_after_summary(self, definition, docstring):
 
         """
         if docstring:
-            lines = eval(docstring).strip().split('\n')
+            lines = ast.literal_eval(docstring).strip().split('\n')
             if len(lines) > 1:
                 post_summary_blanks = list(map(is_blank, lines[1:]))
                 blanks_count = sum(takewhile(bool, post_summary_blanks))
@@ -1487,7 +1499,8 @@ def check_newline_after_last_paragraph(self, definition, docstring):
 
         """
         if docstring:
-            lines = [l for l in eval(docstring).split('\n') if not is_blank(l)]
+            lines = [l for l in ast.literal_eval(docstring).split('\n')
+                     if not is_blank(l)]
             if len(lines) > 1:
                 if docstring.split("\n")[-1].strip() not in ['"""', "'''"]:
                     return D209()
@@ -1496,7 +1509,7 @@ def check_newline_after_last_paragraph(self, definition, docstring):
     def check_surrounding_whitespaces(self, definition, docstring):
         """D210: No whitespaces allowed surrounding docstring text."""
         if docstring:
-            lines = eval(docstring).split('\n')
+            lines = ast.literal_eval(docstring).split('\n')
             if lines[0].startswith(' ') or \
                     len(lines) == 1 and lines[0].endswith(' '):
                 return D210()
@@ -1514,8 +1527,8 @@ def check_triple_double_quotes(self, definition, docstring):
               """ quotes in its body.
 
         '''
-        if docstring and '"""' in eval(docstring) and docstring.startswith(
-                ("'''", "r'''", "u'''", "ur'''")):
+        if (docstring and '"""' in ast.literal_eval(docstring) and
+                docstring.startswith(("'''", "r'''", "u'''", "ur'''"))):
             # Allow ''' quotes if docstring contains """, because otherwise """
             # quotes could not be expressed inside docstring.  Not in PEP 257.
             return
@@ -1563,7 +1576,7 @@ def check_ends_with_period(self, definition, docstring):
 
         """
         if docstring:
-            summary_line = eval(docstring).strip().split('\n')[0]
+            summary_line = ast.literal_eval(docstring).strip().split('\n')[0]
             if not summary_line.endswith('.'):
                 return D400(summary_line[-1])
 
@@ -1577,7 +1590,7 @@ def check_imperative_mood(self, function, docstring):  # def context
 
         """
         if docstring:
-            stripped = eval(docstring).strip()
+            stripped = ast.literal_eval(docstring).strip()
             if stripped:
                 first_word = stripped.split()[0]
                 if first_word.endswith('s') and not first_word.endswith('ss'):
@@ -1592,10 +1605,22 @@ def check_no_signature(self, function, docstring):  # def context
 
         """
         if docstring:
-            first_line = eval(docstring).strip().split('\n')[0]
+            first_line = ast.literal_eval(docstring).strip().split('\n')[0]
             if function.name + '(' in first_line.replace(' ', ''):
                 return D402()
 
+    @check_for(Function)
+    def check_capitalized(self, function, docstring):
+        """D403: First word of the first line should be properly capitalized.
+
+        The [first line of a] docstring is a phrase ending in a period.
+
+        """
+        if docstring:
+            first_word = ast.literal_eval(docstring).split()[0]
+            if first_word != first_word.capitalize():
+                return D403(first_word.capitalize(), first_word)
+
     # Somewhat hard to determine if return value is mentioned.
     # @check(Function)
     def SKIP_check_return_type(self, function, docstring):

src/tests/test_cases/capitalization.py

@@ -0,0 +1,24 @@
+"""A valid module docstring."""
+
+from .expected import Expectation
+
+expectation = Expectation()
+expect = expectation.expect
+
+
+@expect("D403: First word of the first line should be properly capitalized "
+        "('Do', not 'do')")
+def not_capitalized():
+    """do something."""
+
+
+# Make sure empty docstrings don't generate capitalization errors.
+@expect("D103: Missing docstring in public function")
+def empty_docstring():
+    """"""
+
+
+@expect("D403: First word of the first line should be properly capitalized "
+        "('Get', not 'GET')")
+def all_caps():
+    """GET the request."""

src/tests/test_cases/comment_after_def_bug.py

@@ -0,0 +1,14 @@
+"""Check for a bug in parsing comments after definitions."""
+
+from .expected import Expectation
+
+expectation = Expectation()
+expect = expectation.expect
+
+
+def should_be_ok():
+    """Just a function without violations."""
+
+
+# This is a comment that triggers a bug that causes the previous function
+# to generate a D202 error.

src/tests/test_definitions.py

@@ -255,14 +255,19 @@ def test_token_stream():
 
 def test_pep257():
     """Run domain-specific tests from test.py file."""
-    test_cases = ('test', 'unicode_literals', 'nested_class')
+    test_cases = (
+        'test',
+        'unicode_literals',
+        'nested_class',
+        'capitalization',
+        'comment_after_def_bug',
+    )
     for test_case in test_cases:
         case_module = __import__('test_cases.{0}'.format(test_case),
                                  globals=globals(),
                                  locals=locals(),
                                  fromlist=['expectation'],
                                  level=1)
-        # from .test_cases import test
         results = list(check([os.path.join(os.path.dirname(__file__),
                                            'test_cases', test_case + '.py')],
                              select=set(ErrorRegistry.get_error_codes())))

src/tests/test_pep257.py

@@ -135,7 +135,7 @@ def function_with_bad_docstring(foo):
             return foo
     ''')
     expected_error_codes = set(('D100', 'D400', 'D401', 'D205', 'D209',
-                                'D210'))
+                                'D210', 'D403'))
     mock_open = mock.mock_open(read_data=function_to_check)
     from .. import pep257
     with mock.patch.object(pep257, 'tokenize_open', mock_open, create=True):