[YANGTOOLS-838] Yang parser should support detect grouping name collisions Created: 19/Dec/17 Updated: 25/Dec/17 Resolved: 25/Dec/17 |
|
| Status: | Resolved |
| Project: | yangtools |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 2.0.1 |
| Type: | Improvement | Priority: | Medium |
| Reporter: | Jie Han | Assignee: | Robert Varga |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Description |
|
All grouping names defined within a parent node or at the top module test { }
|
| Comments |
| Comment by Robert Varga [ 20/Dec/17 ] |
|
The description is a quote from https://tools.ietf.org/html/rfc7950#section-6.2.1. Furthermore section 5.5 explicitly forbids identifier shadowing: |
| Comment by Robert Varga [ 20/Dec/17 ] |
| Comment by Jie Han [ 22/Dec/17 ] |
|
The codes here seems doesn't work as I have tried, since it's deep first, when the deepest grouping was added to grouping namespace but the ancestor groupings have not been walked then, so here it would always return null by getFromNamespace, I have been considering to add another check phase after full declare phase to resolve it by then all groupings are in the grouping namespace. |
| Comment by Robert Varga [ 23/Dec/17 ] |
|
Adding another phase would be too intrusive and would require another complete walk of the statement tree. The patch has been fixed to check conflicts at declared statement instantiation time, which should be sufficient. |
| Comment by Jie Han [ 25/Dec/17 ] |
|
Yes, adding a new phase is the last option, it's better to reuse the original progress. |