Report line number and source references for forbidden HTML tags

src/checkmk_weblate_syncer/html_tags.py

@@ -1,4 +1,5 @@
 import re
+from dataclasses import dataclass
 
 # keep in sync with tests/pylint/checker_localization.py:HTMLTagsChecker
 _TAG_PATTERN = re.compile("<.*?>")
@@ -7,12 +8,18 @@
 )
 
 
-def forbidden_tags(text: str) -> set[str]:
-    return {
-        tag
-        for tag in re.findall(
-            _TAG_PATTERN,
-            text,
+@dataclass(frozen=True)
+class ForbiddenTag:
+    line: int
+    tag: str
+
+
+def forbidden_tags(text: str) -> frozenset[ForbiddenTag]:
+    return frozenset(
+        ForbiddenTag(
+            line=text.count("\n", 0, match.start()) + 1,
+            tag=match.group(),
         )
-        if not re.match(_ALLOWED_TAGS_PATTERN, tag)
-    }
+        for match in _TAG_PATTERN.finditer(text)
+        if not _ALLOWED_TAGS_PATTERN.match(match.group())
+    )

src/checkmk_weblate_syncer/portable_object.py

@@ -1,18 +1,53 @@
 import re
+from collections.abc import Iterable
 from pathlib import Path
 
+from .html_tags import ForbiddenTag
 
-def remove_header(portable_object_content: str) -> str:
-    pattern = re.compile(r"^#: .*?:\d+$")
+_SOURCE_REFERENCE_PATTERN = re.compile(r"^#: (.+?:\d+)$")
+
+
+def remove_header(portable_object_content: str) -> tuple[str, int]:
     lines = portable_object_content.splitlines()
     index_first_source_string_location = 0
     for index, line in enumerate(lines):
-        if re.match(pattern, line):
+        if _SOURCE_REFERENCE_PATTERN.match(line):
             index_first_source_string_location = index
             break
-    return "\n".join(lines[index_first_source_string_location:]) + (
+    body = "\n".join(lines[index_first_source_string_location:]) + (
         "\n" if portable_object_content.endswith("\n") else ""
     )
+    return body, index_first_source_string_location
+
+
+def source_references_at(content: str, line: int) -> tuple[str, ...]:
+    lines = content.splitlines()
+    if not 1 <= line <= len(lines):
+        return ()
+    block_start = 0
+    for i in range(line - 1, -1, -1):
+        if lines[i] == "":
+            block_start = i + 1
+            break
+    return tuple(
+        match.group(1)
+        for line_text in lines[block_start:line]
+        if (match := _SOURCE_REFERENCE_PATTERN.match(line_text))
+    )
+
+
+def format_forbidden_tags_error(
+    portable_object_content: str,
+    header_line_count: int,
+    tags: Iterable[ForbiddenTag],
+) -> str:
+    formatted: list[str] = []
+    for tag in sorted(tags, key=lambda t: (t.line, t.tag)):
+        absolute_line = header_line_count + tag.line
+        references = source_references_at(portable_object_content, absolute_line)
+        suffix = f" ({', '.join(references)})" if references else ""
+        formatted.append(f"  line {absolute_line}: {tag.tag!r}{suffix}")
+    return "Found forbidden HTML tags:\n" + "\n".join(formatted)
 
 
 def make_soure_string_locations_relative(

src/checkmk_weblate_syncer/update_sources.py

@@ -5,7 +5,11 @@
 from .git import commit_and_push_files, repository_in_clean_state
 from .html_tags import forbidden_tags
 from .logger import LOGGER
-from .portable_object import make_soure_string_locations_relative, remove_header
+from .portable_object import (
+    format_forbidden_tags_error,
+    make_soure_string_locations_relative,
+    remove_header,
+)
 
 
 def run(config: UpdateSourcesConfig) -> int:
@@ -32,11 +36,15 @@ def run(config: UpdateSourcesConfig) -> int:
         raise
 
     LOGGER.info("Checking HTML tags")
-    if forbidden_html_tags := forbidden_tags(remove_header(pot_file_content)):
-        error_msg = (
-            f"Found forbidden HTML tags: {', '.join(sorted(forbidden_html_tags))}"
+    body, header_line_count = remove_header(pot_file_content)
+    if forbidden_html_tags := forbidden_tags(body):
+        raise ValueError(
+            format_forbidden_tags_error(
+                pot_file_content,
+                header_line_count,
+                forbidden_html_tags,
+            ),
         )
-        raise ValueError(error_msg)
 
     LOGGER.info("Making source string locations relative")
     pot_file_content = make_soure_string_locations_relative(

src/checkmk_weblate_syncer/update_translations.py

@@ -11,6 +11,7 @@
 from .html_tags import forbidden_tags
 from .logger import LOGGER
 from .portable_object import (
+    format_forbidden_tags_error,
     remove_header,
     remove_last_translator,
     remove_source_string_locations,
@@ -116,9 +117,14 @@ def _process_po_file_pair(
         )
 
     LOGGER.info("Checking HTML tags")
-    if forbidden_html_tags := forbidden_tags(remove_header(po_file_content)):
+    body, header_line_count = remove_header(po_file_content)
+    if forbidden_html_tags := forbidden_tags(body):
         return _Failure(
-            error_message=f"Found forbidden HTML tags: {', '.join(sorted(forbidden_html_tags))}",
+            error_message=format_forbidden_tags_error(
+                po_file_content,
+                header_line_count,
+                forbidden_html_tags,
+            ),
             path=locale_po_file,
         )
 

tests/test_html_tags.py

@@ -1,6 +1,6 @@
 import pytest
 
-from checkmk_weblate_syncer.html_tags import forbidden_tags
+from checkmk_weblate_syncer.html_tags import ForbiddenTag, forbidden_tags
 
 
 @pytest.mark.parametrize(
@@ -25,9 +25,25 @@
         pytest.param(
             "123 <script>injection</script>",
             frozenset(
-                ["<script>", "</script>"],
+                {
+                    ForbiddenTag(line=1, tag="<script>"),
+                    ForbiddenTag(line=1, tag="</script>"),
+                },
             ),
         ),
+        pytest.param(
+            "ok line\nbad <foo> line\nanother bad <bar> line",
+            frozenset(
+                {
+                    ForbiddenTag(line=2, tag="<foo>"),
+                    ForbiddenTag(line=3, tag="<bar>"),
+                },
+            ),
+        ),
+        pytest.param(
+            '"<br><b>Hinweis</>: trailing"',
+            frozenset({ForbiddenTag(line=1, tag="</>")}),
+        ),
         pytest.param(
             # pylint: disable=line-too-long
             """#: /home/weblate/checkmk_weblate_sync/git/checkmk/cmk/gui/wato/pages/host_rename.py:640
@@ -54,6 +70,6 @@
 )
 def test_html_tags_checker(
     text: str,
-    expected_result: frozenset[str],
+    expected_result: frozenset[ForbiddenTag],
 ) -> None:
     assert forbidden_tags(text) == expected_result

tests/test_portable_object.py

@@ -1,17 +1,19 @@
 from pathlib import Path
 
+from checkmk_weblate_syncer.html_tags import ForbiddenTag
 from checkmk_weblate_syncer.portable_object import (
+    format_forbidden_tags_error,
     make_soure_string_locations_relative,
     remove_header,
     remove_last_translator,
     remove_source_string_locations,
+    source_references_at,
 )
 
 
 def test_remove_header() -> None:
-    assert (
-        remove_header(
-            """# Copyright (C) 2019 Checkmk GmbH - License: GNU General Public License v2
+    body, header_line_count = remove_header(
+        """# Copyright (C) 2019 Checkmk GmbH - License: GNU General Public License v2
 # This file is part of Checkmk (https://checkmk.com). It is subject to the terms and
 # conditions defined in the file COPYING, which is part of this source code package.
 
@@ -46,8 +48,10 @@ def test_remove_header() -> None:
 msgid " (Duration: %s)"
 msgstr ""
 """,
-        )
-        # pylint: disable=line-too-long
+    )
+    # pylint: disable=line-too-long
+    assert (
+        body
         == """#: /home/weblate/checkmk_weblate_sync/git/checkmk/cmk/gui/wato/pages/host_rename.py:640
 #, python-format
 msgid " (%d times)"
@@ -67,6 +71,56 @@ def test_remove_header() -> None:
 msgstr ""
 """
     )
+    expected_header_line_count = 27
+    assert header_line_count == expected_header_line_count
+
+
+def test_source_references_at() -> None:
+    content = """#: cmk/gui/foo.py:10
+#: cmk/gui/bar.py:20
+msgid "hello"
+msgstr "hallo"
+
+#: cmk/gui/baz.py:30
+msgid "world"
+msgstr "welt"
+"""
+    # msgstr "hallo" is on line 4 → both refs from the first block
+    assert source_references_at(content, 4) == (
+        "cmk/gui/foo.py:10",
+        "cmk/gui/bar.py:20",
+    )
+    # msgstr "welt" is on line 8 → only the ref from the second block
+    assert source_references_at(content, 8) == ("cmk/gui/baz.py:30",)
+    # out-of-range line returns empty
+    assert source_references_at(content, 999) == ()
+
+
+def test_format_forbidden_tags_error() -> None:
+    content = """# header line
+# header line
+
+#: cmk/gui/foo.py:10
+#: cmk/gui/bar.py:20
+msgid "hello"
+msgstr "<bad>oops</bad>"
+"""
+    # body starts at file line 4 (1-indexed); header_line_count = 3
+    error = format_forbidden_tags_error(
+        content,
+        header_line_count=3,
+        tags=frozenset(
+            {
+                ForbiddenTag(line=4, tag="<bad>"),
+                ForbiddenTag(line=4, tag="</bad>"),
+            },
+        ),
+    )
+    assert error == (
+        "Found forbidden HTML tags:\n"
+        "  line 7: '</bad>' (cmk/gui/foo.py:10, cmk/gui/bar.py:20)\n"
+        "  line 7: '<bad>' (cmk/gui/foo.py:10, cmk/gui/bar.py:20)"
+    )
 
 
 def test_make_soure_string_locations_relative() -> None: