Enforce augment statement argument well-formedness

'augment' statement has dual use, depending on where it is declared.
Each use requires a different schema node identifier:
- plain augment requires SchemaNodeIdentifier.Absolute
- uses/augment requires SchemaNodeIdentifier.Descendant

Make sure we enforce proper parsing, so that our argument will end
up being correctly captured (and can be used for further reasoning).

JIRA: YANGTOOLS-1189
Change-Id: I4e628fd1aee29e103a045cfe90bae58f91ce32ed
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java
index fc62ae4cc1d95b4c667ac00a14def6382642cfb9..9966b174320d8ec52bb794264329f2a537c2fb94 100644 (file)
--- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java
+++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/augment/AbstractAugmentStatementSupport.java
@@ -25,10 +25,13 @@ import org.opendaylight.yangtools.yang.model.api.Status;
 import org.opendaylight.yangtools.yang.model.api.YangStmtMapping;
 import org.opendaylight.yangtools.yang.model.api.meta.DeclaredStatement;
 import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement;
+import org.opendaylight.yangtools.yang.model.api.meta.StatementDefinition;
 import org.opendaylight.yangtools.yang.model.api.stmt.AugmentEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.AugmentStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.DataDefinitionStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier;
+import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier.Absolute;
+import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier.Descendant;
 import org.opendaylight.yangtools.yang.model.api.stmt.StatusEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.UsesStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.WhenEffectiveStatement;
@@ -72,7 +75,23 @@ abstract class AbstractAugmentStatementSupport
             "Augment argument \'%s\' is not valid, it can be only absolute path; or descendant if used in uses",
             value);
 
-        return ArgumentUtils.nodeIdentifierFromPath(ctx, value);
+        // As per:
+        //   https://tools.ietf.org/html/rfc6020#section-7.15
+        //   https://tools.ietf.org/html/rfc7950#section-7.17
+        //
+        // The argument is either Absolute or Descendant based on whether the statement is declared within a 'uses'
+        // statement. The mechanics differs wildly between the two cases, so let's start by ensuring our argument
+        // is in the correct domain.
+        final SchemaNodeIdentifier result = ArgumentUtils.nodeIdentifierFromPath(ctx, value);
+        final StatementDefinition parent = ctx.coerceParentContext().publicDefinition();
+        if (parent == YangStmtMapping.USES) {
+            SourceException.throwIf(result instanceof Absolute, ctx.sourceReference(),
+                "Absolute schema node identifier is not allowed when used within a uses statement");
+        } else {
+            SourceException.throwIf(result instanceof Descendant, ctx.sourceReference(),
+                "Descendant schema node identifier is not allowed when used outside of a uses statement");
+        }
+        return result;
     }
 
     @Override

diff --git a/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1189Test.java b/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1189Test.java
new file mode 100644 (file)
index 0000000..c872050
--- /dev/null
+++ b/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1189Test.java
@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2020 PANTHEON.tech, s.r.o. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.yangtools.yang.stmt;
+
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.startsWith;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertThrows;
+
+import org.junit.Test;
+import org.opendaylight.yangtools.yang.parser.spi.meta.ReactorException;
+import org.opendaylight.yangtools.yang.parser.spi.source.SourceException;
+
+public class YT1189Test {
+    @Test
+    public void testDescendantAugment() {
+        final ReactorException ex = assertThrows(ReactorException.class,
+            () -> StmtTestUtils.parseYangSource("/bugs/YT1189/foo.yang"));
+        final Throwable cause = ex.getCause();
+        assertThat(cause, instanceOf(SourceException.class));
+        assertThat(cause.getMessage(),
+            startsWith("Descendant schema node identifier is not allowed when used outside of a uses statement [at "));
+    }
+
+    @Test
+    public void testAbsoluteUsesAugment() {
+        final ReactorException ex = assertThrows(ReactorException.class,
+            () -> StmtTestUtils.parseYangSource("/bugs/YT1189/bar.yang"));
+        final Throwable cause = ex.getCause();
+        assertThat(cause, instanceOf(SourceException.class));
+        assertThat(cause.getMessage(),
+            startsWith("Absolute schema node identifier is not allowed when used within a uses statement [at "));
+    }
+}

diff --git a/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1189/bar.yang b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1189/bar.yang
new file mode 100644 (file)
index 0000000..d10d552
--- /dev/null
+++ b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1189/bar.yang
@@ -0,0 +1,17 @@
+module bar {
+  namespace "bar";
+  prefix "bar";
+
+  grouping grp {
+    container grp-cont;
+  }
+
+  container cont {
+    uses grp {
+      // Invalid: the path should be descendant
+      augment "/grp-cont" {
+
+      }
+    }
+  }
+}

diff --git a/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1189/foo.yang b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1189/foo.yang
new file mode 100644 (file)
index 0000000..c2896b8
--- /dev/null
+++ b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1189/foo.yang
@@ -0,0 +1,11 @@
+module foo {
+  namespace "foo";
+  prefix "foo";
+
+  container cont;
+
+  // Invalid: the path should be absolute
+  augment "cont" {
+
+  }
+}

diff --git a/yang/yang-parser-rfc7950/src/test/resources/bugs/bug5481/module2.yang b/yang/yang-parser-rfc7950/src/test/resources/bugs/bug5481/module2.yang
index f6b1fa3450b7c07595c8f823bc209ac3ebbba073..22431b077868bb31d54d142d40533c4f00296468 100644 (file)
--- a/yang/yang-parser-rfc7950/src/test/resources/bugs/bug5481/module2.yang
+++ b/yang/yang-parser-rfc7950/src/test/resources/bugs/bug5481/module2.yang
@@ -12,7 +12,7 @@ module module2 {
         description "Initial version.";
     }
 
-    augment "module1:top" {
+    augment "/module1:top" {
         when "module1:top = 'extended'";
         description "text";
         status deprecated;