From 32e896fd0917250f661de3efe6571ba9e2e594a9 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sun, 18 Oct 2026 23:30:58 +0200 Subject: [PATCH] fix(release): lint the changelog entry inside the file it lands in The entry was linted alone, so rules that compare it with the rest of CHANGELOG.md never fired. infinito-nexus 16.0.0 passed this check with "-" bullets on top of older "*" entries, and the repository's own markdown lint then reported 400 MD004 findings and refused the push. update_changelog now also lints the merged file and reports only the findings the existing file does not already produce (lint_changelog_context), so pre-existing issues are not blamed on the entry. markdownlint-cli2 runs with --no-globs, so a target config with its own globs no longer lints the whole repository and blocks a release on unrelated files. Without markdownlint-cli2 the built-in check now requires "-" list markers (MD004 dash), the strictest style. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../actions/release/files/changelog_lint.py | 36 +++++++++++++++++- .../actions/release/files/changelog_md.py | 26 ++++++++----- .../actions/release/test_changelog_lint.py | 37 ++++++++++++++++++- .../unit/pkgmgr/actions/release/test_files.py | 8 ++-- 4 files changed, 90 insertions(+), 17 deletions(-) diff --git a/src/pkgmgr/actions/release/files/changelog_lint.py b/src/pkgmgr/actions/release/files/changelog_lint.py index b9226a9..0818951 100644 --- a/src/pkgmgr/actions/release/files/changelog_lint.py +++ b/src/pkgmgr/actions/release/files/changelog_lint.py @@ -6,10 +6,13 @@ import re import shutil import subprocess import tempfile +from collections import Counter _HEADING = re.compile(r"^\s*(#{1,6})\s+(.*?)\s*#*\s*$") _INLINE_CODE = re.compile(r"`([^`\n]+)`") _FINDING = re.compile(r"\bMD\d{3}\b") +_LOCATION = re.compile(r"^.*?:\d+(?::\d+)?\s+") +_NON_DASH_ITEM = re.compile(r"^\s*[*+]\s") _MULTI_BLANK = re.compile(r"\n{3,}") _ENTRY_LEVEL = 3 @@ -59,6 +62,35 @@ def lint_changelog_entry(reference_path: str, entry: str) -> list[str]: return _builtin_lint(document) +def lint_changelog_context(reference_path: str, existing: str, merged: str) -> list[str]: + """Return the findings *merged* has beyond those *existing* already has. + + Args: + reference_path: path of the target CHANGELOG.md; its directory supplies + the markdownlint config. + existing: the target file before the entry is inserted. + merged: the target file with the entry inserted. + + Returns: + Findings of *merged* that *existing* does not produce. Empty when + ``markdownlint-cli2`` is unavailable. + """ + if not shutil.which("markdownlint-cli2"): + return [] + known = Counter( + _LOCATION.sub("", finding) + for finding in (_markdownlint(reference_path, existing) if existing.strip() else []) + ) + new: list[str] = [] + for finding in _markdownlint(reference_path, merged): + key = _LOCATION.sub("", finding) + if known[key]: + known[key] -= 1 + else: + new.append(finding) + return new + + def _markdownlint(reference_path: str, document: str) -> list[str]: directory = os.path.dirname(os.path.abspath(reference_path)) or "." fd, tmp_path = tempfile.mkstemp( @@ -69,7 +101,7 @@ def _markdownlint(reference_path: str, document: str) -> list[str]: with os.fdopen(fd, "w", encoding="utf-8") as handle: handle.write(document) proc = subprocess.run( - ["markdownlint-cli2", name], + ["markdownlint-cli2", "--no-globs", name], cwd=directory, capture_output=True, text=True, @@ -96,6 +128,8 @@ def _builtin_lint(document: str) -> list[str]: errors.append(f"line {number}: trailing whitespace (MD009)") if "`" in line: errors.append(f"line {number}: backtick is not allowed (use *italic*)") + if _NON_DASH_ITEM.match(line): + errors.append(f"line {number}: list items use '-' (MD004)") if "\n\n\n" in document: errors.append("multiple consecutive blank lines (MD012)") return errors diff --git a/src/pkgmgr/actions/release/files/changelog_md.py b/src/pkgmgr/actions/release/files/changelog_md.py index 2243176..2922f93 100644 --- a/src/pkgmgr/actions/release/files/changelog_md.py +++ b/src/pkgmgr/actions/release/files/changelog_md.py @@ -7,6 +7,7 @@ from datetime import date from .changelog_lint import ( ChangelogLintError, + lint_changelog_context, lint_changelog_entry, transform_changelog_message, ) @@ -86,9 +87,22 @@ def update_changelog( except EOFError: print("[INFO] Re-opening the editor so you can fix the entry...") + changelog = "" + if os.path.exists(changelog_path): + try: + with open(changelog_path, encoding="utf-8") as f: + changelog = f.read() + except (OSError, UnicodeDecodeError) as exc: + print(f"[WARN] Could not read existing CHANGELOG.md: {exc}") + + def _findings_for(entry: str) -> list[str]: + return lint_changelog_entry(changelog_path, entry) or lint_changelog_context( + changelog_path, changelog, _insert_after_h1(changelog, entry) + ) + if message is not None: body, entry = _entry_for(message) - findings = lint_changelog_entry(changelog_path, entry) + findings = _findings_for(entry) if findings: _print_findings(findings) raise ChangelogLintError( @@ -106,7 +120,7 @@ def update_changelog( ) raw = _open_editor_for_changelog(attempt, rejected) body, entry = _entry_for(raw or f"Release {new_version}") - findings = lint_changelog_entry(changelog_path, entry) + findings = _findings_for(entry) if not findings: break _print_findings(findings) @@ -114,14 +128,6 @@ def update_changelog( rejected = findings _wait_for_reader() - changelog = "" - if os.path.exists(changelog_path): - try: - with open(changelog_path, encoding="utf-8") as f: - changelog = f.read() - except (OSError, UnicodeDecodeError) as exc: - print(f"[WARN] Could not read existing CHANGELOG.md: {exc}") - new_changelog = _insert_after_h1(changelog, entry) print("\n================ CHANGELOG ENTRY ================") diff --git a/tests/unit/pkgmgr/actions/release/test_changelog_lint.py b/tests/unit/pkgmgr/actions/release/test_changelog_lint.py index 109df0e..55f3ab2 100644 --- a/tests/unit/pkgmgr/actions/release/test_changelog_lint.py +++ b/tests/unit/pkgmgr/actions/release/test_changelog_lint.py @@ -1,11 +1,18 @@ from __future__ import annotations +import shutil import unittest +from unittest.mock import patch +from pkgmgr.actions.release.files import changelog_lint from pkgmgr.actions.release.files.changelog_lint import ( + lint_changelog_context, lint_changelog_entry, transform_changelog_message, ) +from pkgmgr.actions.release.files.changelog_md import _insert_after_h1 + +_OLD_ASTERISK_FILE = "# Changelog\n\n## [1.0.0] - 2026-01-01\n\n* old bullet\n" class TestTransformChangelogMessage(unittest.TestCase): @@ -38,16 +45,42 @@ class TestTransformChangelogMessage(unittest.TestCase): class TestLintChangelogEntry(unittest.TestCase): def test_clean_entry_has_no_findings(self) -> None: - entry = "## [1.0.0] - 2026-01-01\n\n* a clean bullet\n\n" + entry = "## [1.0.0] - 2026-01-01\n\n- a clean bullet\n\n" self.assertEqual(lint_changelog_entry("CHANGELOG.md", entry), []) def test_a_structured_message_survives_its_own_transform(self) -> None: body = transform_changelog_message( - "# Security\n\n* a bullet\n\n## Fixed\n\n* another bullet" + "# Security\n\n- a bullet\n\n## Fixed\n\n- another bullet" ) entry = f"## [1.0.0] - 2026-01-01\n\n{body}\n\n" self.assertEqual(lint_changelog_entry("CHANGELOG.md", entry), []) + def test_the_builtin_check_refuses_a_non_dash_list_marker(self) -> None: + entry = "## [1.0.0] - 2026-01-01\n\n* a bullet\n\n" + with patch.object(changelog_lint.shutil, "which", return_value=None): + findings = lint_changelog_entry("CHANGELOG.md", entry) + self.assertTrue(any("MD004" in finding for finding in findings), findings) + + +@unittest.skipUnless(shutil.which("markdownlint-cli2"), "markdownlint-cli2 is not installed") +class TestLintChangelogContext(unittest.TestCase): + def test_a_marker_that_breaks_the_files_consistency_is_reported(self) -> None: + entry = "## [1.1.0] - 2026-02-01\n\n- new bullet\n\n" + merged = _insert_after_h1(_OLD_ASTERISK_FILE, entry) + findings = lint_changelog_context("CHANGELOG.md", _OLD_ASTERISK_FILE, merged) + self.assertTrue(any("MD004" in finding for finding in findings), findings) + + def test_a_finding_the_file_already_has_is_not_blamed_on_the_entry(self) -> None: + existing = "# Changelog\n\n## [1.0.0] - 2026-01-01\n\n* old bullet \n" + entry = "## [1.1.0] - 2026-02-01\n\n* new bullet\n\n" + merged = _insert_after_h1(existing, entry) + self.assertEqual(lint_changelog_context("CHANGELOG.md", existing, merged), []) + + def test_a_consistent_entry_has_no_findings(self) -> None: + entry = "## [1.1.0] - 2026-02-01\n\n* new bullet\n\n" + merged = _insert_after_h1(_OLD_ASTERISK_FILE, entry) + self.assertEqual(lint_changelog_context("CHANGELOG.md", _OLD_ASTERISK_FILE, merged), []) + if __name__ == "__main__": # pragma: no cover unittest.main() diff --git a/tests/unit/pkgmgr/actions/release/test_files.py b/tests/unit/pkgmgr/actions/release/test_files.py index e65662a..cd9dd0d 100644 --- a/tests/unit/pkgmgr/actions/release/test_files.py +++ b/tests/unit/pkgmgr/actions/release/test_files.py @@ -313,13 +313,13 @@ class TestUpdateChangelog(unittest.TestCase): def test_update_changelog_does_not_inject_a_leading_bullet(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: path = os.path.join(tmpdir, "CHANGELOG.md") - update_changelog(path, "1.2.3", message="* Provided bullet", preview=False) + update_changelog(path, "1.2.3", message="- Provided bullet", preview=False) with open(path, encoding="utf-8") as f: content = f.read() - self.assertIn("\n\n* Provided bullet\n", content) - self.assertNotIn("* * Provided bullet", content) + self.assertIn("\n\n- Provided bullet\n", content) + self.assertNotIn("- - Provided bullet", content) def test_update_changelog_transforms_heading_and_inline_code(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: @@ -327,7 +327,7 @@ class TestUpdateChangelog(unittest.TestCase): update_changelog( path, "1.2.3", - message="# Summary\n\n* uses `foo` tool", + message="# Summary\n\n- uses `foo` tool", preview=False, )