From fbd31f8673960a073ddb8cde2d4d091875b4e7b0 Mon Sep 17 00:00:00 2001 From: Hassan Kibirige Date: Mon, 17 Aug 2026 17:09:25 +0300 Subject: [PATCH 1/2] fix(lint): detect docstring style mismatches by parsing, not by pattern The style check matched a hand-written list of section headers per style. The list could not track what the parsers actually read: it treated a numpy Examples section as proof of numpy style, though that section is ordinary rST found in docstrings of every style, and it stayed silent on headers absent from the list. Ask the parsers instead. For each docstring the check collects the section kinds every style reads, and warns when a rival style reads a kind the configured one misses, naming both. This reports a foreign section sitting beside native ones, which the old rule suppressed, and stays silent on docstrings genuinely written in the configured style. Reject a configured parser outside numpy, google and sphinx with one config error rather than a warning on every docstring. --- great_docs/_lint.py | 141 +++++++++++++++++++++++++++++--------------- tests/test_lint.py | 124 +++++++++++++++++++++++++++++--------- 2 files changed, 188 insertions(+), 77 deletions(-) diff --git a/great_docs/_lint.py b/great_docs/_lint.py index ac8b2ebb..2d0457df 100644 --- a/great_docs/_lint.py +++ b/great_docs/_lint.py @@ -1,5 +1,6 @@ from __future__ import annotations +import logging import re from dataclasses import asdict, dataclass, field from pathlib import Path @@ -345,45 +346,73 @@ def _check_cross_references( pass -# Patterns for detecting docstring styles -_NUMPY_SECTION = re.compile( - r"^\s*(Parameters|Returns|Yields|Raises|Examples|Attributes|Methods|" - r"See Also|Notes|References|Warnings)\s*\n\s*-{3,}", - re.MULTILINE, -) +_STYLES = ("numpy", "google", "sphinx") +"""The docstring styles a project can be configured for""" -_GOOGLE_SECTION = re.compile( - r"^\s*(Args|Arguments|Returns|Yields|Raises|Examples|Attributes|" - r"Note|Notes|Todo|Warning|Warnings):\s*$", - re.MULTILINE, -) -_SPHINX_FIELD = re.compile( - r"^\s*:(param|type|returns|rtype|raises|var|ivar|cvar)\s", - re.MULTILINE, -) +def _section_kinds(docstring: str, style: str) -> set[str]: + """ + Return the kinds of structured section a style's parser finds in a docstring + Plain text is not a structured section, so a docstring that a parser cannot + read at all yields the empty set. The docstring is parsed detached from any + object: with a parent, griffe additionally reports mismatches against the + signature, which are not this check's concern. -def _detect_style_of_docstring(docstring: str) -> str | None: - """Detect which style a single docstring uses. Returns None if no sections found.""" - has_numpy = bool(_NUMPY_SECTION.search(docstring)) - has_google = bool(_GOOGLE_SECTION.search(docstring)) and not has_numpy - has_sphinx = bool(_SPHINX_FIELD.search(docstring)) - - styles_found = [] - if has_numpy: - styles_found.append("numpy") - if has_google: - styles_found.append("google") - if has_sphinx: - styles_found.append("sphinx") - - if len(styles_found) == 1: - return styles_found[0] - if len(styles_found) > 1: - # Mixed styles — return the first detected for reporting - return styles_found[0] - return None + The check parses each docstring under styles it was not written in, so + griffe's complaints about the text it cannot read are expected and stay + silenced rather than reaching the user as lint output. + + Parameters + ---------- + docstring + The docstring text. + style + The style whose parser reads the text. + + Returns + ------- + The section kinds found, named as griffe names them. + """ + import griffe + + logger = logging.getLogger("griffe") + previous = logger.disabled + logger.disabled = True + try: + parsed = griffe.Docstring( # pyright: ignore[reportArgumentType] + docstring, parser=style + ).parsed + finally: + logger.disabled = previous + return {section.kind.value for section in parsed if section.kind.value != "text"} + + +def _lost_sections(docstring: str, config_style: str) -> dict[str, set[str]]: + """ + Find the structure the configured parser drops but another parser would read + + Parameters + ---------- + docstring + The docstring text. + config_style + The style the project is configured for. + + Returns + ------- + Each rival style mapped to the section kinds it finds and the configured + style misses, empty when the configured style reads everything. + """ + configured = _section_kinds(docstring, config_style) + lost: dict[str, set[str]] = {} + for style in _STYLES: + if style == config_style: + continue + missed = _section_kinds(docstring, style) - configured + if missed: + lost[style] = missed + return lost def _check_docstring_style( @@ -394,24 +423,38 @@ def _check_docstring_style( result: LintResult, ) -> None: """Enforce consistent docstring style across all exports.""" + if config_style not in _STYLES: + result.issues.append( + LintIssue( + check="config", + severity="error", + symbol="great-docs.yml", + message=( + f"parser: {config_style!r} is not one of {', '.join(_STYLES)}, " + f"so docstring style cannot be checked." + ), + ) + ) + return def _check_one(symbol: str, docstring: str) -> None: - detected = _detect_style_of_docstring(docstring) - if detected is None: - # No structured sections found — skip (short docstrings are fine) + lost = _lost_sections(docstring, config_style) + if not lost: return - if detected != config_style: - result.issues.append( - LintIssue( - check="style-mismatch", - severity="warning", - symbol=symbol, - message=( - f"Docstring appears to use '{detected}' style " - f"but project is configured for '{config_style}'." - ), - ) + detail = "; ".join( + f"{style} reads {', '.join(sorted(kinds))}" for style, kinds in sorted(lost.items()) + ) + result.issues.append( + LintIssue( + check="style-mismatch", + severity="warning", + symbol=symbol, + message=( + f"The '{config_style}' parser does not read some of this " + f"docstring's structure: {detail}." + ), ) + ) for name in exports: if name not in pkg.members: diff --git a/tests/test_lint.py b/tests/test_lint.py index 73ce3d91..3c82d60d 100644 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -11,7 +11,8 @@ _check_directive_consistency, _check_docstring_style, _check_missing_docstrings, - _detect_style_of_docstring, + _lost_sections, + _section_kinds, run_lint, ) @@ -78,9 +79,7 @@ def test_to_dict(self): assert len(d["issues"]) == 2 -class TestDetectStyleOfDocstring: - def test_numpy_style(self): - doc = """\ +NUMPY_DOC = """\ Short description. Parameters @@ -88,33 +87,86 @@ def test_numpy_style(self): x : int The value. """ - assert _detect_style_of_docstring(doc) == "numpy" - def test_google_style(self): - doc = """\ +GOOGLE_DOC = """\ Short description. Args: x: The value. """ - assert _detect_style_of_docstring(doc) == "google" - def test_sphinx_style(self): - doc = """\ +SPHINX_DOC = """\ Short description. :param x: The value. :returns: Something. """ - assert _detect_style_of_docstring(doc) == "sphinx" - def test_no_sections(self): - doc = "Just a short description." - assert _detect_style_of_docstring(doc) is None +class TestSectionKinds: + @pytest.mark.parametrize( + ("doc", "style"), + [(NUMPY_DOC, "numpy"), (GOOGLE_DOC, "google"), (SPHINX_DOC, "sphinx")], + ) + def test_own_parser_reads_the_parameters(self, doc: str, style: str): + assert "parameters" in _section_kinds(doc, style) + + @pytest.mark.parametrize( + ("doc", "style"), + [(NUMPY_DOC, "google"), (GOOGLE_DOC, "numpy"), (SPHINX_DOC, "numpy")], + ) + def test_foreign_parser_reads_nothing(self, doc: str, style: str): + assert _section_kinds(doc, style) == set() + + def test_prose_has_no_structure_under_any_parser(self): + for style in ("numpy", "google", "sphinx"): + assert _section_kinds("Just a short description.", style) == set() def test_empty_string(self): - assert _detect_style_of_docstring("") is None + assert _section_kinds("", "numpy") == set() + + +class TestLostSections: + @pytest.mark.parametrize( + ("doc", "style"), + [(NUMPY_DOC, "numpy"), (GOOGLE_DOC, "google"), (SPHINX_DOC, "sphinx")], + ) + def test_docstring_in_the_configured_style_loses_nothing(self, doc: str, style: str): + assert _lost_sections(doc, style) == {} + + def test_prose_loses_nothing(self): + assert _lost_sections("Just a short description.", "numpy") == {} + + def test_numpy_examples_alone_is_not_reported_as_foreign(self): + """ + An `Examples` section is plain rST, so it must not look like another style + + griffe's own style inference omits `Examples` from its numpy patterns for + this reason: the section appears in docstrings of every style. + """ + doc = "Short description.\n\nExamples\n--------\n>>> f(1)\n" + + assert _lost_sections(doc, "numpy") == {} + + def test_google_sections_under_the_numpy_parser_are_reported(self): + assert _lost_sections(GOOGLE_DOC, "numpy") == {"google": {"parameters"}} + + def test_singular_example_header_is_reported(self): + """ + `Example:` reaches the reader as an admonition only under the Google parser + + The header is not one that a section-name pattern would list, which is why + the check asks the parsers instead of matching headers. + """ + doc = "Short description.\n\nExample:\n >>> f(1)\n 1\n" + + assert _lost_sections(doc, "numpy") == {"google": {"admonition"}} + + def test_a_foreign_section_beside_native_ones_is_reported(self): + """A docstring is not excused by the configured parser reading part of it""" + doc = NUMPY_DOC + "\nExamples:\n >>> f(1)\n" + + assert _lost_sections(doc, "numpy") == {"google": {"examples"}} def _make_griffe_obj(kind="function", docstring=None, members=None): @@ -431,6 +483,7 @@ def test_successful_lint_run(self, mock_gd_cls, mock_griffe_load, tmp_path): mock_gd._resolve_importable_name.return_value = "mypkg" mock_gd._get_package_exports.return_value = ["func_a", "func_b"] mock_gd._config.get.return_value = "numpy" + mock_gd._config.__getitem__.return_value = "numpy" mock_gd_cls.return_value = mock_gd func_a = _make_griffe_obj(docstring="Documented.\n\nParameters\n----------\nx : int\n") @@ -461,6 +514,7 @@ def test_resolves_module_name_when_project_name_differs( mock_gd._resolve_importable_name.return_value = "actual_module" mock_gd._get_package_exports.return_value = ["func_a"] mock_gd._config.get.return_value = "numpy" + mock_gd._config.__getitem__.return_value = "numpy" mock_gd_cls.return_value = mock_gd func_a = _make_griffe_obj(docstring="Documented.\n\nParameters\n----------\nx : int\n") @@ -485,6 +539,7 @@ def test_selective_checks(self, mock_gd_cls, mock_griffe_load, tmp_path): mock_gd._normalize_package_name.return_value = "mypkg" mock_gd._get_package_exports.return_value = ["func_a"] mock_gd._config.get.return_value = "numpy" + mock_gd._config.__getitem__.return_value = "numpy" mock_gd_cls.return_value = mock_gd # func_a has Google-style docstring (triggers style-mismatch) and no xref issues @@ -513,6 +568,7 @@ def test_no_exports(self, mock_gd_cls, mock_griffe_load, tmp_path): mock_gd._normalize_package_name.return_value = "mypkg" mock_gd._get_package_exports.return_value = None mock_gd._config.get.return_value = "numpy" + mock_gd._config.__getitem__.return_value = "numpy" mock_gd_cls.return_value = mock_gd mock_pkg = MagicMock() @@ -571,6 +627,7 @@ def test_quiet_suppresses_output(self, mock_gd_cls, mock_griffe_load, tmp_path, mock_gd._normalize_package_name.return_value = "mypkg" mock_gd._get_package_exports.return_value = ["func_a"] mock_gd._config.get.return_value = "numpy" + mock_gd._config.__getitem__.return_value = "numpy" mock_gd_cls.return_value = mock_gd func_a = _make_griffe_obj(docstring="Documented.") @@ -837,9 +894,24 @@ def test_class_outer_exception_in_method_xref(self): _check_cross_references(pkg, "mypkg", ["MyClass", "something"], result) -class TestDetectStyleEdgeCases: - def test_mixed_numpy_and_sphinx(self): - """Docstring with both numpy and sphinx markers returns numpy (first found).""" +class TestMixedStyleDocstrings: + """ + A docstring mixing two styles reports whichever structure the build loses + + The previous header-matching check reported one winning style per docstring + and so stayed silent whenever the configured style was among those matched. + """ + + def test_a_stray_field_of_a_kind_already_present_goes_unreported(self): + """ + Two styles contributing the same section kind cancel out + + The check compares which kinds each parser reads, not what each one puts + in them, so a `:param:` beside a numpy `Parameters` section is invisible: + both parsers report `parameters`. Naming the lost parameter would mean + comparing section contents per kind, which buys little for how rarely a + docstring mixes styles within one kind. + """ doc = """\ Short description. @@ -849,11 +921,9 @@ def test_mixed_numpy_and_sphinx(self): :param y: Another param. """ - result = _detect_style_of_docstring(doc) - assert result == "numpy" + assert _lost_sections(doc, "numpy") == {} - def test_mixed_google_and_sphinx(self): - """Docstring with google and sphinx markers.""" + def test_sphinx_field_with_a_google_section(self): doc = """\ Short description. @@ -862,12 +932,10 @@ def test_mixed_google_and_sphinx(self): Args: y: Another param. """ - # sphinx detected first in code order - result = _detect_style_of_docstring(doc) - - # Both google and sphinx detected; but numpy takes precedence over google - # and sphinx is also found, so styles_found has both - assert result in ("google", "sphinx") + assert _lost_sections(doc, "numpy") == { + "google": {"parameters"}, + "sphinx": {"parameters"}, + } class TestCheckDocstringStyleEdgeCases: From 041f49d8a87e272ef886656d074f999d464b9d9b Mon Sep 17 00:00:00 2001 From: Richard Iannone Date: Mon, 17 Aug 2026 20:44:59 -0400 Subject: [PATCH 2/2] Silence griffe logger propagation in linter --- great_docs/_lint.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/great_docs/_lint.py b/great_docs/_lint.py index 2d0457df..252fa131 100644 --- a/great_docs/_lint.py +++ b/great_docs/_lint.py @@ -377,14 +377,19 @@ def _section_kinds(docstring: str, style: str) -> set[str]: import griffe logger = logging.getLogger("griffe") - previous = logger.disabled + previous_disabled = logger.disabled + previous_propagate = logger.propagate + # disabled suppresses records sent directly to this logger; propagate=False + # stops child-logger records from reaching root handlers via propagation. logger.disabled = True + logger.propagate = False try: parsed = griffe.Docstring( # pyright: ignore[reportArgumentType] docstring, parser=style ).parsed finally: - logger.disabled = previous + logger.disabled = previous_disabled + logger.propagate = previous_propagate return {section.kind.value for section in parsed if section.kind.value != "text"}