diff --git a/docs/release_notes.rst b/docs/release_notes.rst index 693e6465..24d9ea13 100644 --- a/docs/release_notes.rst +++ b/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). diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index 41e3f35f..047fb6a1 100644 --- a/src/pydocstyle/checker.py +++ b/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 diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index a6f1ed0b..1cd4239c 100644 --- a/src/pydocstyle/parser.py +++ b/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): diff --git a/src/pydocstyle/utils.py b/src/pydocstyle/utils.py index 178f778c..8ea5654a 100644 --- a/src/pydocstyle/utils.py +++ b/src/pydocstyle/utils.py @@ -1,5 +1,4 @@ """General shared utilities.""" -import ast import logging import re from itertools import tee, zip_longest diff --git a/src/pydocstyle/violations.py b/src/pydocstyle/violations.py index eb2b6d4c..25699c40 100644 --- a/src/pydocstyle/violations.py +++ b/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( diff --git a/src/tests/test_cases/fstrings.py b/src/tests/test_cases/fstrings.py new file mode 100644 index 00000000..c4835827 --- /dev/null +++ b/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""" """ diff --git a/src/tests/test_definitions.py b/src/tests/test_definitions.py index 3971f0a0..605d9bb9 100644 --- a/src/tests/test_definitions.py +++ b/src/tests/test_definitions.py @@ -20,6 +20,7 @@ 'noqa', 'sections', 'functions', + 'fstrings', 'canonical_google_examples', 'canonical_numpy_examples', 'canonical_pep257_examples', diff --git a/src/tests/test_integration.py b/src/tests/test_integration.py index 22f57857..4c32b610 100644 --- a/src/tests/test_integration.py +++ b/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: