Repository: openconfig/goyang
Pull request #206 (2021-08-30 23:53:15)

Store global `identities` in `typeDictionary`.
Part of #172's effort.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/42526332/badge)](https://coveralls.io/builds/42526332)

Coverage increased (+0.02%) to 81.647% when pulling **b6cd8cc221badf55119911a142e1b77847f2f667 on identities-local** into **c3ce904bf1f40b47415bc4c7941fadca3b25d513 on merge-caches**.


Patch:
diff --git a/pkg/yang/identity.go b/pkg/yang/identity.go
index 6fd464e..7979b83 100644
--- a/pkg/yang/identity.go
+++ b/pkg/yang/identity.go
@@ -22,8 +22,8 @@ import (
 // This file implements data structures and functions that relate to the
 // identity type.
 
-// identityDictionary stores a global set of identities that have been resolved
-// to be identified by their module and name.
+// identityDictionary stores a set of identities across all parsed Modules that
+// have been resolved to be identified by their module and name.
 type identityDictionary struct {
 	mu sync.Mutex
 	// dict is a global cache of identities keyed by
@@ -33,9 +33,6 @@ type identityDictionary struct {
 	dict map[string]resolvedIdentity
 }
 
-// Global dictionary of resolved identities
-var identities = identityDictionary{dict: map[string]resolvedIdentity{}}
-
 // resolvedIdentity is an Identity that has been disambiguated.
 type resolvedIdentity struct {
 	Module   *Module
@@ -89,13 +86,14 @@ func (mod *Module) findIdentityBase(baseStr string) (*resolvedIdentity, []error)
 	basePrefix, baseName := getPrefix(baseStr)
 	rootPrefix := mod.GetPrefix()
 	source := Source(mod)
+	typeDict := mod.Modules.typeDict
 
 	switch basePrefix {
 	case "", rootPrefix:
 		// This is a local identity which is defined within the current
 		// module
 		keyName := fmt.Sprintf("%s:%s", module(mod).Name, baseName)
-		base, ok = identities.dict[keyName]
+		base, ok = typeDict.identities.dict[keyName]
 		if !ok {
 			errs = append(errs, fmt.Errorf("%s: can't resolve the local base %s as %s", source, baseStr, keyName))
 		}
@@ -108,7 +106,7 @@ func (mod *Module) findIdentityBase(baseStr string) (*resolvedIdentity, []error)
 			break
 		}
 		// The identity we are looking for is modulename:basename.
-		if id, ok := identities.dict[fmt.Sprintf("%s:%s", module(extmod).Name, baseName)]; ok {
+		if id, ok := typeDict.identities.dict[fmt.Sprintf("%s:%s", module(extmod).Name, baseName)]; ok {
 			base = id
 			break
 		}
@@ -123,8 +121,8 @@ func (mod *Module) findIdentityBase(baseStr string) (*resolvedIdentity, []error)
 }
 
 func (ms *Modules) resolveIdentities() []error {
-	defer identities.mu.Unlock()
-	identities.mu.Lock()
+	defer ms.typeDict.identities.mu.Unlock()
+	ms.typeDict.identities.mu.Lock()
 
 	var errs []error
 
@@ -135,7 +133,7 @@ func (ms *Modules) resolveIdentities() []error {
 	for _, mod := range ms.Modules {
 		for _, i := range mod.Identities() {
 			keyName, r := newResolvedIdentity(mod, i)
-			identities.dict[keyName] = *r
+			ms.typeDict.identities.dict[keyName] = *r
 		}
 
 		// Hoist up all identities in our included submodules.
@@ -147,7 +145,7 @@ func (ms *Modules) resolveIdentities() []error {
 			}
 			for _, i := range in.Module.Identities() {
 				keyName, r := newResolvedIdentity(in.Module, i)
-				identities.dict[keyName] = *r
+				ms.typeDict.identities.dict[keyName] = *r
 			}
 		}
 	}
@@ -156,7 +154,7 @@ func (ms *Modules) resolveIdentities() []error {
 	// fully resolved identity statement. The intention here is to make sure
 	// that the Children slice is fully populated with pointers to all identities
 	// that have a base, so that we can do inheritance of these later.
-	for _, i := range identities.dict {
+	for _, i := range ms.typeDict.identities.dict {
 		if i.Identity.Base != nil {
 			// This identity inherits from one or more other identities.
 
@@ -176,7 +174,7 @@ func (ms *Modules) resolveIdentities() []error {
 	}
 
 	// Do a final sweep through the identities to build up their children.
-	for _, i := range identities.dict {
+	for _, i := range ms.typeDict.identities.dict {
 		newValues := []*Identity{}
 		for _, j := range i.Identity.Values {
 			newValues = addChildren(j, newValues)
diff --git a/pkg/yang/types.go b/pkg/yang/types.go
index b3efb0e..facf935 100644
--- a/pkg/yang/types.go
+++ b/pkg/yang/types.go
@@ -29,10 +29,15 @@ import (
 type typeDictionary struct {
 	mu   sync.Mutex
 	dict map[Node]map[string]*Typedef
+	// identities contains a dictionary of resolved identities.
+	identities identityDictionary
 }
 
 func newTypeDictionary() *typeDictionary {
-	return &typeDictionary{dict: map[Node]map[string]*Typedef{}}
+	return &typeDictionary{
+		dict:       map[Node]map[string]*Typedef{},
+		identities: identityDictionary{dict: map[string]resolvedIdentity{}},
+	}
 }
 
 // typeDict is a protected global dictionary of all typedefs.


Test patch:
diff --git a/pkg/yang/identity_test.go b/pkg/yang/identity_test.go
index 98b0448..d717866 100644
--- a/pkg/yang/identity_test.go
+++ b/pkg/yang/identity_test.go
@@ -217,7 +217,7 @@ func TestIdentityExtract(t *testing.T) {
 
 			foundIdentity := false
 			var thisID *Identity
-			for _, ri := range identities.dict {
+			for _, ri := range ms.typeDict.identities.dict {
 				moduleName := module(ri.Module).Name
 				if ri.Identity.Name == ti.name && moduleName == ti.module {
 					foundIdentity = true
@@ -227,7 +227,7 @@ func TestIdentityExtract(t *testing.T) {
 			}
 
 			if !foundIdentity {
-				t.Errorf("Could not find identity %s in module %s, identity dict:\n%+v", ti.name, ti.module, identities.dict)
+				t.Errorf("Could not find identity %s in module %s, identity dict:\n%+v", ti.name, ti.module, ms.typeDict.identities.dict)
 				continue
 			}
 
@@ -791,9 +791,5 @@ func TestIdentityTree(t *testing.T) {
 				}
 			}
 		})
-		// Reset this global after the test.
-		// TODO(wenovus): We should implement hermetic file analysis to
-		// prevent needing such hacks.
-		identities = identityDictionary{dict: map[string]resolvedIdentity{}}
 	}
 }