diff --git a/README.md b/README.md index e5e1b74..c77a990 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,14 @@ table header and merged-cell risks, explicitly low-contrast text, and floating objects or text boxes that disturb reading order. These run by default; use `--no-docx-accessibility` to skip them. +DOCX files are also checked for embedded comments and tracked-change markup. +These are errors by default because drafting material can leak into published +documents or change their output. Accept or reject all changes and remove all +comments before committing a template. Use `--no-docx-review-markup` to disable +only this rule while keeping the accessibility checks, or suppress `EG130` with +the standard `--suppress` option. The existing `--no-docx-accessibility` master +switch disables all DOCX checks, including this one. + **Every finding is capped at warning severity by default**, so turning these checks on reports problems without failing the build. Most existing templates have findings today, and the intent is for authors to work through diff --git a/src/dayamlchecker/docx_accessibility.py b/src/dayamlchecker/docx_accessibility.py index d11b74a..fca89f0 100644 --- a/src/dayamlchecker/docx_accessibility.py +++ b/src/dayamlchecker/docx_accessibility.py @@ -72,6 +72,51 @@ # a caption fragment) that having no heading styles is not a real finding. _HEADING_REQUIRED_TEXT_LENGTH = 1500 +_WORDPROCESSINGML_NAMESPACES = { + "http://schemas.openxmlformats.org/wordprocessingml/2006/main", + "http://purl.oclc.org/ooxml/wordprocessingml/main", +} +_COMMENT_MARKUP_NAMES = { + "comment", + "commentRangeStart", + "commentRangeEnd", + "commentReference", +} +_TRACKED_CHANGE_NAMES = { + "ins", + "del", + "delText", + "delInstrText", + "moveFrom", + "moveTo", + "moveFromRangeStart", + "moveFromRangeEnd", + "moveToRangeStart", + "moveToRangeEnd", + "customXmlInsRangeStart", + "customXmlInsRangeEnd", + "customXmlDelRangeStart", + "customXmlDelRangeEnd", + "customXmlMoveFromRangeStart", + "customXmlMoveFromRangeEnd", + "customXmlMoveToRangeStart", + "customXmlMoveToRangeEnd", + "conflictIns", + "conflictDel", + "numberingChange", + "cellIns", + "cellDel", + "cellMerge", + "pPrChange", + "rPrChange", + "sectPrChange", + "tblPrChange", + "tblPrExChange", + "tblGridChange", + "trPrChange", + "tcPrChange", +} + _SEVERITY_RANK = {Severity.INFO: 0, Severity.WARNING: 1, Severity.ERROR: 2} @@ -161,6 +206,60 @@ def check_docx_accessibility( ] +def check_docx_review_markup(path: str | Path) -> list[Finding]: + """Return an error when a DOCX still contains comments or revisions. + + This is intentionally independent from the accessibility severity ceiling: + unresolved review markup can expose drafting material or produce unintended + output, so it is a default-on release-safety check. + """ + path = Path(path) + comment_parts: set[str] = set() + revision_parts: set[str] = set() + + try: + with zipfile.ZipFile(path) as package: + for part_name in package.namelist(): + if not part_name.startswith("word/") or not part_name.endswith(".xml"): + continue + try: + root = ET.fromstring(package.read(part_name)) + except (ET.ParseError, KeyError): + continue + for element in root.iter(): + if _namespace(element.tag) not in _WORDPROCESSINGML_NAMESPACES: + continue + local_name = _local_name(element.tag) + if local_name in _COMMENT_MARKUP_NAMES: + comment_parts.add(part_name) + if local_name in _TRACKED_CHANGE_NAMES: + revision_parts.add(part_name) + except (OSError, zipfile.BadZipFile): + # The accessibility package check owns the existing unreadable-DOCX + # diagnostic. Avoid reporting a misleading review-markup error too. + return [] + + markup = [] + if comment_parts: + markup.append("embedded comments") + if revision_parts: + markup.append("tracked changes") + if not markup: + return [] + + parts = sorted(comment_parts | revision_parts) + return [ + Finding( + message_id=MessageId.DOCX_REVIEW_MARKUP, + file_name=str(path), + context={ + "markup": " and ".join(markup), + "parts": ", ".join(parts), + }, + ) + ] + + def collect_docx_files( paths: list[Path], include_default_ignores: bool = True ) -> list[Path]: @@ -1110,6 +1209,12 @@ def _local_name(tag: str) -> str: return tag +def _namespace(tag: str) -> str: + if tag.startswith("{") and "}" in tag: + return tag[1:].split("}", 1)[0] + return "" + + def _attr(element: Optional[ET.Element], local_name: str) -> str: if element is None: return "" diff --git a/src/dayamlchecker/messages.py b/src/dayamlchecker/messages.py index 4e0d33a..7230652 100644 --- a/src/dayamlchecker/messages.py +++ b/src/dayamlchecker/messages.py @@ -80,6 +80,7 @@ class MessageId(StrEnum): MULTIPLE_MANDATORY_BLOCKS = "multiple_mandatory_blocks" MISSING_METADATA_FIELDS = "missing_metadata_fields" ATTACHMENT_CONDITIONAL_VARIABLE = "attachment_conditional_variable" + DOCX_REVIEW_MARKUP = "docx_review_markup" ACCESSIBILITY_COMBOBOX_NOT_ACCESSIBLE = "accessibility_combobox_not_accessible" ACCESSIBILITY_NO_LABEL_MULTI_FIELD = "accessibility_no_label_multi_field" @@ -965,6 +966,16 @@ class MessageDefinition: "displayed content" ), ), + MessageId.DOCX_REVIEW_MARKUP: MessageDefinition( + code="EG130", + severity=Severity.ERROR, + finding_class=FindingClass.GENERAL, + summary="DOCX contains review markup", + template=( + "the document contains {markup}; accept or reject tracked changes and " + "remove comments before publishing (found in {parts})" + ), + ), # DOCX template accessibility MessageId.ACCESSIBILITY_DOCX_DOCX_UNREADABLE: MessageDefinition( code="EA540", diff --git a/src/dayamlchecker/yaml_structure.py b/src/dayamlchecker/yaml_structure.py index df819e2..6210576 100644 --- a/src/dayamlchecker/yaml_structure.py +++ b/src/dayamlchecker/yaml_structure.py @@ -46,6 +46,7 @@ from dayamlchecker.docx_accessibility import ( DocxAccessibilityOptions, check_docx_accessibility, + check_docx_review_markup, collect_docx_files, ) @@ -2842,6 +2843,15 @@ def main(argv: Optional[list[str]] = None) -> int: "error to fail on documents with accessibility errors" ), ) + parser.add_argument( + "--docx-review-markup", + action=argparse.BooleanOptionalAction, + default=True, + help=( + "Fail when DOCX templates contain comments or tracked changes " + "(default: on)" + ), + ) parser.add_argument( "--format", choices=("text", "github"), @@ -2907,6 +2917,8 @@ def main(argv: Optional[list[str]] = None) -> int: docx_options = runtime_options.docx_accessibility_options() for docx_file in docx_files: all_findings.extend(check_docx_accessibility(docx_file, docx_options)) + if args.docx_review_markup: + all_findings.extend(check_docx_review_markup(docx_file)) if args.url_check and yaml_files: url_check_root = ( diff --git a/tests/test_docx_accessibility.py b/tests/test_docx_accessibility.py index 6e2bb82..ff4f146 100644 --- a/tests/test_docx_accessibility.py +++ b/tests/test_docx_accessibility.py @@ -7,6 +7,7 @@ from dayamlchecker.docx_accessibility import ( DocxAccessibilityOptions, check_docx_accessibility, + check_docx_review_markup, ) from dayamlchecker.messages import Severity from dayamlchecker.yaml_structure import main @@ -259,6 +260,71 @@ def test_unreadable_docx_reports_a_package_finding(): assert strict[0].severity == ERROR +# --------------------------------------------------------------------------- +# Comments and tracked changes +# --------------------------------------------------------------------------- + + +def test_docx_comments_are_errors_by_default(): + with TemporaryDirectory() as tmp: + path = _build( + tmp, + "commented", + 'Draft' + '' + "", + ) + with zipfile.ZipFile(path, "a") as package: + package.writestr( + "word/comments.xml", + '' + '' + "Do not publish this note" + "", + ) + + findings = check_docx_review_markup(path) + + assert len(findings) == 1 + assert findings[0].code == "EG130" + assert findings[0].severity == ERROR + assert findings[0].finding_class == "general" + assert "embedded comments" in findings[0].message + assert "word/comments.xml" in findings[0].message + + +def test_docx_tracked_changes_are_errors_by_default(): + with TemporaryDirectory() as tmp: + path = _build( + tmp, + "revisions", + 'Old' + 'New', + ) + + findings = check_docx_review_markup(path) + + assert len(findings) == 1 + assert findings[0].code == "EG130" + assert "tracked changes" in findings[0].message + assert "word/document.xml" in findings[0].message + + +def test_track_changes_setting_without_revision_markup_is_allowed(): + with TemporaryDirectory() as tmp: + path = _build(tmp, "tracking-on", f"{PROSE}") + with zipfile.ZipFile(path, "a") as package: + package.writestr( + "word/settings.xml", + '' + '', + ) + + assert check_docx_review_markup(path) == [] + + # --------------------------------------------------------------------------- # Regressions # --------------------------------------------------------------------------- @@ -553,7 +619,7 @@ def test_cli_checks_docx_by_default_without_failing(): assert "WA541" in output, "image-alt-missing, demoted to a warning" -def test_cli_can_skip_docx_checks(): +def test_cli_can_skip_all_docx_checks(): with TemporaryDirectory() as tmp: path = Path(tmp) / "inaccessible.docx" _write_docx(path, _base_files(_inaccessible_document_xml())) @@ -562,7 +628,57 @@ def test_cli_can_skip_docx_checks(): ["--no-docx-accessibility", "--no-url-check", str(path)] ) - assert exit_code == 1, "nothing left to check" + assert exit_code == 1, "nothing is enabled to check the DOCX" + + +def test_cli_review_markup_fails_independently_of_accessibility_severity(): + with TemporaryDirectory() as tmp: + path = _build( + tmp, + "revisions", + 'New', + ) + + exit_code, output = _run_cli(["--no-url-check", str(path)]) + + assert exit_code == 1 + assert "EG130" in output + + +def test_cli_can_disable_docx_review_markup_rule(): + with TemporaryDirectory() as tmp: + path = _build( + tmp, + "revisions", + 'New', + ) + + exit_code, output = _run_cli( + [ + "--no-docx-review-markup", + "--no-url-check", + str(path), + ] + ) + + assert exit_code == 0, "warning-level accessibility findings do not fail" + assert "EG130" not in output + + +def test_cli_can_suppress_docx_review_markup_by_code(): + with TemporaryDirectory() as tmp: + path = _build( + tmp, + "revisions", + 'New', + ) + + exit_code, output = _run_cli( + ["--suppress", "EG130", "--no-url-check", str(path)] + ) + + assert exit_code == 0 + assert "EG130" not in output def test_cli_error_severity_fails_the_command():