From 2429d34e05f521eee83bc4be5227949aa167c915 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Wed, 23 Mar 2022 14:23:10 +0100 Subject: [PATCH] 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 (cherry picked from commit eb017d49e00a06c1b29ac1a91d4d95007fa7c1a6) --- .../reactor/YangValidationBundles.java | 1 + .../stmt/meta/ChoiceStatementSupport.java | 20 +++++--- .../yangtools/yang/stmt/YT1410Test.java | 47 +++++++++++++++++++ .../src/test/resources/bugs/YT1410/bar.yang | 11 +++++ .../src/test/resources/bugs/YT1410/foo.yang | 10 ++++ 5 files changed, 83 insertions(+), 6 deletions(-) create mode 100644 parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1410Test.java create mode 100644 parser/yang-parser-rfc7950/src/test/resources/bugs/YT1410/bar.yang create mode 100644 parser/yang-parser-rfc7950/src/test/resources/bugs/YT1410/foo.yang 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 97ebbd67de..e1a0e8791c 100644 --- 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 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 784c70b3b6..c2a1354f15 100644 --- 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 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 RFC7950_CASE_SHORTHANDS = ImmutableSet.of( + YangStmtMapping.ANYDATA, YangStmtMapping.ANYXML, YangStmtMapping.CHOICE, YangStmtMapping.CONTAINER, + YangStmtMapping.LEAF, YangStmtMapping.LIST, YangStmtMapping.LEAF_LIST); + private final ImmutableSet caseShorthands; private final CaseStatementSupport implicitCase; private ChoiceStatementSupport(final YangParserConfiguration config, final SubstatementValidator validator, - final CaseStatementSupport implicitCase) { + final ImmutableSet 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> 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 index 0000000000..2d1b0ea557 --- /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 index 0000000000..fb6a156a52 --- /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 index 0000000000..31ecbaf47c --- /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; + } +} + -- 2.36.6