Merge pull request #54 from openconfig/fix-lists-no-sibling-check

Fix sibling check for lists.

.github/workflows/py.yml

@@ -12,7 +12,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python-version: [ '2.7', '3.7', '3.8', '3.9', '3.x' ]
+        python-version: [ '3.9', '3.10', '3.11' ]
       fail-fast: false
     name: Python ${{ matrix.python-version }} Packaging Test
 

openconfig_pyang/plugins/openconfig.py

@@ -304,12 +304,11 @@ def setup_ctx(self, ctx):
         "OC_OPSTATE_APPLIED_CONFIG", ErrorLevel.MAJOR,
         "\"%s\" is not mirrored in the state container at %s")
 
-    # a list is within a container that has elements other than the list
-    # within it
+    # a list that has siblings.
     error.add_error_code(
-        "OC_LIST_SURROUNDING_CONTAINER", ErrorLevel.MAJOR,
-        "List %s is within a container (%s) that has other elements "
-        "within it: %s")
+        "OC_LIST_HAS_SIBLING", ErrorLevel.MAJOR,
+        "Parent node (%s) has multiple children where one is a list, but a list "
+        "is not allowed to have siblings: (%s)")
 
     # a list that does not have a container above it
     error.add_error_code(
@@ -506,6 +505,9 @@ def openconfig_reference(ctx, stmt):
         stmt: pyang.Statement matching the validation call
     """
     validmap = {
+        "*": [
+            OCLintFunctions.check_list_no_sibling,
+        ],
         u"LEAVES": [
             OCLintFunctions.check_opstate,
         ],
@@ -881,6 +883,33 @@ def check_opstate(ctx, stmt):
           err_add(ctx.errors, stmt.pos, "OC_OPSTATE_CONTAINER_NAME",
                   (stmt.arg, pathstr))
 
+  @staticmethod
+  def check_list_no_sibling(ctx, stmt):
+    """Check that a list has no sibling, and a non-list doesn't have a list as a
+    sibling.
+
+    Args:
+      ctx: pyang.Context for the validation
+      stmt: pyang.Statement for the node
+    """
+
+    if stmt.parent is None:
+      return
+    siblings = [(i.keyword, i.arg) for i in stmt.parent.i_children
+                       if i.keyword in INSTANTIATED_DATA_KEYWORDS]
+
+    has_list = False
+    for s in siblings:
+      if s[0] == "list":
+        has_list = True
+        break
+
+    if has_list and len(siblings) > 1:
+      err_add(ctx.errors, stmt.parent.pos,
+              "OC_LIST_HAS_SIBLING",
+              (stmt.parent.arg,
+               ", ".join((f"{s[0]}: {s[1]}" for s in siblings))))
+
   @staticmethod
   def check_list_enclosing_container(ctx, stmt):
     """Check that a list has an enclosing container and that its
@@ -899,22 +928,14 @@ def check_list_enclosing_container(ctx, stmt):
               "OC_LIST_NO_ENCLOSING_CONTAINER", stmt.arg)
 
     grandparent = stmt.parent.parent
-    for ch in grandparent.i_children:
-      if ch.keyword == "container" and ch.arg != stmt.parent.arg:
-        if len(ch.i_children) == 1 and ch.i_children[0].arg == stmt.arg \
-            and ch.i_children[0].keyword == "list":
-          err_add(ctx.errors, stmt.parent.pos,
-                  "OC_LIST_DUPLICATE_COMPRESSED_NAME",
-                  (stmt.arg, stmt.parent.arg))
-
-    if parent_substmts != [stmt.arg]:
-      remaining_parent_substmts = [i.arg for i in stmt.parent.i_children
-                                   if i.arg != stmt.arg and i.keyword
-                                   in INSTANTIATED_DATA_KEYWORDS]
-      err_add(ctx.errors, stmt.parent.pos,
-              "OC_LIST_SURROUNDING_CONTAINER",
-              (stmt.arg, stmt.parent.arg,
-               ", ".join(remaining_parent_substmts)))
+    if grandparent:
+      for ch in grandparent.i_children:
+        if ch.keyword == "container" and ch.arg != stmt.parent.arg:
+          if len(ch.i_children) == 1 and ch.i_children[0].arg == stmt.arg \
+              and ch.i_children[0].keyword == "list":
+            err_add(ctx.errors, stmt.parent.pos,
+                    "OC_LIST_DUPLICATE_COMPRESSED_NAME",
+                    (stmt.arg, stmt.parent.arg))
 
   @staticmethod
   def check_leaf_mirroring(ctx, stmt):

tests/oclinter/lists-no-sibling-augment/Makefile

@@ -0,0 +1,11 @@
+ROOT_DIR:=$(shell dirname $(realpath $(lastword $(MAKEFILE_LIST))))
+
+ok:
+	pyang --plugindir $(PLUGIN_DIR) \
+		--openconfig --oc-only -p ${ROOT_DIR}/../common \
+		${ROOT_DIR}/openconfig-testcase-succeed.yang
+
+broken:
+	pyang --plugindir $(PLUGIN_DIR) \
+	    --openconfig --oc-only -p ${ROOT_DIR}/../common \
+			    ${ROOT_DIR}/openconfig-testcase-fail.yang ${ROOT_DIR}/openconfig-testcase-fail-augment.yang

tests/oclinter/lists-no-sibling-augment/openconfig-testcase-fail-augment.yang

@@ -0,0 +1,33 @@
+module openconfig-testcase-fail-augment {
+  prefix "oc-tc2";
+  namespace "http://openconfig.net/linter/testcase2";
+
+  import openconfig-extensions { prefix oc-ext; }
+  import openconfig-testcase-fail { prefix oc-tc; }
+
+  description
+    "Failure test case for a list having siblings.";
+
+  oc-ext:openconfig-version "0.0.1";
+
+  revision 2016-09-28 {
+    reference "0.0.1";
+    description
+      "Revision statement";
+  }
+
+  grouping augment-top {
+    container hello {
+      container state {
+        config false;
+        leaf hello-leaf { type string; }
+      }
+    }
+  }
+
+
+  augment '/oc-tc:surrounding-container' {
+    uses augment-top;
+  }
+
+}

tests/oclinter/lists-no-sibling-augment/openconfig-testcase-fail.yang

@@ -0,0 +1,47 @@
+module openconfig-testcase-fail {
+  prefix "oc-tc";
+  namespace "http://openconfig.net/linter/testcase";
+
+  import openconfig-extensions { prefix oc-ext; }
+
+  description
+    "Failure test case for a list having siblings.";
+
+  oc-ext:openconfig-version "0.0.1";
+
+  revision 2016-09-28 {
+    reference "0.0.1";
+    description
+      "Revision statement";
+  }
+
+  grouping list-config {
+    leaf keyleaf { type string; }
+  }
+
+  grouping foo-top {
+    container surrounding-container {
+      list the-list {
+        key "keyleaf";
+
+        leaf keyleaf {
+          type leafref {
+            path "../config/keyleaf";
+          }
+        }
+
+        container config {
+          uses list-config;
+        }
+
+        container state {
+          config false;
+          uses list-config;
+        }
+      }
+    }
+  }
+
+  uses foo-top;
+
+}

tests/oclinter/lists-no-sibling-augment/openconfig-testcase-succeed.yang

@@ -0,0 +1,47 @@
+module openconfig-testcase-succeed {
+  prefix "oc-tc";
+  namespace "http://openconfig.net/linter/testcase";
+
+  import openconfig-extensions { prefix oc-ext; }
+
+  description
+    "Success test case for a list having siblings.";
+
+  oc-ext:openconfig-version "0.0.1";
+
+  revision 2016-09-28 {
+    reference "0.0.1";
+    description
+      "Revision statement";
+  }
+
+  grouping list-config {
+    leaf keyleaf { type string; }
+  }
+
+  grouping foo-top {
+    container surrounding-container {
+      list the-list {
+        key "keyleaf";
+
+        leaf keyleaf {
+          type leafref {
+            path "../config/keyleaf";
+          }
+        }
+
+        container config {
+          uses list-config;
+        }
+
+        container state {
+          config false;
+          uses list-config;
+        }
+      }
+    }
+  }
+
+  uses foo-top;
+
+}

tests/oclinter/lists-no-sibling/Makefile

@@ -0,0 +1,11 @@
+ROOT_DIR:=$(shell dirname $(realpath $(lastword $(MAKEFILE_LIST))))
+
+ok:
+	pyang --plugindir $(PLUGIN_DIR) \
+		--openconfig --oc-only -p ${ROOT_DIR}/../common \
+		${ROOT_DIR}/openconfig-testcase-succeed.yang
+
+broken:
+	pyang --plugindir $(PLUGIN_DIR) \
+	    --openconfig --oc-only -p ${ROOT_DIR}/../common \
+			    ${ROOT_DIR}/openconfig-testcase-fail.yang

tests/oclinter/lists-no-sibling/openconfig-testcase-fail.yang

@@ -0,0 +1,54 @@
+module openconfig-testcase-fail {
+  prefix "oc-tc";
+  namespace "http://openconfig.net/linter/testcase";
+
+  import openconfig-extensions { prefix oc-ext; }
+
+  description
+    "Failure test case for a list having siblings.";
+
+  oc-ext:openconfig-version "0.0.1";
+
+  revision 2016-09-28 {
+    reference "0.0.1";
+    description
+      "Revision statement";
+  }
+
+  grouping list-config {
+    leaf keyleaf { type string; }
+  }
+
+  grouping foo-top {
+    container surrounding-container {
+      container hello {
+        container state {
+          config false;
+          leaf hello-leaf { type string; }
+        }
+      }
+
+      list the-list {
+        key "keyleaf";
+
+        leaf keyleaf {
+          type leafref {
+            path "../config/keyleaf";
+          }
+        }
+
+        container config {
+          uses list-config;
+        }
+
+        container state {
+          config false;
+          uses list-config;
+        }
+      }
+    }
+  }
+
+  uses foo-top;
+
+}

tests/oclinter/lists-no-sibling/openconfig-testcase-succeed.yang

@@ -0,0 +1,47 @@
+module openconfig-testcase-succeed {
+  prefix "oc-tc";
+  namespace "http://openconfig.net/linter/testcase";
+
+  import openconfig-extensions { prefix oc-ext; }
+
+  description
+    "Success test case for a list having siblings.";
+
+  oc-ext:openconfig-version "0.0.1";
+
+  revision 2016-09-28 {
+    reference "0.0.1";
+    description
+      "Revision statement";
+  }
+
+  grouping list-config {
+    leaf keyleaf { type string; }
+  }
+
+  grouping foo-top {
+    container surrounding-container {
+      list the-list {
+        key "keyleaf";
+
+        leaf keyleaf {
+          type leafref {
+            path "../config/keyleaf";
+          }
+        }
+
+        container config {
+          uses list-config;
+        }
+
+        container state {
+          config false;
+          uses list-config;
+        }
+      }
+    }
+  }
+
+  uses foo-top;
+
+}

tests/packaging.sh

@@ -15,6 +15,8 @@
 
 TESTDIR="$(cd -P "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 
+pip install setuptools==58.2.0
+
 cd $TESTDIR/..
 rm -rf $TESTDIR/tvirtenv $TESTDIR/../dist $TESTDIR/../build $TESTDIR/../openconfig_pyang.egg-info