From c9875588a51d8c0ea88ee60b82c53aa4602be502 Mon Sep 17 00:00:00 2001 From: Daniel Pope Date: Wed, 17 Jul 2019 18:32:40 +0100 Subject: [PATCH 01/10] Add error D303 when an f-string is used as a docstring --- docs/release_notes.rst | 2 ++ src/pydocstyle/checker.py | 32 ++++++++++++++++++++++++++++++++ src/pydocstyle/violations.py | 1 + src/tests/test_cases/fstrings.py | 11 +++++++++++ src/tests/test_definitions.py | 17 ++++++++++++++++- 5 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 src/tests/test_cases/fstrings.py diff --git a/docs/release_notes.rst b/docs/release_notes.rst index 265bf5fc..d0c45a78 100644 --- a/docs/release_notes.rst +++ b/docs/release_notes.rst @@ -15,6 +15,8 @@ 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. Bug Fixes diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index 834ef7b8..d18b5d44 100644 --- a/src/pydocstyle/checker.py +++ b/src/pydocstyle/checker.py @@ -46,6 +46,24 @@ def decorator(f): return decorator + +FSTRING_RE = re(r'^[rR]?[fF]') + + +def is_fstring(docstring): + r"""Return True if docstring is an f-string. + + >>> is_fstring('rf"abc"') + True + >>> is_fstring('F"abc"') + True + >>> is_fstring("u'''abc'''") + False + """ + return FSTRING_RE.match(str(docstring)) + + + class ConventionChecker: """Checker for PEP 257, NumPy and Google conventions. @@ -180,6 +198,17 @@ def checks(self): ] return sorted(all, key=lambda this_check: not this_check._terminal) + @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. @@ -194,6 +223,9 @@ def check_docstring_missing(self, definition, docstring): with a single underscore. """ + if is_fstring(docstring): + return # checked above in check_docstring_fstring + if ( not docstring and definition.is_public diff --git a/src/pydocstyle/violations.py b/src/pydocstyle/violations.py index 6bf3963d..870095f0 100644 --- a/src/pydocstyle/violations.py +++ b/src/pydocstyle/violations.py @@ -312,6 +312,7 @@ 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..9d4243cb --- /dev/null +++ b/src/tests/test_cases/fstrings.py @@ -0,0 +1,11 @@ +"""Test for warning about f-strings as docstrings.""" + +from .expected import Expectation + +expectation = Expectation() +expect = expectation.expect + + +@expect("D303: f-strings are not valid as docstrings") +def fstring(): + f"""Toggle the gizmo.""" diff --git a/src/tests/test_definitions.py b/src/tests/test_definitions.py index 3971f0a0..a0fb4088 100644 --- a/src/tests/test_definitions.py +++ b/src/tests/test_definitions.py @@ -1,5 +1,6 @@ """Old parser tests.""" +import sys import os import re import pytest @@ -24,7 +25,21 @@ 'canonical_numpy_examples', 'canonical_pep257_examples', ]) -def test_complex_file(test_case): +def test_all_interpreters(test_case): + """Complex test cases that run under all interpreters.""" + run_case(test_case) + + +@pytest.mark.skipif( + sys.version_info < (3, 6), + reason="f-string support needed" +) +def test_fstrings(): + """Run the f-string test case under Python 3.6+ only.""" + run_case('fstrings') + + +def run_case(test_case): """Run domain-specific tests from test.py file.""" case_module = __import__(f'test_cases.{test_case}', globals=globals(), From 0854a959c04288edf56a85f5deb2cdabaa41665b Mon Sep 17 00:00:00 2001 From: Daniel Pope Date: Wed, 20 Nov 2019 16:17:44 +0000 Subject: [PATCH 02/10] Workaround mypy un-ignoreable error in Py35 --- .../test_cases/{fstrings.py => fstrings.py36} | 0 src/tests/test_definitions.py | 18 ++++++++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) rename src/tests/test_cases/{fstrings.py => fstrings.py36} (100%) diff --git a/src/tests/test_cases/fstrings.py b/src/tests/test_cases/fstrings.py36 similarity index 100% rename from src/tests/test_cases/fstrings.py rename to src/tests/test_cases/fstrings.py36 diff --git a/src/tests/test_definitions.py b/src/tests/test_definitions.py index a0fb4088..6326bfd8 100644 --- a/src/tests/test_definitions.py +++ b/src/tests/test_definitions.py @@ -36,6 +36,18 @@ def test_all_interpreters(test_case): ) def test_fstrings(): """Run the f-string test case under Python 3.6+ only.""" + # When run under Python 3.5, mypy reports a parse error for the test file, + # because Python 3.5 doesn't support f-strings. It does not support + # ignoring parse errors. + # + # To work around this, we put our code in a file that mypy cannot see. This + # code reveals it to Python. + from . import test_cases + import importlib.machinery + test_cases_dir = test_cases.__path__[0] + loader = sys.path_importer_cache[test_cases_dir] + loader._loaders.append(('.py36', importlib.machinery.SourceFileLoader)) + run_case('fstrings') @@ -46,10 +58,8 @@ def run_case(test_case): locals=locals(), fromlist=['expectation'], level=1) - test_case_dir = os.path.normcase(os.path.dirname(__file__)) - test_case_file = os.path.join(test_case_dir, - 'test_cases', - test_case + '.py') + + test_case_file = case_module.__file__ results = list(check([test_case_file], select=set(ErrorRegistry.get_error_codes()), ignore_decorators=re.compile( From 9e96a78046967bb34754a32a386338aff625c703 Mon Sep 17 00:00:00 2001 From: Daniel Pope Date: Thu, 21 Nov 2019 13:31:52 +0000 Subject: [PATCH 03/10] Use relative paths to avoid case issues on Windows --- src/tests/test_definitions.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tests/test_definitions.py b/src/tests/test_definitions.py index 6326bfd8..4e9accb3 100644 --- a/src/tests/test_definitions.py +++ b/src/tests/test_definitions.py @@ -2,6 +2,7 @@ import sys import os +import os.path import re import pytest from pydocstyle.violations import Error, ErrorRegistry @@ -59,7 +60,7 @@ def run_case(test_case): fromlist=['expectation'], level=1) - test_case_file = case_module.__file__ + test_case_file = os.path.relpath(case_module.__file__) results = list(check([test_case_file], select=set(ErrorRegistry.get_error_codes()), ignore_decorators=re.compile( From 71463536e62e67e2f7d72be3b1040d7e70a3d44e Mon Sep 17 00:00:00 2001 From: Sambhav Kothari Date: Sun, 6 Sep 2020 17:23:03 +0100 Subject: [PATCH 04/10] Reformat the files --- src/pydocstyle/checker.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index d18b5d44..d75e6827 100644 --- a/src/pydocstyle/checker.py +++ b/src/pydocstyle/checker.py @@ -46,7 +46,6 @@ def decorator(f): return decorator - FSTRING_RE = re(r'^[rR]?[fF]') @@ -63,7 +62,6 @@ def is_fstring(docstring): return FSTRING_RE.match(str(docstring)) - class ConventionChecker: """Checker for PEP 257, NumPy and Google conventions. From 52e1e100f5b86fb22046f55dab4b287e4d647be5 Mon Sep 17 00:00:00 2001 From: Sambhav Kothari Date: Sun, 6 Sep 2020 17:27:49 +0100 Subject: [PATCH 05/10] Remove py3.5 compat from tests --- src/tests/test_cases/fstrings.py36 | 11 --------- src/tests/test_definitions.py | 36 +++++------------------------- 2 files changed, 5 insertions(+), 42 deletions(-) delete mode 100644 src/tests/test_cases/fstrings.py36 diff --git a/src/tests/test_cases/fstrings.py36 b/src/tests/test_cases/fstrings.py36 deleted file mode 100644 index 9d4243cb..00000000 --- a/src/tests/test_cases/fstrings.py36 +++ /dev/null @@ -1,11 +0,0 @@ -"""Test for warning about f-strings as docstrings.""" - -from .expected import Expectation - -expectation = Expectation() -expect = expectation.expect - - -@expect("D303: f-strings are not valid as docstrings") -def fstring(): - f"""Toggle the gizmo.""" diff --git a/src/tests/test_definitions.py b/src/tests/test_definitions.py index 4e9accb3..3971f0a0 100644 --- a/src/tests/test_definitions.py +++ b/src/tests/test_definitions.py @@ -1,8 +1,6 @@ """Old parser tests.""" -import sys import os -import os.path import re import pytest from pydocstyle.violations import Error, ErrorRegistry @@ -26,41 +24,17 @@ 'canonical_numpy_examples', 'canonical_pep257_examples', ]) -def test_all_interpreters(test_case): - """Complex test cases that run under all interpreters.""" - run_case(test_case) - - -@pytest.mark.skipif( - sys.version_info < (3, 6), - reason="f-string support needed" -) -def test_fstrings(): - """Run the f-string test case under Python 3.6+ only.""" - # When run under Python 3.5, mypy reports a parse error for the test file, - # because Python 3.5 doesn't support f-strings. It does not support - # ignoring parse errors. - # - # To work around this, we put our code in a file that mypy cannot see. This - # code reveals it to Python. - from . import test_cases - import importlib.machinery - test_cases_dir = test_cases.__path__[0] - loader = sys.path_importer_cache[test_cases_dir] - loader._loaders.append(('.py36', importlib.machinery.SourceFileLoader)) - - run_case('fstrings') - - -def run_case(test_case): +def test_complex_file(test_case): """Run domain-specific tests from test.py file.""" case_module = __import__(f'test_cases.{test_case}', globals=globals(), locals=locals(), fromlist=['expectation'], level=1) - - test_case_file = os.path.relpath(case_module.__file__) + test_case_dir = os.path.normcase(os.path.dirname(__file__)) + test_case_file = os.path.join(test_case_dir, + 'test_cases', + test_case + '.py') results = list(check([test_case_file], select=set(ErrorRegistry.get_error_codes()), ignore_decorators=re.compile( From f2a944f15d179d5a715dac2683041d6d9a2cb832 Mon Sep 17 00:00:00 2001 From: Sambhav Kothari Date: Sun, 6 Sep 2020 17:30:30 +0100 Subject: [PATCH 06/10] Add PR Id --- docs/release_notes.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/release_notes.rst b/docs/release_notes.rst index d0c45a78..52ce9d6a 100644 --- a/docs/release_notes.rst +++ b/docs/release_notes.rst @@ -16,7 +16,8 @@ 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. + docstring. Also fixes a bug where using f-strings as docstrings returned + ValueError: malformed node or string. (#381) Bug Fixes From e39f80be8e7976d280817985720c07a23f3919ba Mon Sep 17 00:00:00 2001 From: Sambhav Kothari Date: Sun, 6 Sep 2020 17:41:52 +0100 Subject: [PATCH 07/10] Add extensive tests --- src/pydocstyle/checker.py | 20 ++++---------- src/pydocstyle/violations.py | 5 +++- src/tests/test_cases/fstrings.py | 47 ++++++++++++++++++++++++++++++++ src/tests/test_definitions.py | 1 + 4 files changed, 58 insertions(+), 15 deletions(-) create mode 100644 src/tests/test_cases/fstrings.py diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index d75e6827..29cbff9d 100644 --- a/src/pydocstyle/checker.py +++ b/src/pydocstyle/checker.py @@ -46,20 +46,12 @@ def decorator(f): return decorator -FSTRING_RE = re(r'^[rR]?[fF]') +_FSTRING_REGEX = re(r'^[rR]?[fF]') -def is_fstring(docstring): - r"""Return True if docstring is an f-string. - - >>> is_fstring('rf"abc"') - True - >>> is_fstring('F"abc"') - True - >>> is_fstring("u'''abc'''") - False - """ - return FSTRING_RE.match(str(docstring)) +def _is_fstring(docstring): + """Return True if docstring is an f-string.""" + return _FSTRING_REGEX.match(str(docstring)) class ConventionChecker: @@ -204,7 +196,7 @@ def check_docstring_fstring(self, definition, docstring): 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): + if _is_fstring(docstring): return violations.D303() @check_for(Definition, terminal=True) @@ -221,7 +213,7 @@ def check_docstring_missing(self, definition, docstring): with a single underscore. """ - if is_fstring(docstring): + if _is_fstring(docstring): return # checked above in check_docstring_fstring if ( diff --git a/src/pydocstyle/violations.py b/src/pydocstyle/violations.py index 870095f0..a5bf5c1e 100644 --- a/src/pydocstyle/violations.py +++ b/src/pydocstyle/violations.py @@ -312,7 +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') +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..ee8756ca --- /dev/null +++ b/src/tests/test_cases/fstrings.py @@ -0,0 +1,47 @@ +"""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. + """ 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', From e6a7c826e9d4c77144ff8c18834692cd9bcdcaa7 Mon Sep 17 00:00:00 2001 From: Sambhav Kothari Date: Sun, 6 Sep 2020 18:22:51 +0100 Subject: [PATCH 08/10] Make the missing docstring check more robust --- src/pydocstyle/checker.py | 10 ++++++---- src/pydocstyle/utils.py | 9 ++++++++- src/tests/test_cases/fstrings.py | 5 +++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index 29cbff9d..f59042b6 100644 --- a/src/pydocstyle/checker.py +++ b/src/pydocstyle/checker.py @@ -186,8 +186,13 @@ 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. @@ -213,14 +218,11 @@ def check_docstring_missing(self, definition, docstring): with a single underscore. """ - if _is_fstring(docstring): - return # checked above in check_docstring_fstring - if ( not docstring and definition.is_public or docstring - and is_blank(ast.literal_eval(docstring)) + and is_blank(docstring, literal_eval=True) ): codes = { Module: violations.D100, diff --git a/src/pydocstyle/utils.py b/src/pydocstyle/utils.py index 178f778c..c4c233f0 100644 --- a/src/pydocstyle/utils.py +++ b/src/pydocstyle/utils.py @@ -13,8 +13,15 @@ NON_ALPHANUMERIC_STRIP_RE = re.compile(r'[\W_]+') -def is_blank(string: str) -> bool: +def is_blank(string: str, literal_eval: bool = False) -> bool: """Return True iff the string contains only whitespaces.""" + if literal_eval: + try: + string = ast.literal_eval(string) + except ValueError: + # This happens in case of an fstring + # in which case let's return false + return False return not string.strip() diff --git a/src/tests/test_cases/fstrings.py b/src/tests/test_cases/fstrings.py index ee8756ca..c4835827 100644 --- a/src/tests/test_cases/fstrings.py +++ b/src/tests/test_cases/fstrings.py @@ -45,3 +45,8 @@ def fstring_with_other_errors(arg=1, missing_arg=2): This should not raise any other errors since fstrings are a terminal check. """ + + +@D303 +def fstring_with_blank_doc_string(): + f""" """ From afd0030b53ce9ab8ecc4e5d4864872dd9f45bccd Mon Sep 17 00:00:00 2001 From: Sambhav Kothari Date: Sun, 6 Sep 2020 19:02:45 +0100 Subject: [PATCH 09/10] Safely evaluate literals in case fstring check has been disbaled --- src/pydocstyle/checker.py | 38 +++++++++++++------------------- src/pydocstyle/utils.py | 30 ++++++++++++++++++------- src/tests/test_cases/fstrings.py | 5 +++++ 3 files changed, 42 insertions(+), 31 deletions(-) diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index f59042b6..0bbda050 100644 --- a/src/pydocstyle/checker.py +++ b/src/pydocstyle/checker.py @@ -2,8 +2,6 @@ import ast import string -import sys -import textwrap import tokenize as tk from collections import namedtuple from itertools import chain, takewhile @@ -28,8 +26,10 @@ from .utils import ( common_prefix_length, is_blank, + is_fstring, log, pairwise, + safe_literal_eval, strip_non_alphanumeric, ) from .wordlists import IMPERATIVE_BLACKLIST, IMPERATIVE_VERBS, stem @@ -46,14 +46,6 @@ 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)) - - class ConventionChecker: """Checker for PEP 257, NumPy and Google conventions. @@ -201,7 +193,7 @@ def check_docstring_fstring(self, definition, docstring): 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): + if is_fstring(docstring): return violations.D303() @check_for(Definition, terminal=True) @@ -222,7 +214,7 @@ def check_docstring_missing(self, definition, docstring): not docstring and definition.is_public or docstring - and is_blank(docstring, literal_eval=True) + and is_blank(safe_literal_eval(docstring)) ): codes = { Module: violations.D100, @@ -252,7 +244,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: @@ -328,7 +320,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)) @@ -381,7 +373,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: @@ -392,7 +384,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 @@ -420,7 +412,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: @@ -442,7 +434,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. @@ -486,7 +478,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]) @@ -521,7 +513,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() @@ -547,7 +539,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() @@ -559,7 +551,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: @@ -579,7 +571,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/utils.py b/src/pydocstyle/utils.py index c4c233f0..c8916ea4 100644 --- a/src/pydocstyle/utils.py +++ b/src/pydocstyle/utils.py @@ -13,15 +13,8 @@ NON_ALPHANUMERIC_STRIP_RE = re.compile(r'[\W_]+') -def is_blank(string: str, literal_eval: bool = False) -> bool: +def is_blank(string: str) -> bool: """Return True iff the string contains only whitespaces.""" - if literal_eval: - try: - string = ast.literal_eval(string) - except ValueError: - # This happens in case of an fstring - # in which case let's return false - return False return not string.strip() @@ -54,3 +47,24 @@ def common_prefix_length(a: str, b: str) -> int: def strip_non_alphanumeric(string: str) -> str: """Strip string from any non-alphanumeric characters.""" return NON_ALPHANUMERIC_STRIP_RE.sub('', string) + + +FSTRING_REGEX = re.compile(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: + # In case we hit a value error due to an fstring + # we do a literal eval by subtituting the fstring + # with a normal string. + # We keep the first captured group if any. This includes + # the raw identifiers (r/R) and replace f/F with a blank. + return ast.literal_eval(FSTRING_REGEX.sub(r"\1", string)) diff --git a/src/tests/test_cases/fstrings.py b/src/tests/test_cases/fstrings.py index c4835827..cad163a3 100644 --- a/src/tests/test_cases/fstrings.py +++ b/src/tests/test_cases/fstrings.py @@ -50,3 +50,8 @@ def fstring_with_other_errors(arg=1, missing_arg=2): @D303 def fstring_with_blank_doc_string(): f""" """ + + +@expect("D103: Missing docstring in public function") +def fstring_with_ignores(): # noqa: D303 + f""" """ From 2f9f337dbbb439143e5f8ce0a8d4b80c5044e1cb Mon Sep 17 00:00:00 2001 From: Sambhav Kothari Date: Sun, 6 Sep 2020 19:19:47 +0100 Subject: [PATCH 10/10] Raise ParseError on fstring docstrings as they are not valid docstrings --- src/pydocstyle/checker.py | 23 +++++++++++++++++++++-- src/pydocstyle/parser.py | 6 +++++- src/pydocstyle/utils.py | 22 ---------------------- src/tests/test_cases/fstrings.py | 5 ----- src/tests/test_integration.py | 15 +++++++++++++++ 5 files changed, 41 insertions(+), 30 deletions(-) diff --git a/src/pydocstyle/checker.py b/src/pydocstyle/checker.py index 0bbda050..0c118832 100644 --- a/src/pydocstyle/checker.py +++ b/src/pydocstyle/checker.py @@ -26,10 +26,8 @@ from .utils import ( common_prefix_length, is_blank, - is_fstring, log, pairwise, - safe_literal_eval, strip_non_alphanumeric, ) from .wordlists import IMPERATIVE_BLACKLIST, IMPERATIVE_VERBS, stem @@ -46,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. diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index 12c0b16d..346d804d 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 c8916ea4..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 @@ -47,24 +46,3 @@ def common_prefix_length(a: str, b: str) -> int: def strip_non_alphanumeric(string: str) -> str: """Strip string from any non-alphanumeric characters.""" return NON_ALPHANUMERIC_STRIP_RE.sub('', string) - - -FSTRING_REGEX = re.compile(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: - # In case we hit a value error due to an fstring - # we do a literal eval by subtituting the fstring - # with a normal string. - # We keep the first captured group if any. This includes - # the raw identifiers (r/R) and replace f/F with a blank. - return ast.literal_eval(FSTRING_REGEX.sub(r"\1", string)) diff --git a/src/tests/test_cases/fstrings.py b/src/tests/test_cases/fstrings.py index cad163a3..c4835827 100644 --- a/src/tests/test_cases/fstrings.py +++ b/src/tests/test_cases/fstrings.py @@ -50,8 +50,3 @@ def fstring_with_other_errors(arg=1, missing_arg=2): @D303 def fstring_with_blank_doc_string(): f""" """ - - -@expect("D103: Missing docstring in public function") -def fstring_with_ignores(): # noqa: D303 - f""" """ diff --git a/src/tests/test_integration.py b/src/tests/test_integration.py index 7de3b755..f0755347 100644 --- a/src/tests/test_integration.py +++ b/src/tests/test_integration.py @@ -571,6 +571,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: