From 20e020c14009e3c064ba46c5357877694937e84f Mon Sep 17 00:00:00 2001 From: romanodanilo <62891297+romanodanilo@users.noreply.github.com> Date: Fri, 12 Jul 2024 08:56:38 +0200 Subject: [PATCH 1/5] Add scoped params filtering to parameter search (#28) Signed-off-by: romanodanilo Signed-off-by: romanodanilo <62891297+romanodanilo@users.noreply.github.com> --- .../resolvable_entity_references.py | 4 +- qc_openscenario/checks/utils.py | 47 ++++++++++--------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/qc_openscenario/checks/reference_checker/resolvable_entity_references.py b/qc_openscenario/checks/reference_checker/resolvable_entity_references.py index 5d360dc..d550c80 100644 --- a/qc_openscenario/checks/reference_checker/resolvable_entity_references.py +++ b/qc_openscenario/checks/reference_checker/resolvable_entity_references.py @@ -92,8 +92,8 @@ def check_rule(checker_data: models.CheckerData) -> None: == models.AttributeType.PARAMETER ): current_entity_param_name = current_entity_ref[1:] - current_entity_param_value = utils.get_parameter_value( - root, current_entity_param_name + current_entity_param_value = utils.get_parameter_value_from_node( + root, node_with_entity_ref, current_entity_param_name ) logging.debug(f"current_entity_param_name: {current_entity_param_name}") logging.debug(f"current_entity_param_value: {current_entity_param_value}") diff --git a/qc_openscenario/checks/utils.py b/qc_openscenario/checks/utils.py index 7199f96..2965a80 100644 --- a/qc_openscenario/checks/utils.py +++ b/qc_openscenario/checks/utils.py @@ -48,46 +48,49 @@ def compare_versions(version1: str, version2: str) -> int: return 0 -def get_parameter_value( - root: etree._ElementTree, parameter_name: str +def get_parameter_value_from_node( + tree: etree._ElementTree, node: etree._Element, parameter_name: str ) -> Union[None, str, int, float]: - """Read all ParameterDeclaration nodes from root and get the value of parameter_name if present + """Read all ParameterDeclaration visible from node and get the value of parameter_name if present Args: root (etree._ElementTree): root node of the xml document + node (etree._Element): node to start the upward search from parameter_name (str): the parameter name to search Returns: Union[None, str, int, float]: the parameter value is present, with its type. None if the parameter_name is not found """ - param_declarations = root.findall(".//ParameterDeclaration") - if param_declarations is None: + # Dictionary to hold parameters + params_dict = {} + parameter_xpath = "./ParameterDeclarations/ParameterDeclaration" + + current = node + while current is not None: + for param in current.xpath(parameter_xpath): + name = param.get("name") + value = param.get("value") + if name not in params_dict: + params_dict[name] = value + current = current.getparent() + + if parameter_name in params_dict: + return params_dict[parameter_name] + else: return None - for param_declaration in param_declarations: - current_name = param_declaration.get("name") - current_value = param_declaration.get("value") - if ( - current_name is not None - and current_value is not None - and current_name == parameter_name - ): - return current_value - - return None - -def get_xodr_road_network(root: etree._ElementTree) -> Union[etree._ElementTree, None]: - """Get parsed xodr tree indicated in the RoadNetwork/LogicFile node of the input root +def get_xodr_road_network(tree: etree._ElementTree) -> Union[etree._ElementTree, None]: + """Get parsed xodr tree indicated in the RoadNetwork/LogicFile node of the input tree Args: - root (etree._ElementTree): root node of the xml document that refers to a xodr file + tree (etree._ElementTree): xml document tree that refers to a xodr file Returns: Union[etree._ElementTree, None]: the parsed road network tree. None if the specified nodes in the root or the road network file are not found """ - road_network = root.find("RoadNetwork") + road_network = tree.find("RoadNetwork") if road_network is None: return None logic_file = road_network.find("LogicFile") @@ -100,7 +103,7 @@ def get_xodr_road_network(root: etree._ElementTree) -> Union[etree._ElementTree, # If filepath is specified using param, get all param declaration and update the filepath if get_attribute_type(filepath) == models.AttributeType.PARAMETER: filepath_param = filepath[1:] - filepath = get_parameter_value(root, filepath_param) + filepath = get_parameter_value_from_node(tree, tree.getroot(), filepath_param) if filepath is None: return None From ce8873cefb339422beca32ebb4c0594aeb89d1a2 Mon Sep 17 00:00:00 2001 From: romanodanilo <62891297+romanodanilo@users.noreply.github.com> Date: Wed, 17 Jul 2024 17:12:29 +0200 Subject: [PATCH 2/5] Add storyboard reference and level checks (#34) Signed-off-by: romanodanilo Signed-off-by: romanodanilo <62891297+romanodanilo@users.noreply.github.com> --- .../checks/reference_checker/__init__.py | 4 + .../reference_checker/reference_checker.py | 4 + ...resolvable_storyboard_element_reference.py | 152 ++++++++++++ .../unique_element_names_on_same_level.py | 121 +++++++++ ..._element_reference.negative.parameter.xosc | 118 +++++++++ ...storyboard_element_reference.negative.xosc | 118 +++++++++ ..._element_reference.positive.parameter.xosc | 117 +++++++++ ...storyboard_element_reference.positive.xosc | 115 +++++++++ ...names_on_same_level.negative.multiple.xosc | 169 +++++++++++++ ..._element_names_on_same_level.negative.xosc | 105 ++++++++ ...names_on_same_level.positive.multiple.xosc | 164 ++++++++++++ ..._element_names_on_same_level.positive.xosc | 103 ++++++++ tests/test_reference_checks.py | 234 ++++++++++++++++++ 13 files changed, 1524 insertions(+) create mode 100644 qc_openscenario/checks/reference_checker/resolvable_storyboard_element_reference.py create mode 100644 qc_openscenario/checks/reference_checker/unique_element_names_on_same_level.py create mode 100644 tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.negative.parameter.xosc create mode 100644 tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.negative.xosc create mode 100644 tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.positive.parameter.xosc create mode 100644 tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.positive.xosc create mode 100644 tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.negative.multiple.xosc create mode 100644 tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.negative.xosc create mode 100644 tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.positive.multiple.xosc create mode 100644 tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.positive.xosc diff --git a/qc_openscenario/checks/reference_checker/__init__.py b/qc_openscenario/checks/reference_checker/__init__.py index bb05131..1ccee4b 100644 --- a/qc_openscenario/checks/reference_checker/__init__.py +++ b/qc_openscenario/checks/reference_checker/__init__.py @@ -14,3 +14,7 @@ ) from . import resolvable_entity_references as resolvable_entity_references from . import resolvable_variable_reference as resolvable_variable_reference +from . import ( + resolvable_storyboard_element_reference as resolvable_storyboard_element_reference, +) +from . import unique_element_names_on_same_level as unique_element_names_on_same_level diff --git a/qc_openscenario/checks/reference_checker/reference_checker.py b/qc_openscenario/checks/reference_checker/reference_checker.py index ecbd12b..94f407e 100644 --- a/qc_openscenario/checks/reference_checker/reference_checker.py +++ b/qc_openscenario/checks/reference_checker/reference_checker.py @@ -15,6 +15,8 @@ valid_actor_reference_in_private_actions, resolvable_entity_references, resolvable_variable_reference, + resolvable_storyboard_element_reference, + unique_element_names_on_same_level, ) @@ -62,6 +64,8 @@ def run_checks(checker_data: models.CheckerData) -> None: valid_actor_reference_in_private_actions.check_rule, resolvable_entity_references.check_rule, resolvable_variable_reference.check_rule, + resolvable_storyboard_element_reference.check_rule, + unique_element_names_on_same_level.check_rule, ] for rule in rule_list: diff --git a/qc_openscenario/checks/reference_checker/resolvable_storyboard_element_reference.py b/qc_openscenario/checks/reference_checker/resolvable_storyboard_element_reference.py new file mode 100644 index 0000000..6724871 --- /dev/null +++ b/qc_openscenario/checks/reference_checker/resolvable_storyboard_element_reference.py @@ -0,0 +1,152 @@ +import os, logging + +from dataclasses import dataclass +from typing import List + +from lxml import etree + +from qc_baselib import Configuration, Result, IssueSeverity + +from qc_openscenario import constants +from qc_openscenario.schema import schema_files +from qc_openscenario.checks import utils, models + +from qc_openscenario.checks.reference_checker import reference_constants +from collections import deque, defaultdict + +MIN_RULE_VERSION = "1.2.0" +RULE_SEVERITY = IssueSeverity.ERROR +STORYBOARD_ELEMENTS = ["Act", "Action", "Event", "Maneuver", "ManeuverGroup", " Story"] + + +def check_rule(checker_data: models.CheckerData) -> None: + """ + Rule ID: asam.net:xosc:1.2.0:reference_control.resolvable_storyboard_element_reference + + Description: The attribute storyboardElementRef shall point to an existing element + of the corresponding type and shall be uniquely resolvable. + Severity: ERROR + + Version range: [1.2.0, ) + + Remark: + None + + More info at + - https://github.com/asam-ev/qc-openscenarioxml/issues/29 + """ + logging.info("Executing resolvable_storyboard_element_reference check") + + schema_version = checker_data.schema_version + if schema_version is None: + logging.info(f"- Version not found in the file. Skipping check") + return + + if utils.compare_versions(schema_version, MIN_RULE_VERSION) < 0: + logging.info( + f"- Version {schema_version} is less than minimum required version {MIN_RULE_VERSION}. Skipping check" + ) + return + + rule_uid = checker_data.result.register_rule( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=reference_constants.CHECKER_ID, + emanating_entity="asam.net", + standard="xosc", + definition_setting=MIN_RULE_VERSION, + rule_full_name="reference_control.resolvable_storyboard_element_reference", + ) + + root = checker_data.input_file_xml_root + + storyboard_node = root.find("Storyboard") + if storyboard_node is None: + logging.error( + "Cannot find Storyboard node in provided XOSC file. Skipping check" + ) + return + + xpath_expr = "|".join([f"//{node}" for node in STORYBOARD_ELEMENTS]) + storyboard_elements = storyboard_node.xpath(xpath_expr) + if storyboard_elements is None: + logging.error( + "Cannot find Storyboard elements node in provided XOSC file. Skipping check" + ) + return + + storyboard_element_type = {} + storyboard_element_occurrences = {} + # Store storyboard elements along with + # - their type (for type matching) + # - number of occurrences (for unique resolution) + for storyboard_element in list(storyboard_elements): + current_name = storyboard_element.get("name") + current_type = storyboard_element.tag + if current_name is not None: + if current_name not in storyboard_element_occurrences: + storyboard_element_occurrences[current_name] = 1 + else: + storyboard_element_occurrences[current_name] += 1 + + storyboard_element_type[current_name] = current_type + + logging.debug(f"storyboard_element_type_dict: {storyboard_element_type}") + logging.debug(f"storyboard_element_occurrences: {storyboard_element_occurrences}") + + nodes_with_storyboard_el_ref = storyboard_node.xpath(".//*[@storyboardElementRef]") + + for node_with_storyboard_el_ref in nodes_with_storyboard_el_ref: + current_storyboard_el_ref = node_with_storyboard_el_ref.get( + "storyboardElementRef" + ) + current_storyboard_type = node_with_storyboard_el_ref.get( + "storyboardElementType" + ) + logging.debug(f"current_storyboard_el_ref: {current_storyboard_el_ref}") + logging.debug(f"current_storyboard_type: {current_storyboard_type}") + + # Check if entityRef points to a declared param + if ( + utils.get_attribute_type(current_storyboard_el_ref) + == models.AttributeType.PARAMETER + ): + current_entity_param_name = current_storyboard_el_ref[1:] + current_entity_param_value = utils.get_parameter_value_from_node( + root, node_with_storyboard_el_ref, current_entity_param_name + ) + logging.debug(f"current_st_el_param_name: {current_entity_param_name}") + logging.debug(f"current_st_el_param_value: {current_entity_param_value}") + # Parameter value is assigned to the current_storyboard_el_ref to search + # If parameter is not found, None is assigned to current_storyboard_el_ref + current_storyboard_el_ref = current_entity_param_value + + is_valid = ( + # Is found or its parameter can be resolved + current_storyboard_el_ref is not None + # Is found among storyboard elements + and current_storyboard_el_ref in storyboard_element_occurrences + # Is uniquely resolvable + and storyboard_element_occurrences[current_storyboard_el_ref] == 1 + # Type matches + and storyboard_element_type[current_storyboard_el_ref].lower() + == current_storyboard_type + ) + + if not is_valid: + xpath = root.getpath(node_with_storyboard_el_ref) + + issue_id = checker_data.result.register_issue( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=reference_constants.CHECKER_ID, + description="Issue flagging when a storyboardElementRef does not point to an existing element", + level=RULE_SEVERITY, + rule_uid=rule_uid, + ) + issue_description = f"Storyboard element reference {current_storyboard_el_ref} not found among Storyboard elements " + checker_data.result.add_xml_location( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=reference_constants.CHECKER_ID, + issue_id=issue_id, + xpath=xpath, + description=issue_description, + ) diff --git a/qc_openscenario/checks/reference_checker/unique_element_names_on_same_level.py b/qc_openscenario/checks/reference_checker/unique_element_names_on_same_level.py new file mode 100644 index 0000000..ee21199 --- /dev/null +++ b/qc_openscenario/checks/reference_checker/unique_element_names_on_same_level.py @@ -0,0 +1,121 @@ +import os, logging + +from dataclasses import dataclass +from typing import List, Union + +from lxml import etree + +from qc_baselib import Configuration, Result, IssueSeverity + +from qc_openscenario import constants +from qc_openscenario.schema import schema_files +from qc_openscenario.checks import utils, models + +from qc_openscenario.checks.reference_checker import reference_constants +from collections import deque, defaultdict + +MIN_RULE_VERSION = "1.2.0" +RULE_SEVERITY = IssueSeverity.ERROR + + +@dataclass +class QueueNode: + element: etree._ElementTree + parent_xpath: Union[str, None] + + +@dataclass +class DuplicateOccurrence: + name: str + xpath: str + + +def are_names_unique_at_each_level(tree: etree._ElementTree, root: etree._Element): + # Initialize a queue for breadth-first traversal + queue = deque([QueueNode(root, None)]) + levels_dict = defaultdict(set) + + duplicates = [] + + while queue: + queue_node = queue.popleft() + + current_element = queue_node.element + current_name = current_element.get("name") + parent_xpath = queue_node.parent_xpath + current_xpath = tree.getpath(current_element) + # Check if the element has a 'name' attribute + if current_name is not None: + # Check for duplicate names at the current level + if current_name in levels_dict[parent_xpath]: + logging.debug(f"Duplicated name found : {current_name}") + duplicates.append(DuplicateOccurrence(current_name, current_xpath)) + levels_dict[parent_xpath].add(current_name) + + # Add children to the stack with incremented level + for child in current_element.getchildren(): + queue.append(QueueNode(child, current_xpath)) + + return duplicates + + +def check_rule(checker_data: models.CheckerData) -> None: + """ + Rule ID: asam.net:xosc:1.2.0:reference_control.unique_element_names_on_same_level + + Description: Element names at each level shall be unique at that level. + There shall not be more than one element with the same name at the same level + (within the same directly enclosing element). + For example, within one Story, every Act shall use a unique name ("MyStory1": "MyAct1", "MyAct2"…​), + but the names of the acts may be reused in another story ("MyStory2": "MyAct1", "MyAct2"…​). + Severity: ERROR + + Version range: [1.2.0, ) + + Remark: + None + + More info at + - https://github.com/asam-ev/qc-openscenarioxml/issues/30 + """ + logging.info("Executing unique_element_names_on_same_level check") + + schema_version = checker_data.schema_version + if schema_version is None: + logging.info(f"- Version not found in the file. Skipping check") + return + + if utils.compare_versions(schema_version, MIN_RULE_VERSION) < 0: + logging.info( + f"- Version {schema_version} is less than minimum required version {MIN_RULE_VERSION}. Skipping check" + ) + return + + rule_uid = checker_data.result.register_rule( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=reference_constants.CHECKER_ID, + emanating_entity="asam.net", + standard="xosc", + definition_setting=MIN_RULE_VERSION, + rule_full_name="reference_control.unique_element_names_on_same_level", + ) + + tree = checker_data.input_file_xml_root + duplicates_found = are_names_unique_at_each_level(tree, tree.getroot()) + + for duplicate in duplicates_found: + issue_id = checker_data.result.register_issue( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=reference_constants.CHECKER_ID, + description="Issue flagging when a element name is not unique within its level", + level=RULE_SEVERITY, + rule_uid=rule_uid, + ) + issue_description = f"Element {duplicate.name} is duplicated" + checker_data.result.add_xml_location( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=reference_constants.CHECKER_ID, + issue_id=issue_id, + xpath=duplicate.xpath, + description=issue_description, + ) diff --git a/tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.negative.parameter.xosc b/tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.negative.parameter.xosc new file mode 100644 index 0000000..580749e --- /dev/null +++ b/tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.negative.parameter.xosc @@ -0,0 +1,118 @@ + + + + + + + + + + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.negative.xosc b/tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.negative.xosc new file mode 100644 index 0000000..7aaea6f --- /dev/null +++ b/tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.negative.xosc @@ -0,0 +1,118 @@ + + + + + + + + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.positive.parameter.xosc b/tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.positive.parameter.xosc new file mode 100644 index 0000000..c178f16 --- /dev/null +++ b/tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.positive.parameter.xosc @@ -0,0 +1,117 @@ + + + + + + + + + + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.positive.xosc b/tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.positive.xosc new file mode 100644 index 0000000..ff693bc --- /dev/null +++ b/tests/data/resolvable_storyboard_element_reference/resolvable_storyboard_element_reference.positive.xosc @@ -0,0 +1,115 @@ + + + + + + + + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.negative.multiple.xosc b/tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.negative.multiple.xosc new file mode 100644 index 0000000..a8b7165 --- /dev/null +++ b/tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.negative.multiple.xosc @@ -0,0 +1,169 @@ + + + + + + + + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.negative.xosc b/tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.negative.xosc new file mode 100644 index 0000000..b588e86 --- /dev/null +++ b/tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.negative.xosc @@ -0,0 +1,105 @@ + + + + + + + + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.positive.multiple.xosc b/tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.positive.multiple.xosc new file mode 100644 index 0000000..a53a6e0 --- /dev/null +++ b/tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.positive.multiple.xosc @@ -0,0 +1,164 @@ + + + + + + + + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.positive.xosc b/tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.positive.xosc new file mode 100644 index 0000000..6eb19f3 --- /dev/null +++ b/tests/data/unique_element_names_on_same_level/unique_element_names_on_same_level.positive.xosc @@ -0,0 +1,103 @@ + + + + + + + + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/test_reference_checks.py b/tests/test_reference_checks.py index ea07003..eb4fbf5 100644 --- a/tests/test_reference_checks.py +++ b/tests/test_reference_checks.py @@ -573,3 +573,237 @@ def test_resolvable_variable_reference_multiple( ) assert "foo" in reference_issues[1].locations[0].description test_utils.cleanup_files() + + +def test_resolvable_storyboard_element_reference_positive( + monkeypatch, +) -> None: + base_path = "tests/data/resolvable_storyboard_element_reference/" + target_file_name = f"resolvable_storyboard_element_reference.positive.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + _ = result.get_checker_result( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=reference_constants.CHECKER_ID, + ) + assert ( + len( + result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:reference_control.resolvable_storyboard_element_reference" + ) + ) + == 0 + ) + + +def test_resolvable_storyboard_element_reference_positive_parameter( + monkeypatch, +) -> None: + base_path = "tests/data/resolvable_storyboard_element_reference/" + target_file_name = ( + f"resolvable_storyboard_element_reference.positive.parameter.xosc" + ) + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + _ = result.get_checker_result( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=reference_constants.CHECKER_ID, + ) + assert ( + len( + result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:reference_control.resolvable_storyboard_element_reference" + ) + ) + == 0 + ) + + +def test_resolvable_storyboard_element_reference_negative( + monkeypatch, +) -> None: + base_path = "tests/data/resolvable_storyboard_element_reference/" + target_file_name = f"resolvable_storyboard_element_reference.negative.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + _ = result.get_checker_result( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=reference_constants.CHECKER_ID, + ) + + reference_issues = result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:reference_control.resolvable_storyboard_element_reference" + ) + assert len(reference_issues) == 2 + assert reference_issues[0].level == IssueSeverity.ERROR + assert reference_issues[1].level == IssueSeverity.ERROR + assert "Act2" in reference_issues[0].locations[0].description + assert "Act1" in reference_issues[1].locations[0].description + + test_utils.cleanup_files() + + +def test_resolvable_storyboard_element_reference_negative_parameter( + monkeypatch, +) -> None: + base_path = "tests/data/resolvable_storyboard_element_reference/" + target_file_name = ( + f"resolvable_storyboard_element_reference.negative.parameter.xosc" + ) + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + _ = result.get_checker_result( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=reference_constants.CHECKER_ID, + ) + + reference_issues = result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:reference_control.resolvable_storyboard_element_reference" + ) + assert len(reference_issues) == 1 + assert reference_issues[0].level == IssueSeverity.ERROR + test_utils.cleanup_files() + + +def test_unique_element_names_on_same_level_positive( + monkeypatch, +) -> None: + base_path = "tests/data/unique_element_names_on_same_level/" + target_file_name = f"unique_element_names_on_same_level.positive.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + _ = result.get_checker_result( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=reference_constants.CHECKER_ID, + ) + assert ( + len( + result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:reference_control.unique_element_names_on_same_level" + ) + ) + == 0 + ) + + +def test_unique_element_names_on_same_level_positive_multiple( + monkeypatch, +) -> None: + base_path = "tests/data/unique_element_names_on_same_level/" + target_file_name = f"unique_element_names_on_same_level.positive.multiple.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + _ = result.get_checker_result( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=reference_constants.CHECKER_ID, + ) + assert ( + len( + result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:reference_control.unique_element_names_on_same_level" + ) + ) + == 0 + ) + + +def test_unique_element_names_on_same_level_negative( + monkeypatch, +) -> None: + base_path = "tests/data/unique_element_names_on_same_level/" + target_file_name = f"unique_element_names_on_same_level.negative.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + _ = result.get_checker_result( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=reference_constants.CHECKER_ID, + ) + + reference_issues = result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:reference_control.unique_element_names_on_same_level" + ) + assert len(reference_issues) == 1 + assert reference_issues[0].level == IssueSeverity.ERROR + test_utils.cleanup_files() + + +def test_unique_element_names_on_same_level_negative_multiple( + monkeypatch, +) -> None: + base_path = "tests/data/unique_element_names_on_same_level/" + target_file_name = f"unique_element_names_on_same_level.negative.multiple.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + _ = result.get_checker_result( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=reference_constants.CHECKER_ID, + ) + + reference_issues = result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:reference_control.unique_element_names_on_same_level" + ) + assert len(reference_issues) == 3 + assert reference_issues[0].level == IssueSeverity.ERROR + assert reference_issues[1].level == IssueSeverity.ERROR + assert reference_issues[2].level == IssueSeverity.ERROR + + assert "Story1" in reference_issues[0].locations[0].description + assert "Event222" in reference_issues[1].locations[0].description + assert "Event1" in reference_issues[2].locations[0].description + test_utils.cleanup_files() From 4b178501275cef595e7b42088998d5a47d3bcb0c Mon Sep 17 00:00:00 2001 From: romanodanilo <62891297+romanodanilo@users.noreply.github.com> Date: Fri, 19 Jul 2024 11:00:26 +0200 Subject: [PATCH 3/5] Add data type checks (#35) Signed-off-by: romanodanilo Signed-off-by: romanodanilo <62891297+romanodanilo@users.noreply.github.com> --- main.py | 4 + .../checks/data_type_checker/__init__.py | 7 + .../data_type_checker/allowed_operators.py | 162 ++++++++ .../data_type_checker/data_type_checker.py | 70 ++++ .../data_type_checker/data_type_constants.py | 1 + ...e_transition_time_in_light_state_action.py | 116 ++++++ .../positive_duration_in_phase.py | 107 ++++++ qc_openscenario/checks/utils.py | 7 +- .../allowed_operators/negative_example.xosc | 29 ++ .../negative_example_multiple.xosc | 32 ++ .../negative_example_typo.xosc | 29 ++ .../positive_example.param.xosc | 28 ++ .../allowed_operators/positive_example.xosc | 27 ++ .../positive_example_multiple.xosc | 28 ++ .../negative_example.parameter.xosc | 26 ++ .../negative_example.xosc | 24 ++ .../positive_example.parameter.xosc | 25 ++ .../positive_example.xosc | 23 ++ .../negative_example.xosc | 28 ++ .../negative_example_param.xosc | 32 ++ .../positive_example.xosc | 27 ++ tests/test_data_type_checks.py | 357 ++++++++++++++++++ 22 files changed, 1187 insertions(+), 2 deletions(-) create mode 100644 qc_openscenario/checks/data_type_checker/__init__.py create mode 100644 qc_openscenario/checks/data_type_checker/allowed_operators.py create mode 100644 qc_openscenario/checks/data_type_checker/data_type_checker.py create mode 100644 qc_openscenario/checks/data_type_checker/data_type_constants.py create mode 100644 qc_openscenario/checks/data_type_checker/non_negative_transition_time_in_light_state_action.py create mode 100644 qc_openscenario/checks/data_type_checker/positive_duration_in_phase.py create mode 100644 tests/data/allowed_operators/negative_example.xosc create mode 100644 tests/data/allowed_operators/negative_example_multiple.xosc create mode 100644 tests/data/allowed_operators/negative_example_typo.xosc create mode 100644 tests/data/allowed_operators/positive_example.param.xosc create mode 100644 tests/data/allowed_operators/positive_example.xosc create mode 100644 tests/data/allowed_operators/positive_example_multiple.xosc create mode 100644 tests/data/positive_duration_in_phase/negative_example.parameter.xosc create mode 100644 tests/data/positive_duration_in_phase/negative_example.xosc create mode 100644 tests/data/positive_duration_in_phase/positive_example.parameter.xosc create mode 100644 tests/data/positive_duration_in_phase/positive_example.xosc create mode 100644 tests/data/transition_time_should_be_non_negative/negative_example.xosc create mode 100644 tests/data/transition_time_should_be_non_negative/negative_example_param.xosc create mode 100644 tests/data/transition_time_should_be_non_negative/positive_example.xosc create mode 100644 tests/test_data_type_checks.py diff --git a/main.py b/main.py index 902b589..00433de 100644 --- a/main.py +++ b/main.py @@ -9,6 +9,7 @@ from qc_openscenario.checks.basic_checker import basic_checker from qc_openscenario.checks.reference_checker import reference_checker from qc_openscenario.checks.parameters_checker import parameters_checker +from qc_openscenario.checks.data_type_checker import data_type_checker logging.basicConfig(format="%(asctime)s - %(message)s", level=logging.INFO) @@ -59,6 +60,9 @@ def main(): # 4. Run parameters checks parameters_checker.run_checks(checker_data) + # 5. Run data_type checks + data_type_checker.run_checks(checker_data) + result.write_to_file( config.get_checker_bundle_param( checker_bundle_name=constants.BUNDLE_NAME, param_name="resultFile" diff --git a/qc_openscenario/checks/data_type_checker/__init__.py b/qc_openscenario/checks/data_type_checker/__init__.py new file mode 100644 index 0000000..cf63920 --- /dev/null +++ b/qc_openscenario/checks/data_type_checker/__init__.py @@ -0,0 +1,7 @@ +from . import data_type_constants as data_type_constants +from . import data_type_checker as data_type_checker +from . import allowed_operators as allowed_operators +from . import ( + non_negative_transition_time_in_light_state_action as non_negative_transition_time_in_light_state_action, +) +from . import positive_duration_in_phase as positive_duration_in_phase diff --git a/qc_openscenario/checks/data_type_checker/allowed_operators.py b/qc_openscenario/checks/data_type_checker/allowed_operators.py new file mode 100644 index 0000000..c1282b5 --- /dev/null +++ b/qc_openscenario/checks/data_type_checker/allowed_operators.py @@ -0,0 +1,162 @@ +import os, logging + +from dataclasses import dataclass +from typing import List, Union + +from lxml import etree + +from qc_baselib import Configuration, Result, IssueSeverity + +from qc_openscenario import constants +from qc_openscenario.schema import schema_files +from qc_openscenario.checks import utils, models + +from qc_openscenario.checks.data_type_checker import data_type_constants + +import re + +MIN_RULE_VERSION = "1.2.0" +RULE_SEVERITY = IssueSeverity.ERROR +ALLOWED_OPERANDS = set() +ALLOWED_OPERANDS.add("-") +ALLOWED_OPERANDS.add("round") +ALLOWED_OPERANDS.add("floor") +ALLOWED_OPERANDS.add("ceil") +ALLOWED_OPERANDS.add("sqrt") +ALLOWED_OPERANDS.add("pow") +ALLOWED_OPERANDS.add("*") +ALLOWED_OPERANDS.add("/") +ALLOWED_OPERANDS.add("%") +ALLOWED_OPERANDS.add("+") +ALLOWED_OPERANDS.add("-") +ALLOWED_OPERANDS.add("not") +ALLOWED_OPERANDS.add("and") +ALLOWED_OPERANDS.add("or") + + +@dataclass +class QueueNode: + element: etree._ElementTree + xpath: Union[str, None] + + +@dataclass +class AttributeInfo: + name: str + value: str + xpath: str + + +def get_all_attributes(tree: etree._ElementTree, root: etree._Element): + attributes = [] + stack = [ + QueueNode(root, tree.getpath(root)) + ] # Initialize stack with the root element + + while stack: + current_node = stack.pop() + current_element = current_node.element + current_xpath = current_node.xpath + + # Process attributes of the current element + for attr, value in current_element.attrib.items(): + attributes.append(AttributeInfo(attr, value, current_xpath)) + + # Push children to the stack for further processing + stack.extend( + reversed( + [QueueNode(x, tree.getpath(x)) for x in current_element.getchildren()] + ) + ) + + return attributes + + +def filter_expressions(attribute): + return ( + attribute.value.startswith("$") + and utils.get_attribute_type(attribute.value) != models.AttributeType.PARAMETER + ) + + +def check_rule(checker_data: models.CheckerData) -> None: + """ + Rule ID: asam.net:xosc:1.2.0:data_type.allowed_operators + + Description: Expressions in OpenSCENARIO must only use the following operands: + -, round, floor, ceil, sqrt, pow, *, /, %, +, -, not, and, or. + Severity: ERROR + + Version range: [1.2.0, ) + + Remark: + None + + More info at + - https://github.com/asam-ev/qc-openscenarioxml/issues/31 + """ + logging.info("Executing allowed_operators check") + + schema_version = checker_data.schema_version + if schema_version is None: + logging.info(f"- Version not found in the file. Skipping check") + return + + if utils.compare_versions(schema_version, MIN_RULE_VERSION) < 0: + logging.info( + f"- Version {schema_version} is less than minimum required version {MIN_RULE_VERSION}. Skipping check" + ) + return + + rule_uid = checker_data.result.register_rule( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + emanating_entity="asam.net", + standard="xosc", + definition_setting=MIN_RULE_VERSION, + rule_full_name="data_type.allowed_operators", + ) + + tree = checker_data.input_file_xml_root + root = tree.getroot() + attributes = get_all_attributes(tree, root) + filtered_attributes = list(filter(filter_expressions, attributes)) + + logging.debug(f"attributes: {attributes}") + logging.debug(f"filtered_attributes: {filtered_attributes}") + + for attribute in filtered_attributes: + # Remove starting "${" and trailing "}" + expression_candidate = attribute.value[2:-1] + logging.debug(f"expression_candidate: {expression_candidate}") + # Define the regex pattern to match digits and allowed chars ( ) . and , + pattern = r"[\d()., ]+" + # Split the input string based on the regex pattern + operand_candidates = re.split(pattern, expression_candidate) + # Filter out empty strings from the resulting list + operand_candidates = [ + part + for part in operand_candidates + if part and part != "" and part != " " and not part.startswith("$") + ] + logging.debug(f"operand candidates : {operand_candidates}") + for operand in operand_candidates: + has_issue = operand not in ALLOWED_OPERANDS + if has_issue: + logging.debug(f"Invalid operand {operand}") + xpath = attribute.xpath + + issue_id = checker_data.result.register_issue( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + description="Issue flagging invalid operand is used within expression", + level=RULE_SEVERITY, + rule_uid=rule_uid, + ) + checker_data.result.add_xml_location( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + issue_id=issue_id, + xpath=xpath, + description=f"Invalid operand {operand} used", + ) diff --git a/qc_openscenario/checks/data_type_checker/data_type_checker.py b/qc_openscenario/checks/data_type_checker/data_type_checker.py new file mode 100644 index 0000000..29ff6b3 --- /dev/null +++ b/qc_openscenario/checks/data_type_checker/data_type_checker.py @@ -0,0 +1,70 @@ +import logging + +from lxml import etree + +from qc_baselib import Configuration, Result, StatusType + +from qc_openscenario import constants +from qc_openscenario.checks import utils, models +from qc_openscenario.schema import schema_files + +from qc_openscenario.checks.data_type_checker import ( + data_type_constants, + allowed_operators, + non_negative_transition_time_in_light_state_action, + positive_duration_in_phase, +) + + +def run_checks(checker_data: models.CheckerData) -> None: + logging.info("Executing data_type checks") + + checker_data.result.register_checker( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + description="Check if data_type properties of input file are properly set", + summary="", + ) + + if checker_data.input_file_xml_root is None: + logging.error( + f"Invalid xml input file. Checker {data_type_constants.CHECKER_ID} skipped" + ) + checker_data.result.set_checker_status( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + status=StatusType.SKIPPED, + ) + return + + if checker_data.schema_version not in schema_files.SCHEMA_FILES: + + logging.error( + f"Version {checker_data.schema_version} unsupported. Checker {data_type_constants.CHECKER_ID} skipped" + ) + checker_data.result.set_checker_status( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + status=StatusType.SKIPPED, + ) + return + + rule_list = [ + allowed_operators.check_rule, + non_negative_transition_time_in_light_state_action.check_rule, + positive_duration_in_phase.check_rule, + ] + + for rule in rule_list: + rule(checker_data=checker_data) + + logging.info( + f"Issues found - {checker_data.result.get_checker_issue_count(checker_bundle_name=constants.BUNDLE_NAME, checker_id=data_type_constants.CHECKER_ID)}" + ) + + # TODO: Add logic to deal with error or to skip it + checker_data.result.set_checker_status( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + status=StatusType.COMPLETED, + ) diff --git a/qc_openscenario/checks/data_type_checker/data_type_constants.py b/qc_openscenario/checks/data_type_checker/data_type_constants.py new file mode 100644 index 0000000..a23d24b --- /dev/null +++ b/qc_openscenario/checks/data_type_checker/data_type_constants.py @@ -0,0 +1 @@ +CHECKER_ID = "data_type_xosc" diff --git a/qc_openscenario/checks/data_type_checker/non_negative_transition_time_in_light_state_action.py b/qc_openscenario/checks/data_type_checker/non_negative_transition_time_in_light_state_action.py new file mode 100644 index 0000000..548a2fb --- /dev/null +++ b/qc_openscenario/checks/data_type_checker/non_negative_transition_time_in_light_state_action.py @@ -0,0 +1,116 @@ +import os, logging + +from dataclasses import dataclass +from typing import List + +from lxml import etree + +from qc_baselib import Configuration, Result, IssueSeverity + +from qc_openscenario import constants +from qc_openscenario.schema import schema_files +from qc_openscenario.checks import utils, models + +from qc_openscenario.checks.data_type_checker import data_type_constants + +MIN_RULE_VERSION = "1.2.0" +RULE_SEVERITY = IssueSeverity.ERROR + + +def check_rule(checker_data: models.CheckerData) -> None: + """ + Rule ID: asam.net:xosc:1.2.0:data_type.non_negative_transition_time_in_light_state_action + + Description: transitionTime in LightStateAction should be non-negative + Severity: ERROR + + Version range: [1.2.0, ) + + Remark: + None + + More info at + - https://github.com/asam-ev/qc-openscenarioxml/issues/32 + """ + logging.info("Executing non_negative_transition_time_in_light_state_action check") + + schema_version = checker_data.schema_version + if schema_version is None: + logging.info(f"- Version not found in the file. Skipping check") + return + + if utils.compare_versions(schema_version, MIN_RULE_VERSION) < 0: + logging.info( + f"- Version {schema_version} is less than minimum required version {MIN_RULE_VERSION}. Skipping check" + ) + return + + rule_uid = checker_data.result.register_rule( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + emanating_entity="asam.net", + standard="xosc", + definition_setting=MIN_RULE_VERSION, + rule_full_name="data_type.non_negative_transition_time_in_light_state_action", + ) + + root = checker_data.input_file_xml_root + + light_state_nodes = root.findall(".//LightStateAction") + + for light_state_node in light_state_nodes: + current_transition_time = light_state_node.get("transitionTime") + if current_transition_time is None: + continue + + current_transition_type = utils.get_attribute_type(current_transition_time) + logging.debug(f"current_transition_type: {current_transition_type}") + if current_transition_type == models.AttributeType.EXPRESSION: + logging.debug( + f"Skipping transitionTime with value {current_transition_time} since sign cannot be evaluated " + ) + continue + if current_transition_type == models.AttributeType.PARAMETER: + current_transition_param_name = current_transition_time[1:] + current_transition_param_value = utils.get_parameter_value_from_node( + root, light_state_node, current_transition_param_name + ) + logging.debug( + f"current_transition_param_name: {current_transition_param_name}" + ) + logging.debug( + f"current_transition_param_name: {current_transition_param_name}" + ) + # Parameter value is assigned to the current_transition_time to search + # If parameter is not found, None is assigned to current_transition_time + if current_transition_param_value is None: + continue + current_transition_time = current_transition_param_value + # Case of incomplete expression to skip + if ( + current_transition_type == models.AttributeType.VALUE + and current_transition_time.startswith("$") + ): + continue + + logging.debug(f"current_transition_time: {current_transition_time}") + current_numeric_value = float(current_transition_time) + has_issue = current_numeric_value < 0 + + if has_issue: + xpath = root.getpath(light_state_node) + + issue_id = checker_data.result.register_issue( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + description="Issue transitionTime in LightStateAction node is negative", + level=RULE_SEVERITY, + rule_uid=rule_uid, + ) + checker_data.result.add_xml_location( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + issue_id=issue_id, + xpath=xpath, + description=f"transitionTime duration {current_numeric_value} is negative", + ) diff --git a/qc_openscenario/checks/data_type_checker/positive_duration_in_phase.py b/qc_openscenario/checks/data_type_checker/positive_duration_in_phase.py new file mode 100644 index 0000000..ad29707 --- /dev/null +++ b/qc_openscenario/checks/data_type_checker/positive_duration_in_phase.py @@ -0,0 +1,107 @@ +import os, logging + +from dataclasses import dataclass +from typing import List + +from lxml import etree + +from qc_baselib import Configuration, Result, IssueSeverity + +from qc_openscenario import constants +from qc_openscenario.schema import schema_files +from qc_openscenario.checks import utils, models + +from qc_openscenario.checks.data_type_checker import data_type_constants + +MIN_RULE_VERSION = "1.2.0" +RULE_SEVERITY = IssueSeverity.ERROR + + +def check_rule(checker_data: models.CheckerData) -> None: + """ + Rule ID: asam.net:xosc:1.2.0:data_type.positive_duration_in_phase + + Description: Attribute “duration” in the complex type “Phase” (e.g., for TrafficSignalController”) shall be non-negative. + Severity: ERROR + + Version range: [1.2.0, ) + + Remark: + None + + More info at + - https://github.com/asam-ev/qc-openscenarioxml/issues/33 + """ + logging.info("Executing positive_duration_in_phase check") + + schema_version = checker_data.schema_version + if schema_version is None: + logging.info(f"- Version not found in the file. Skipping check") + return + + if utils.compare_versions(schema_version, MIN_RULE_VERSION) < 0: + logging.info( + f"- Version {schema_version} is less than minimum required version {MIN_RULE_VERSION}. Skipping check" + ) + return + + rule_uid = checker_data.result.register_rule( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + emanating_entity="asam.net", + standard="xosc", + definition_setting=MIN_RULE_VERSION, + rule_full_name="data_type.positive_duration_in_phase", + ) + + root = checker_data.input_file_xml_root + + phase_nodes = root.findall(".//Phase") + + for phase_node in phase_nodes: + current_duration = phase_node.get("duration") + logging.debug(f"current_duration: {current_duration}") + current_duration_type = utils.get_attribute_type(current_duration) + + if current_duration is None: + continue + if current_duration_type == models.AttributeType.EXPRESSION: + logging.debug( + f"Skipping duration with value {current_duration} since sign cannot be evaluated " + ) + continue + if current_duration_type == models.AttributeType.PARAMETER: + current_duration_param_name = current_duration[1:] + current_duration_param_value = utils.get_parameter_value_from_node( + root, phase_node, current_duration_param_name + ) + logging.debug(f"current_duration_param_name: {current_duration_param_name}") + logging.debug( + f"current_duration_param_value: {current_duration_param_value}" + ) + # Parameter value is assigned to the current_duration to search + # If parameter is not found, None is assigned to current_duration + if current_duration_param_value is None: + continue + current_duration = current_duration_param_value + + current_numeric_value = float(current_duration) + has_issue = current_numeric_value < 0 + + if has_issue: + xpath = root.getpath(phase_node) + + issue_id = checker_data.result.register_issue( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + description="Issue flagging when attribute “duration” in the complex type “Phase” is negative", + level=RULE_SEVERITY, + rule_uid=rule_uid, + ) + checker_data.result.add_xml_location( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + issue_id=issue_id, + xpath=xpath, + description=f"Phase duration {current_numeric_value} is negative", + ) diff --git a/qc_openscenario/checks/utils.py b/qc_openscenario/checks/utils.py index 2965a80..d1e0eaa 100644 --- a/qc_openscenario/checks/utils.py +++ b/qc_openscenario/checks/utils.py @@ -2,9 +2,10 @@ from typing import Union from qc_openscenario.checks import models import re +import logging -EXPRESSION_PATTERN = re.compile("[$][{][ A-Za-z0-9_\+\-\*/%$\(\)\.,]*[\}]") -PARAMETER_PATTERN = re.compile("[$][A-Za-z_][A-Za-z0-9_]*") +EXPRESSION_PATTERN = re.compile(r"[$][{][ A-Za-z0-9_\+\-\*/%$\(\)\.,]*[\}]") +PARAMETER_PATTERN = re.compile(r"[$][A-Za-z_][A-Za-z0-9_]*") def get_standard_schema_version(root: etree._ElementTree) -> Union[str, None]: @@ -74,6 +75,8 @@ def get_parameter_value_from_node( params_dict[name] = value current = current.getparent() + logging.debug(f"Visible parameters dictionary: {params_dict}") + if parameter_name in params_dict: return params_dict[parameter_name] else: diff --git a/tests/data/allowed_operators/negative_example.xosc b/tests/data/allowed_operators/negative_example.xosc new file mode 100644 index 0000000..1ed3419 --- /dev/null +++ b/tests/data/allowed_operators/negative_example.xosc @@ -0,0 +1,29 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/allowed_operators/negative_example_multiple.xosc b/tests/data/allowed_operators/negative_example_multiple.xosc new file mode 100644 index 0000000..b0d46f3 --- /dev/null +++ b/tests/data/allowed_operators/negative_example_multiple.xosc @@ -0,0 +1,32 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/allowed_operators/negative_example_typo.xosc b/tests/data/allowed_operators/negative_example_typo.xosc new file mode 100644 index 0000000..480605b --- /dev/null +++ b/tests/data/allowed_operators/negative_example_typo.xosc @@ -0,0 +1,29 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/allowed_operators/positive_example.param.xosc b/tests/data/allowed_operators/positive_example.param.xosc new file mode 100644 index 0000000..6873485 --- /dev/null +++ b/tests/data/allowed_operators/positive_example.param.xosc @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/allowed_operators/positive_example.xosc b/tests/data/allowed_operators/positive_example.xosc new file mode 100644 index 0000000..7e4bf8d --- /dev/null +++ b/tests/data/allowed_operators/positive_example.xosc @@ -0,0 +1,27 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/allowed_operators/positive_example_multiple.xosc b/tests/data/allowed_operators/positive_example_multiple.xosc new file mode 100644 index 0000000..b399349 --- /dev/null +++ b/tests/data/allowed_operators/positive_example_multiple.xosc @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/positive_duration_in_phase/negative_example.parameter.xosc b/tests/data/positive_duration_in_phase/negative_example.parameter.xosc new file mode 100644 index 0000000..144ea82 --- /dev/null +++ b/tests/data/positive_duration_in_phase/negative_example.parameter.xosc @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/positive_duration_in_phase/negative_example.xosc b/tests/data/positive_duration_in_phase/negative_example.xosc new file mode 100644 index 0000000..24cc1f5 --- /dev/null +++ b/tests/data/positive_duration_in_phase/negative_example.xosc @@ -0,0 +1,24 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/positive_duration_in_phase/positive_example.parameter.xosc b/tests/data/positive_duration_in_phase/positive_example.parameter.xosc new file mode 100644 index 0000000..2132aab --- /dev/null +++ b/tests/data/positive_duration_in_phase/positive_example.parameter.xosc @@ -0,0 +1,25 @@ + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/positive_duration_in_phase/positive_example.xosc b/tests/data/positive_duration_in_phase/positive_example.xosc new file mode 100644 index 0000000..e4a326e --- /dev/null +++ b/tests/data/positive_duration_in_phase/positive_example.xosc @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/transition_time_should_be_non_negative/negative_example.xosc b/tests/data/transition_time_should_be_non_negative/negative_example.xosc new file mode 100644 index 0000000..377c522 --- /dev/null +++ b/tests/data/transition_time_should_be_non_negative/negative_example.xosc @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/transition_time_should_be_non_negative/negative_example_param.xosc b/tests/data/transition_time_should_be_non_negative/negative_example_param.xosc new file mode 100644 index 0000000..7389d54 --- /dev/null +++ b/tests/data/transition_time_should_be_non_negative/negative_example_param.xosc @@ -0,0 +1,32 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/transition_time_should_be_non_negative/positive_example.xosc b/tests/data/transition_time_should_be_non_negative/positive_example.xosc new file mode 100644 index 0000000..828a9fe --- /dev/null +++ b/tests/data/transition_time_should_be_non_negative/positive_example.xosc @@ -0,0 +1,27 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/test_data_type_checks.py b/tests/test_data_type_checks.py new file mode 100644 index 0000000..bf7becb --- /dev/null +++ b/tests/test_data_type_checks.py @@ -0,0 +1,357 @@ +import os +import pytest +import test_utils +from qc_openscenario import constants +from qc_openscenario.checks.data_type_checker import data_type_constants +from qc_baselib import Result, IssueSeverity + + +def test_positive_duration_in_phase_positive( + monkeypatch, +) -> None: + base_path = "tests/data/positive_duration_in_phase/" + target_file_name = f"positive_example.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + assert ( + len( + result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:data_type.positive_duration_in_phase" + ) + ) + == 0 + ) + + test_utils.cleanup_files() + + +def test_positive_duration_in_phase_positive_parameter( + monkeypatch, +) -> None: + base_path = "tests/data/positive_duration_in_phase/" + target_file_name = f"positive_example.parameter.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + assert ( + len( + result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:data_type.positive_duration_in_phase" + ) + ) + == 0 + ) + + test_utils.cleanup_files() + + +def test_positive_duration_in_phase_negative( + monkeypatch, +) -> None: + base_path = "tests/data/positive_duration_in_phase/" + target_file_name = f"negative_example.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + _ = result.get_checker_result( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + ) + + data_type_issues = result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:data_type.positive_duration_in_phase" + ) + assert len(data_type_issues) == 1 + assert data_type_issues[0].level == IssueSeverity.ERROR + test_utils.cleanup_files() + + +def test_positive_duration_in_phase_negative_parameter( + monkeypatch, +) -> None: + base_path = "tests/data/positive_duration_in_phase/" + target_file_name = f"negative_example.parameter.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + _ = result.get_checker_result( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + ) + + data_type_issues = result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:data_type.positive_duration_in_phase" + ) + assert len(data_type_issues) == 1 + assert data_type_issues[0].level == IssueSeverity.ERROR + test_utils.cleanup_files() + + +def test_allowed_operators_positive( + monkeypatch, +) -> None: + base_path = "tests/data/allowed_operators/" + target_file_name = f"positive_example.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + assert ( + len( + result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:data_type.allowed_operators" + ) + ) + == 0 + ) + + test_utils.cleanup_files() + + +def test_allowed_operators_positive_param( + monkeypatch, +) -> None: + base_path = "tests/data/allowed_operators/" + target_file_name = f"positive_example.param.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + assert ( + len( + result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:data_type.allowed_operators" + ) + ) + == 0 + ) + + test_utils.cleanup_files() + + +def test_allowed_operators_positive_multiple( + monkeypatch, +) -> None: + base_path = "tests/data/allowed_operators/" + target_file_name = f"positive_example_multiple.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + assert ( + len( + result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:data_type.allowed_operators" + ) + ) + == 0 + ) + + test_utils.cleanup_files() + + +def test_allowed_operators_negative( + monkeypatch, +) -> None: + base_path = "tests/data/allowed_operators/" + target_file_name = f"negative_example.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + _ = result.get_checker_result( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + ) + + data_type_issues = result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:data_type.allowed_operators" + ) + assert len(data_type_issues) == 1 + assert data_type_issues[0].level == IssueSeverity.ERROR + test_utils.cleanup_files() + + +def test_allowed_operators_negative_typo( + monkeypatch, +) -> None: + base_path = "tests/data/allowed_operators/" + target_file_name = f"negative_example_typo.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + _ = result.get_checker_result( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + ) + + data_type_issues = result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:data_type.allowed_operators" + ) + assert len(data_type_issues) == 1 + assert data_type_issues[0].level == IssueSeverity.ERROR + test_utils.cleanup_files() + + +def test_allowed_operators_negative_multiple( + monkeypatch, +) -> None: + base_path = "tests/data/allowed_operators/" + target_file_name = f"negative_example_multiple.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + _ = result.get_checker_result( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + ) + + data_type_issues = result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:data_type.allowed_operators" + ) + assert len(data_type_issues) == 4 + assert data_type_issues[0].level == IssueSeverity.ERROR + assert data_type_issues[1].level == IssueSeverity.ERROR + assert data_type_issues[2].level == IssueSeverity.ERROR + assert "}" in data_type_issues[0].locations[0].description + assert "powerer" in data_type_issues[1].locations[0].description + assert "^" in data_type_issues[2].locations[0].description + assert "^" in data_type_issues[3].locations[0].description + test_utils.cleanup_files() + + +def test_non_negative_transition_time_in_light_state_action_positive( + monkeypatch, +) -> None: + base_path = "tests/data/transition_time_should_be_non_negative/" + target_file_name = f"positive_example.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + assert ( + len( + result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:data_type.non_negative_transition_time_in_light_state_action" + ) + ) + == 0 + ) + + test_utils.cleanup_files() + + +def test_non_negative_transition_time_in_light_state_action_negative( + monkeypatch, +) -> None: + base_path = "tests/data/transition_time_should_be_non_negative/" + target_file_name = f"negative_example.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + _ = result.get_checker_result( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + ) + + data_type_issues = result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:data_type.non_negative_transition_time_in_light_state_action" + ) + assert len(data_type_issues) == 1 + assert data_type_issues[0].level == IssueSeverity.ERROR + test_utils.cleanup_files() + + +def test_non_negative_transition_time_in_light_state_action_negative_param( + monkeypatch, +) -> None: + base_path = "tests/data/transition_time_should_be_non_negative/" + target_file_name = f"negative_example_param.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + _ = result.get_checker_result( + checker_bundle_name=constants.BUNDLE_NAME, + checker_id=data_type_constants.CHECKER_ID, + ) + + data_type_issues = result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:data_type.non_negative_transition_time_in_light_state_action" + ) + assert len(data_type_issues) == 1 + assert data_type_issues[0].level == IssueSeverity.ERROR + test_utils.cleanup_files() From bd05f7cf642f9850c20ecbe7cf5db6997f48beee Mon Sep 17 00:00:00 2001 From: romanodanilo <62891297+romanodanilo@users.noreply.github.com> Date: Fri, 26 Jul 2024 12:08:46 +0200 Subject: [PATCH 4/5] Add trailer connect test to improve allowed operator check (#36) Signed-off-by: romanodanilo Signed-off-by: romanodanilo <62891297+romanodanilo@users.noreply.github.com> --- .../data_type_checker/allowed_operators.py | 50 +++-- .../negative_example_multiple.xosc | 2 +- .../TrailerConnect.xosc | 210 ++++++++++++++++++ tests/test_data_type_checks.py | 26 +++ 4 files changed, 272 insertions(+), 16 deletions(-) create mode 100644 tests/data/parametric_operator_in_expression/TrailerConnect.xosc diff --git a/qc_openscenario/checks/data_type_checker/allowed_operators.py b/qc_openscenario/checks/data_type_checker/allowed_operators.py index c1282b5..db8a484 100644 --- a/qc_openscenario/checks/data_type_checker/allowed_operators.py +++ b/qc_openscenario/checks/data_type_checker/allowed_operators.py @@ -14,6 +14,7 @@ from qc_openscenario.checks.data_type_checker import data_type_constants import re +import enum MIN_RULE_VERSION = "1.2.0" RULE_SEVERITY = IssueSeverity.ERROR @@ -34,6 +35,14 @@ ALLOWED_OPERANDS.add("or") +class ExpressionMember(enum.IntEnum): + INVALID = 0 + VARIABLE = 1 + OPERATOR = 2 + NUMBER = 3 + PARENTHESIS = 4 + + @dataclass class QueueNode: element: etree._ElementTree @@ -129,21 +138,32 @@ def check_rule(checker_data: models.CheckerData) -> None: # Remove starting "${" and trailing "}" expression_candidate = attribute.value[2:-1] logging.debug(f"expression_candidate: {expression_candidate}") - # Define the regex pattern to match digits and allowed chars ( ) . and , - pattern = r"[\d()., ]+" - # Split the input string based on the regex pattern - operand_candidates = re.split(pattern, expression_candidate) - # Filter out empty strings from the resulting list - operand_candidates = [ - part - for part in operand_candidates - if part and part != "" and part != " " and not part.startswith("$") - ] - logging.debug(f"operand candidates : {operand_candidates}") - for operand in operand_candidates: - has_issue = operand not in ALLOWED_OPERANDS + + # Tokenize the expression using regular expressions + token_pattern = r"\$[A-Za-z_]\w*|\d+\.\d+|\d+|[\+\-\*/\^%()**//\{\}]|\w+" + tokens = re.findall(token_pattern, expression_candidate) + + token_type = None + logging.debug(f"tokens: {tokens}") + for token in tokens: + if token.startswith("$"): + token_type = ExpressionMember.VARIABLE + elif token in ALLOWED_OPERANDS: + token_type = ExpressionMember.OPERATOR + elif re.match(r"\d+\.\d+", token) or token.isdigit(): + token_type = ExpressionMember.NUMBER + elif token in ["(", ")"]: + token_type = ExpressionMember.PARENTHESIS + else: + token_type = ExpressionMember.INVALID + + logging.debug(f"token {token} - type {token_type}") + if token_type is None: + continue + + has_issue = token_type == ExpressionMember.INVALID if has_issue: - logging.debug(f"Invalid operand {operand}") + logging.debug(f"Invalid operand {token}") xpath = attribute.xpath issue_id = checker_data.result.register_issue( @@ -158,5 +178,5 @@ def check_rule(checker_data: models.CheckerData) -> None: checker_id=data_type_constants.CHECKER_ID, issue_id=issue_id, xpath=xpath, - description=f"Invalid operand {operand} used", + description=f"Invalid operand {token} used", ) diff --git a/tests/data/allowed_operators/negative_example_multiple.xosc b/tests/data/allowed_operators/negative_example_multiple.xosc index b0d46f3..7a36b1f 100644 --- a/tests/data/allowed_operators/negative_example_multiple.xosc +++ b/tests/data/allowed_operators/negative_example_multiple.xosc @@ -21,7 +21,7 @@ - + diff --git a/tests/data/parametric_operator_in_expression/TrailerConnect.xosc b/tests/data/parametric_operator_in_expression/TrailerConnect.xosc new file mode 100644 index 0000000..5dd5637 --- /dev/null +++ b/tests/data/parametric_operator_in_expression/TrailerConnect.xosc @@ -0,0 +1,210 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/test_data_type_checks.py b/tests/test_data_type_checks.py index bf7becb..c787b6b 100644 --- a/tests/test_data_type_checks.py +++ b/tests/test_data_type_checks.py @@ -190,6 +190,32 @@ def test_allowed_operators_positive_multiple( test_utils.cleanup_files() +def test_trailer_connect_standard_example( + monkeypatch, +) -> None: + base_path = "tests/data/parametric_operator_in_expression/" + target_file_name = f"TrailerConnect.xosc" + target_file_path = os.path.join(base_path, target_file_name) + + test_utils.create_test_config(target_file_path) + + test_utils.launch_main(monkeypatch) + + result = Result() + result.load_from_file(test_utils.REPORT_FILE_PATH) + + assert ( + len( + result.get_issues_by_rule_uid( + "asam.net:xosc:1.2.0:data_type.allowed_operators" + ) + ) + == 0 + ) + + test_utils.cleanup_files() + + def test_allowed_operators_negative( monkeypatch, ) -> None: From 821caf1995515f969e7ebd67ca65af7d265c178e Mon Sep 17 00:00:00 2001 From: romanodanilo <62891297+romanodanilo@users.noreply.github.com> Date: Mon, 29 Jul 2024 14:21:19 +0200 Subject: [PATCH 5/5] Check input string before double conversion in non negative checks (#37) Signed-off-by: romanodanilo Signed-off-by: romanodanilo <62891297+romanodanilo@users.noreply.github.com> --- ...gative_transition_time_in_light_state_action.py | 6 ++++++ .../positive_duration_in_phase.py | 6 ++++++ qc_openscenario/checks/utils.py | 14 ++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/qc_openscenario/checks/data_type_checker/non_negative_transition_time_in_light_state_action.py b/qc_openscenario/checks/data_type_checker/non_negative_transition_time_in_light_state_action.py index 548a2fb..9e9bd98 100644 --- a/qc_openscenario/checks/data_type_checker/non_negative_transition_time_in_light_state_action.py +++ b/qc_openscenario/checks/data_type_checker/non_negative_transition_time_in_light_state_action.py @@ -94,6 +94,12 @@ def check_rule(checker_data: models.CheckerData) -> None: continue logging.debug(f"current_transition_time: {current_transition_time}") + if not utils.is_xsd_double(current_transition_time): + logging.error( + f"Cannot convert '{current_transition_time}' to double as it does not match xsd:double pattern. Skipping check..." + ) + return + current_numeric_value = float(current_transition_time) has_issue = current_numeric_value < 0 diff --git a/qc_openscenario/checks/data_type_checker/positive_duration_in_phase.py b/qc_openscenario/checks/data_type_checker/positive_duration_in_phase.py index ad29707..60306bd 100644 --- a/qc_openscenario/checks/data_type_checker/positive_duration_in_phase.py +++ b/qc_openscenario/checks/data_type_checker/positive_duration_in_phase.py @@ -85,6 +85,12 @@ def check_rule(checker_data: models.CheckerData) -> None: continue current_duration = current_duration_param_value + if not utils.is_xsd_double(current_duration): + logging.error( + f"Cannot convert '{current_duration}' to double as it does not match xsd:double pattern. Skipping check..." + ) + return + current_numeric_value = float(current_duration) has_issue = current_numeric_value < 0 diff --git a/qc_openscenario/checks/utils.py b/qc_openscenario/checks/utils.py index d1e0eaa..15c3d1c 100644 --- a/qc_openscenario/checks/utils.py +++ b/qc_openscenario/checks/utils.py @@ -132,3 +132,17 @@ def get_attribute_type(attribute_value: str) -> models.AttributeType: return models.AttributeType.PARAMETER return models.AttributeType.VALUE + + +def is_xsd_double(input_str: str) -> bool: + """Checks if input string follows xsd double specification + The pattern is built following xsd double definition from http://www.datypic.com/sc/xsd/t-xsd_double.html + + Args: + input_str (str): The input string to check + + Returns: + bool: True if the input string represent a valid xsd:double value. False otherwise + """ + pattern = re.compile(r"^([+-]?(\d+(\.\d*)?|\.\d+)([eE][+-]?\d+)?|INF|-INF|NaN)$") + return pattern.match(input_str) is not None