Filter implicitly-created declared substatements 63/98963/14
authorRobert Varga <robert.varga@pantheon.tech>
Tue, 14 Dec 2021 01:48:08 +0000 (02:48 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Wed, 15 Dec 2021 06:51:40 +0000 (07:51 +0100)
When we create an implicit parent statement in createDeclaredChild()
we need to exclude that statement from the statements we pass to
createDeclared(). Otherwise the statement would be an 'undeclared'
declared statement, which our downstreams are still working with.

JIRA: YANGTOOLS-1383
Change-Id: I81141b1db5653fa4a0e3457e13c868dd25596905
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/AbstractResumedStatement.java
parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java
parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1383Test.java [new file with mode: 0644]
parser/yang-parser-rfc7950/src/test/resources/bugs/YT1383/foo.yang [new file with mode: 0644]

index ad26af0a58b73f7bcdd1dde659fe04233a60649b..50fbbcd23cff059c9e8c4764836003fb4986a98c 100644 (file)
@@ -141,8 +141,20 @@ abstract class AbstractResumedStatement<A, D extends DeclaredStatement<A>, E ext
     }
 
     private @NonNull Stream<DeclaredStatement<?>> substatementsAsDeclared() {
-        // FIXME: YANGTOOLS-1383: this stream includes implicit case statements, but it should not
-        return substatements.stream().map(AbstractResumedStatement::declared);
+        var stream = substatements.stream();
+        if (getImplicitDeclaredFlag()) {
+            stream = stream.map(stmt -> {
+                var ret = stmt;
+                while (ret.origin() == StatementOrigin.CONTEXT) {
+                    final var stmts = ret.substatements;
+                    verify(stmts.size() == 1, "Unexpected substatements %s", stmts);
+                    ret = verifyNotNull(stmts.get(0));
+                }
+                return ret;
+            });
+        }
+
+        return stream.map(AbstractResumedStatement::declared);
     }
 
     @Override
@@ -320,6 +332,7 @@ abstract class AbstractResumedStatement<A, D extends DeclaredStatement<A>, E ext
 
     private AbstractResumedStatement<?, ?, ?> createImplicitParent(final int offset,
             final StatementSupport<?, ?, ?> implicitParent, final StatementSourceReference ref, final String argument) {
+        setImplicitDeclaredFlag();
         return createSubstatement(offset, new StatementDefinitionContext<>(implicitParent),
             ImplicitSubstatement.of(ref), argument);
     }
index 4663d2d5b15911fb3a6009731c6636b8c77fc94d..c5cf622953e22153c194eeba442e43985a3a50ac 100644 (file)
@@ -149,6 +149,15 @@ abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E extends
      */
     private byte executionOrder;
 
+    /**
+     * This field should live in AbstractResumedStatement, but is placed here for memory efficiency to squat in the
+     * alignment shadow of {@link #bitsAight} and {@link #executionOrder}.
+     */
+    private boolean implicitDeclaredFlag;
+
+    // TODO: we a single byte of alignment shadow left, we should think how we can use it to cache information we build
+    //       during buildEffective()
+
     // Copy constructor used by subclasses to implement reparent()
     StatementContextBase(final StatementContextBase<A, D, E> original) {
         super(original);
@@ -860,4 +869,13 @@ abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E extends
      * @return True if {@link #allSubstatements()} and {@link #allSubstatementsStream()} would return an empty stream.
      */
     abstract boolean hasEmptySubstatements();
+
+    // Note: these two are exposed for AbstractResumedStatement only
+    final boolean getImplicitDeclaredFlag() {
+        return implicitDeclaredFlag;
+    }
+
+    final void setImplicitDeclaredFlag() {
+        implicitDeclaredFlag = true;
+    }
 }
diff --git a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1383Test.java b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1383Test.java
new file mode 100644 (file)
index 0000000..0ab2824
--- /dev/null
@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) 2021 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.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+import org.junit.Test;
+import org.opendaylight.yangtools.yang.model.api.stmt.CaseEffectiveStatement;
+import org.opendaylight.yangtools.yang.model.api.stmt.CaseStatement;
+import org.opendaylight.yangtools.yang.model.api.stmt.ChoiceEffectiveStatement;
+import org.opendaylight.yangtools.yang.model.api.stmt.ContainerStatement;
+
+public class YT1383Test extends AbstractYangTest {
+    @Test
+    public void testDeclaredImplicitCase() {
+        final var foo = assertEffectiveModel("/bugs/YT1383/foo.yang").getModuleStatements().values().iterator()
+            .next().findFirstEffectiveSubstatement(ChoiceEffectiveStatement.class).orElseThrow();
+
+        // Effective view of things
+        final var effStatements = foo.effectiveSubstatements();
+        assertEquals(2, effStatements.size());
+        final var bar = effStatements.get(0);
+        assertThat(bar, instanceOf(CaseEffectiveStatement.class));
+        assertNotNull(bar.getDeclared());
+        final var baz = effStatements.get(1);
+        assertThat(baz, instanceOf(CaseEffectiveStatement.class));
+        assertNull(baz.getDeclared());
+
+        // Declared view of things
+        final var fooDecl = foo.getDeclared();
+        assertNotNull(fooDecl);
+        final var declStatements = fooDecl.declaredSubstatements();
+        assertEquals(2, declStatements.size());
+        final var barDecl = declStatements.get(0);
+        assertThat(barDecl, instanceOf(CaseStatement.class));
+        final var bazDecl = declStatements.get(1);
+        assertThat(bazDecl, instanceOf(ContainerStatement.class));
+    }
+}
diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1383/foo.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1383/foo.yang
new file mode 100644 (file)
index 0000000..6142118
--- /dev/null
@@ -0,0 +1,9 @@
+module foo {
+  namespace foo;
+  prefix foo;
+
+  choice foo {
+    case bar;
+    container baz;
+  }
+}