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

Move global ParseOptions into Modules.
Part of the effort of #172.
**Backwards-incompatible.**

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

Coverage remained the same at 81.715% when pulling **21cb85f526385aaed5ab8b7d772cb8c11de65c5e on move-global-parseoptions** into **8379e36309dac68afa6c5daa001a252c52c23076 on remove-global-typedict**.

robshakir: Oh, sorry - I missed the backwards-incompatible in the description. Same comment above still applies.
robshakir: @wenovus - Can you check whether you're waiting on something from me here? thanks!

Patch:
diff --git a/pkg/yang/entry.go b/pkg/yang/entry.go
index 371466f..7dd8bfd 100644
--- a/pkg/yang/entry.go
+++ b/pkg/yang/entry.go
@@ -779,7 +779,7 @@ func ToEntry(n Node) (e *Entry) {
 					ms.mergedSubmodule[srcToIncluded] = true
 					ms.mergedSubmodule[includedToParent] = true
 					e.merge(a.Module.Prefix, nil, ToEntry(a.Module))
-				case ParseOptions.IgnoreSubmoduleCircularDependencies:
+				case ms.ParseOptions.IgnoreSubmoduleCircularDependencies:
 					continue
 				default:
 					e.addError(fmt.Errorf("%s: has a circular dependency, importing %s", n.NName(), a.Module.NName()))
@@ -850,7 +850,7 @@ func ToEntry(n Node) (e *Entry) {
 			for _, a := range fv.Interface().([]*Uses) {
 				grouping := ToEntry(a)
 				e.merge(nil, nil, grouping)
-				if ParseOptions.StoreUses {
+				if ms.ParseOptions.StoreUses {
 					e.Uses = append(e.Uses, &UsesStmt{a, grouping.shallowDup()})
 				}
 			}
diff --git a/pkg/yang/modules.go b/pkg/yang/modules.go
index 5f12d01..d54389d 100644
--- a/pkg/yang/modules.go
+++ b/pkg/yang/modules.go
@@ -29,6 +29,7 @@ type Modules struct {
 	Modules    map[string]*Module // All "module" nodes
 	SubModules map[string]*Module // All "submodule" nodes
 	includes   map[*Module]bool   // Modules we have already done include on
+	nsMu       sync.Mutex         // nsMu protects the byNS map.
 	byNS       map[string]*Module // Cache of namespace lookup
 	typeDict   *typeDictionary    // Cache for type definitions.
 	// entryCache is used to prevent unnecessary recursion into previously
@@ -39,7 +40,10 @@ type Modules struct {
 	// ignored. The keys of the map are a string that is formed by concatenating
 	// the name of the including (sub)module and the included submodule.
 	mergedSubmodule map[string]bool
-	mu              sync.Mutex // Mutex to protect byNS map
+	// ParseOptions sets the options for the current YANG module parsing. It can be
+	// directly set by the caller to influence how goyang will behave in the presence
+	// of certain exceptional cases.
+	ParseOptions Options
 }
 
 // NewModules returns a newly created and initialized Modules.
@@ -220,8 +224,8 @@ func (ms *Modules) FindModule(n Node) *Module {
 // or returns an error.
 func (ms *Modules) FindModuleByNamespace(ns string) (*Module, error) {
 	// Protect the byNS map from concurrent accesses
-	ms.mu.Lock()
-	defer ms.mu.Unlock()
+	ms.nsMu.Lock()
+	defer ms.nsMu.Unlock()
 
 	if m, ok := ms.byNS[ns]; ok {
 		return m, nil
diff --git a/pkg/yang/options.go b/pkg/yang/options.go
index ee259e7..879ab4e 100644
--- a/pkg/yang/options.go
+++ b/pkg/yang/options.go
@@ -28,8 +28,3 @@ type Options struct {
 	// is derived.
 	StoreUses bool
 }
-
-// ParseOptions sets the options for the current YANG module parsing. It can be
-// directly set by the caller to influence how goyang will behave in the presence
-// of certain exceptional cases.
-var ParseOptions = Options{}
diff --git a/yang.go b/yang.go
index b709c0c..8edfeeb 100644
--- a/yang.go
+++ b/yang.go
@@ -92,11 +92,12 @@ func main() {
 	var traceP string
 	var help bool
 	var paths []string
+	var ignoreSubmoduleCircularDependencies bool
 	getopt.ListVarLong(&paths, "path", 'p', "comma separated list of directories to add to search path", "DIR[,DIR...]")
 	getopt.StringVarLong(&format, "format", 'f', "format to display: "+strings.Join(formats, ", "), "FORMAT")
 	getopt.StringVarLong(&traceP, "trace", 't', "write trace into to TRACEFILE", "TRACEFILE")
 	getopt.BoolVarLong(&help, "help", 'h', "display help")
-	getopt.BoolVarLong(&yang.ParseOptions.IgnoreSubmoduleCircularDependencies, "ignore-circdep", 'g', "ignore circular dependencies between submodules")
+	getopt.BoolVarLong(&ignoreSubmoduleCircularDependencies, "ignore-circdep", 'g', "ignore circular dependencies between submodules")
 	getopt.SetParameters("[FORMAT OPTIONS] [SOURCE] [...]")
 
 	if err := getopt.Getopt(func(o getopt.Option) bool {
@@ -169,6 +170,7 @@ Formats:
 	files := getopt.Args()
 
 	ms := yang.NewModules()
+	ms.ParseOptions.IgnoreSubmoduleCircularDependencies = ignoreSubmoduleCircularDependencies
 
 	if len(files) == 0 {
 		data, err := ioutil.ReadAll(os.Stdin)


Test patch:
diff --git a/pkg/yang/entry_test.go b/pkg/yang/entry_test.go
index b1c1cc6..d6a8c59 100644
--- a/pkg/yang/entry_test.go
+++ b/pkg/yang/entry_test.go
@@ -504,10 +504,8 @@ module when {
 }
 
 func TestGetWhenXPath(t *testing.T) {
-	ParseOptions.StoreUses = true
-	defer func() { ParseOptions.StoreUses = false }()
-
 	ms := NewModules()
+	ms.ParseOptions.StoreUses = true
 	for _, tt := range testWhenModules {
 		if err := ms.Parse(tt.in, tt.name); err != nil {
 			t.Fatalf("could not parse module %s: %v", tt.name, err)
@@ -775,10 +773,8 @@ func TestAugmentedEntry(t *testing.T) {
 }
 
 func TestUsesEntry(t *testing.T) {
-	ParseOptions.StoreUses = true
-	defer func() { ParseOptions.StoreUses = false }()
-
 	ms := NewModules()
+	ms.ParseOptions.StoreUses = true
 	for _, tt := range testAugmentAndUsesModules {
 		if err := ms.Parse(tt.in, tt.name); err != nil {
 			t.Fatalf("could not parse module %s: %v", tt.name, err)
@@ -1024,7 +1020,7 @@ func TestIgnoreCircularDependencies(t *testing.T) {
 
 	for _, tt := range tests {
 		ms := NewModules()
-		ParseOptions.IgnoreSubmoduleCircularDependencies = tt.inIgnoreCircDep
+		ms.ParseOptions.IgnoreSubmoduleCircularDependencies = tt.inIgnoreCircDep
 		for n, m := range tt.inModules {
 			if err := ms.Parse(m, n); err != nil {
 				if !tt.wantErrs {
@@ -1674,7 +1670,7 @@ func TestFullModuleProcess(t *testing.T) {
 	for _, tt := range tests {
 		ms := NewModules()
 
-		ParseOptions.IgnoreSubmoduleCircularDependencies = tt.inIgnoreCircDeps
+		ms.ParseOptions.IgnoreSubmoduleCircularDependencies = tt.inIgnoreCircDeps
 		for n, m := range tt.inModules {
 			if err := ms.Parse(m, n); err != nil {
 				t.Errorf("%s: error parsing module %s, got: %v, want: nil", tt.name, n, err)