From a65048076df9539300214678a616fb96378383b4 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Tue, 14 Dec 2021 02:48:08 +0100 Subject: [PATCH] Filter implicitly-created declared substatements 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 --- .../reactor/AbstractResumedStatement.java | 17 ++++++- .../stmt/reactor/StatementContextBase.java | 18 +++++++ .../yangtools/yang/stmt/YT1383Test.java | 48 +++++++++++++++++++ .../src/test/resources/bugs/YT1383/foo.yang | 9 ++++ 4 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1383Test.java create mode 100644 parser/yang-parser-rfc7950/src/test/resources/bugs/YT1383/foo.yang 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 ad26af0a58..50fbbcd23c 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 @@ -141,8 +141,20 @@ abstract class AbstractResumedStatement, E ext } private @NonNull Stream> 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, 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); } diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java index 4663d2d5b1..c5cf622953 100644 --- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java +++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java @@ -149,6 +149,15 @@ abstract class StatementContextBase, 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 original) { super(original); @@ -860,4 +869,13 @@ abstract class StatementContextBase, 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 index 0000000000..0ab2824d84 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1383Test.java @@ -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 index 0000000000..6142118dfd --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1383/foo.yang @@ -0,0 +1,9 @@ +module foo { + namespace foo; + prefix foo; + + choice foo { + case bar; + container baz; + } +} -- 2.36.6