changelog: validate rst entry line lengths

py/envoy.code.check/envoy/code/check/abstract/changelog.py

@@ -7,6 +7,8 @@
 from collections.abc import Iterator
 
 from packaging import version as _version
+from yamllint import linter as yamllint_linter  # type:ignore
+from yamllint.config import YamlLintConfig  # type:ignore
 
 import abstracts
 
@@ -19,9 +21,11 @@
     ENTRY_SEPARATOR,
 )
 from envoy.code.check import abstract, interface
+from envoy.code.check.abstract.yamllint import YAMLLINT_CONFIG
 
 
 MAX_VERSION_FOR_CHANGES_SECTION = "1.16"
+YAML_ENTRY_INDENT = "    "
 VALID_CHANGELOG_AREA_RE = re.compile(r"^[a-z0-9_\-/]+$")
 VALID_CHANGELOG_AREA_PATTERN = r"[a-z0-9_\-/]+"
 
@@ -35,9 +39,14 @@ class AChangelogChangesChecker(metaclass=abstracts.Abstraction):
     def __init__(
             self,
             sections: utils.typing.ChangelogSectionsDict,
-            areas: "utils.typing.ChangelogAreasDict") -> None:
+            areas: "utils.typing.ChangelogAreasDict",
+            *,
+            yamllint_config: YamlLintConfig | None = None,
+            yamllint_config_path: pathlib.Path | None = None) -> None:
         self.sections = sections
         self.areas = areas
+        self._yamllint_config = yamllint_config
+        self._yamllint_config_path = yamllint_config_path
 
     @property  # type:ignore
     @abstracts.interfacemethod
@@ -52,6 +61,17 @@ def config_path(self) -> pathlib.Path:
     def max_version_for_changes_section(self) -> _version.Version:
         return _version.Version(MAX_VERSION_FOR_CHANGES_SECTION)
 
+    @cached_property
+    def yamllint_config(self) -> YamlLintConfig:
+        if self._yamllint_config is not None:
+            return self._yamllint_config
+        config_path = (
+            self._yamllint_config_path or pathlib.Path(YAMLLINT_CONFIG))
+        return (
+            YamlLintConfig(file=config_path)
+            if config_path.exists()
+            else YamlLintConfig("extends: default"))
+
     def check_entry(
             self,
             version: _version.Version,
@@ -174,6 +194,19 @@ def check_entry_content(
                 f"{path}: Entry file is empty or contains only whitespace")
         return None
 
+    def check_entry_line_lengths(
+            self,
+            path: pathlib.Path) -> tuple[str, ...]:
+        content = "".join(
+            f"{YAML_ENTRY_INDENT}{line}\n"
+            for line
+            in path.read_text().splitlines())
+        return tuple(
+            f"{path}:{problem.line}: {problem.desc}"
+            for problem
+            in yamllint_linter.run(content, self.yamllint_config)
+            if problem.rule == "line-length")
+
     def check_entry_files(
             self,
             paths: list[pathlib.Path]) -> tuple[str, ...]:
@@ -183,6 +216,7 @@ def check_entry_files(
                 errors.append(err)
             if err := self.check_entry_content(path):
                 errors.append(err)
+            errors.extend(self.check_entry_line_lengths(path))
         return tuple(errors)
 
 
@@ -384,7 +418,8 @@ def changes_checker_class(
     def changes_checker(self) -> interface.IChangelogChangesChecker:
         return self.changes_checker_class(
             self.project.changelogs.sections,
-            self.project.changelogs.areas)
+            self.project.changelogs.areas,
+            yamllint_config_path=self.project.path.joinpath(YAMLLINT_CONFIG))
 
     @property  # type:ignore
     @abstracts.interfacemethod

py/envoy.code.check/tests/test_abstract_changelog.py

@@ -4,6 +4,7 @@
 
 from packaging import version as _version
 import pytest
+from yamllint.config import YamlLintConfig
 
 from envoy.base import utils
 from envoy.code import check
@@ -83,7 +84,9 @@ def test_changelogcheck_changes_checker(patches):
     assert (
         m_class.return_value.call_args
         == [(project.changelogs.sections,
-             project.changelogs.areas), {}])
+             project.changelogs.areas),
+            dict(yamllint_config_path=project.path.joinpath(".yamllint"))])
+    assert project.path.joinpath.call_args == [(".yamllint", ), {}]
     assert "changes_checker" in changelog.__dict__
 
 
@@ -701,8 +704,8 @@ async def test_changelogstatus_check_areas_file(
 
 class DummyChangelogChangesChecker(check.AChangelogChangesChecker):
 
-    def __init__(self, sections, areas=None):
-        super().__init__(sections, {} if areas is None else areas)
+    def __init__(self, sections, areas=None, **kwargs):
+        super().__init__(sections, {} if areas is None else areas, **kwargs)
 
     @property
     def change_checkers(self):
@@ -716,10 +719,48 @@ def test_changeschecker_constructor():
     changelog = DummyChangelogChangesChecker("SECTIONS")
     assert changelog.sections == "SECTIONS"
     assert changelog.areas == {}
+    assert changelog._yamllint_config is None
+    assert changelog._yamllint_config_path is None
     with pytest.raises(NotImplementedError):
         changelog.change_checkers
 
 
+def test_changeschecker_yamllint_config_passed(tmp_path):
+    config = YamlLintConfig("extends: default")
+    config_path = tmp_path / ".yamllint"
+    config_path.write_text("invalid config")
+    changelog = DummyChangelogChangesChecker(
+        "SECTIONS",
+        yamllint_config=config,
+        yamllint_config_path=config_path)
+
+    assert changelog._yamllint_config is config
+    assert changelog._yamllint_config_path == config_path
+    assert changelog.yamllint_config is config
+    assert changelog.__dict__["yamllint_config"] is config
+
+
+@pytest.mark.parametrize("exists", [True, False])
+@pytest.mark.parametrize("explicit_path", [True, False])
+def test_changeschecker_yamllint_config(
+        tmp_path, monkeypatch, exists, explicit_path):
+    monkeypatch.chdir(tmp_path)
+    config_path = tmp_path / ".yamllint"
+    if exists:
+        config_path.write_text(
+            "extends: default\nrules:\n  line-length: {max: 140}\n")
+    changelog = DummyChangelogChangesChecker(
+        "SECTIONS",
+        yamllint_config_path=config_path if explicit_path else None)
+
+    config = changelog.yamllint_config
+
+    assert isinstance(config, YamlLintConfig)
+    assert config.rules["line-length"]["max"] == (140 if exists else 80)
+    assert changelog.__dict__["yamllint_config"] is config
+    assert changelog.yamllint_config is config
+
+
 def test_changeschecker_max_version_for_changes_section(patches):
     changelog = DummyChangelogChangesChecker("SECTIONS")
     patched = patches(
@@ -1031,31 +1072,87 @@ def test_changeschecker_check_entry_content(content, expected):
         assert substring in result
 
 
+@pytest.mark.parametrize(
+    "content,expected",
+    [(f"{'a' * 134} b", ()),
+     ("a" * 137, ()),
+     (f"{'a' * 135} b",
+      ("entry.rst:1: line too long (141 > 140 characters)", )),
+     (f"valid\n{'a' * 135} b\n{'a' * 136} c",
+      ("entry.rst:2: line too long (141 > 140 characters)",
+       "entry.rst:3: line too long (142 > 140 characters)")),
+     (":ref:`envoy_v3_api_field_extensions.bootstrap.reverse_tunnel."
+      "downstream_socket_interface.v3.DownstreamReverseConnection"
+      "SocketInterface.maintain_interval`", ()),
+     (f"key: {'a' * 137}", ()),
+     ("The downstream reverse-tunnel initiator\n"
+      "(``envoy.bootstrap.reverse_tunnel.downstream_socket_interface``) "
+      "now accepts\n"
+      ":ref:`maintain_interval "
+      "<envoy_v3_api_field_extensions.bootstrap.reverse_tunnel."
+      "downstream_socket_interface.v3.DownstreamReverseConnection"
+      "SocketInterface.maintain_interval>`\n"
+      "to control how often each host is re-checked and missing tunnels "
+      "are dialed.\n",
+      ("entry.rst:3: line too long (177 > 140 characters)", )),
+     ("valid  \n", ()),
+     ("", ())])
+def test_changeschecker_check_entry_line_lengths(
+        tmp_path, content, expected):
+    changelog = DummyChangelogChangesChecker(
+        "SECTIONS",
+        yamllint_config=YamlLintConfig(
+            "extends: default\n"
+            "rules:\n"
+            "  line-length:\n"
+            "    max: 140\n"
+            "    allow-non-breakable-words: true\n"
+            "    allow-non-breakable-inline-mappings: true\n"))
+    path = tmp_path / "entry.rst"
+    path.write_text(content)
+
+    assert (
+        changelog.check_entry_line_lengths(path)
+        == tuple(f"{tmp_path}/{error}" for error in expected))
+
+
 def test_changeschecker_check_entry_files(patches):
     changelog = DummyChangelogChangesChecker("SECTIONS")
     patched = patches(
         "AChangelogChangesChecker.check_entry_content",
         "AChangelogChangesChecker.check_entry_filename",
+        "AChangelogChangesChecker.check_entry_line_lengths",
         prefix="envoy.code.check.abstract.changelog")
     paths = [MagicMock(), MagicMock(), MagicMock()]
-    # path 0: filename error + content error
+    # path 0: filename error + content error + line length error
     # path 1: no errors
-    # path 2: content error only
+    # path 2: content error + line length errors
     filename_returns = ["FILENAME_ERR", None, None]
     content_returns = ["CONTENT_ERR", None, "CONTENT_ERR2"]
+    line_length_returns = [
+        ("LINE_LENGTH_ERR", ),
+        (),
+        ("LINE_LENGTH_ERR2", "LINE_LENGTH_ERR3")]
 
-    with patched as (m_content, m_filename):
+    with patched as (m_content, m_filename, m_line_lengths):
         m_filename.side_effect = filename_returns
         m_content.side_effect = content_returns
+        m_line_lengths.side_effect = line_length_returns
         result = changelog.check_entry_files(paths)
 
-    assert result == ("FILENAME_ERR", "CONTENT_ERR", "CONTENT_ERR2")
+    assert (
+        result
+        == ("FILENAME_ERR", "CONTENT_ERR", "LINE_LENGTH_ERR",
+            "CONTENT_ERR2", "LINE_LENGTH_ERR2", "LINE_LENGTH_ERR3"))
     assert (
         m_filename.call_args_list
         == [[(p, ), {}] for p in paths])
     assert (
         m_content.call_args_list
         == [[(p, ), {}] for p in paths])
+    assert (
+        m_line_lengths.call_args_list
+        == [[(p, ), {}] for p in paths])
 
 
 @pytest.mark.parametrize("is_current", [True, False])
@@ -1066,11 +1163,9 @@ def test_changelogstatus_entry_dir(patches, is_current):
          dict(new_callable=PropertyMock)),
         ("AChangelogStatus.project",
          dict(new_callable=PropertyMock)),
-        ("AChangelogStatus.version",
-         dict(new_callable=PropertyMock)),
         prefix="envoy.code.check.abstract.changelog")
 
-    with patched as (m_current, m_project, m_version):
+    with patched as (m_current, m_project):
         m_current.return_value = is_current
         m_project.return_value.changelogs.current_dir_path = (
             MagicMock(name="CURRENT_DIR_PATH"))
@@ -1079,7 +1174,6 @@ def test_changelogstatus_entry_dir(patches, is_current):
     if not is_current:
         assert result is None
         assert not m_project.called
-        assert not m_version.called
         return
     assert result == m_project.return_value.changelogs.current_dir_path
     assert not m_project.return_value.changelogs.changelog_path.called
@@ -1088,7 +1182,6 @@ def test_changelogstatus_entry_dir(patches, is_current):
                  .changelog_path.return_value
                  .with_suffix.called)
     assert "entry_dir" not in status.__dict__
-    assert not m_version.called
 
 
 @pytest.mark.parametrize("entry_dir_exists", [None, False, True])