pyang icon indicating copy to clipboard operation
pyang copied to clipboard

Expanded `uses` do not set parent element correctly on `type` substatements

Open ian-h-chamberlain opened this issue 6 years ago • 2 comments

Consider the following module:

module example-model {

    namespace "urn:xml:ns:test";

    prefix ex;

    grouping group-with-ref {
        leaf grouped-ref {
            type leafref {
                path "../root-leaf";
            }
        }
    }

    container root-container {

        leaf root-leaf {
            type string;
        }

        uses group-with-ref;
    }
}

And the following plugin:

from pyang import plugin


def pyang_plugin_init():
    plugin.register_plugin(ExamplePlugin())


class ExamplePlugin(plugin.PyangPlugin):

    def add_output_format(self, formats):
        formats['example'] = self

    def emit(self, _ctx, modules, writef):
        all_stmts = self._walk_statements(modules[0])
        writef.write("\n".join(all_stmts))

    def _walk_statements(self, statement, indent=0):
        result = []
        result.append("{}{}".format(
            " " * indent,
            self._format_stmt(statement)))

        for substatement in statement.substmts:
            result.extend(self._walk_statements(substatement, indent + 2))

        return result

    def _format_stmt(self, statement):
        if statement is None:
            return "None"

        if statement.keyword == "type" and statement.arg == "leafref":
            data_node = statement.parent
            referenced_leaf, pos = getattr(
                data_node, "i_leafref_ptr", None) or (None, None)

            if referenced_leaf:
                return "type: leafref -> {}".format(
                    self._format_stmt(referenced_leaf.search_one('type')))
            else:
                return "type: leafref -> None"

        return "{}: {}".format(
            statement.keyword,
            getattr(statement, "arg", None))

Expected output:

$ pyang --plugindir pyang-example/ -f example pyang-example/example-model.yang
module: example-model
  namespace: urn:xml:ns:test
  prefix: ex
  grouping: group-with-ref
    leaf: grouped-ref
      type: leafref -> type: string
        path: ../root-leaf
  container: root-container
    leaf: root-leaf
      type: string
    uses: group-with-ref

Actual output:

$ pyang --plugindir pyang-example/ -f example pyang-example/example-model.yang
module: example-model
  namespace: urn:xml:ns:test
  prefix: ex
  grouping: group-with-ref
    leaf: grouped-ref
      type: leafref -> None
        path: ../root-leaf
  container: root-container
    leaf: root-leaf
      type: string
    uses: group-with-ref

This happens because in the implementation of statements.Statement.copy(), the parent is not updated when a substatement is nocopy:

        for s in self.substmts:
            if s.keyword in ignore:
                pass
            elif s.keyword in nocopy:
                new.substmts.append(s)
                s.parent = new # <--- add this line to get expected output
            else:
                new.substmts.append(s.copy(new, uses, False,
                                           nocopy, ignore, copyf))

Since type is in nocopywhen a uses is being expanded, the expected output does not contain type information, since the newly copied substatement still references the old parent which has not been expanded.

ian-h-chamberlain avatar Nov 27 '19 17:11 ian-h-chamberlain

Hmm. What do you think is the expected output from this module:

module example-model {
    namespace "urn:xml:ns:test";
    prefix ex;

    grouping group-with-ref {
        leaf grouped-ref {
            type leafref {
                path "../root-leaf";
            }
        }
    }

    container root-container {
        leaf root-leaf {
            type string;
        }
        uses group-with-ref;
    }

    container root-container2 {
        leaf root-leaf {
            type int32;
        }
        uses group-with-ref;
    }
}

mbj4668 avatar Dec 02 '19 09:12 mbj4668

Good point, I think I wrote up my example a little too quickly and missed that case. Really, what I want is for leafrefs within a grouping to be able to resolve their type where the uses is instantiated.

Here is an updated example:

from pyang import plugin


def pyang_plugin_init():
    plugin.register_plugin(ExamplePlugin())


_DATA_KEYWORDS = {
    "container",
    "leaf",
    "leaf-list",
    "list",
    "choice",
    "anyxml",
    "anydata",
    "uses",
    "augment",
}


class ExamplePlugin(plugin.PyangPlugin):
    def add_output_format(self, formats):
        formats["example"] = self

    def emit(self, _ctx, modules, writef):
        all_stmts = self._walk_statements(modules[0])
        writef.write("\n".join(all_stmts))
        writef.write("\n")

    def _walk_statements(self, statement, indent=0):
        result = []
        result.append("{}{}".format(" " * indent, self._format_stmt(statement)))

        for substmt in getattr(statement, "substmts", []):
            if substmt.keyword not in _DATA_KEYWORDS:
                result.extend(self._walk_statements(substmt, indent + 2))

        for child in getattr(statement, "i_children", []):
            result.extend(self._walk_statements(child, indent + 2))

        return result

    def _format_stmt(self, statement):
        if statement is None:
            return "None"

        if statement.keyword == "type" and statement.arg == "leafref":
            data_node = statement.parent
            referenced_leaf, pos = getattr(data_node, "i_leafref_ptr", None) or (
                None,
                None,
            )

            if referenced_leaf:
                return "type: leafref -> {}".format(
                    self._format_stmt(referenced_leaf.search_one("type"))
                )
            else:
                return "type: leafref -> None"

        return "{}: {}".format(statement.keyword, getattr(statement, "arg", None))

Model:

module example-model {
    namespace "urn:xml:ns:test";
    prefix ex;

    grouping group-with-ref {
        leaf grouped-ref {
            type leafref {
                path "../root-leaf";
            }
        }
    }

    container root-container {
        leaf root-leaf {
            type string;
        }
        uses group-with-ref;

        leaf ungrouped-ref {
            type leafref {
                path "../root-leaf";
            }
        }
    }

    container root-container2 {
        leaf root-leaf {
            type int32;
        }
        uses group-with-ref;

        leaf ungrouped-ref {
            type leafref {
                path "../root-leaf";
            }
        }
    }
}

Actual output:

$ pyang --plugindir . -f example example-model.yang
module: example-model
  namespace: urn:xml:ns:test
  prefix: ex
  grouping: group-with-ref
    leaf: grouped-ref
      type: leafref -> None
        path: ../root-leaf
  container: root-container
    leaf: root-leaf
      type: string
    leaf: grouped-ref
      type: leafref -> None
        path: ../root-leaf
    leaf: ungrouped-ref
      type: leafref -> type: string
        path: ../root-leaf
  container: root-container2
    leaf: root-leaf
      type: int32
    leaf: grouped-ref
      type: leafref -> None
        path: ../root-leaf
    leaf: ungrouped-ref
      type: leafref -> type: int32
        path: ../root-leaf

Expected output:

$ pyang --plugindir . -f example example-model.yang
module: example-model
  namespace: urn:xml:ns:test
  prefix: ex
  grouping: group-with-ref
    leaf: grouped-ref
      type: leafref -> None
        path: ../root-leaf
  container: root-container
    leaf: root-leaf
      type: string
    leaf: grouped-ref
      type: leafref -> type: string
        path: ../root-leaf
    leaf: ungrouped-ref
      type: leafref -> type: string
        path: ../root-leaf
  container: root-container2
    leaf: root-leaf
      type: int32
    leaf: grouped-ref
      type: leafref -> type: int32
        path: ../root-leaf
    leaf: ungrouped-ref
      type: leafref -> type: int32
        path: ../root-leaf

It's clear that my proposed change does not solve this. There is a comment in statement.py which indicates the non-copy of the type is intentional:

        # don't copy the type since it cannot be modified anyway.
        # not copying the type also works better for some plugins that
        # generate output from the i_children list.
        def post_copy(old, new):

It appears I can get my desired output by undoing this:

diff --git a/pyang/statements.py b/pyang/statements.py
index d1408c5..9366314 100644
--- a/pyang/statements.py
+++ b/pyang/statements.py
@@ -1565,12 +1565,12 @@ def v_expand_1_uses(ctx, stmt):
                     else:
                         # otherwise, copy the i_child
                         newx = x.copy(new, stmt,
-                                      nocopy=['type','uses', 'unique',
+                                      nocopy=['uses', 'unique',
                                               'typedef','grouping'],
                                       copyf=post_copy)
                         new.i_children.append(newx)
         newg = g.copy(stmt.parent, stmt,
-                      nocopy=['type','uses','unique','typedef','grouping'],
+                      nocopy=['uses','unique','typedef','grouping'],
                       copyf=post_copy)
         newg.substmts.extend(whens)
         newg.substmts.extend(iffeatures)

Is this likely to break other plugins? If so, is there another way the type information could be updated per-instantiation so I can retrieve the referenced type within the uses?

ian-h-chamberlain avatar Dec 02 '19 16:12 ian-h-chamberlain