thema: Fix backwards compatibility checking bugs

The entire fix here was:

- Checking a closed def (`_#schema`) instead of the original, typically
  open `schema` field
- Getting rid of cue.Schema() and cue.Final() as options to Subsume(),
  because they weren't doing what we thought they did

Then all the backwards compatibility checks just started working,
exactly like we'd originally hoped.

bind.go

@@ -95,7 +95,7 @@ func (ml *maybeLineage) checkGoValidity(cfg *bindConfig) error {
 		sch.ref = schiter.Value()
 		sch.def = sch.ref.LookupPath(pathSchDef)
 		if previous != nil && !cfg.skipbuggychecks {
-			compaterr := compat.ThemaCompatible(previous.ref.LookupPath(pathSch), sch.ref.LookupPath(pathSch))
+			compaterr := compat.ThemaCompatible(previous.def, sch.def)
 			if sch.v[1] == 0 && compaterr == nil {
 				// Major version change, should be backwards incompatible
 				return errors.Mark(mkerror(sch.ref.LookupPath(pathSch), "schema %s must be backwards incompatible with schema %s: introduce a breaking change, or redeclare as version %s", sch.v, previous.v, synv(previous.v[0], previous.v[1]+1)), terrors.ErrInvalidLineage)

internal/compat/compat.go

@@ -5,9 +5,13 @@ import (
 )
 
 // ThemaCompatible is the canonical Thema algorithm for checking that the
-// cue.Value s is (backwards) compatible with p.
+// [cue.Value] s is (backwards) compatible with p. A nil return indicates
+// compatibility.
+//
+// The behavior of this function is undefined if s and p are not closed
+// structs. TODO check this and error if conditions aren't met
 func ThemaCompatible(p, s cue.Value) error {
-	return s.Subsume(p, cue.Raw(), cue.Schema(), cue.Definitions(true), cue.All(), cue.Final())
+	return s.Subsume(p, cue.Raw(), cue.All())
 }
 
 // type CompatInvariantError struct {

lineage_test.go

@@ -10,6 +10,7 @@ import (
 	"cuelang.org/go/cue/cuecontext"
 	"cuelang.org/go/cue/errors"
 	"cuelang.org/go/cue/load"
+
 	"github.com/grafana/thema/internal/txtartest/vanilla"
 )
 
@@ -64,11 +65,8 @@ func TestInvalidLineages(t *testing.T) {
 		Name:    "bindfail",
 		ThemaFS: CueJointFS,
 		ToDo: map[string]string{
-			"invalidlineage/joindef":                "no invariant checker written to disallow definitions from joinSchema",
-			"invalidlineage/onlydef":                "Lineage schema non-emptiness constraints are temporarily suspended while migrating grafana to flattened lineage structure",
-			"invalidlineage/compat/change-default":  "Thema compat analyzer fails to classify changes to default values as breaking",
-			"invalidlineage/compat/remove-required": "Required field removal is not detected as breaking changes",
-			"invalidlineage/compat/remove-optional": "Optional field removal is not detected as breaking changes",
+			"invalidlineage/joindef": "no invariant checker written to disallow definitions from joinSchema",
+			"invalidlineage/onlydef": "Lineage schema non-emptiness constraints are temporarily suspended while migrating grafana to flattened lineage structure",
 		},
 	}
 

testdata/invalidlineage/compat/change-default.txtar

@@ -14,6 +14,12 @@ schemas: [
     },
     {
         version: [0, 1]
+		schema: {
+		    aunion: *"foo" | "bar" | "baz"
+        }
+    },
+    {
+        version: [0, 2]
 		schema: {
 		    aunion: "foo" | "bar" | *"baz"
         }
@@ -29,6 +35,17 @@ lenses: [
             aunion: input.aunion
         }
     },
+    {
+        to: [0, 1]
+        from: [0, 2]
+        input: _
+        result: {
+            aunion: input.aunion
+        }
+    },
 ]
 -- out/bindfail --
-schema 0.1 must be backwards compatible with schema 0.0
+schema 0.2 is not backwards compatible with schema 0.1:
+field aunion not present in {aunion:*"foo" | "bar" | "baz"}:
+    /cue.mod/pkg/github.com/grafana/thema/lineage.cue:234:12
+missing field "aunion"

testdata/invalidlineage/compat/remove-optional.txtar

@@ -29,4 +29,6 @@ lin: lenses: [{
 	}
 }]
 -- out/bindfail --
-schema 0.1 must be backwards compatible with schema 0.0
+schema 0.1 is not backwards compatible with schema 0.0:
+field not allowed in closed struct: getsRemoved
+value not an instance

testdata/invalidlineage/compat/remove-required.txtar

@@ -30,4 +30,6 @@ lin: lenses: [{
 	}
 }]
 -- out/bindfail --
-schema 0.1 must be backwards compatible with schema 0.0
+schema 0.1 is not backwards compatible with schema 0.0:
+field not allowed in closed struct: getsRemoved
+value not an instance

testdata/invalidlineage/compat/union-reduction.txtar

@@ -0,0 +1,43 @@
+# reducing the permitted options in a union/disjunction is backwards incompatible
+#lineagePath: lin
+-- in.cue --
+
+import "github.com/grafana/thema"
+
+lin: thema.#Lineage
+lin: name: "union-reduction"
+lin: schemas: [{
+    version: [0, 0]
+    schema: {
+        concreteCross: "foo" | "bar" | 42
+        concreteString: "foo" | "bar" | "baz"
+        crossKind3: string | int32 | bytes
+        crossKind2: string | int32
+    }
+},
+{
+    version: [0, 1]
+    schema: {
+        concreteCross: "foo" | 42
+        concreteString: "foo" | "bar"
+        crossKind3: string | int32
+        crossKind2: string
+    }
+}]
+
+lin: lenses: [{
+	from: [0, 1]
+	to: [0, 0]
+	input: _
+	result: {
+        concreteCross: input.concreteCross
+        concreteString: input.concreteString
+        crossKind3: input.crossKind3
+        crossKind2: input.crossKind2
+	}
+}]
+-- out/bindfail --
+schema 0.1 is not backwards compatible with schema 0.0:
+field concreteCross not present in {concreteCross:"foo" | "bar" | 42,concreteString:"foo" | "bar" | "baz",crossKind3:string | >=-2147483648 & <=2147483647 & int | bytes,crossKind2:string | >=-2147483648 & <=2147483647 & int}:
+    /cue.mod/pkg/github.com/grafana/thema/lineage.cue:234:12
+missing field "concreteCross"