Recognize 'choice' in 'choice' with YANG 1.1

RFC7950 allows for 'choice' to appear directly in another 'choice'.
Make sure we allow for that possibility. This also requires
disconnecting shorthands from global state -- they are version-specific
after all.

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

diff --git a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/reactor/YangValidationBundles.java b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/reactor/YangValidationBundles.java
index 97ebbd67deee3b6e3f1409018d685c140cffa0b5..5e0349dc18afa312300ebcf0232c4f8cbe6457e6 100644 (file)
--- a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/reactor/YangValidationBundles.java
+++ b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/reactor/YangValidationBundles.java
@@ -84,6 +84,7 @@ public final class YangValidationBundles {
     // FIXME: 7.0.0: consider hiding this list, as choice nodes are handling creation of implied shorthands themselves.
     //               This has implications on other members of this class, as they really seem like something which
     //               should live in corresponding StatementSupport classes.
+    @Deprecated(forRemoval = true, since = "8.0.2")
     public static final Set<StatementDefinition> SUPPORTED_CASE_SHORTHANDS = ImmutableSet.of(
         YangStmtMapping.CONTAINER, YangStmtMapping.LIST, YangStmtMapping.LEAF, YangStmtMapping.LEAF_LIST,
         YangStmtMapping.ANYXML, YangStmtMapping.ANYDATA);

diff --git a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/meta/ChoiceStatementSupport.java b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/meta/ChoiceStatementSupport.java
index a9b9200dc19be343c55b6a192515c9edd32ff328..eec559721196e33994fb2d22605a79f2c8120a8a 100644 (file)
--- a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/meta/ChoiceStatementSupport.java
+++ b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/meta/ChoiceStatementSupport.java
@@ -13,6 +13,7 @@ import static java.util.Objects.requireNonNull;
 
 import com.google.common.annotations.Beta;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import java.util.Collection;
 import java.util.Optional;
 import org.eclipse.jdt.annotation.NonNull;
@@ -36,7 +37,6 @@ import org.opendaylight.yangtools.yang.model.ri.stmt.EffectiveStatements;
 import org.opendaylight.yangtools.yang.model.spi.meta.EffectiveStatementMixins.EffectiveStatementWithFlags.FlagsBuilder;
 import org.opendaylight.yangtools.yang.model.spi.meta.SubstatementIndexingException;
 import org.opendaylight.yangtools.yang.parser.api.YangParserConfiguration;
-import org.opendaylight.yangtools.yang.parser.rfc7950.reactor.YangValidationBundles;
 import org.opendaylight.yangtools.yang.parser.spi.meta.AbstractSchemaTreeStatementSupport;
 import org.opendaylight.yangtools.yang.parser.spi.meta.BoundStmtCtx;
 import org.opendaylight.yangtools.yang.parser.spi.meta.EffectiveStatementState;
@@ -70,11 +70,15 @@ public final class ChoiceStatementSupport
             .addOptional(YangStmtMapping.STATUS)
             .addOptional(YangStmtMapping.WHEN)
             .build();
+    private static final ImmutableSet<StatementDefinition> RFC6020_CASE_SHORTHANDS = ImmutableSet.of(
+            YangStmtMapping.ANYXML, YangStmtMapping.CONTAINER, YangStmtMapping.LEAF, YangStmtMapping.LIST,
+            YangStmtMapping.LEAF_LIST);
 
     private static final SubstatementValidator RFC7950_VALIDATOR = SubstatementValidator.builder(YangStmtMapping.CHOICE)
             .addAny(YangStmtMapping.ANYDATA)
             .addAny(YangStmtMapping.ANYXML)
             .addAny(YangStmtMapping.CASE)
+            .addAny(YangStmtMapping.CHOICE)
             .addOptional(YangStmtMapping.CONFIG)
             .addAny(YangStmtMapping.CONTAINER)
             .addOptional(YangStmtMapping.DEFAULT)
@@ -88,23 +92,30 @@ public final class ChoiceStatementSupport
             .addOptional(YangStmtMapping.STATUS)
             .addOptional(YangStmtMapping.WHEN)
             .build();
+    private static final ImmutableSet<StatementDefinition> RFC7950_CASE_SHORTHANDS = ImmutableSet.of(
+        YangStmtMapping.ANYDATA, YangStmtMapping.ANYXML, YangStmtMapping.CHOICE, YangStmtMapping.CONTAINER,
+        YangStmtMapping.LEAF, YangStmtMapping.LIST, YangStmtMapping.LEAF_LIST);
 
-    private ChoiceStatementSupport(final YangParserConfiguration config, final SubstatementValidator validator) {
+    private final ImmutableSet<StatementDefinition> caseShorthands;
+
+    private ChoiceStatementSupport(final YangParserConfiguration config, final SubstatementValidator validator,
+            final ImmutableSet<StatementDefinition> caseShorthands) {
         super(YangStmtMapping.CHOICE, instantiatedPolicy(), config, requireNonNull(validator));
+        this.caseShorthands = requireNonNull(caseShorthands);
     }
 
     public static @NonNull ChoiceStatementSupport rfc6020Instance(final YangParserConfiguration config) {
-        return new ChoiceStatementSupport(config, RFC6020_VALIDATOR);
+        return new ChoiceStatementSupport(config, RFC6020_VALIDATOR, RFC6020_CASE_SHORTHANDS);
     }
 
     public static @NonNull ChoiceStatementSupport rfc7950Instance(final YangParserConfiguration config) {
-        return new ChoiceStatementSupport(config, RFC7950_VALIDATOR);
+        return new ChoiceStatementSupport(config, RFC7950_VALIDATOR, RFC7950_CASE_SHORTHANDS);
     }
 
     @Override
     public Optional<StatementSupport<?, ?, ?>> getImplicitParentFor(final NamespaceStmtCtx parent,
             final StatementDefinition stmtDef) {
-        if (!YangValidationBundles.SUPPORTED_CASE_SHORTHANDS.contains(stmtDef)) {
+        if (!caseShorthands.contains(stmtDef)) {
             return Optional.empty();
         }
         return Optional.of(verifyNotNull(parent.getFromNamespace(StatementSupportNamespace.class,

diff --git a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1410Test.java b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1410Test.java
new file mode 100644 (file)
index 0000000..9d3bb79
--- /dev/null
+++ b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1410Test.java
@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2022 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 org.junit.Test;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.model.api.stmt.CaseEffectiveStatement;
+import org.opendaylight.yangtools.yang.model.api.stmt.ChoiceEffectiveStatement;
+
+public class YT1410Test extends AbstractYangTest {
+    @Test
+    public void testRFC6020() {
+        assertInvalidSubstatementException(
+            startsWith("CHOICE is not valid for CHOICE. Error in module foo (QNameModule{ns=foo}) [at "),
+            "/bugs/YT1410/foo.yang");
+    }
+
+    @Test
+    public void testRFC7950() {
+        final var module = assertEffectiveModel("/bugs/YT1410/bar.yang").getModuleStatement(QName.create("bar", "bar"));
+        final var one = module.findSchemaTreeNode(QName.create("bar", "one")).orElseThrow();
+        assertThat(one, instanceOf(ChoiceEffectiveStatement.class));
+        final var two = ((ChoiceEffectiveStatement) one).findSchemaTreeNode(QName.create("bar", "two")).orElseThrow();
+        assertThat(two, instanceOf(CaseEffectiveStatement.class));
+        assertThat(((CaseEffectiveStatement) two).findSchemaTreeNode(QName.create("bar", "two")).orElseThrow(),
+            instanceOf(ChoiceEffectiveStatement.class));
+    }
+}

diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1410/bar.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1410/bar.yang
new file mode 100644 (file)
index 0000000..fb6a156
--- /dev/null
+++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1410/bar.yang
@@ -0,0 +1,11 @@
+module bar {
+  namespace bar;
+  prefix bar;
+  yang-version 1.1;
+
+  choice one {
+    // RFC7950: completely fine
+    choice two;
+  }
+}
+

diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1410/foo.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1410/foo.yang
new file mode 100644 (file)
index 0000000..31ecbaf
--- /dev/null
+++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1410/foo.yang
@@ -0,0 +1,10 @@
+module foo {
+  namespace foo;
+  prefix foo;
+
+  choice one {
+    // RFC6020: not allowed
+    choice two;
+  }
+}
+