diff --git a/great_docs/_lint.py b/great_docs/_lint.py index ac8b2ebb..252fa131 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,78 @@ 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_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_disabled + logger.propagate = previous_propagate + 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 +428,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: