Belongs-To parent module scope is not consider in Submodule

[huyuwen]

Let's discuss based on yangster-test.yang (it could be found in io.typefox.yang/src/test/resources). If we consider to create a submodule of the yangster-test.yang with the following content.

submodule yangster-test-sub {
    yang-version 1.1;
    belongs-to yangster-test {
        prefix ytest;
    }
    augment /ytest:c1 { }
}

When we are trying to load it first (purpose is to update this submodule and serialize it afterwards), we found the SchemaNodeIdentifier /ytest:c1 is not linked correctly. schema node c1 is still an eProxy.

So we tried to add yangster-test as an import module, like:

   belongs-to yangster-test { prefix ytest; }
   import yangster-test { prefix ytest; }

Then yang parser solve the link correctly. But accutally, this is a weired behavior for yang lsp. As to a submodule, if belongs-to has already been defined, the submodule should have the parent module scope by default. Back to the example, the additional 'import' of yangster shouldn't be used in this example.

Same scope problem also happens when we are trying to serialize a new create submodule. Serializer couldn't finish to serialize Augment because missing target node in the scope. yangster-test-sub.yang.txt

[kittaakos]

Although I am still having serialization test issues, I have fixed the flawed belongs-to case. I have a question; let's assume we have the following two Yang models:

foo.yang:

module foo {
    yang-version 1.1;
    namespace "foo:bar";
    prefix x;

    typedef foo {
        type int32 {
            range "1..40 | 60..100";
        }
    } 
    typedef foo2 {
        type foo {
            range "4..20";
        }
    }
    typedef foo3 {
        type foo2 {
            range "5..15";
        }
    }

    container c12 {
            container cool {}
    }
}

foosub2.yang:

submodule foosub2 {
    yang-version 1.1;
    belongs-to foo {
        prefix "fooprefix";
    }
    grouping mygrouping2 {
        container c12 {
        }
    }
    uses "fooprefix:mygrouping2" {
        augment "c12" {
            container augmented {}
        }
    }
}

Would you expect an error in the foosub2.yang file at foosub2.mygrouping2.c12 as it is already defined in foo.yang too. Thank you!

[huyuwen]

Hi Kittaakos, It is a tricky case, really. From my understanding (according to RFC7950), submodule has full view of parent module scope, due to belongs-to. But parent module may not have any information about submodule, if there is no 'include' for a certain submodule in parent module. So basically, in this particular case, I think, there should has a warning on foosub2.yang, because foosub2.yang defines an duplicate data definition node c12. Why do I propose warning here? Because, in current case, foosub2.yang is not included into foo.yang yet. The duplicate issue hasn't be involved into foo.yang. (Submodule couldn't be used without parent module). But when foo.yang includes foosub2.yang, I think both foo.yang and foosub2.yang should report error. This is my opnion. Meanwhile, I compared with Pyang validation result. Pyang doesn't report any error for current case (I don't think this is correct). Of cause, Pyang does report error when 'include' is used in foo.yang. Because duplicate really happens in both foo.yang and foosub2.yang.

I'm also asking our Yang Expert to share their idea, I will follow up ASAP.

[huyuwen]

Hi Kittaakos, After discussing with our Yang Expert, I think we could handle the case according the following rules.

1. For submodule (foosub2.yang), it shouldn't be an error, because submodule couldn't decide when it is used by parent module. Personally, I recommend to raise a warning in submodule, because it can detect this kind of duplicate as potential error.
2. For parent module (foo.yang), when include is not used, there shouldn't be an error. But when submodule is 'include' into parent module, this kind of duplicate must be raised as an error, (prefer to report in parent module, the same reason, parent module decide when to involve submodule).

Does this make sense?

[kittaakos]

Does this make sense?

Absolutely, thank you for the clarification, @huyuwen.

Please note, these additional constraints make the scoping (and the validation) a bit more tricky, in other words, it will take some time to have this additional feature on the master branch.