_yaml: Remove 'node_items' and add 'MappingNode.items()'bschubert/node-api-keys

One difference is that 'MappingNode.items()' does not strip the
provenance from scalars and lists, which ends up not affecting the
code much.

10 files changed, 32 insertions, 86 deletions

diff --git a/doc/bst2html.py b/doc/bst2html.py
index c6dcf3673..9d178bc18 100755
--- a/doc/bst2html.py
+++ b/doc/bst2html.py
@@ -348,7 +348,8 @@ def run_session(description, tempdir, source_cache, palette, config_file, force)
     #        a source distribution tarball.
     #
     symlinks = desc.get_mapping('workaround-symlinks', default={})
-    for symlink, target in _yaml.node_items(symlinks):
+    for symlink, target in symlinks.items():
+        target = target.as_str()
 
         # Resolve real path to where symlink should be
         symlink = os.path.join(desc_dir, symlink)
diff --git a/src/buildstream/_options/optionpool.py b/src/buildstream/_options/optionpool.py
index af0f99b97..96cff898d 100644
--- a/src/buildstream/_options/optionpool.py
+++ b/src/buildstream/_options/optionpool.py
@@ -65,7 +65,7 @@ class OptionPool():
     #
     def load(self, options):
 
-        for option_name, option_definition in _yaml.node_items(options):
+        for option_name, option_definition in options.items():
 
             # Assert that the option name is a valid symbol
             p = _yaml.node_get_provenance(options, option_name)
@@ -272,7 +272,7 @@ class OptionPool():
             del node['(?)']
 
             for condition, p in zip(conditions, provenance):
-                tuples = list(_yaml.node_items(condition))
+                tuples = list(condition.items())
                 if len(tuples) > 1:
                     raise LoadError(LoadErrorReason.INVALID_DATA,
                                     "{}: Conditional statement has more than one key".format(p))
@@ -284,7 +284,7 @@ class OptionPool():
                     # Prepend the provenance of the error
                     raise LoadError(e.reason, "{}: {}".format(p, e)) from e
 
-                if not _yaml.is_node(value):
+                if type(value) is not _yaml.MappingNode:  # pylint: disable=unidiomatic-typecheck
                     raise LoadError(LoadErrorReason.ILLEGAL_COMPOSITE,
                                     "{}: Only values of type 'dict' can be composed.".format(p))
 
diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py
index 3c3dc566f..2793b161a 100644
--- a/src/buildstream/_project.py
+++ b/src/buildstream/_project.py
@@ -815,9 +815,9 @@ class Project():
             _yaml.node_validate(mirror, allowed_mirror_fields)
             mirror_name = mirror.get_str('name')
             alias_mappings = {}
-            for alias_mapping, uris in _yaml.node_items(mirror.get_mapping('aliases')):
-                assert isinstance(uris, list)
-                alias_mappings[alias_mapping] = list(uris)
+            for alias_mapping, uris in mirror.get_mapping('aliases').items():
+                assert type(uris) is _yaml.SequenceNode  # pylint: disable=unidiomatic-typecheck
+                alias_mappings[alias_mapping] = uris.as_str_list()
             output.mirrors[mirror_name] = alias_mappings
             if not output.default_mirror:
                 output.default_mirror = mirror_name
diff --git a/src/buildstream/_workspaces.py b/src/buildstream/_workspaces.py
index 3847c7a85..a674ce1f5 100644
--- a/src/buildstream/_workspaces.py
+++ b/src/buildstream/_workspaces.py
@@ -577,16 +577,14 @@ class Workspaces():
 
         if version == 0:
             # Pre-versioning format can be of two forms
-            for element, config in _yaml.node_items(workspaces):
-                if _yaml.is_node(config):
-                    # Get a dict
-                    config = _yaml.node_sanitize(config, dict_type=dict)
+            for element, config in workspaces.items():
+                config_type = type(config)
 
-                if isinstance(config, str):
+                if config_type is _yaml.ScalarNode:
                     pass
 
-                elif isinstance(config, dict):
-                    sources = list(config.items())
+                elif config_type is _yaml.MappingNode:
+                    sources = list(config.values())
                     if len(sources) > 1:
                         detail = "There are multiple workspaces open for '{}'.\n" + \
                                  "This is not supported anymore.\n" + \
@@ -594,21 +592,21 @@ class Workspaces():
                         raise LoadError(LoadErrorReason.INVALID_DATA,
                                         detail.format(element, self._get_filename()))
 
-                    _yaml.node_set(workspaces, element, sources[0][1])
+                    _yaml.node_set(workspaces, element, sources[0])
 
                 else:
                     raise LoadError(LoadErrorReason.INVALID_DATA,
                                     "Workspace config is in unexpected format.")
 
             res = {
-                element: Workspace(self._toplevel_project, path=config)
-                for element, config in _yaml.node_items(workspaces)
+                element: Workspace(self._toplevel_project, path=config.as_str())
+                for element, config in workspaces.items()
             }
 
         elif 1 <= version <= BST_WORKSPACE_FORMAT_VERSION:
             workspaces = workspaces.get_mapping("workspaces", default={})
             res = {element: self._load_workspace(node)
-                   for element, node in _yaml.node_items(workspaces)}
+                   for element, node in workspaces.items()}
 
         else:
             raise LoadError(LoadErrorReason.INVALID_DATA,
diff --git a/src/buildstream/_yaml.pxd b/src/buildstream/_yaml.pxd
index e80df7115..3e0ee86c2 100644
--- a/src/buildstream/_yaml.pxd
+++ b/src/buildstream/_yaml.pxd
@@ -39,6 +39,7 @@ cdef class MappingNode(Node):
     cpdef bint get_bool(self, str key, default=*) except *
     cpdef int get_int(self, str key, default=*) except *
     cpdef str get_str(self, str key, object default=*)
+    cpdef object items(self)
     cpdef list keys(self)
     cpdef void safe_del(self, str key)
     cpdef object values(self)
diff --git a/src/buildstream/_yaml.pyx b/src/buildstream/_yaml.pyx
index e3d3f7df3..ae00924bc 100644
--- a/src/buildstream/_yaml.pyx
+++ b/src/buildstream/_yaml.pyx
@@ -232,6 +232,9 @@ cdef class MappingNode(Node):
         cdef ScalarNode scalar = self.get_scalar(key, default)
         return scalar.as_str()
 
+    cpdef object items(self):
+        return self.value.items()
+
     cpdef list keys(self):
         return list(self.value.keys())
 
@@ -736,20 +739,6 @@ cpdef ProvenanceInformation node_get_provenance(Node node, str key=None, list in
     return ProvenanceInformation(nodeish)
 
 
-cdef list __trim_list_provenance(list value):
-    cdef list ret = []
-    cdef Node entry
-
-    for entry in value:
-        if type(entry.value) is list:
-            ret.append(__trim_list_provenance(entry.value))
-        elif type(entry.value) is dict:
-            ret.append(entry)
-        else:
-            ret.append(entry.value)
-    return ret
-
-
 # node_set()
 #
 # Set an item within the node.  If using `indices` be aware that the entry must
@@ -833,31 +822,6 @@ def node_extend_list(Node node, str key, Py_ssize_t length, object default):
         the_list.append(_create_node(value, file_index, line_num, next_synthetic_counter()))
 
 
-# node_items()
-#
-# A convenience generator for iterating over loaded key/value
-# tuples in a dictionary loaded from project YAML.
-#
-# Args:
-#    node (Node): The dictionary node
-#
-# Yields:
-#    (str): The key name
-#    (anything): The value for the key
-#
-def node_items(Node node):
-    cdef str key
-    cdef Node value
-
-    for key, value in node.value.items():
-        if type(value.value) is dict:
-            yield (key, value)
-        elif type(value.value) is list:
-            yield (key, __trim_list_provenance(value.value))
-        else:
-            yield (key, value.value)
-
-
 # is_node()
 #
 # A test method which returns whether or not the passed in value
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 242fe7b25..13a4b88ef 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -2700,10 +2700,10 @@ class Element(Plugin):
         element_splits = element_bst.get_mapping('split-rules', default={})
 
         # Resolve any variables in the public split rules directly
-        for domain, splits in self.node_items(element_splits):
+        for domain, splits in element_splits.items():
             splits = [
                 self.__variables.subst(split.strip())
-                for split in splits
+                for split in splits.as_str_list()
             ]
             _yaml.node_set(element_splits, domain, splits)
 
@@ -2713,8 +2713,8 @@ class Element(Plugin):
         bstdata = self.get_public_data('bst')
         splits = bstdata.get_mapping('split-rules')
         self.__splits = {
-            domain: re.compile('^(?:' + '|'.join([utils._glob2re(r) for r in rules]) + ')$')
-            for domain, rules in self.node_items(splits)
+            domain: re.compile('^(?:' + '|'.join([utils._glob2re(r) for r in rules.as_str_list()]) + ')$')
+            for domain, rules in splits.items()
         }
 
     # __split_filter():
diff --git a/src/buildstream/plugin.py b/src/buildstream/plugin.py
index f2bd90b35..400b6802a 100644
--- a/src/buildstream/plugin.py
+++ b/src/buildstream/plugin.py
@@ -347,23 +347,6 @@ class Plugin():
         """
         return self.__kind
 
-    def node_items(self, node):
-        """Iterate over a dictionary loaded from YAML
-
-        Args:
-            node (Node): The YAML loaded dictionary object
-
-        Returns:
-           list: List of key/value tuples to iterate over
-
-        BuildStream holds some private data in dictionaries loaded from
-        the YAML in order to preserve information to report in errors.
-
-        This convenience function should be used instead of the dict.items()
-        builtin function provided by python.
-        """
-        yield from _yaml.node_items(node)
-
     def node_provenance(self, node, member_name=None):
         """Gets the provenance for `node` and `member_name`
 
@@ -819,10 +802,10 @@ class Plugin():
             silenced_warnings = set()
             project = self.__project
 
-            for key, value in self.node_items(project.element_overrides):
+            for key, value in project.element_overrides.items():
                 if value.get_bool('suppress-deprecation-warnings', default=False):
                     silenced_warnings.add(key)
-            for key, value in self.node_items(project.source_overrides):
+            for key, value in project.source_overrides.items():
                 if value.get_bool('suppress-deprecation-warnings', default=False):
                     silenced_warnings.add(key)
 
diff --git a/src/buildstream/plugins/elements/junction.py b/src/buildstream/plugins/elements/junction.py
index d30e34828..b21ef0777 100644
--- a/src/buildstream/plugins/elements/junction.py
+++ b/src/buildstream/plugins/elements/junction.py
@@ -188,7 +188,7 @@ class JunctionElement(Element):
         # 3. config['path']
         if self.target and any(self.sources()):
             raise ElementError("junction elements cannot define both 'sources' and 'target' config option")
-        if self.target and any(self.node_items(self.options)):
+        if self.target and any(self.options.items()):
             raise ElementError("junction elements cannot define both 'options' and 'target'")
         if self.target and self.path:
             raise ElementError("junction elements cannot define both 'path' and 'target'")
diff --git a/tests/frontend/project/sources/fetch_source.py b/tests/frontend/project/sources/fetch_source.py
index 7c0df4595..ead5bc3f5 100644
--- a/tests/frontend/project/sources/fetch_source.py
+++ b/tests/frontend/project/sources/fetch_source.py
@@ -40,11 +40,10 @@ class FetchSource(Source):
     def configure(self, node):
         self.original_urls = node.get_sequence('urls').as_str_list()
         self.output_file = node.get_str('output-text')
-        self.fetch_succeeds = {}
-        if 'fetch-succeeds' in node:
-            fetch_succeeds_node = node.get_mapping('fetch-succeeds')
-            for key, value in self.node_items(fetch_succeeds_node):
-                self.fetch_succeeds[key] = value in ('True', 'true')
+        self.fetch_succeeds = {
+            key: value.as_bool()
+            for key, value in node.get_mapping('fetch-succeeds', {}).items()
+        }
 
         # First URL is the primary one for this test
         #