From: Peter Suna Date: Tue, 3 May 2022 12:33:42 +0000 (+0200) Subject: Fix if-feature propagation for implicit case statements X-Git-Tag: v9.0.0~99 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=yangtools.git;a=commitdiff_plain;h=bc84355a85cafb4d8257a0e397c1efb2409aaf2e Fix if-feature propagation for implicit case statements Implicit statements created during parse must not be propagated if their child statement is not supported by features or otherwise unavailable. This really can only happen when we are creating an implicit declaration on the parser side, as in other code paths the substatement is already completely defined. JIRA: YANGTOOLS-1431 Change-Id: I5797f09c795587c91f10c7f92a6a8c9fd2480213 Signed-off-by: Peter Suna Signed-off-by: Robert Varga --- diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/AbstractResumedStatement.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/AbstractResumedStatement.java index e085978af3..03d20e81d9 100644 --- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/AbstractResumedStatement.java +++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/AbstractResumedStatement.java @@ -176,10 +176,10 @@ abstract class AbstractResumedStatement, E ext final var implicitParent = definition().getImplicitParentFor(this, support.getPublicView()); if (implicitParent.isPresent()) { final var parent = createUndeclared(offset, implicitParent.orElseThrow(), ref, argument); - ret = new UndeclaredStmtCtx<>(parent, support, argument); + ret = new ImplicitStmtCtx<>(parent, support, argument); parent.addEffectiveSubstatement(ret); } else { - ret = new UndeclaredStmtCtx<>(this, support, argument); + ret = new ImplicitStmtCtx<>(this, support, argument); substatements = substatements.put(offset, ret); } support.onStatementAdded(ret); diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ImplicitStmtCtx.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ImplicitStmtCtx.java new file mode 100644 index 0000000000..3461d1e071 --- /dev/null +++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ImplicitStmtCtx.java @@ -0,0 +1,58 @@ +/* + * 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.parser.stmt.reactor; + +import static com.google.common.base.Verify.verify; + +import org.opendaylight.yangtools.yang.model.api.meta.DeclaredStatement; +import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement; +import org.opendaylight.yangtools.yang.parser.spi.meta.StatementSupport; + +/** + * A statement which has not been declared, but exists in the statement hierarchy through being an implicit intermediate + * statement -- like a {@code case} created by a {@code leaf} inside a {@code choice}. + * + *

+ * That the contract of this class requires the caller to add child effective statement, if that does not happen, you + * get to keep the pieces. + * + * @param Argument type + * @param Declared Statement representation + * @param Effective Statement representation + */ +final class ImplicitStmtCtx, E extends EffectiveStatement> + extends UndeclaredStmtCtx { + private boolean checkedSubstatement; + + // Exposed for AbstractResumedStatement.createUndeclared() + ImplicitStmtCtx(final StatementContextBase parent, final StatementSupport support, + final String rawArgument) { + super(parent, support, rawArgument); + } + + @Override + public boolean isSupportedToBuildEffective() { + // The availability of this statement depends on its first substatement, added by the sole user of this class. + // We do not really have a reasonable lifecycle hook (yet?), so here we deactivate just before the state could + // be leaked. + if (!checkedSubstatement) { + final var substatements = effectiveSubstatements(); + verify(!substatements.isEmpty(), "Unexpected empty substatements in %s", this); + final var substatement = substatements.iterator().next(); + // Note: isSupportedByFeatures() is implemented as a walk towards root and we are walking in the opposite + // direction. This check therefore must never be checked as part of isSupportedByFeatures() -- for + // that reason we first in our direction (which may involve recursive calls to this method in a + // substatement) + if (!substatement.isSupportedToBuildEffective() || !substatement.isSupportedByFeatures()) { + setUnsupported(); + } + checkedSubstatement = true; + } + return super.isSupportedToBuildEffective(); + } +} diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java index 73aef53d66..f2a33e9f74 100644 --- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java +++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java @@ -472,8 +472,9 @@ abstract class ReactorStmtCtx, E extends Effec // // + // Non-final form ImplicitStmtCtx @Override - public final boolean isSupportedToBuildEffective() { + public boolean isSupportedToBuildEffective() { return isSupportedToBuildEffective; } diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/UndeclaredStmtCtx.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/UndeclaredStmtCtx.java index beef8d92fb..52a1291849 100644 --- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/UndeclaredStmtCtx.java +++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/UndeclaredStmtCtx.java @@ -27,14 +27,13 @@ import org.opendaylight.yangtools.yang.parser.spi.meta.UndeclaredStatementFactor import org.opendaylight.yangtools.yang.parser.spi.source.ImplicitSubstatement; /** -/** - * Core reactor statement implementation of {@link Mutable}. + * A statement which has not been declared, but exists in the statement hierarchy through some inference. * * @param Argument type * @param Declared Statement representation * @param Effective Statement representation */ -final class UndeclaredStmtCtx, E extends EffectiveStatement> +class UndeclaredStmtCtx, E extends EffectiveStatement> extends OriginalStmtCtx implements UndeclaredCurrent { private final StatementContextBase parent; private final A argument; @@ -68,7 +67,7 @@ final class UndeclaredStmtCtx, E extends Effec this.argument = castArgument(original); } - // Exposed for AbstractResumedStatement + // Exposed for ImplicitStmtCtx UndeclaredStmtCtx(final StatementContextBase parent, final StatementSupport support, final String rawArgument) { super(new StatementDefinitionContext<>(support), ImplicitSubstatement.of(parent.sourceReference())); diff --git a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1431Test.java b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1431Test.java new file mode 100644 index 0000000000..7275be69a0 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1431Test.java @@ -0,0 +1,39 @@ +/* + * 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.junit.Assert.assertEquals; + +import java.util.List; +import java.util.Set; +import org.junit.Test; +import org.opendaylight.yangtools.yang.common.QName; +import org.opendaylight.yangtools.yang.model.api.stmt.ChoiceEffectiveStatement; +import org.opendaylight.yangtools.yang.model.api.stmt.ContainerEffectiveStatement; + +public class YT1431Test { + @Test + public void testUnsupportedChoiceLeaf() throws Exception { + final var module = StmtTestUtils.parseYangSource("/bugs/YT1431/foo.yang", Set.of()) + .findModuleStatement(QName.create("foo", "foo")) + .orElseThrow(); + final var choice = module.findFirstEffectiveSubstatement(ChoiceEffectiveStatement.class).orElseThrow(); + assertEquals(List.of(), choice.effectiveSubstatements()); + } + + @Test + public void testUnsupportedChoiceLeafAugment() throws Exception { + final var module = StmtTestUtils.parseYangSource("/bugs/YT1431/bar.yang", Set.of()) + .findModuleStatement(QName.create("bar", "bar")) + .orElseThrow(); + final var choice = module.findFirstEffectiveSubstatement(ContainerEffectiveStatement.class).orElseThrow() + .findFirstEffectiveSubstatement(ChoiceEffectiveStatement.class) + .orElseThrow(); + assertEquals(List.of(), choice.effectiveSubstatements()); + } +} diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1431/bar.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1431/bar.yang new file mode 100644 index 0000000000..ae6a3dbbb6 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1431/bar.yang @@ -0,0 +1,17 @@ +module bar { + namespace bar; + prefix bar; + + feature bar; + + container foo; + + augment /foo { + choice bar { + leaf baz { + if-feature bar; + type string; + } + } + } +} diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1431/foo.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1431/foo.yang new file mode 100644 index 0000000000..49b064fb5d --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1431/foo.yang @@ -0,0 +1,13 @@ +module foo { + namespace foo; + prefix foo; + + feature foo; + + choice foo { + leaf bar { + if-feature foo; + type string; + } + } +}