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 <peter.suna@pantheon.tech>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

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 e085978af3ba34ee5d52b967e913f42c0e3e8422..03d20e81d900b0adcbcb670e5501c6fdd2d061aa 100644 (file)
--- 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<A, D extends DeclaredStatement<A>, 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 (file)
index 0000000..3461d1e
--- /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}.
+ *
+ * <p>
+ * 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 <A> Argument type
+ * @param <D> Declared Statement representation
+ * @param <E> Effective Statement representation
+ */
+final class ImplicitStmtCtx<A, D extends DeclaredStatement<A>, E extends EffectiveStatement<A, D>>
+        extends UndeclaredStmtCtx<A, D, E> {
+    private boolean checkedSubstatement;
+
+    // Exposed for AbstractResumedStatement.createUndeclared()
+    ImplicitStmtCtx(final StatementContextBase<?, ?, ?> parent, final StatementSupport<A, D, E> 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 73aef53d66b7e140caba5424019ecadf95927450..f2a33e9f74734d631569f2b56760e7a9af73b7f5 100644 (file)
--- 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<A, D extends DeclaredStatement<A>, 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 beef8d92fb27740baa099a686c3d35e44ec9b435..52a12918492cfe4ad0d3706ffb03a620a0cec542 100644 (file)
--- 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 <A> Argument type
  * @param <D> Declared Statement representation
  * @param <E> Effective Statement representation
  */
-final class UndeclaredStmtCtx<A, D extends DeclaredStatement<A>, E extends EffectiveStatement<A, D>>
+class UndeclaredStmtCtx<A, D extends DeclaredStatement<A>, E extends EffectiveStatement<A, D>>
         extends OriginalStmtCtx<A, D, E> implements UndeclaredCurrent<A, D> {
     private final StatementContextBase<?, ?, ?> parent;
     private final A argument;
@@ -68,7 +67,7 @@ final class UndeclaredStmtCtx<A, D extends DeclaredStatement<A>, E extends Effec
         this.argument = castArgument(original);
     }
 
-    // Exposed for AbstractResumedStatement
+    // Exposed for ImplicitStmtCtx
     UndeclaredStmtCtx(final StatementContextBase<?, ?, ?> parent, final StatementSupport<A, D, E> 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 (file)
index 0000000..7275be6
--- /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 (file)
index 0000000..ae6a3db
--- /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 (file)
index 0000000..49b064f
--- /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;
+    }
+  }
+}