Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions src/oca_pre_commit_hooks/checks_odoo_module_xml.py
Original file line number Diff line number Diff line change
Expand Up @@ -679,14 +679,16 @@ def check_xml_double_quotes_py(self):
and (new_py_code := self.is_compatible_single_quote(py_code))
):
# Process text <field name="context">{}</field>
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"&quot;" 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"&quot;", b"'")
if self.autofix and during2 != node_content.content_node:
Expand All @@ -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"&quot;" 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"&quot;", b"'")
if self.autofix and during2 != node_content.content_node:
Expand Down
52 changes: 36 additions & 16 deletions src/oca_pre_commit_hooks/node_xml.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# Based on https://github.com/mitsuhiko/sloppy-xml-py
import re
from dataclasses import dataclass


Expand Down Expand Up @@ -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""
Expand All @@ -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. "<li>text<strong").
if node_start_idx is None:
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_start_re = re.compile(b"<" + node_tag + b"(?:[ /><\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
Expand Down
18 changes: 18 additions & 0 deletions test_repo/test_module/model_view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,23 @@
<filter name="in_filter" string="In Filter" domain="[(&quot;state&quot;, &quot;=&quot;, &quot;done&quot;)]"/>
</record>

<template id="template_doc2">
<t t-call="base.external_layout">
<t t-set="o" t-value="o.with_context(lang=lang)" />
<div class="page o_base_execution_request_page" t-translation="off">
<ul class="bullet-list o_base_execution_request_bullet_tab" t-translation="off">
<li>Name: <strong t-field="o.name" />, date <strong
t-field="o.date_start"
/></li>
<li>Percentage <strong
t-out="o.coverage*100"
t-options="{&quot;widget&quot;: &quot;float&quot;, &quot;precision&quot;: 2}"
/>
%</li>
</ul>
</div>
</t>
</template>

</data>
</openerp>
2 changes: 1 addition & 1 deletion tests/test_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
33 changes: 33 additions & 0 deletions tests/test_node_xml.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,36 @@ def test_xml_record_id_autofixes_preserve_menuitem_layout():
xml_content = (module_dst / "model_view_odoo2.xml").read_text()
assert "<menuitem id='menu_root' name=\"Root\" />" in xml_content
assert '<menuitem id=\'menu_root2\'\n name="Root 2"\n parent="menu_root"' in xml_content


def test_xml_double_quotes_py_reports_correct_line_for_inline_and_multiline_tags():
"""Regression test: xml-double-quotes-py must report the line where the
*attribute* is declared, not the closing line of the opening tag.

Covers two scenarios:
1. model_view.xml line 35: a <strong> tag with t-options spanning multiple
lines and starting AFTER other text on the same line as <li>.
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: <strong t-out="o.coverage*100" t-options="{&quot;...&quot;}">
# The tag <strong starts after <li>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 &quot;), "
f"got lines: {sorted(model_view_lines)}"
)

# website_templates.xml: <strong t-options on line 33
tmpl_lines = {e.position.line for e in errors if "website_templates.xml" in e.position.filepath}
assert 33 in tmpl_lines, (
f"Expected xml-double-quotes-py error on website_templates.xml:33 (t-options with &quot;), "
f"got lines: {sorted(tmpl_lines)}"
)
Loading