https://git.opendaylight.org/gerrit/66848
https://git.opendaylight.org/gerrit/66850

I think this needs a way better description and justification. What is the precise problem we are trying to solve here?

Note that usual run-time users do not concern themselves with groupings (which really are a leak from declared model and should end up being eliminated from the effective model).

The problem was actually met in class ModuleContext at first, for there're many
maps which take schema path as the key, such as below:

private final Map<SchemaPath,GeneratedTOBuilder> genTOs = new HashMap<>();
private final Map<SchemaPath, Type> typedefs = new HashMap<>();
private final Map<SchemaPath, GeneratedTypeBuilder> childNodes = new HashMap<>();
private final BiMap<String, GeneratedTypeBuilder> dataTypes = HashBiMap.create();
private final Map<SchemaPath, GeneratedTypeBuilder> groupings = new HashMap<>();
private final Map<SchemaPath, GeneratedTypeBuilder> cases = new HashMap<>();

See the blocked issue "MDSAL-271 About Namespace":
https://jira.opendaylight.org/browse/MDSAL-271
in that case, all these maps above may conflicts for the same schema path but diffrent schema nodes.
Another case here is the implementation from uses node which is a litte complex, for example:

container test {
container test {
}
}
grouping test {
container test {
}
}
grouping test2

The generated class Test for container test in grouing test2 (added by uses) would implement the class Test generated by container test in grouping test, but when we try to use an orginal schema path of the uses node to get the original node, it would find the wrong one - the inner container test in container test at the top level.
Such problems would always happen in many cases in the generator.

Indeed that usual users don't care about the groupings but data schema nodes.
So we had better to provide two kinds of schema paths：
1) DataSchemaPath - for usual runtime users, which would try to find node from only data schema node, not groupings.
2) SchemaPath - for generator most, which would find node from both data schema node and groupings.
And as a result two APIs comes after them:
1) findDataSchemaNode(..., DataSchemaPath dataPath) - for usual users
2) findSchemaNode(..., SchemaPath schemaPath) - for generator

Another way to reslove this is to eliminate groupings from effective models, by then the generator would generate classes from uses nodes alone without implementations , and also it would clean everything about groupings.

As described in RFC7950 section 5.4:

For example, if a module defines a grouping in which a type is
referenced, when the grouping is used in a second module, the type is
resolved in the context of the original module, not the second
module. There is no ambiguity if both modules define the type.

it seems necessary to keep groupings.
And also for yang snippet,
grouping grp {
leaf my-leaf {
type union {
type int32;
type enumeration

}
}
The union SHOULD only be generated in grouping grp.

Well, from the overall direction at some point we should be ditching SchemaNodes in their current shape and form and we cannot really change the semantic meaning of SchemaPath – no matter how flawed it currently is, as it is used all over the place in contexts which actually work okay, as they simply do not deal with groupings and their instantiations.

Use of SchemaPath in those maps is flawed and was supposed to be fixed (note how these things are essentially copy&pasted from binding v1) – I would suggest binding v2 defining a SchemaNodeIdentifier for non-conflicting identification and construct the tracking maps using that. If/when that approach is qualified as sound we can promote that concept to be the replacement for SchemaPath.

Defining a SchemaNodeIdentifier in binding v2 would still not work, for the original schema path from uses node which was constructed by yangtools
was still SchemaPath by using which we still could not find the correct original schema node and then could not add class of implementation.

Right. Another question: would it be feasible to use IdentityHashMaps keyed by actual SchemaNodes? A cursory glance shows that that's how they are effective accessed...

In that way, APIs in yang-model-api such as UsesNode which provides getGroupingPath or AugmentationSchemaNode provides getTargetPath and so on, all these sort of nodes should also provide like 'getGroupingNode' or getTargetNode', in one word, 'getXXXNode' should appear any where getXXXPath appears.

Well, SchemaNodes are on their way out, so I do not believe we want to retrofit SchemaPath (or anything else) to fit their twisted model. EffectiveStatement is the replacement, where any statement can be addressed using a combination of:

• List<QName> along schema tree
• List<QName> along data tree
• Statement type + offset

I do not believe we need addressing beyond the first two items of that list. Hence I am inclined to turn this issue into a 'Define StatementPath to address effective statements'. Binding concerns, such as finding in what grouping a particular definition comes from should really be computed/cached in MD-SAL, not SchemaContext (and its equivalents).

This issue centers around SchemaPath, but we have an alternative in YANGTOOLS-738. The difference is that while this issue request each node to retain a global identifier (path), YANGTOOLS-738 retains an identifier within the context of a parent.

This effectively means that instead of having a SchemaNode->Path and SchemaContext->Path->SchemaNode lookup we would maintain the identifier externally, if that is even useful.