Add error D303 when an f-string is used as a docstring

docs/release_notes.rst

@@ -15,10 +15,14 @@ Major Updates
 New Features
 
 * Add flag to disable `# noqa` comment processing in API (#485).
+* New error code D303 is emitted when an f-string is found in place of a
+  docstring. Also fixes a bug where using f-strings as docstrings returned
+  ValueError: malformed node or string. (#381)
 * Methods, Functions and Nested functions that have a docstring now throw D418 (#511).
 * Methods decorated with @overload no longer reported as D102 (#511).
 * Functions and nested functions decorated with @overload no longer reported as D103 (#511).
 
+
 Bug Fixes
 
 * Treat "package" as an imperative verb for D401 (#356).

src/pydocstyle/checker.py

@@ -44,6 +44,27 @@ def decorator(f):
     return decorator
 
 
+FSTRING_REGEX = re(r'^([rR]?)[fF]')
+
+
+def is_fstring(docstring):
+    """Return True if docstring is an f-string."""
+    return FSTRING_REGEX.match(str(docstring))
+
+
+def safe_literal_eval(string):
+    """Safely evaluate a literal even if it is an fstring."""
+    try:
+        return ast.literal_eval(string)
+    except ValueError as error:
+        # If the docstring is a fstring, it is
+        # not considered a valid docstring. See
+        # https://bugs.python.org/issue28739
+        raise ParseError(
+            info="f-strings are not valid as docstrings."
+        ) from error
+
+
 class ConventionChecker:
     """Checker for PEP 257, NumPy and Google conventions.
 
@@ -176,8 +197,24 @@ def checks(self):
             for this_check in vars(type(self)).values()
             if hasattr(this_check, '_check_for')
         ]
+        # This returns the checks in the order they are
+        # listed in the file (since py3.6) if their priority is the same
         return sorted(all, key=lambda this_check: not this_check._terminal)
 
+    # Note - this needs to be listed before other checks
+    # as f string evalutaion may cause malformed AST Nodes.
+    # So we need to run this check first and terminate early.
+    @check_for(Definition, terminal=True)
+    def check_docstring_fstring(self, definition, docstring):
+        """D303: Docstrings may not be f-strings.
+
+        f-strings are not treated as string literals, but they look similar
+        and users may attempt to use them as docstrings. This is an
+        outright mistake so we issue a specific error code.
+        """
+        if is_fstring(docstring):
+            return violations.D303()
+
     @check_for(Definition, terminal=True)
     def check_docstring_missing(self, definition, docstring):
         """D10{0,1,2,3}: Public definitions should have docstrings.
@@ -196,7 +233,7 @@ def check_docstring_missing(self, definition, docstring):
             not docstring
             and definition.is_public
             or docstring
-            and is_blank(ast.literal_eval(docstring))
+            and is_blank(safe_literal_eval(docstring))
         ):
             codes = {
                 Module: violations.D100,
@@ -232,7 +269,7 @@ def check_one_liners(self, definition, docstring):
 
         """
         if docstring:
-            lines = ast.literal_eval(docstring).split('\n')
+            lines = safe_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:
@@ -308,7 +345,7 @@ def check_blank_after_summary(self, definition, docstring):
 
         """
         if docstring:
-            lines = ast.literal_eval(docstring).strip().split('\n')
+            lines = safe_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))
@@ -361,7 +398,7 @@ def check_newline_after_last_paragraph(self, definition, docstring):
         if docstring:
             lines = [
                 l
-                for l in ast.literal_eval(docstring).split('\n')
+                for l in safe_literal_eval(docstring).split('\n')
                 if not is_blank(l)
             ]
             if len(lines) > 1:
@@ -372,7 +409,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 = ast.literal_eval(docstring).split('\n')
+            lines = safe_literal_eval(docstring).split('\n')
             if (
                 lines[0].startswith(' ')
                 or len(lines) == 1
@@ -400,7 +437,7 @@ def check_multi_line_summary_start(self, definition, docstring):
                 "ur'''",
             ]
 
-            lines = ast.literal_eval(docstring).split('\n')
+            lines = safe_literal_eval(docstring).split('\n')
             if len(lines) > 1:
                 first = docstring.split("\n")[0].strip().lower()
                 if first in start_triple:
@@ -422,7 +459,7 @@ def check_triple_double_quotes(self, definition, docstring):
 
         '''
         if docstring:
-            if '"""' in ast.literal_eval(docstring):
+            if '"""' in safe_literal_eval(docstring):
                 # Allow ''' quotes if docstring contains """, because
                 # otherwise """ quotes could not be expressed inside
                 # docstring. Not in PEP 257.
@@ -466,7 +503,7 @@ def _check_ends_with(docstring, chars, violation):
 
         """
         if docstring:
-            summary_line = ast.literal_eval(docstring).strip().split('\n')[0]
+            summary_line = safe_literal_eval(docstring).strip().split('\n')[0]
             if not summary_line.endswith(chars):
                 return violation(summary_line[-1])
 
@@ -501,7 +538,7 @@ def check_imperative_mood(self, function, docstring):  # def context
 
         """
         if docstring and not function.is_test:
-            stripped = ast.literal_eval(docstring).strip()
+            stripped = safe_literal_eval(docstring).strip()
             if stripped:
                 first_word = strip_non_alphanumeric(stripped.split()[0])
                 check_word = first_word.lower()
@@ -527,7 +564,7 @@ def check_no_signature(self, function, docstring):  # def context
 
         """
         if docstring:
-            first_line = ast.literal_eval(docstring).strip().split('\n')[0]
+            first_line = safe_literal_eval(docstring).strip().split('\n')[0]
             if function.name + '(' in first_line.replace(' ', ''):
                 return violations.D402()
 
@@ -539,7 +576,7 @@ def check_capitalized(self, function, docstring):
 
         """
         if docstring:
-            first_word = ast.literal_eval(docstring).split()[0]
+            first_word = safe_literal_eval(docstring).split()[0]
             if first_word == first_word.upper():
                 return
             for char in first_word:
@@ -571,7 +608,7 @@ def check_starts_with_this(self, function, docstring):
         if not docstring:
             return
 
-        stripped = ast.literal_eval(docstring).strip()
+        stripped = safe_literal_eval(docstring).strip()
         if not stripped:
             return
 

src/pydocstyle/parser.py

@@ -30,8 +30,12 @@
 class ParseError(Exception):
     """An error parsing contents of a Python file."""
 
+    def __init__(self, info=""):
+        """Initialize the error with a more specific message."""
+        self.info = info
+
     def __str__(self):
-        return "Cannot parse file."
+        return f"Cannot parse file. {self.info}".strip()
 
 
 class UnexpectedTokenError(ParseError):

src/pydocstyle/utils.py

@@ -1,5 +1,4 @@
 """General shared utilities."""
-import ast
 import logging
 import re
 from itertools import tee, zip_longest

src/pydocstyle/violations.py

@@ -312,6 +312,10 @@ def to_rst(cls) -> str:
     'D302',
     'Deprecated: Use u""" for Unicode docstrings',
 )
+D303 = D3xx.create_error(
+    'D303',
+    'f-strings are not valid as docstrings',
+)
 
 D4xx = ErrorRegistry.create_group('D4', 'Docstring Content Issues')
 D400 = D4xx.create_error(

src/tests/test_cases/fstrings.py

@@ -0,0 +1,52 @@
+"""Test for warning about f-strings as docstrings."""
+
+from .expected import Expectation
+
+expectation = Expectation()
+expect = expectation.expect
+_GIZMO = "gizmo"
+D303 = expect("D303: f-strings are not valid as docstrings")
+
+
+@D303
+def fstring():
+    f"""Toggle the gizmo."""
+
+
+@D303
+def another_fstring():
+    F"""Toggle the gizmo."""
+
+
+@D303
+def fstring_with_raw():
+    rF"""Toggle the gizmo."""
+
+
+@D303
+def fstring_with_raw_caps():
+    RF"""Toggle the gizmo."""
+
+
+@D303
+def fstring_with_raw_variable():
+    RF"""Toggle the {_GIZMO}."""
+
+
+@D303
+def fstring_with_variable():
+    f"""Toggle the {_GIZMO.upper()}."""
+
+
+@D303
+def fstring_with_other_errors(arg=1, missing_arg=2):
+    f"""Toggle the {_GIZMO.upper()}
+
+    This should not raise any other errors since fstrings
+    are a terminal check.
+    """
+
+
+@D303
+def fstring_with_blank_doc_string():
+    f"""  """

src/tests/test_definitions.py

@@ -20,6 +20,7 @@
     'noqa',
     'sections',
     'functions',
+    'fstrings',
     'canonical_google_examples',
     'canonical_numpy_examples',
     'canonical_pep257_examples',

src/tests/test_integration.py

@@ -753,6 +753,21 @@ def foo():
     assert code == 0
 
 
+def test_fstring_excluded(env):
+    """Test excluding D303 fstring error."""
+    with env.open('example.py', 'wt') as example:
+        example.write(textwrap.dedent("""\
+            def foo():  # noqa: D303
+                f'''Test'''
+        """))
+
+    env.write_config(add_ignore="D100")
+    out, err, code = env.invoke()
+    assert code == 1
+    assert out == ""
+    assert "f-strings are not valid as docstrings." in err
+
+
 def test_empty_select_with_added_error(env):
     """Test excluding all errors but one."""
     with env.open('example.py', 'wt') as example: