From 8d9e489a2e7055db24ae3791d0f0540675a7a138 Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Wed, 3 Jul 2019 19:40:20 +0100 Subject: _yaml: Move 'value' of Node in each sub node This allows us to type its value more strictly, allowing for more efficient access. - Also implement 'node_at' on 'SequenceNode', to allow some tests to test some internals generically. --- src/buildstream/_yaml.pxd | 8 ++++- src/buildstream/_yaml.pyx | 81 +++++++++++++++++++++++++++++------------------ tests/internals/yaml.py | 21 ++++++++---- 3 files changed, 71 insertions(+), 39 deletions(-) diff --git a/src/buildstream/_yaml.pxd b/src/buildstream/_yaml.pxd index f0f3a4267..a3edd83f0 100644 --- a/src/buildstream/_yaml.pxd +++ b/src/buildstream/_yaml.pxd @@ -22,7 +22,6 @@ cdef class Node: - cdef object value cdef int file_index cdef int line cdef int column @@ -38,6 +37,8 @@ cdef class Node: cdef class MappingNode(Node): + cdef dict value + cpdef void composite(self, MappingNode target) except * cpdef void composite_under(self, MappingNode target) except * cdef Node get(self, str key, default, default_constructor) @@ -61,6 +62,8 @@ cdef class MappingNode(Node): cdef class ScalarNode(Node): + cdef str value + cpdef bint as_bool(self) except * cpdef int as_int(self) except * cpdef str as_str(self) @@ -68,8 +71,11 @@ cdef class ScalarNode(Node): cdef class SequenceNode(Node): + cdef list value + cpdef void append(self, object value) cpdef MappingNode mapping_at(self, int index) + cpdef Node node_at(self, int index, list allowed_types=*) cpdef SequenceNode sequence_at(self, int index) cpdef list as_str_list(self) diff --git a/src/buildstream/_yaml.pyx b/src/buildstream/_yaml.pyx index ba58b39f0..606db0a4a 100644 --- a/src/buildstream/_yaml.pyx +++ b/src/buildstream/_yaml.pyx @@ -54,7 +54,6 @@ _sentinel = object() # than a plain tuple, to distinguish them in things like node_sanitize) # # Members: -# value (str/list/dict): The loaded value. # file_index (int): Index within _FILE_LIST (a list of loaded file paths). # Negative indices indicate synthetic nodes so that # they can be referenced. @@ -63,8 +62,7 @@ _sentinel = object() # cdef class Node: - def __init__(self, object value, int file_index, int line, int column): - self.value = value + def __init__(self, int file_index, int line, int column): self.file_index = file_index self.line = line self.column = column @@ -72,7 +70,7 @@ cdef class Node: @classmethod def from_dict(cls, dict value): if value: - return _new_node_from_dict(value, Node(None, _SYNTHETIC_FILE_INDEX, 0, next_synthetic_counter())) + return _new_node_from_dict(value, MappingNode({}, _SYNTHETIC_FILE_INDEX, 0, next_synthetic_counter())) else: # We got an empty dict, we can shortcut return MappingNode({}, _SYNTHETIC_FILE_INDEX, 0, next_synthetic_counter()) @@ -87,7 +85,7 @@ cdef class Node: # Delegate to the inner value, though this will likely not work # very well if the node is a list or string, it's unlikely that # code which has access to such nodes would do this. - return what in self.value + return what in ( self).value cpdef Node copy(self): raise NotImplementedError() @@ -135,12 +133,25 @@ cdef class Node: cdef class ScalarNode(Node): def __init__(self, object value, int file_index, int line, int column): - if type(value) is str: + super().__init__(file_index, line, column) + + cdef value_type = type(value) + + if value_type is str: value = value.strip() + elif value_type is bool: + if value: + value = "True" + else: + value = "False" + elif value_type is int: + value = str(value) + elif value is None: + pass + else: + raise ValueError("ScalarNode can only hold str, int, bool or None objects") + self.value = value - self.file_index = file_index - self.line = line - self.column = column cpdef ScalarNode copy(self): return self @@ -207,10 +218,8 @@ cdef class ScalarNode(Node): cdef class MappingNode(Node): def __init__(self, dict value, int file_index, int line, int column): + super().__init__(file_index, line, column) self.value = value - self.file_index = file_index - self.line = line - self.column = column cpdef MappingNode copy(self): cdef dict copy = {} @@ -472,22 +481,22 @@ cdef class MappingNode(Node): if prefix is not None: target.value["(<)"] = prefix elif "(<)" in target.value: - target.value["(<)"].value.clear() + ( target.value["(<)"]).value.clear() if suffix is not None: target.value["(>)"] = suffix elif "(>)" in target.value: - target.value["(>)"].value.clear() + ( target.value["(>)"]).value.clear() else: # Not clobbering, so prefix the prefix and suffix the suffix if prefix is not None: if "(<)" in target.value: for v in reversed(prefix.value): - target.value["(<)"].value.insert(0, v) + ( target.value["(<)"]).value.insert(0, v) else: target.value["(<)"] = prefix if suffix is not None: if "(>)" in target.value: - target.value["(>)"].value.extend(suffix.value) + ( target.value["(>)"]).value.extend(suffix.value) else: target.value["(>)"] = suffix @@ -580,10 +589,8 @@ cdef class MappingNode(Node): cdef class SequenceNode(Node): def __init__(self, list value, int file_index, int line, int column): + super().__init__(file_index, line, column) self.value = value - self.file_index = file_index - self.line = line - self.column = column cpdef void append(self, object value): if type(value) in [MappingNode, ScalarNode, SequenceNode]: @@ -612,6 +619,17 @@ cdef class SequenceNode(Node): .format(provenance, path, MappingNode.__name__)) return value + cpdef Node node_at(self, int key, list allowed_types = None): + cdef value = self.value[key] + + if allowed_types and type(value) not in allowed_types: + provenance = node_get_provenance(self) + raise LoadError(LoadErrorReason.INVALID_DATA, + "{}: Value of '{}' is not one of the following: {}.".format( + provenance, key, ", ".join(allowed_types))) + + return value + cpdef SequenceNode sequence_at(self, int index): value = self.value[index] @@ -679,6 +697,8 @@ cdef class SequenceNode(Node): return reversed(self.value) def __setitem__(self, int key, object value): + cdef Node old_value + if type(value) in [MappingNode, ScalarNode, SequenceNode]: self.value[key] = value else: @@ -914,14 +934,14 @@ cdef class Representer: cdef RepresenterState _handle_wait_value_ScalarEvent(self, object ev): key = self.keys.pop() - ( ( self.output[-1]).value)[key] = \ + ( self.output[-1]).value[key] = \ ScalarNode(ev.value, self._file_index, ev.start_mark.line, ev.start_mark.column) return RepresenterState.wait_key cdef RepresenterState _handle_wait_value_MappingStartEvent(self, object ev): cdef RepresenterState new_state = self._handle_doc_MappingStartEvent(ev) key = self.keys.pop() - ( ( self.output[-2]).value)[key] = self.output[-1] + ( self.output[-2]).value[key] = self.output[-1] return new_state cdef RepresenterState _handle_wait_key_MappingEndEvent(self, object ev): @@ -929,7 +949,7 @@ cdef class Representer: # unless it's the last one in which case we leave it if len(self.output) > 1: self.output.pop() - if type(( self.output[-1]).value) is list: + if type(self.output[-1]) is SequenceNode: return RepresenterState.wait_list_item else: return RepresenterState.wait_key @@ -938,13 +958,13 @@ cdef class Representer: cdef RepresenterState _handle_wait_value_SequenceStartEvent(self, object ev): self.output.append(SequenceNode([], self._file_index, ev.start_mark.line, ev.start_mark.column)) - ( ( self.output[-2]).value)[self.keys[-1]] = self.output[-1] + ( self.output[-2]).value[self.keys[-1]] = self.output[-1] return RepresenterState.wait_list_item cdef RepresenterState _handle_wait_list_item_SequenceStartEvent(self, object ev): - self.keys.append(len(( self.output[-1]).value)) + self.keys.append(len(( self.output[-1]).value)) self.output.append(SequenceNode([], self._file_index, ev.start_mark.line, ev.start_mark.column)) - ( ( self.output[-2]).value).append(self.output[-1]) + ( self.output[-2]).value.append(self.output[-1]) return RepresenterState.wait_list_item cdef RepresenterState _handle_wait_list_item_SequenceEndEvent(self, object ev): @@ -959,13 +979,13 @@ cdef class Representer: return RepresenterState.wait_key cdef RepresenterState _handle_wait_list_item_ScalarEvent(self, object ev): - ( self.output[-1]).value.append( + ( self.output[-1]).value.append( ScalarNode(ev.value, self._file_index, ev.start_mark.line, ev.start_mark.column)) return RepresenterState.wait_list_item cdef RepresenterState _handle_wait_list_item_MappingStartEvent(self, object ev): cdef RepresenterState new_state = self._handle_doc_MappingStartEvent(ev) - ( ( self.output[-2]).value).append(self.output[-1]) + ( self.output[-2]).value.append(self.output[-1]) return new_state cdef RepresenterState _handle_doc_DocumentEndEvent(self, object ev): @@ -1109,11 +1129,11 @@ cpdef ProvenanceInformation node_get_provenance(Node node, str key=None, list in return ProvenanceInformation(node) if key and not indices: - return ProvenanceInformation(node.value.get(key)) + return ProvenanceInformation(( node).value.get(key)) - cdef Node nodeish = node.value.get(key) + cdef Node nodeish = ( node).value.get(key) for idx in indices: - nodeish = nodeish.value[idx] + nodeish = ( nodeish).value[idx] return ProvenanceInformation(nodeish) @@ -1179,7 +1199,6 @@ cdef Node _new_node_from_list(list inlist, Node ref_node): ret.value.append(_new_node_from_list(v, ref_node)) else: ret.value.append(ScalarNode(str(v), ref_node.file_index, ref_node.line, next_synthetic_counter())) - return ret diff --git a/tests/internals/yaml.py b/tests/internals/yaml.py index f50d68585..37a2c9931 100644 --- a/tests/internals/yaml.py +++ b/tests/internals/yaml.py @@ -21,7 +21,7 @@ def test_load_yaml(datafiles): 'basics.yaml') loaded = _yaml.load(filename) - assert loaded.value.get('kind').value == 'pony' + assert loaded.get_str('kind') == 'pony' def assert_provenance(filename, line, col, node, key=None, indices=None): @@ -42,7 +42,7 @@ def test_basic_provenance(datafiles): 'basics.yaml') loaded = _yaml.load(filename) - assert loaded.value.get('kind').value == 'pony' + assert loaded.get_str('kind') == 'pony' assert_provenance(filename, 1, 0, loaded) @@ -55,7 +55,7 @@ def test_member_provenance(datafiles): 'basics.yaml') loaded = _yaml.load(filename) - assert loaded.value.get('kind').value == 'pony' + assert loaded.get_str('kind') == 'pony' assert_provenance(filename, 2, 13, loaded, 'description') @@ -67,7 +67,7 @@ def test_element_provenance(datafiles): 'basics.yaml') loaded = _yaml.load(filename) - assert loaded.value.get('kind').value == 'pony' + assert loaded.get_str('kind') == 'pony' assert_provenance(filename, 5, 2, loaded, 'moods', [1]) @@ -101,7 +101,7 @@ def test_node_get(datafiles): 'basics.yaml') base = _yaml.load(filename) - assert base.value.get('kind').value == 'pony' + assert base.get_str('kind') == 'pony' children = base.get_sequence('children') assert isinstance(children, _yaml.SequenceNode) @@ -504,9 +504,16 @@ def test_node_find_target(datafiles, case): # laid out. Client code should never do this. def _walk(node, entry, rest): if rest: - return _walk(node.value[entry], rest[0], rest[1:]) + if isinstance(entry, int): + new_node = node.node_at(entry) + else: + new_node = node.get_node(entry) + + return _walk(new_node, rest[0], rest[1:]) else: - return node.value[entry] + if isinstance(entry, int): + return node.node_at(entry) + return node.get_node(entry) want = _walk(loaded, case[0], case[1:]) found_path = toplevel._find(want) -- cgit v1.2.1