From f40bd151e5be5541d4cf04a787a08c4d571374d4 Mon Sep 17 00:00:00 2001 From: "Moises Lopez - https://www.vauxoo.com/" Date: Tue, 28 Apr 2026 20:19:32 -0600 Subject: [PATCH] [FIX] xml-double-quotes-py: fix tag detection and report exact attribute line Previously, `NodeContent._read_node` failed to identify the start of an XML tag when it appeared after other content on the same line (e.g. `
  • text{} - node_content = node_xml.NodeContent(manifest_data["filename"], elem) + node_content = node_xml.NodeContent( + manifest_data["filename"], elem, locator=self._get_tag_locator(manifest_data) + ) if b""" in node_content.content_node: self.register_error( code="xml-double-quotes-py", message='Escaped double quotes " for python code detected', info=f"Use single quote instead: `{new_py_code}`", filepath=manifest_data["filename_short"], - line=elem.sourceline, + line=node_content.start_sourceline or elem.sourceline, ) during2 = node_content.content_node.replace(b""", b"'") if self.autofix and during2 != node_content.content_node: @@ -702,15 +704,22 @@ def check_xml_double_quotes_py(self): continue if not (new_py_code := self.is_compatible_single_quote(attr_value)): continue - node_content = node_xml.NodeContent(manifest_data["filename"], elem) + locator = self._get_tag_locator(manifest_data) + node_content = node_xml.NodeContent(manifest_data["filename"], elem, locator=locator) if b""" not in node_content.content_node: continue + # Use the attribute's exact line from the locator (most precise) + attr_span = locator.get_attr(elem, attr_name) + if attr_span is not None: + line_no = locator.content[: attr_span.name_start].count(b"\n") + 1 + else: + line_no = node_content.start_sourceline or elem.sourceline self.register_error( code="xml-double-quotes-py", message='Escaped double quotes " for python code detected use', info=f"Use single quote instead: `{new_py_code}`", filepath=manifest_data["filename_short"], - line=elem.sourceline, + line=line_no, ) during2 = node_content.content_node.replace(b""", b"'") if self.autofix and during2 != node_content.content_node: diff --git a/src/oca_pre_commit_hooks/node_xml.py b/src/oca_pre_commit_hooks/node_xml.py index 004cd69..ec600da 100644 --- a/src/oca_pre_commit_hooks/node_xml.py +++ b/src/oca_pre_commit_hooks/node_xml.py @@ -1,4 +1,5 @@ # Based on https://github.com/mitsuhiko/sloppy-xml-py +import re from dataclasses import dataclass @@ -217,15 +218,19 @@ def rewrite_start_tag( class NodeContent: """Represents the content and metadata of an XML node.""" - def __init__(self, filename, node): + def __init__(self, filename, node, locator=None): """Initialize by reading and parsing the node from the file. Args: filename: Path to the XML file node: lxml node element to extract + locator: Optional XMLStartTagLocator already built for this file. + When provided it is used as the authoritative source for + tag start position and line, avoiding fragile heuristics. """ self.filename = filename self.node = node + self._locator = locator # Initialize attributes self.content_before = b"" @@ -240,28 +245,43 @@ def __init__(self, filename, node): def _read_node(self): # noqa:C901 pylint:disable=too-complex """Internal method to read the content of the file and extract node information.""" - # TODO: Get the sourceline of a particular attribute - # Determine the search start line - if (node_previous := self.node.getprevious()) is not None: - search_start_line = node_previous.sourceline + 1 - elif (node_parent := self.node.getparent()) is not None: - search_start_line = node_parent.sourceline + 1 - else: - search_start_line = 2 # first element and it is the root - - search_end_line = self.node.sourceline node_tag = self.node.tag.encode() if isinstance(self.node.tag, str) else self.node.tag # Read all lines from file with open(self.filename, "rb") as f_content: all_lines = list((i, line) for i, line in enumerate(f_content, start=1)) - # Find the actual node start by looking for the tag + # --- Determine tag start using the locator when available --- + # XMLStartTagLocator already did a full byte-accurate parse of the file, + # so use it as the authoritative source instead of re-scanning with heuristics. node_start_idx = None - for idx, (no_line, line) in enumerate(all_lines): - if search_start_line <= no_line <= search_end_line: - stripped_line = line.lstrip() - if stripped_line.startswith(b"<" + node_tag): + if self._locator is not None: + tag_info = self._locator.get_tag(self.node) + if tag_info is not None: + self.start_sourceline = tag_info.line + # Find the matching line index in all_lines + for idx, (no_line, _line) in enumerate(all_lines): + if no_line == tag_info.line: + node_start_idx = idx + break + + # --- Fallback: scan backwards from sourceline using a regex --- + # This handles tags that appear mid-line (e.g. "
  • text<\n\r\t]|$)") + end_idx = min(search_end_line - 1, len(all_lines) - 1) + start_idx = max(search_start_line - 1, 0) + for idx in range(end_idx, start_idx - 1, -1): + no_line, line = all_lines[idx] + if node_start_re.search(line): node_start_idx = idx self.start_sourceline = no_line break diff --git a/test_repo/test_module/model_view.xml b/test_repo/test_module/model_view.xml index 1fef119..b5179df 100644 --- a/test_repo/test_module/model_view.xml +++ b/test_repo/test_module/model_view.xml @@ -22,5 +22,23 @@ + + diff --git a/tests/test_checks.py b/tests/test_checks.py index 9b863ca..b4c5535 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -38,7 +38,7 @@ "xml-deprecated-qweb-directive-15": 4, "xml-deprecated-qweb-directive": 2, "xml-deprecated-tree-attribute": 3, - "xml-double-quotes-py": 3, + "xml-double-quotes-py": 5, "xml-duplicate-fields": 3, "xml-duplicate-record-id": 2, "xml-not-valid-char-link": 2, diff --git a/tests/test_node_xml.py b/tests/test_node_xml.py index 455e35d..268ce6a 100644 --- a/tests/test_node_xml.py +++ b/tests/test_node_xml.py @@ -93,3 +93,36 @@ def test_xml_record_id_autofixes_preserve_menuitem_layout(): xml_content = (module_dst / "model_view_odoo2.xml").read_text() assert "" in xml_content assert ' tag with t-options spanning multiple + lines and starting AFTER other text on the same line as
  • . + 2. website_templates.xml line 33: same pattern in a different file. + """ + manifest_path = str(REPO_ROOT / "test_repo/test_module/__openerp__.py") + result = checks_odoo_module.run( + [manifest_path], + enable={"xml-double-quotes-py"}, + no_exit=True, + ) + errors = [e for e in result if getattr(e, "code", None) == "xml-double-quotes-py"] + + # model_view.xml: + # The tag text on line 33, t-options is on line 35 + model_view_lines = {e.position.line for e in errors if "model_view.xml" in e.position.filepath} + assert 35 in model_view_lines, ( + f"Expected xml-double-quotes-py error on model_view.xml:35 (t-options with "), " + f"got lines: {sorted(model_view_lines)}" + ) + + # website_templates.xml: