Skip to content

Handle unresolved identity bases safely - #55

Open
Deepya1502 wants to merge 2 commits into
yang-central:mainfrom
Deepya1502:pr/partial-identity-base-resolution
Open

Deepya1502 wants to merge 2 commits into
yang-central:mainfrom
Deepya1502:pr/partial-identity-base-resolution

Conversation

@Deepya1502

@Deepya1502 Deepya1502 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Separates Base/Identity resolution from YANG Library integration PR #48, as requested in its review.

  • Resolve identity bases during GRAMMAR when imported modules are available, then retry during SCHEMA_BUILD when they are not.
  • Register BASE exactly once for GRAMMAR and SCHEMA_BUILD, so unresolved bases are reported and later-loaded imported identities resolve.
  • Skip unresolved bases safely in isDerived and isDerivedOrSelf.

Verification

IdentityBaseResolutionTest covers an unresolved base error, a base imported from a later-loaded module, and null-safe derived checks. The branch is rebased onto current main; a clean full-reactor mvn test passes.

@lllyfeng lllyfeng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deferred resolution makes sense, but on its own this PR regresses: BaseImpl's GRAMMAR case (L44-45) now swallows the ModelException, and the SCHEMA_BUILD case (L49-59) never runs because BASE is registered for GRAMMAR only on main. A module with base x:does-not-exist validates OK with this PR alone (main reports UNRECOGNIZED_IDENTITY). Please include the YangStatementImplRegister change (BASE → GRAMMAR, SCHEMA_BUILD) in this PR (and drop it from #48), and add tests: unresolved base → error; base in another module resolved in SCHEMA_BUILD → OK; isDerived/isDerivedOrSelf with an unresolved base doesn't NPE.

@Deepya1502

Copy link
Copy Markdown
Contributor Author

Addressed the follow-up: BASE is registered for GRAMMAR and SCHEMA_BUILD in this PR, with tests for unresolved bases, later-loaded imported bases, and null-safe derived checks. Full mvn test passes.

@lllyfeng lllyfeng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — the BASE registration change now lives here, an unresolved base is reported again standalone (verified locally), and IdentityBaseResolutionTest covers the unresolved, cross-module and null-safe derived cases. #48 has been merged; please rebase onto main (YangStatementImplRegister conflict; keep #48's removal of the duplicate BASE registration, with a single BASE line registered for GRAMMAR and SCHEMA_BUILD).

@Deepya1502
Deepya1502 force-pushed the pr/partial-identity-base-resolution branch from db9b1a7 to 1d9ad43 Compare October 13, 2026 14:03
@Deepya1502

Copy link
Copy Markdown
Contributor Author

Rebased onto current main after #48 merged. The conflict is resolved with a single BASE registration for GRAMMAR and SCHEMA_BUILD. IdentityBaseResolutionTest verifies unresolved bases remain errors, later-loaded imports resolve, and derived checks are null-safe; clean full-reactor mvn test passes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants