From 391e216b666c8146504447ae707279f741628b8b Mon Sep 17 00:00:00 2001 From: Jan Gutsche Date: Mon, 5 Oct 2026 21:07:25 +0200 Subject: [PATCH] fix: validate DSD structure with source diagnostics --- docs/language.md | 73 +++ .../dynamic_stack_decider/parser.py | 498 ++++++++---------- .../dynamic_stack_decider/tree.py | 2 + .../test/test_parser_errors.py | 95 +++- 4 files changed, 390 insertions(+), 278 deletions(-) create mode 100644 docs/language.md diff --git a/docs/language.md b/docs/language.md new file mode 100644 index 0000000..459476e --- /dev/null +++ b/docs/language.md @@ -0,0 +1,73 @@ +# Behavior language + +A file contains named subtree definitions and one behavior entry point. Subtrees +must be defined before they are referenced. `-->` introduces the main behavior; +an optional name can follow it, such as `-->HeadBehavior`. Each definition has one +unindented root expression. + +```text +#Search +@LookAround, @SearchBall + +-->HeadBehavior +$BallSeen + YES --> @TrackBall + NO --> #Search +``` + +Decisions start with `$`, actions with `@`, and subtree calls with `#`. Each +decision outcome has one branch. Use `ELSE` to handle otherwise unmapped outcomes. +Names resolve to registered Python classes when the behavior is loaded. + +Indent each decision's branches one level deeper than its expression. Use spaces +in groups of four. An action or subtree call cannot receive further branches. +Only a decision written directly at the end of an action chain can receive +branches on following lines: + +```text +--> +$Mode + ACTIVE --> @Prepare, $BallSeen + YES --> @TrackBall + NO --> @SearchBall + ELSE --> @Idle +``` + +Comma-separated expressions form a chain. Every expression except the last must +be an action; the last can be an action, decision, or subtree call. Completing an +action advances to its successor. Execution timing is defined by the runtime. + +Parameters use `+ key:value`. Literal values are decoded with `yaml.safe_load`. +`%name` looks up a ROS parameter and requires a node. Subtree definitions declare +parameter names with `+ name`, and elements within them reference those values +with `*name`: + +```text +#TimedSearch + duration +@SearchBall + time:*duration + +--> +$BallSeen + YES --> @TrackBall + NO --> #TimedSearch + duration:10 +``` + +The existing parameter syntax does not provide general escaping for DSL +separators. Spaces in parameter values are rejected; commas, colons, plus signs, +and comment markers have syntactic meaning. The parameter grammar and optional +schemas are being discussed in [issue #117](https://github.com/bit-bots/dynamic_stack_decider/issues/117). + +`//` starts a line comment. Block comments start with `//**` and end with `**//`; +they can share lines with code. A block comment must be closed. + +The parser rejects missing or duplicate entry points, duplicate roots, branches, +subtrees or parameters, invalid indentation, decisions without branches, +incomplete definitions, unresolved references, and malformed YAML values. +`ParseError` includes the file and line. Parsed elements retain their definition +locations in `source_file` and `source_line`. Invalid behavior files should be +fixed before loading them into a running agent. + +The legacy subtree alias form `RESULT --> #OtherSubtree` is accepted as the +unindented body of a subtree definition. It copies the referenced subtree and +resolves its parameters. New behavior files can normally call the referenced +subtree directly. diff --git a/dynamic_stack_decider/dynamic_stack_decider/parser.py b/dynamic_stack_decider/dynamic_stack_decider/parser.py index 3c981c4..7e04f36 100644 --- a/dynamic_stack_decider/dynamic_stack_decider/parser.py +++ b/dynamic_stack_decider/dynamic_stack_decider/parser.py @@ -1,7 +1,10 @@ +"""Parse the DSD language into decisions and action chains.""" + from __future__ import annotations import copy import re +from collections.abc import Iterator from pathlib import Path from typing import Generic @@ -12,215 +15,210 @@ from dynamic_stack_decider.types import BlackboardT, ParameterReferences, Parameters +class ParseError(AssertionError): + """Invalid DSD input, with the source file and line when available.""" + + def __init__(self, message: str, path: Path | None = None, line: int | None = None) -> None: + self.path = path + self.line = line + location = f"{path}:{line}" if path is not None else f"line {line}" + super().__init__(f"{location}: {message}" if line is not None else message) + + class DsdParser(Generic[BlackboardT]): def __init__(self, node: Node | None = None) -> None: self.node = node + self._source: Path | None = None + + def _error(self, line: int, message: str) -> ParseError: + return ParseError(message, self._source, line) + + def _lines(self, path: Path) -> Iterator[tuple[int, str]]: + """Strip comments while preserving indentation on the remaining code.""" + block_start: int | None = None + with path.open(encoding="utf-8") as stream: + for line_number, raw_line in enumerate(stream, start=1): + remaining = raw_line + parts: list[str] = [] + while remaining: + if block_start is not None: + _, marker, remaining = remaining.partition("**//") + if not marker: + break + block_start = None + else: + before, marker, after = remaining.partition("//") + parts.append(before) + if not marker: + break + if not after.startswith("**"): + break + block_start = line_number + remaining = after[2:] + line = "".join(parts).rstrip() + if line: + yield line_number, line + if block_start is not None: + raise self._error(block_start, "Unterminated block comment") def parse(self, file_path: str | Path) -> Tree[BlackboardT]: - """ - Parse a .dsd file to a Tree - - :param file_path: the path to the .dsd file to be parsed - :return: a Tree object containing the parsed elements - """ - # The root tree + """Parse and validate a behavior before any implementation is instantiated.""" + self._source = Path(file_path) tree: Tree[BlackboardT] = Tree() - # Dictionary of subtrees that are created - # The key is the name and the value is the corresponding TreeElement subtrees: dict[str, Tree[BlackboardT]] = {} - - current_subtree = tree - current_tree_element: AbstractTreeElement[BlackboardT] | None = None - next_is_start = False - next_is_comment = False - comment = False - last_indent = 0 - lnr = 0 - with open(file_path) as bfile: - for line in bfile: - lnr += 1 - comment = next_is_comment - - line = re.sub(r"//\*\*.*?\*\*//", "", line) # Block comments starting and ending in the same line - - if "**//" in line: - # Block comments ending in this line - # This line as well as the following will contain valid code - next_is_comment = False - comment = False - line = re.sub(r".*\*\*//", "", line) - if "//**" in line: - # Block comments starting in this line - # This line may contain valid code, the next ones won't - next_is_comment = True - line = re.sub(r"//\*\*.*", "", line) - - line = re.sub(r"//.*", "", line) # Line comments - - line = line.rstrip() - if not line: - continue - - if not comment: - indent = len(line) - len(line.lstrip()) - if indent % 4 != 0: - raise ParseError(f"Error parsing line {lnr}: Indent is not a multiple of 4") - - line_content = line.lstrip() - - if indent == 0 and line_content.startswith("-->"): - # This is the declaration of the start. Next line contains root element - next_is_start = True - current_subtree = tree - last_indent = indent - continue - - if next_is_start: - # This line contains the root element of the main tree - next_is_start = False - element = self._create_tree_element(line_content, current_tree_element, lnr) - tree.set_root_element(element) - current_tree_element = element - - if indent == 0 and line_content.startswith("#"): - # This is the declaration of a new subtree - current_subtree = Tree() - name = line_content[1:] - parameter_list = re.split(r"\s*\+\s*", name) - name = parameter_list.pop(0) - current_subtree.parameter_list = parameter_list - subtrees[name] = current_subtree - current_tree_element = None - last_indent = indent - continue - - if indent < last_indent: - # Go layers up, depending on indent difference - for _ in range(indent, last_indent, 4): - if current_tree_element is None: - raise ParseError(f"Error parsing line {lnr}: Invalid indentation") - current_tree_element = current_tree_element.parent - - if re.search(r"\s*-?->\s*", line_content): - # Arrow in line, split in decision result and call - result, call = re.split(r"\s*-?->\s*", line_content, maxsplit=1) - - if call.startswith("#"): - # A subtree is called here. - if current_tree_element is None: - # The current subtree is empty, set the subtree as its root element - subtree, _, _ = self._lookup_subtree(call, lnr, subtrees) - if subtree.root_element is None: - raise ParseError(f"Error parsing line {lnr}: Empty subtree") - current_subtree.set_root_element(subtree.root_element) - else: - # Copy this subtree and place it in the current tree position - subtree_copy = self._instantiate_subtree(call, lnr, subtrees) - if not isinstance(current_tree_element, DecisionTreeElement): - raise ParseError(f"Error parsing line {lnr}: Branch parent must be a decision") - current_tree_element.add_child_element(subtree_copy, result) - - elif re.search(r"\s*,\s*", call): - # A sequence: a chain of actions, optionally ending in a decision or a subtree - tokens = re.split(r"\s*,\s*", call) - head, tail = self._create_chain(tokens, current_tree_element, lnr, subtrees) - if not isinstance(current_tree_element, DecisionTreeElement): - raise ParseError(f"Error parsing line {lnr}: Branch parent must be a decision") - current_tree_element.add_child_element(head, result) - if tokens[-1].startswith("$"): - # A decision at the end of a sequence receives its children from the - # following indented lines - current_tree_element = tail - - elif call.startswith("@"): - # An action is called - element = self._create_tree_element(call, current_tree_element, lnr) - if not isinstance(current_tree_element, DecisionTreeElement): - raise ParseError(f"Error parsing line {lnr}: Branch parent must be a decision") - current_tree_element.add_child_element(element, result) - - elif call.startswith("$"): - # A decision is called - element = self._create_tree_element(call, current_tree_element, lnr) - if not isinstance(current_tree_element, DecisionTreeElement): - raise ParseError(f"Error parsing line {lnr}: Branch parent must be a decision") - current_tree_element.add_child_element(element, result) - current_tree_element = element - - else: - raise ParseError( - f"Error parsing line {lnr}: Element {call} is neither an action nor a decision" - ) - - else: - # No arrow, must be the beginning of a new subtree - if re.search(r"\s*,\s*", line_content): - tokens = re.split(r"\s*,\s*", line_content) - head, tail = self._create_chain(tokens, current_tree_element, lnr, subtrees) - current_subtree.set_root_element(head) - # A decision at the end of a sequence receives its children from the - # following indented lines - current_tree_element = tail if tokens[-1].startswith("$") else head - else: - element = self._create_tree_element(line_content, current_tree_element, lnr) - current_subtree.set_root_element(element) - current_tree_element = element - - last_indent = indent + current: Tree[BlackboardT] | None = None + current_name: str | None = None + current_header = 1 + parents: dict[int, DecisionTreeElement[BlackboardT]] = {} + entry_seen = False + last_line = 1 + + for line_number, line in self._lines(self._source): + last_line = line_number + whitespace = line[: len(line) - len(line.lstrip())] + if "\t" in whitespace: + raise self._error(line_number, "Indentation must use spaces, not tabs") + indent = len(whitespace) + if indent % 4: + raise self._error(line_number, "Indent is not a multiple of 4") + content = line.lstrip() + + if indent == 0 and (content.startswith("-->") or content.startswith("#")): + if current is not None: + self._validate_tree(current, current_header) + if current_name is not None: + subtrees[current_name] = current + parents.clear() + current_header = line_number + if content.startswith("-->"): + if entry_seen: + raise self._error(line_number, "Duplicate behavior entry point") + entry_seen = True + current = tree + current_name = None + else: + tokens = re.split(r"\s*\+\s*", content[1:]) + name, declared = tokens[0], tokens[1:] + if not name or any(not parameter or ":" in parameter for parameter in declared): + raise self._error(line_number, "Invalid subtree declaration") + if name in subtrees: + raise self._error(line_number, f"Duplicate subtree {name!r}") + if len(set(declared)) != len(declared): + raise self._error(line_number, "Duplicate subtree parameter") + current = Tree() + current.parameter_list = declared + current_name = name + continue + + if current is None: + raise self._error(line_number, "Expected a subtree declaration or behavior entry point (-->)") + + if current.root_element is None: + if indent: + raise self._error(line_number, "Root element must not be indented") + # Preserve the legacy labelled subtree alias syntax: RESULT --> #Subtree. + if "->" in content: + _, content = re.split(r"\s*-?->\s*", content, maxsplit=1) + if not content.startswith("#"): + raise self._error(line_number, "A root alias must reference a subtree") + head, tail = self._create_chain(re.split(r"\s*,\s*", content), None, line_number, subtrees) + current.set_root_element(head) + if isinstance(tail, DecisionTreeElement) and not content.split(",")[-1].strip().startswith("#"): + parents[4] = tail + continue + + if indent == 0: + raise self._error(line_number, "Duplicate root element") + parent = parents.get(indent) + if parent is None: + raise self._error(line_number, "Invalid indentation: branches must belong to a decision") + if "->" not in content: + raise self._error(line_number, "Expected a decision outcome followed by -->") + result, call = re.split(r"\s*-?->\s*", content, maxsplit=1) + if not result: + raise self._error(line_number, "Decision outcome must not be empty") + if result in parent.children: + raise self._error(line_number, f"Duplicate outcome {result!r} on decision {parent.name!r}") + tokens = re.split(r"\s*,\s*", call) + head, tail = self._create_chain(tokens, parent, line_number, subtrees) + parent.add_child_element(head, result) + parents = {level: decision for level, decision in parents.items() if level <= indent} + if isinstance(tail, DecisionTreeElement) and not tokens[-1].startswith("#"): + parents[indent + 4] = tail + + if current is not None: + self._validate_tree(current, current_header) + if not entry_seen: + raise self._error(last_line, "Missing behavior entry point (-->)") return tree + def _validate_tree(self, tree: Tree[BlackboardT], line: int) -> None: + if tree.root_element is None: + raise self._error(line, "Definition has no root element") + pending = [tree.root_element] + while pending: + element = pending.pop() + unknown = set(element.unset_parameters.values()) - set(tree.parameter_list) + if unknown: + raise self._error(element.source_line or line, f"Unknown reference: {', '.join(sorted(unknown))}") + if isinstance(element, DecisionTreeElement): + if not element.children: + raise self._error(element.source_line or line, f"Decision {element.name!r} has no branches") + pending.extend(element.children.values()) + elif isinstance(element, ActionTreeElement) and element.next_element is not None: + pending.append(element.next_element) + def _extract_parameters(self, token: str, lnr: int) -> tuple[str, Parameters, ParameterReferences]: - """ - Extract parameters from a token string in the form of name + key1:value1 + key2:value2 - :param token: the string containing the name and the parameters - :param lnr: Line number of the current line (used for error messages) - :return: the name, a dict of set parameters, a dict of unset parameters - """ - parameters = re.split(r"\s*\+\s*", token) - name = parameters.pop(0) - parameter_dict: Parameters = {} - unset_parameters: ParameterReferences = {} - for parameter in parameters: + tokens = re.split(r"\s*\+\s*", token) + name = tokens.pop(0) + if not name: + raise self._error(lnr, "Element name must not be empty") + parameters: Parameters = {} + references: ParameterReferences = {} + for token in tokens: try: - parameter_key, parameter_value = parameter.split(":") - except ValueError as e: - raise ParseError(f"Error parsing line {lnr}: Invalid parameter list") from e - - if " " in parameter_value: - raise ParseError( - f"Error parsing line {lnr}: Parameter values should not contain spaces. Did you forget a comma?" - ) - - if parameter_value.startswith("%"): - parameter_dict[parameter_key] = ( - self.node.get_parameter(parameter_value[1:]).value if self.node is not None else None - ) - elif parameter_value.startswith("*"): - # This is a reference to the value specified in the subtree - unset_parameters[parameter_key] = parameter_value[1:] + key, value = token.split(":") + except ValueError as error: + raise self._error(lnr, "Invalid parameter list") from error + if not key or not value: + raise self._error(lnr, "Parameter key and value must not be empty") + if key in parameters or key in references: + raise self._error(lnr, f"Duplicate parameter {key!r}") + if " " in value: + raise self._error(lnr, "Parameter values should not contain spaces. Did you forget a comma?") + if value in ("%", "*"): + raise self._error(lnr, "Parameter reference must not be empty") + if value.startswith("%"): + if self.node is None: + raise self._error(lnr, "ROS parameter lookup requires a ROS node") + try: + parameters[key] = self.node.get_parameter(value[1:]).value + except Exception as error: + raise self._error(lnr, f"Cannot resolve ROS parameter {value[1:]!r}: {error}") from error + elif value.startswith("*"): + references[key] = value[1:] else: - parameter_dict[parameter_key] = yaml.safe_load(parameter_value) - return name, parameter_dict, unset_parameters + try: + parameters[key] = yaml.safe_load(value) + except yaml.YAMLError as error: + raise self._error(lnr, f"Invalid YAML parameter {key!r}: {error}") from error + return name, parameters, references def _create_tree_element( self, token: str, parent: AbstractTreeElement[BlackboardT] | None, lnr: int ) -> AbstractTreeElement[BlackboardT]: - """ - Create a tree element given a token and a parent. - The method derives the type (Action/Decision) and optional parameters from the token - :param token: the string describing the element in the dsd description - :param parent: the parent element of the new element, None for root - :param lnr: Line number of the current line (used for error messages) - :return: a TreeElement containing the information given in token - """ - name, parameter_dict, unset_parameters = self._extract_parameters(token[1:], lnr) + if not token.startswith(("$", "@")): + raise self._error( + lnr, "An element has to start with either $ or @; neither an action nor a decision was given" + ) + name, parameters, references = self._extract_parameters(token[1:], lnr) element: AbstractTreeElement[BlackboardT] if token.startswith("$"): - element = DecisionTreeElement(name, parent, parameter_dict, unset_parameters) - elif token.startswith("@"): - element = ActionTreeElement(name, parent, parameter_dict, unset_parameters) + element = DecisionTreeElement(name, parent, parameters, references) else: - raise ParseError("An element has to start with either $ or @") + element = ActionTreeElement(name, parent, parameters, references) + element.source_file = str(self._source) if self._source is not None else None + element.source_line = lnr return element def _create_chain( @@ -230,88 +228,53 @@ def _create_chain( lnr: int, subtrees: dict[str, Tree[BlackboardT]], ) -> tuple[AbstractTreeElement[BlackboardT], AbstractTreeElement[BlackboardT]]: - """ - Create a sequence as a chain of tree elements linked via ``next_element``. - - Every element except the last must be an action, as only actions advance to their successor - by popping themselves. The last element may be an action, a decision, or a subtree call, which - allows a decision or a subtree to follow a sequence of actions. - - :param tokens: The call tokens of the sequence (e.g. ``["@A", "@B", "$D"]``) - :param parent: The element the chain is attached to - :param lnr: Line number of the current line (used for error messages) - :param subtrees: The already parsed subtrees, needed to resolve a trailing subtree call - :return: A tuple of the head element (the child of ``parent``) and the tail element of the chain - """ head: AbstractTreeElement[BlackboardT] | None = None previous: AbstractTreeElement[BlackboardT] | None = None - for i, token in enumerate(tokens): - is_last = i == len(tokens) - 1 - if is_last and token.startswith("#"): - # A subtree may terminate a sequence - element = self._instantiate_subtree(token, lnr, subtrees) - elif is_last: - # The last element may be an action or a decision - element = self._create_tree_element(token, parent, lnr) - else: - # Only actions may be followed by another element in a sequence - if not token.startswith("@"): - raise ParseError( - f"Error parsing line {lnr}: Only actions may be followed by another element " - f"in a sequence, but got '{token}'" - ) - element = self._create_tree_element(token, parent, lnr) - + for index, token in enumerate(tokens): + if index < len(tokens) - 1 and not token.startswith("@"): + raise self._error(lnr, f"Only actions may be followed by another element in a sequence, got {token!r}") + element = ( + self._instantiate_subtree(token, lnr, subtrees) + if token.startswith("#") + else self._create_tree_element(token, parent, lnr) + ) + element.parent = parent if previous is None: head = element - else: - if not isinstance(previous, ActionTreeElement): - raise ParseError(f"Error parsing line {lnr}: Only actions may be followed") + elif isinstance(previous, ActionTreeElement): previous.next_element = element previous = element if head is None or previous is None: - raise ParseError(f"Error parsing line {lnr}: Empty action chain") + raise self._error(lnr, "Empty action chain") return head, previous def _lookup_subtree( self, call: str, lnr: int, subtrees: dict[str, Tree[BlackboardT]] ) -> tuple[Tree[BlackboardT], Parameters, ParameterReferences]: - """ - Look up the subtree referenced by a ``#``-call and validate that its parameters match. - - :param call: The call token, e.g. ``#SubBehavior + key:value`` - :param lnr: Line number of the current line (used for error messages) - :param subtrees: The already parsed subtrees - :return: A tuple of the referenced subtree and the set and unset parameters of the call - """ - name, parameters, unset_parameters = self._extract_parameters(call.strip("#"), lnr) + name, parameters, references = self._extract_parameters(call[1:], lnr) if name not in subtrees: - raise ParseError(f"Error parsing line {lnr}: {name} not defined") + raise self._error(lnr, f"Subtree {name!r} not defined") subtree = subtrees[name] - if (set(parameters.keys()) | set(unset_parameters.keys())) != set(subtree.parameter_list): - raise ParseError( - f"Error parsing line {lnr}: Invalid parameters specified.\n" - f"Available parameters are {set(parameters.keys()) | set(unset_parameters.keys())}, specified parameters are {set(subtree.parameter_list)}." + specified = set(parameters) | set(references) + available = set(subtree.parameter_list) + if specified != available: + raise self._error( + lnr, + f"Invalid parameters specified. Available parameters are {sorted(available)}, " + f"specified parameters are {sorted(specified)}.", ) - return subtree, parameters, unset_parameters + return subtree, parameters, references def _instantiate_subtree( self, call: str, lnr: int, subtrees: dict[str, Tree[BlackboardT]] ) -> AbstractTreeElement[BlackboardT]: - """ - Create a deep copy of a referenced subtree with its parameters evaluated. - - :param call: The call token, e.g. ``#SubBehavior + key:value`` - :param lnr: Line number of the current line (used for error messages) - :param subtrees: The already parsed subtrees - :return: The root element of the instantiated subtree - """ - subtree, parameters, unset_parameters = self._lookup_subtree(call, lnr, subtrees) + subtree, parameters, references = self._lookup_subtree(call, lnr, subtrees) if subtree.root_element is None: - raise ParseError(f"Error parsing line {lnr}: Empty subtree") - subtree_copy = copy.deepcopy(subtree.root_element) - self._evaluate_subtree_parameters(subtree_copy, parameters, unset_parameters, lnr) - return subtree_copy + raise self._error(lnr, "Empty subtree") + root = copy.deepcopy(subtree.root_element) + root.parent = None + self._evaluate_subtree_parameters(root, parameters, references, lnr) + return root def _evaluate_subtree_parameters( self, @@ -320,33 +283,18 @@ def _evaluate_subtree_parameters( unset_parameters: ParameterReferences, lnr: int, ) -> None: - """ - Resolve the unset parameters of a copied subtree against the parameters given in the call. - - :param root: The root element of the copied subtree - :param parameters: The set parameters of the subtree call - :param unset_parameters: The unset (referenced) parameters of the subtree call - :param lnr: Line number of the current line (used for error messages) - """ - to_evaluate = [root] - while len(to_evaluate) > 0: - current = to_evaluate.pop(0) + pending = [root] + while pending: + current = pending.pop() if isinstance(current, DecisionTreeElement): - to_evaluate.extend(current.children.values()) + pending.extend(current.children.values()) elif isinstance(current, ActionTreeElement) and current.next_element is not None: - # Follow the chain of actions (i.e. a sequence) - to_evaluate.append(current.next_element) - - for name, reference in current.unset_parameters.items(): + pending.append(current.next_element) + for name, reference in list(current.unset_parameters.items()): if reference in parameters: current.parameters[name] = parameters[reference] + del current.unset_parameters[name] elif reference in unset_parameters: current.unset_parameters[name] = unset_parameters[reference] else: - raise ParseError(f"Error evaluating subtree call in line {lnr}: Unknown reference to {reference}.") - - -class ParseError(AssertionError): - """An error during the DSD file parsing occurred""" - - pass + raise self._error(lnr, f"Unknown reference to {reference!r}") diff --git a/dynamic_stack_decider/dynamic_stack_decider/tree.py b/dynamic_stack_decider/dynamic_stack_decider/tree.py index 9752b65..496e8f0 100644 --- a/dynamic_stack_decider/dynamic_stack_decider/tree.py +++ b/dynamic_stack_decider/dynamic_stack_decider/tree.py @@ -50,6 +50,8 @@ def __init__( parameters: Parameters | None = None, unset_parameters: ParameterReferences | None = None, ) -> None: + self.source_file: str | None = None + self.source_line: int | None = None self.name = name self.parent = parent self.parameters = parameters or dict() diff --git a/dynamic_stack_decider/test/test_parser_errors.py b/dynamic_stack_decider/test/test_parser_errors.py index 4afaf32..4e0664f 100644 --- a/dynamic_stack_decider/test/test_parser_errors.py +++ b/dynamic_stack_decider/test/test_parser_errors.py @@ -94,6 +94,95 @@ def test_node_provided_parameter(): assert tree.root_element.get_child("GO").parameters == {"key": 5} -def test_node_provided_parameter_without_node_is_none(): - tree = parse("-->T\n$Chooser\n GO --> @FirstAction + key:%speed\n", node=None) - assert tree.root_element.get_child("GO").parameters == {"key": None} +def test_node_provided_parameter_without_node_is_rejected(): + with pytest.raises(ParseError, match="requires a ROS node"): + parse("-->T\n$Chooser\n GO --> @FirstAction + key:%speed\n", node=None) + + +@pytest.mark.parametrize( + ("source", "message"), + [ + ("", "Missing behavior entry"), + ("@Idle\n", "Expected a subtree declaration"), + ("-->\n", "no root element"), + ("-->\n@Idle\n-->\n@Run\n", "Duplicate behavior entry"), + ("-->\n@Idle\n@Run\n", "Duplicate root"), + ("-->\n @Idle\n", "Root element must not be indented"), + ("-->\n$Guard\n GO --> @Idle\n", "Invalid indentation"), + ("-->\n$Guard\n\tGO --> @Idle\n", "not tabs"), + ("-->\n$Guard\n GO --> @Idle\n BAD --> @Run\n", "Invalid indentation"), + ( + "#Sub\n$Guard\n GO --> @Idle\n-->\n$Chooser\n GO --> #Sub\n BAD --> @Run\n", + "Invalid indentation", + ), + ("-->\n$Guard\n GO --> @StepA, @Idle\n BAD --> @Run\n", "Invalid indentation"), + ("-->\n$Guard\n GO --> @Idle\n GO --> @Run\n", "Duplicate outcome"), + ("-->\n$Guard\n --> @Idle\n", "outcome must not be empty"), + ("-->\n$Guard\n", "has no branches"), + ("#Sub\n@Idle\n#Sub\n@Run\n-->\n@Idle\n", "Duplicate subtree"), + ("#Sub\n-->\n@Idle\n", "no root element"), + ("#Sub + x + x\n@Idle\n-->\n@Idle\n", "Duplicate subtree parameter"), + ("-->\n@Idle + x:1 + x:2\n", "Duplicate parameter"), + ("-->\n@Idle + x:1 + x:*other\n", "Duplicate parameter"), + ("-->\n@Idle + x:*missing\n", "Unknown reference"), + ("-->\n@Idle + x:[\n", "Invalid YAML parameter"), + ("-->\n@Idle + x:\n", "must not be empty"), + ("-->\n@Idle + :1\n", "must not be empty"), + ("-->\n@\n", "name must not be empty"), + ("-->\n@Idle + x:*\n", "reference must not be empty"), + ("#Sub\n$Guard\n A --> @Idle\n B --> #Sub\n-->\n@Idle\n", "not defined"), + ("-->\n@Idle\n//** never closed\n", "Unterminated block comment"), + ], +) +def test_malformed_structure_is_rejected(source, message): + with pytest.raises(ParseError, match=message): + parse(source) + + +def test_error_has_source_location(tmp_path): + path = tmp_path / "broken.dsd" + path.write_text("-->\n$Guard\n GO --> @Idle\n GO --> @Run\n") + with pytest.raises(ParseError) as caught: + DsdParser().parse(path) + assert caught.value.path == path + assert caught.value.line == 4 + assert f"{path}:4:" in str(caught.value) + + +def test_sibling_after_chain_decision_attaches_to_outer_decision(): + tree = parse("-->\n$Chooser\n FIRST --> @StepA, $Guard\n GO --> @Idle\n SECOND --> @Run\n") + assert set(tree.root_element.children) == {"FIRST", "SECOND"} + assert set(tree.root_element.get_child("FIRST").next_element.children) == {"GO"} + + +def test_resolved_subtree_references_do_not_leak_into_outer_calls(): + tree = parse( + "#Inner + value\n@Idle + x:*value\n" + "#Outer + other\n$Guard\n GO --> #Inner + value:fixed\n" + " ELSE --> @Run + x:*other\n" + "-->\n$Chooser\n GO --> #Outer + other:outer\n" + ) + inner = tree.root_element.get_child("GO").get_child("GO") + assert inner.parameters == {"x": "fixed"} + assert inner.unset_parameters == {} + + +def test_parameterized_root_alias_is_copied_and_resolved(): + tree = parse( + "#Inner + value\n@Idle + x:*value\n" + "#Outer + other\nALIAS --> #Inner + value:*other\n" + "-->\n$Chooser\n GO --> #Outer + other:resolved\n" + ) + assert tree.root_element.get_child("GO").parameters == {"x": "resolved"} + + +def test_copied_subtree_root_has_its_calling_parent(): + tree = parse("#Sub\n@Idle\n-->\n$Chooser\n GO --> #Sub\n") + child = tree.root_element.get_child("GO") + assert child.parent is tree.root_element + assert child.source_line == 2 + + +def test_sibling_after_nested_decision_closes_deeper_indentation(): + with pytest.raises(ParseError, match="Invalid indentation"): + parse("-->\n$Chooser\n FIRST --> $Guard\n GO --> @Idle\n SECOND --> @Run\n BAD --> @Idle\n")