fix: refuse a null for an optional, non-nullable field (#4)

README.md

@@ -4,7 +4,7 @@ The Codefly language agent for typed, finite Go operations: generate a handler a
 
 **Status: the contract-determined generation slice is implemented; the agent process, harness and qualification are deferred until the Python/native/k3d contract is qualified.** No agent executable or release is available yet. [Implementation issue #1](https://github.com/codefly-dev/runnable-go/issues/1) records the original scope; [issue #2](https://github.com/codefly-dev/runnable-go/issues/2) tracks the remaining execution lifecycle.
 
-`pkg/generate` turns a runnable's declared contract into the typed Go bindings and the author handler scaffold. The bounded profile maps to `string`, `int64`, `bool`, generated structs and slices, and the contract's independent `optional` (the key may be absent) and `nullable` (the value may be null) declarations map to three distinct Go spellings so neither state can stand for the other.
+`pkg/generate` turns a runnable's declared contract into the typed Go bindings and the author handler scaffold. The bounded profile maps to `string`, `int64`, `bool`, generated structs and slices, and the contract's independent `optional` (the key may be absent) and `nullable` (the value may be null) declarations map to four distinct Go spellings, one per combination, so neither state can stand for the other.
 
 The agent gRPC process, the invocation harness, build evidence and native/k3d qualification wait on [core #472](https://github.com/codefly-dev/core/issues/472), which freezes the agent/CLI handoff `runnable.codefly.yaml` names but does not define: carrying a `RunnableIdentity` through `Builder.Load`, returning native launch and build evidence from `Builder.Package`, and the `codefly.runnable/v1` framing a harness must agree with byte for byte.
 

pkg/generate/bindings.go

@@ -11,22 +11,23 @@ import (
 	"github.com/codefly-dev/core/resources"
 )
 
-// Type names of a generated binding file: the two payload roots and the
-// carrier of a field declared both optional and nullable.
+// Type names of a generated binding file: the two payload roots and the two
+// carriers of a field the contract declares optional.
 const (
 	inputType    = "Input"
 	outputType   = "Output"
 	optionalType = "Optional"
+	presentType  = "Present"
 )
 
 // Bindings renders the typed Go bindings of a contract's input and output.
 //
 // The bounded profile maps to Go by value type and by the two declarations the
 // contract keeps independent: optional means the key may be absent, nullable
-// means the value may be null. A field that is only optional is a pointer
-// omitted when absent, a field that is only nullable is a pointer written as
-// null, and a field that is both is an Optional, which round-trips absent and
-// null as the distinct states they are.
+// means the value may be null. A field that is only optional is a Present,
+// omitted when absent and refusing the null it does not declare, a field that
+// is only nullable is a pointer written as null, and a field that is both is an
+// Optional, which round-trips absent and null as the distinct states they are.
 //
 // A Go nil slice encodes as null whether or not the array field is declared
 // nullable, so an array the contract never allows to be null is caught by
@@ -49,7 +50,10 @@ func Bindings(pkg string, contract *resources.RunnableContract) ([]byte, error)
 	var b strings.Builder
 	b.WriteString("// Code generated by runnable-go. DO NOT EDIT.\n\n")
 	fmt.Fprintf(&b, "package %s\n", pkg)
-	if g.usesOptional {
+	switch {
+	case g.usesPresent:
+		b.WriteString("\nimport (\n\t\"encoding/json\"\n\t\"errors\"\n)\n")
+	case g.usesOptional:
 		b.WriteString("\nimport \"encoding/json\"\n")
 	}
 	for _, decl := range g.decls {
@@ -60,6 +64,10 @@ func Bindings(pkg string, contract *resources.RunnableContract) ([]byte, error)
 		b.WriteString("\n")
 		b.WriteString(optionalDeclaration)
 	}
+	if g.usesPresent {
+		b.WriteString("\n")
+		b.WriteString(presentDeclaration)
+	}
 	return format.Source([]byte(b.String()))
 }
 
@@ -74,6 +82,7 @@ type generator struct {
 	queue        []pending
 	declared     map[string]struct{}
 	usesOptional bool
+	usesPresent  bool
 }
 
 // enqueue schedules one object type. Nested objects are rendered after the type
@@ -142,7 +151,10 @@ func (g *generator) fieldType(hint string, field *resources.RunnableField) (stri
 	case field.Optional && field.Nullable:
 		g.usesOptional = true
 		return fmt.Sprintf("%s[%s]", optionalType, base), nil
-	case field.Optional, field.Nullable:
+	case field.Optional:
+		g.usesPresent = true
+		return fmt.Sprintf("%s[%s]", presentType, base), nil
+	case field.Nullable:
 		return "*" + base, nil
 	default:
 		return base, nil
@@ -233,3 +245,57 @@ func (o *Optional[T]) UnmarshalJSON(data []byte) error {
 	return nil
 }
 `
+
+const presentDeclaration = `// Present carries a field the contract declares optional and not nullable: the
+// key may be absent, and no value it carries may be null.
+//
+// It is a value and not a pointer because encoding/json sets a pointer field to
+// nil for an explicit null without ever consulting an unmarshaler, which admits
+// a null the declaration forbids and leaves it indistinguishable from the key
+// being absent.
+//
+// The value is reachable only through Value, and the zero Present is absent, so
+// no literal can spell a key that is absent and carries a value at once.
+type Present[T any] struct {
+	set   bool
+	value T
+}
+
+// Value is the state of a key the payload carries.
+func Value[T any](value T) Present[T] {
+	return Present[T]{set: true, value: value}
+}
+
+// Get returns the carried value, and false when the key was absent.
+func (p Present[T]) Get() (T, bool) {
+	return p.value, p.set
+}
+
+// IsZero reports an absent key, which is what omits it from an encoded payload.
+func (p Present[T]) IsZero() bool {
+	return !p.set
+}
+
+// MarshalJSON encodes the carried value. The absent state has no encoding as a
+// value: every generated field carries omitzero, so an absent Present is
+// omitted with its key and never reaches here. Reaching here absent means the
+// value is being encoded where a key cannot be omitted, where null would stand
+// in for absent and would itself be a value the declaration forbids.
+func (p Present[T]) MarshalJSON() ([]byte, error) {
+	if !p.set {
+		return nil, errors.New("an absent optional value has no encoding: it is omitted with its key, never written as null")
+	}
+	return json.Marshal(p.value)
+}
+
+func (p *Present[T]) UnmarshalJSON(data []byte) error {
+	if string(data) == "null" {
+		return errors.New("value is null but the contract does not declare the field nullable")
+	}
+	if err := json.Unmarshal(data, &p.value); err != nil {
+		return err
+	}
+	p.set = true
+	return nil
+}
+`

pkg/generate/bindings_test.go

@@ -75,25 +75,55 @@ func TestOptionalAndNullableAreIndependent(t *testing.T) {
 	)
 
 	requireLine(t, source, "Required string `json:\"required\"`")
-	requireLine(t, source, "AbsentOrSet *string `json:\"absent_or_set,omitzero\"`")
+	requireLine(t, source, "AbsentOrSet Present[string] `json:\"absent_or_set,omitzero\"`")
 	requireLine(t, source, "NullOrSet *string `json:\"null_or_set\"`")
 	requireLine(t, source, "AbsentOrNullOrSet Optional[string] `json:\"absent_or_null_or_set,omitzero\"`")
 	if !strings.Contains(source, "type Optional[T any] struct") {
 		t.Fatalf("Optional is used but not declared:\n%s", source)
 	}
+	if !strings.Contains(source, "type Present[T any] struct") {
+		t.Fatalf("Present is used but not declared:\n%s", source)
+	}
+}
+
+// An optional, non-nullable field is a Present and never a pointer:
+// encoding/json sets a pointer field to nil for an explicit null without
+// consulting an unmarshaler, which would admit a null the declaration forbids
+// and leave it indistinguishable from the key being absent.
+func TestOptionalAndNotNullableIsNeverAPointer(t *testing.T) {
+	source := generated(t,
+		[]*resources.RunnableField{
+			{Name: "label", Type: resources.RunnableFieldString, Optional: true},
+			{Name: "count", Type: resources.RunnableFieldInteger, Optional: true},
+			{Name: "enabled", Type: resources.RunnableFieldBoolean, Optional: true},
+			{Name: "options", Type: resources.RunnableFieldObject, Optional: true, Fields: []*resources.RunnableField{
+				field("limit", resources.RunnableFieldInteger),
+			}},
+			{Name: "tags", Type: resources.RunnableFieldArray, Optional: true, Items: field("", resources.RunnableFieldString)},
+		},
+		[]*resources.RunnableField{field("total", resources.RunnableFieldInteger)},
+	)
+
+	requireLine(t, source, "Label Present[string] `json:\"label,omitzero\"`")
+	requireLine(t, source, "Count Present[int64] `json:\"count,omitzero\"`")
+	requireLine(t, source, "Enabled Present[bool] `json:\"enabled,omitzero\"`")
+	requireLine(t, source, "Options Present[InputOptions] `json:\"options,omitzero\"`")
+	requireLine(t, source, "Tags Present[[]string] `json:\"tags,omitzero\"`")
 }
 
-// Optional is only declared when the contract needs it.
-func TestOptionalIsNotDeclaredWhenUnused(t *testing.T) {
+// A carrier is only declared when the contract needs it.
+func TestCarriersAreNotDeclaredWhenUnused(t *testing.T) {
 	source := generated(t,
 		[]*resources.RunnableField{{Name: "note", Type: resources.RunnableFieldString, Nullable: true}},
 		[]*resources.RunnableField{field("total", resources.RunnableFieldInteger)},
 	)
-	if strings.Contains(source, "Optional") {
-		t.Fatalf("Optional declared for a contract that has no optional-and-nullable field:\n%s", source)
+	for _, carrier := range []string{"Optional", "Present"} {
+		if strings.Contains(source, carrier) {
+			t.Fatalf("%s declared for a contract that has no optional field:\n%s", carrier, source)
+		}
 	}
 	if strings.Contains(source, "encoding/json") {
-		t.Fatalf("bindings import encoding/json without Optional:\n%s", source)
+		t.Fatalf("bindings import encoding/json without a carrier:\n%s", source)
 	}
 }
 
@@ -116,7 +146,7 @@ func TestNestedObjectsAreNamedByPath(t *testing.T) {
 		},
 	)
 
-	requireLine(t, source, "Options *InputOptions `json:\"options,omitzero\"`")
+	requireLine(t, source, "Options Present[InputOptions] `json:\"options,omitzero\"`")
 	requireLine(t, source, "StopWords *[]string `json:\"stop_words\"`")
 	requireLine(t, source, "Records []InputRecordsItem `json:\"records\"`")
 	requireLine(t, source, "type InputRecordsItem struct {")

pkg/generate/compile_test.go

@@ -84,10 +84,10 @@ func TestAbsentNullAndSetAreThreeStates(t *testing.T) {
 	if absent.Flag.Present {
 		t.Error("an absent optional-and-nullable key decoded as present")
 	}
-	if absent.Count != nil {
+	if _, ok := absent.Count.Get(); ok {
 		t.Error("an absent optional key decoded as set")
 	}
-	if absent.Options != nil {
+	if _, ok := absent.Options.Get(); ok {
 		t.Error("an absent optional object decoded as set")
 	}
 	if absent.Note != nil {
@@ -103,13 +103,13 @@ func TestAbsentNullAndSetAreThreeStates(t *testing.T) {
 	if !set.Flag.Present || set.Flag.Value == nil || !*set.Flag.Value {
 		t.Errorf("a set optional-and-nullable key decoded as %+v", set.Flag)
 	}
-	if set.Count == nil || *set.Count != 7 {
-		t.Errorf("count decoded as %v", set.Count)
+	if count, ok := set.Count.Get(); !ok || count != 7 {
+		t.Errorf("count decoded as %+v", set.Count)
 	}
 	if set.Note == nil || *set.Note != "n" {
 		t.Errorf("note decoded as %v", set.Note)
 	}
-	if set.Options == nil || set.Options.Limit != 3 {
+	if options, ok := set.Options.Get(); !ok || options.Limit != 3 {
 		t.Errorf("options decoded as %+v", set.Options)
 	}
 }
@@ -138,6 +138,30 @@ func TestEncodingOmitsAbsentKeysAndWritesNull(t *testing.T) {
 	if !strings.Contains(null, ` + "`" + `"flag":null` + "`" + `) {
 		t.Errorf("a present-and-null key was not encoded as null: %s", null)
 	}
+
+	set := encoded(t, Input{Text: "a", Scores: []int64{}, Count: Value(int64(7))})
+	if !strings.Contains(set, ` + "`" + `"count":7` + "`" + `) {
+		t.Errorf("a set optional key was not encoded: %s", set)
+	}
+}
+
+// A field the contract declares optional and not nullable has no null state, so
+// a null is refused rather than decoded as the absent key it is not.
+func TestNullIsRefusedForAnOptionalFieldThatIsNotNullable(t *testing.T) {
+	for _, payload := range []string{
+		` + "`" + `{"text":"a","note":null,"scores":[],"count":null}` + "`" + `,
+		` + "`" + `{"text":"a","note":null,"scores":[],"options":null}` + "`" + `,
+	} {
+		var in Input
+		if err := json.Unmarshal([]byte(payload), &in); err == nil {
+			t.Errorf("%s decoded without error", payload)
+		}
+	}
+
+	var in Input
+	if err := json.Unmarshal([]byte(` + "`" + `{"text":"a","note":null,"flag":null,"scores":[]}` + "`" + `), &in); err != nil {
+		t.Errorf("a null for a key the contract declares nullable was refused: %v", err)
+	}
 }
 
 func TestIntegerIsSigned64Bit(t *testing.T) {