Apply deviations in two passes to fix non-deterministic errors

pkg/yang/entry.go

@@ -1129,14 +1129,33 @@ func (e *Entry) Augment(addErrors bool) (processed, skipped int) {
 func (e *Entry) ApplyDeviate(deviateOpts ...DeviateOpt) []error {
 	var errs []error
 	appendErr := func(err error) { errs = append(errs, err) }
+
+	pass := getDeviationPass(deviateOpts)
+
 	for _, d := range e.Deviations {
+		_, hasNotSupported := d.Deviate[DeviationNotSupported]
+		onlyHasNotSupported := hasNotSupported && len(d.Deviate) == 1
+		if pass == deviateSkipNotSupported && onlyHasNotSupported {
+			continue
+		}
+		if pass == deviateOnlyNotSupported && !hasNotSupported {
+			continue
+		}
+
 		deviatedNode := e.Find(d.DeviatedPath)
 		if deviatedNode == nil {
 			appendErr(fmt.Errorf("cannot find target node to deviate, %s", d.DeviatedPath))
 			continue
 		}
 
 		for dt, dv := range d.Deviate {
+			if pass == deviateSkipNotSupported && dt == DeviationNotSupported {
+				continue
+			}
+			if pass == deviateOnlyNotSupported && dt != DeviationNotSupported {
+				continue
+			}
+
 			for _, devSpec := range dv {
 				switch dt {
 				case DeviationAdd, DeviationReplace:

pkg/yang/entry_test.go

@@ -3658,6 +3658,50 @@ func TestDeviation(t *testing.T) {
 				},
 			},
 		},
+	}, {
+		desc: "not-supported and replace targeting the same node",
+		inFiles: map[string]string{
+			"source": `
+				module source {
+					prefix "s";
+					namespace "urn:s";
+
+					leaf a { type string; }
+					leaf b { type string; }
+				}`,
+			"dev-notsup": `
+				module dev-notsup {
+					prefix "dn";
+					namespace "urn:dn";
+
+					import source { prefix s; }
+
+					deviation /s:a {
+						deviate not-supported;
+					}
+				}`,
+			"dev-replace": `
+				module dev-replace {
+					prefix "dr";
+					namespace "urn:dr";
+
+					import source { prefix s; }
+
+					deviation /s:a {
+						deviate replace {
+							type uint16;
+						}
+					}
+				}`,
+		},
+		wants: map[string][]deviationTest{
+			"source": {{
+				path: "/a",
+			}, {
+				path:  "/b",
+				entry: &Entry{},
+			}},
+		},
 	}}
 
 	for _, tt := range tests {

pkg/yang/modules.go

@@ -392,18 +392,33 @@ func (ms *Modules) Process() []error {
 	// rather we can just walk all modules and submodules *after* entries
 	// are resolved. This means we do not need to concern ourselves that
 	// an entry does not exist.
+	// Deviations are applied in two passes so that not-supported deviations
+	// do not delete nodes before add/replace/delete deviations are applied.
+	// Without this, non-deterministic errors depend on map iteration order.
+	// Pass 1: Apply add/replace/delete deviations, skip not-supported.
+	errs = append(errs, ms.applyDeviations(
+		DeviateOptions{pass: deviateSkipNotSupported})...)
+	// Pass 2: Apply not-supported deviations only.
+	errs = append(errs, ms.applyDeviations(
+		DeviateOptions{pass: deviateOnlyNotSupported})...)
+
+	return errorSort(errs)
+}
+
+func (ms *Modules) applyDeviations(opts ...DeviateOpt) []error {
+	opts = append([]DeviateOpt{ms.ParseOptions.DeviateOptions}, opts...)
+	var errs []error
 	dvP := map[string]bool{} // cache the modules we've handled since we have both modname and modname@revision-date
 	for _, devmods := range []map[string]*Module{ms.Modules, ms.SubModules} {
 		for _, m := range devmods {
 			e := ToEntry(m)
 			if !dvP[e.Name] {
-				errs = append(errs, e.ApplyDeviate(ms.ParseOptions.DeviateOptions)...)
+				errs = append(errs, e.ApplyDeviate(opts...)...)
 				dvP[e.Name] = true
 			}
 		}
 	}
-
-	return errorSort(errs)
+	return errs
 }
 
 // include resolves all the include and import statements for m.  It returns

pkg/yang/options.go

@@ -39,6 +39,9 @@ type DeviateOptions struct {
 	// different support for a leaf without having to use a second instance
 	// of an AST.
 	IgnoreDeviateNotSupported bool
+
+	// pass controls what deviations are applied.
+	pass deviationPass
 }
 
 // IsDeviateOpt ensures that DeviateOptions satisfies the DeviateOpt interface.
@@ -49,6 +52,23 @@ type DeviateOpt interface {
 	IsDeviateOpt()
 }
 
+type deviationPass int
+
+const (
+	deviateAll              deviationPass = iota // Apply all deviations.
+	deviateSkipNotSupported                      // Skip not-supported deviations.
+	deviateOnlyNotSupported                      // Apply only not-supported deviations.
+)
+
+func getDeviationPass(opts []DeviateOpt) deviationPass {
+	for _, o := range opts {
+		if opt, ok := o.(DeviateOptions); ok && opt.pass != deviateAll {
+			return opt.pass
+		}
+	}
+	return deviateAll
+}
+
 func hasIgnoreDeviateNotSupported(opts []DeviateOpt) bool {
 	for _, o := range opts {
 		if opt, ok := o.(DeviateOptions); ok {