Skip to content
Draft
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
37 changes: 22 additions & 15 deletions ament_xmllint/ament_xmllint/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,21 @@
import subprocess
import sys
import time
from typing import Any
from typing import Literal
from typing_extensions import TypeAlias
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will effectively become a new dependency for all of ROS 2. Is there any way to avoid it?

from xml.etree import ElementTree
from xml.sax import make_parser
from xml.sax import SAXParseException
from xml.sax.xmlreader import AttributesImpl
from xml.sax.handler import ContentHandler
from xml.sax.saxutils import escape
from xml.sax.saxutils import quoteattr

ReturnCode: TypeAlias = Literal[0, 1, "Could not find 'xmllint' executable"]

def main(argv=sys.argv[1:]):

def main(argv: list[str] = sys.argv[1:]) -> ReturnCode:
default_extensions = ['xml']

parser = argparse.ArgumentParser(
Expand Down Expand Up @@ -72,16 +78,16 @@ def main(argv=sys.argv[1:]):
if not xmllint_bin:
return "Could not find 'xmllint' executable"

report = []
report: list[tuple[str, Any]] = []

# invoke xmllint on all files
for filename in files:
# parse file to extract desired validation information
parser = make_parser()
xml_parser = make_parser()
handler = CustomHandler()
parser.setContentHandler(handler)
xml_parser.setContentHandler(handler)
try:
parser.parse(filename)
xml_parser.parse(filename)
except SAXParseException:
pass

Expand Down Expand Up @@ -131,7 +137,7 @@ def main(argv=sys.argv[1:]):
error_count = sum(1 if r[1] else 0 for r in report)
if not error_count:
print('No problems found')
rc = 0
rc: ReturnCode = 0
else:
print('%d files are invalid' % error_count, file=sys.stderr)
rc = 1
Expand All @@ -158,8 +164,9 @@ def main(argv=sys.argv[1:]):
return rc


def get_files(paths, extensions, excludes=[]):
files = []
def get_files(paths: list[str], extensions: list[str],
excludes: list[str] = []) -> list[str]:
files: list[str] = []
for path in paths:
if os.path.isdir(path):
for dirpath, dirnames, filenames in os.walk(path):
Expand Down Expand Up @@ -187,31 +194,31 @@ def get_files(paths, extensions, excludes=[]):

class CustomHandler(ContentHandler):

def __init__(self):
def __init__(self) -> None:
super().__init__()
self.xml_model_attributes = []
self.root_attributes = {}
self.xml_model_attributes: list[dict[str, str]] = []
self.root_attributes: dict[str, str] = {}
self._first_node = False

def processingInstruction(self, target, data):
def processingInstruction(self, target: str, data: str) -> None:
if target != 'xml-model':
return

root = ElementTree.fromstring('<data ' + data + '/>')
self.xml_model_attributes.append(root.attrib)

def startDocument(self):
def startDocument(self) -> None:
self._first_node = True

def startElement(self, name, attrs):
def startElement(self, name: str, attrs: AttributesImpl) -> None:
if not self._first_node:
return
self._first_node = False
for attr_name in attrs.getNames():
self.root_attributes[attr_name] = attrs.getValue(attr_name)


def get_xunit_content(report, testname, elapsed):
def get_xunit_content(report: list[tuple[str, Any]], testname: str, elapsed: float) -> str:
test_count = len(report)
error_count = len([r for r in report if r[1]])
data = {
Expand Down
4 changes: 3 additions & 1 deletion ament_xmllint/ament_xmllint/pytest_marker.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
# limitations under the License.


def pytest_configure(config):
from _pytest.config import Config
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should be importing from a private module.

Suggested change
from _pytest.config import Config
from pytest import Config


def pytest_configure(config: Config) -> None:
config.addinivalue_line(
'markers',
'xmllint: marks tests checking XML files being well formed and valid')
2 changes: 2 additions & 0 deletions ament_xmllint/package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@

<exec_depend>ament_lint</exec_depend>
<exec_depend>libxml2-utils</exec_depend>
<exec_depend>python3-typing-extensions</exec_depend>

<test_depend>ament_copyright</test_depend>
<test_depend>ament_flake8</test_depend>
<test_depend>ament_pep257</test_depend>
<test_depend>ament_mypy</test_depend>
<test_depend>python3-pytest</test_depend>

<export>
Expand Down
2 changes: 1 addition & 1 deletion ament_xmllint/test/test_copyright.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@

@pytest.mark.copyright
@pytest.mark.linter
def test_copyright():
def test_copyright() -> None:
rc = main(argv=[])
assert rc == 0, 'Found errors'
2 changes: 1 addition & 1 deletion ament_xmllint/test/test_flake8.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

@pytest.mark.flake8
@pytest.mark.linter
def test_flake8():
def test_flake8() -> None:
rc, errors = main_with_errors(argv=[])
assert rc == 0, \
'Found %d code style errors / warnings:\n' % len(errors) + \
Expand Down
21 changes: 21 additions & 0 deletions ament_xmllint/test/test_mypy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Copyright 2026 Open Source Robotics Foundation, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following the pattern in other files, only a single newline between copyright and imports.

Suggested change


from ament_mypy.main import main


def test_mypy() -> None:
rc = main(argv=[])
assert rc == 0, 'Found code style errors / warnings'
2 changes: 1 addition & 1 deletion ament_xmllint/test/test_pep257.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@

@pytest.mark.linter
@pytest.mark.pep257
def test_pep257():
def test_pep257() -> None:
rc = main(argv=[])
assert rc == 0, 'Found docblock style errors'
2 changes: 1 addition & 1 deletion ament_xmllint/test/test_xmllint.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@

@pytest.mark.linter
@pytest.mark.xmllint
def test_xmllint():
def test_xmllint() -> None:
rc = main(argv=[])
assert rc == 0, 'Found errors'