This is an automated email from the ASF dual-hosted git repository. tvb pushed a commit to branch tristan/yaml-fixes in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit 111e4c02225e63c07f85d1cd9a21daa1582ae3c5 Author: Tristan van Berkom <[email protected]> AuthorDate: Sun Jan 24 13:39:02 2021 +0900 node.pyx, node.pxd, node.pyi: Type checking in MappingNode.get_sequence() The MappingNode.get_sequence() accessor now has an allowed_types parameter allowing the caller to obtain a squence with prevalidated element types. --- src/buildstream/node.pxd | 2 +- src/buildstream/node.pyi | 26 +++++++++++++------ src/buildstream/node.pyx | 67 ++++++++++++++++++++++++++++++++---------------- 3 files changed, 64 insertions(+), 31 deletions(-) diff --git a/src/buildstream/node.pxd b/src/buildstream/node.pxd index c26d78a..e18b26f 100644 --- a/src/buildstream/node.pxd +++ b/src/buildstream/node.pxd @@ -51,7 +51,7 @@ cdef class MappingNode(Node): cpdef MappingNode get_mapping(self, str key, default=*) cpdef Node get_node(self, str key, list allowed_types=*, bint allow_none=*) cpdef ScalarNode get_scalar(self, str key, default=*) - cpdef SequenceNode get_sequence(self, str key, object default=*) + cpdef SequenceNode get_sequence(self, str key, object default=*, list allowed_types=*) cpdef str get_str(self, str key, object default=*) cpdef list get_str_list(self, str key, object default=*) cpdef object items(self) diff --git a/src/buildstream/node.pyi b/src/buildstream/node.pyi index 34dc363..a3b0ed9 100644 --- a/src/buildstream/node.pyi +++ b/src/buildstream/node.pyi @@ -12,6 +12,14 @@ class Node: def get_provenance(self) -> ProvenanceInformation: ... def strip_node_info(self) -> Dict[str, Any]: ... +class ScalarNode(Node): + def as_str(self) -> str: ... + def clone(self) -> "ScalarNode": ... + +class SequenceNode(Node, Generic[TNode]): + def as_str_list(self) -> List[str]: ... + def clone(self) -> "SequenceNode[TNode]": ... + class MappingNode(Node, Generic[TNode]): def __init__(self, file_index: int, line: int, column: int, value: Mapping[str, TValidNodeValue]) -> None: ... def clone(self) -> MappingNode[TNode]: ... @@ -46,20 +54,22 @@ class MappingNode(Node, Generic[TNode]): @overload def get_mapping(self, key: str, default: Optional["MappingNode"]) -> Optional["MappingNode"]: ... @overload + def get_sequence(self, key: str, *, allowed_types: Optional[List[Type[Node]]]) -> SequenceNode: ... + @overload + def get_sequence( + self, key: str, default: List[Any], *, allowed_types: Optional[List[Type[Node]]] + ) -> SequenceNode: ... + @overload + def get_sequence( + self, key: str, default: Optional[List[Any]], *, allowed_types: Optional[List[Type[Node]]] + ) -> Optional[SequenceNode]: ... + @overload def get_node(self, key: str) -> Node: ... @overload def get_node(self, key: str, allowed_types: Optional[List[Type[Node]]]) -> Node: ... @overload def get_node(self, key: str, allowed_types: Optional[List[Type[Node]]], allow_none: bool) -> Optional[Node]: ... -class ScalarNode(Node): - def as_str(self) -> str: ... - def clone(self) -> "ScalarNode": ... - -class SequenceNode(Node, Generic[TNode]): - def as_str_list(self) -> List[str]: ... - def clone(self) -> "SequenceNode[TNode]": ... - def _assert_symbol_name( symbol_name: str, purpose: str, *, ref_node: Optional[Node], allow_dashes: bool = True ) -> None: ... diff --git a/src/buildstream/node.pyx b/src/buildstream/node.pyx index 7b46809..48f0739 100644 --- a/src/buildstream/node.pyx +++ b/src/buildstream/node.pyx @@ -683,18 +683,7 @@ cdef class MappingNode(Node): raise LoadError("{}: Dictionary did not contain expected key '{}'".format(provenance, key), LoadErrorReason.INVALID_DATA) - if allowed_types and type(value) not in allowed_types: - provenance = self.get_provenance() - human_types = [] - if MappingNode in allowed_types: - human_types.append("dict") - if SequenceNode in allowed_types: - human_types.append('list') - if ScalarNode in allowed_types: - human_types.append('scalar') - - raise LoadError("{}: Value of '{}' is not one of the following: {}.".format( - provenance, key, ", ".join(human_types)), LoadErrorReason.INVALID_DATA) + __validate_node_type(value, allowed_types, key) return value @@ -728,7 +717,7 @@ cdef class MappingNode(Node): return value - cpdef SequenceNode get_sequence(self, str key, object default=_sentinel): + cpdef SequenceNode get_sequence(self, str key, object default=_sentinel, list allowed_types = None): """get_sequence(key, default=sentinel) Get the value of the node for `key` as a :class:`.SequenceNode`. @@ -737,6 +726,7 @@ cdef class MappingNode(Node): key (str): key for which to get the value default (list): default value to return if `key` is not in the mapping. It will be converted to a :class:`.SequenceNode` before being returned + allowed_types (list): list of valid subtypes of :class:`.Node` that are valid for nodes in the sequence. Raises: :class:`buildstream._exceptions.LoadError`: if the value at `key` is not a @@ -745,14 +735,19 @@ cdef class MappingNode(Node): Returns: :class:`.SequenceNode`: the value at `key` or the default """ - value = self._get(key, default, SequenceNode) + cdef Node value = self._get(key, default, SequenceNode) + cdef Node node if type(value) is not SequenceNode and value is not None: provenance = value.get_provenance() raise LoadError("{}: Value of '{}' is not of the expected type 'list'" .format(provenance, key), LoadErrorReason.INVALID_DATA) - return value + if allowed_types: + for node in value: + __validate_node_type(node, allowed_types) + + return <SequenceNode> value cpdef str get_str(self, str key, object default=_sentinel): """get_str(key, default=sentinel) @@ -1319,13 +1314,7 @@ cdef class SequenceNode(Node): :class:`.Node`: the value at `index` """ cdef value = self.value[index] - - if allowed_types and type(value) not in allowed_types: - provenance = self.get_provenance() - raise LoadError("{}: Value of '{}' is not one of the following: {}.".format( - provenance, index, ", ".join(allowed_types)), - LoadErrorReason.INVALID_DATA) - + __validate_node_type(value, allowed_types, str(index)) return value cpdef ScalarNode scalar_at(self, int index): @@ -1715,3 +1704,37 @@ cdef Node __new_node_from_list(list inlist, Node ref_node): ret.value.append(__create_node_recursive(v, ref_node)) return ret + + +# __validate_node_type(node, allowed_types, key) +# +# Validates that this node is of the expected node type, +# and raises a user facing LoadError if not. +# +# Args: +# allowed_types (list): list of valid subtypes of Node, or None +# key (str): A key, in case the validated node is a value for a key +# +# Raises: +# (LoadError): If this node is not of the expected type +# +cdef void __validate_node_type(Node node, list allowed_types = None, str key = None) except *: + cdef ProvenanceInformation provenance + cdef list human_types + cdef str message + + if allowed_types and type(node) not in allowed_types: + provenance = node.get_provenance() + human_types = [] + if MappingNode in allowed_types: + human_types.append("dict") + if SequenceNode in allowed_types: + human_types.append('list') + if ScalarNode in allowed_types: + human_types.append('scalar') + + message = "{}: Value ".format(provenance) + if key: + message += "of '{}' ".format(key) + message += "is not one of the following: {}.".format(", ".join(human_types)) + raise LoadError(message, LoadErrorReason.INVALID_DATA)
