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>
(cherry picked from commit eb017d49e00a06c1b29ac1a91d4d95007fa7c1a6)

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..e1a0e8791c63faf0dc0878f41cdf99902f596d62 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 = "7.0.15")
     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 784c70b3b6662de1a77564170b68fc9991ec2cb8..c2a1354f15e9af27c6a16b67e46df3ce37d8b8c0 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
@@ -11,6 +11,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;
@@ -34,7 +35,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.EffectiveStmtCtx.Current;
 import org.opendaylight.yangtools.yang.parser.spi.meta.ImplicitParentAwareStatementSupport;
@@ -64,11 +64,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)
@@ -82,29 +86,33 @@ 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 final ImmutableSet<StatementDefinition> caseShorthands;
     private final CaseStatementSupport implicitCase;
 
     private ChoiceStatementSupport(final YangParserConfiguration config, final SubstatementValidator validator,
-            final CaseStatementSupport implicitCase) {
+            final ImmutableSet<StatementDefinition> caseShorthands, final CaseStatementSupport implicitCase) {
         super(YangStmtMapping.CHOICE, instantiatedPolicy(), config, requireNonNull(validator));
+        this.caseShorthands = requireNonNull(caseShorthands);
         this.implicitCase = requireNonNull(implicitCase);
     }
 
     public static @NonNull ChoiceStatementSupport rfc6020Instance(final YangParserConfiguration config,
             final CaseStatementSupport implicitCase) {
-        return new ChoiceStatementSupport(config, RFC6020_VALIDATOR, implicitCase);
+        return new ChoiceStatementSupport(config, RFC6020_VALIDATOR, RFC6020_CASE_SHORTHANDS, implicitCase);
     }
 
     public static @NonNull ChoiceStatementSupport rfc7950Instance(final YangParserConfiguration config,
             final CaseStatementSupport implicitCase) {
-        return new ChoiceStatementSupport(config, RFC7950_VALIDATOR, implicitCase);
+        return new ChoiceStatementSupport(config, RFC7950_VALIDATOR, RFC7950_CASE_SHORTHANDS, implicitCase);
     }
 
     @Override
     public Optional<StatementSupport<?, ?, ?>> getImplicitParentFor(final StatementDefinition stmtDef) {
-        return YangValidationBundles.SUPPORTED_CASE_SHORTHANDS.contains(stmtDef) ? Optional.of(implicitCase)
-            : Optional.empty();
+        return caseShorthands.contains(stmtDef) ? Optional.of(implicitCase) : Optional.empty();
     }
 
     @Override

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..2d1b0ea
--- /dev/null
+++ b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1410Test.java
@@ -0,0 +1,47 @@
+/*
+ * 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 static org.junit.Assert.assertThrows;
+
+import java.io.IOException;
+import java.net.URISyntaxException;
+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;
+import org.opendaylight.yangtools.yang.parser.api.YangSyntaxErrorException;
+import org.opendaylight.yangtools.yang.parser.spi.meta.ReactorException;
+import org.opendaylight.yangtools.yang.parser.spi.meta.SomeModifiersUnresolvedException;
+import org.opendaylight.yangtools.yang.parser.spi.source.SourceException;
+
+public class YT1410Test {
+    @Test
+    public void testRFC6020() {
+        final var ex = assertThrows(SomeModifiersUnresolvedException.class,
+            () -> StmtTestUtils.parseYangSource("/bugs/YT1410/foo.yang")).getCause();
+        assertThat(ex, instanceOf(SourceException.class));
+        assertThat(ex.getMessage(),
+            startsWith("CHOICE is not valid for CHOICE. Error in module foo (QNameModule{ns=foo}) [at "));
+    }
+
+    @Test
+    public void testRFC7950() throws YangSyntaxErrorException, ReactorException, URISyntaxException, IOException {
+        final var module = StmtTestUtils.parseYangSource("/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;
+  }
+}
+