Skip to content

Commit e36fb81

Browse files
authored
feat(dart): use proto presence info to generate null-safe code (#2726)
Use proto presence info to generate null-safe code: - `annotateField` updated to use information from proto optional annotations - toJson() implementations write values if they're nullable and have a non-null value, or if they're not nullable and have a non-default value for that type - fromJson parses a value for parse a value for nullable fields if one is given; it will parse any value for non-null field or fall back to using the default value for that type - properly dereference null values when referring to request fields for use in url paths - enums definitions now get a default value (`EnumClass.$default`) - removed the older annotate.requiredFields and tests (based on usage in service calls) - closes googleapis/google-cloud-dart#24 Tests are updated for `buildQueryLines()` to include optional fields. Tests were added for the `createFromJsonLine()` and `createToJsonLine()` methods; previously we got our testing here from being on the same repo as the dart code, and running the dart unit tests on the generator's putput. In-lined from the PR: > - proto 3 fields default to implicit presence > - the 'optional' keyword changes a field to explicit presence > - types like lists (repeated) and maps are always implicit presence > > Explicit presence means that you can know whether the user set a value or > not. Implicit presence means you can always retrieve a value; if one had > not been set, you'll see the default value for that type. > > We translate explicit presence (a optional annotation) to using a nullable > type for that field. We translate implicit presence (always returning some > value) to a non-null type. > > Some short-hand: > - optional == explicit == nullable > - implicit == non-nullable > - lists and maps == implicit == non-nullable > - singular message == explicit == nullable Sample output can be seen at googleapis/google-cloud-dart#58.
1 parent e9fc828 commit e36fb81

File tree

6 files changed

+472
-133
lines changed

6 files changed

+472
-133
lines changed

internal/sidekick/internal/dart/annotate.go

Lines changed: 189 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"errors"
1919
"fmt"
2020
"log/slog"
21+
"slices"
2122
"sort"
2223
"strconv"
2324
"strings"
@@ -156,19 +157,22 @@ type operationInfoAnnotation struct {
156157
}
157158

158159
type fieldAnnotation struct {
159-
Name string
160-
Type string
161-
DocLines []string
162-
Required bool
163-
Nullable bool
164-
FromJson string
165-
ToJson string
160+
Name string
161+
Type string
162+
DocLines []string
163+
Required bool
164+
Nullable bool
165+
FieldBehaviorRequired bool
166+
DefaultValue string
167+
FromJson string
168+
ToJson string
166169
}
167170

168171
type enumAnnotation struct {
169-
Name string
170-
DocLines []string
171-
Model *api.API
172+
Name string
173+
DocLines []string
174+
DefaultValue string
175+
Model *api.API
172176
}
173177

174178
type enumValueAnnotation struct {
@@ -200,8 +204,6 @@ type annotateModel struct {
200204
packageMapping map[string]string
201205
// The protobuf packages that need to be imported with prefixes.
202206
packagePrefixes map[string]string
203-
// A mapping from field IDs to fields for the fields we know to be required.
204-
requiredFields map[string]*api.Field
205207
// A mapping from a package name (e.g. "http") to its version constraint (e.g. "^1.3.0").
206208
dependencyConstraints map[string]string
207209
}
@@ -213,7 +215,6 @@ func newAnnotateModel(model *api.API) *annotateModel {
213215
imports: map[string]bool{},
214216
packageMapping: map[string]string{},
215217
packagePrefixes: map[string]string{},
216-
requiredFields: map[string]*api.Field{},
217218
dependencyConstraints: map[string]string{},
218219
}
219220
}
@@ -331,9 +332,6 @@ func (annotate *annotateModel) annotateModel(options map[string]string) error {
331332

332333
model := annotate.model
333334

334-
// Calculate required fields.
335-
annotate.requiredFields = calculateRequiredFields(model)
336-
337335
// Traverse and annotate the enums defined in this API.
338336
for _, e := range model.Enums {
339337
annotate.annotateEnum(e)
@@ -422,31 +420,6 @@ func (annotate *annotateModel) annotateModel(options map[string]string) error {
422420
return nil
423421
}
424422

425-
func calculateRequiredFields(model *api.API) map[string]*api.Field {
426-
required := map[string]*api.Field{}
427-
428-
for _, s := range model.Services {
429-
// Some methods are skipped.
430-
methods := language.FilterSlice(s.Methods, func(m *api.Method) bool {
431-
return shouldGenerateMethod(m)
432-
})
433-
434-
for _, method := range methods {
435-
for _, field := range language.PathParams(method, model.State) {
436-
required[field.ID] = field
437-
}
438-
439-
for _, field := range method.InputType.Fields {
440-
if field.Name == method.PathInfo.BodyFieldPath {
441-
required[field.ID] = field
442-
}
443-
}
444-
}
445-
}
446-
447-
return required
448-
}
449-
450423
// calculatePubPackages returns a set of package names (e.g. "http"), given a
451424
// set of imports (e.g. "package:http/http.dart as http").
452425
func calculatePubPackages(imports map[string]bool) map[string]bool {
@@ -650,7 +623,7 @@ func (annotate *annotateModel) annotateMethod(method *api.Method) {
650623
queryParams := language.QueryParams(method, method.PathInfo.Bindings[0])
651624
queryLines := []string{}
652625
for _, field := range queryParams {
653-
queryLines = buildQueryLines(queryLines, "request.", "", field, state)
626+
queryLines = annotate.buildQueryLines(queryLines, "request.", "", field, state)
654627
}
655628

656629
annotation := &methodAnnotation{
@@ -687,35 +660,149 @@ func (annotate *annotateModel) annotateOneOf(oneof *api.OneOf) {
687660
}
688661

689662
func (annotate *annotateModel) annotateField(field *api.Field) {
690-
_, required := annotate.requiredFields[field.ID]
663+
// Here, we calculate the nullability / required status of a field. For this
664+
// we use the proto field presence information.
665+
//
666+
// For edification of our readers:
667+
// - proto 3 fields default to implicit presence
668+
// - the 'optional' keyword changes a field to explicit presence
669+
// - types like lists (repeated) and maps are always implicit presence
670+
//
671+
// Explicit presence means that you can know whether the user set a value or
672+
// not. Implicit presence means you can always retrieve a value; if one had
673+
// not been set, you'll see the default value for that type.
674+
//
675+
// We translate explicit presence (a optional annotation) to using a nullable
676+
// type for that field. We translate implicit presence (always returning some
677+
// value) to a non-null type.
678+
//
679+
// Some short-hand:
680+
// - optional == explicit == nullable
681+
// - implicit == non-nullable
682+
// - lists and maps == implicit == non-nullable
683+
// - singular message == explicit == nullable
684+
//
685+
// See also https://protobuf.dev/programming-guides/field_presence/.
686+
687+
var implicitPresence bool
688+
689+
if field.Repeated || field.Map {
690+
// Repeated fields and maps have implicit presence (non-nullable).
691+
implicitPresence = true
692+
} else if field.Typez == api.MESSAGE_TYPE {
693+
// In proto3, singular message fields have explicit presence and are nullable.
694+
implicitPresence = false
695+
} else {
696+
if field.IsOneOf {
697+
// If this field is part of a oneof, it may or may not have a value; we
698+
// translate that as nullable (explicit presence).
699+
implicitPresence = false
700+
} else if field.Optional {
701+
// The optional keyword makes the field have explicit presence (nullable).
702+
implicitPresence = false
703+
} else {
704+
// Proto3 does not track presence for basic types (implicit presence).
705+
implicitPresence = true
706+
}
707+
}
708+
709+
// We interpret proto implicit presence as non-nullable for Dart.
710+
required := implicitPresence
711+
712+
// We can't make 'bytes' required, as UInt8List in Dart can't be const.
713+
if field.Typez == api.BYTES_TYPE {
714+
required = false
715+
}
716+
717+
// Calculate the default field value.
718+
defaultValues := map[api.Typez]string{
719+
api.BOOL_TYPE: "false",
720+
api.DOUBLE_TYPE: "0",
721+
api.FIXED32_TYPE: "0",
722+
api.FIXED64_TYPE: "0",
723+
api.FLOAT_TYPE: "0",
724+
api.INT32_TYPE: "0",
725+
api.INT64_TYPE: "0",
726+
api.SFIXED32_TYPE: "0",
727+
api.SFIXED64_TYPE: "0",
728+
api.SINT32_TYPE: "0",
729+
api.SINT64_TYPE: "0",
730+
api.STRING_TYPE: "''",
731+
api.UINT32_TYPE: "0",
732+
api.UINT64_TYPE: "0",
733+
}
734+
defaultValue := ""
735+
if required {
736+
switch {
737+
case field.Repeated:
738+
defaultValue = "const []"
739+
case field.Map:
740+
defaultValue = "const {}"
741+
case field.Typez == api.ENUM_TYPE:
742+
// The default value for enums are the generated MyEnum.$default field,
743+
// always set to the first value of that enum.
744+
typeName := enumName(annotate.state.EnumByID[field.TypezID])
745+
defaultValue = fmt.Sprintf("%s.$default", typeName)
746+
default:
747+
defaultValue = defaultValues[field.Typez]
748+
}
749+
}
750+
691751
state := annotate.state
692752

693753
field.Codec = &fieldAnnotation{
694-
Name: fieldName(field),
695-
Type: annotate.fieldType(field),
696-
DocLines: formatDocComments(field.Documentation, state),
697-
Required: required,
698-
Nullable: !required,
699-
FromJson: annotate.createFromJsonLine(field, state, required),
700-
ToJson: createToJsonLine(field, state, required),
754+
Name: fieldName(field),
755+
Type: annotate.fieldType(field),
756+
DocLines: formatDocComments(field.Documentation, state),
757+
Required: required,
758+
Nullable: !required,
759+
FieldBehaviorRequired: slices.Contains(field.Behavior, api.FIELD_BEHAVIOR_REQUIRED),
760+
DefaultValue: defaultValue,
761+
FromJson: annotate.createFromJsonLine(field, state, required),
762+
ToJson: createToJsonLine(field, state, required),
701763
}
702764
}
703765

704766
func (annotate *annotateModel) createFromJsonLine(field *api.Field, state *api.APIState, required bool) string {
705767
message := state.MessageByID[field.TypezID]
706768

707-
isList := field.Repeated
708-
isMap := message != nil && message.IsMap
709-
710769
data := fmt.Sprintf("json['%s']", field.JSONName)
711770

712-
bang := "!"
713-
if !required {
714-
bang = ""
771+
bang := ""
772+
if required {
773+
switch {
774+
case field.Repeated:
775+
bang = " ?? []"
776+
case field.Map:
777+
bang = " ?? {}"
778+
case field.Typez == api.ENUM_TYPE:
779+
// 'ExecutableCode_Language.$default'
780+
typeName := enumName(annotate.state.EnumByID[field.TypezID])
781+
bang = fmt.Sprintf(" ?? %s.$default", typeName)
782+
default:
783+
defaultValues := map[api.Typez]string{
784+
api.BOOL_TYPE: "false",
785+
api.BYTES_TYPE: "Uint8List()",
786+
api.DOUBLE_TYPE: "0",
787+
api.FIXED32_TYPE: "0",
788+
api.FIXED64_TYPE: "0",
789+
api.FLOAT_TYPE: "0",
790+
api.INT32_TYPE: "0",
791+
api.INT64_TYPE: "0",
792+
api.SFIXED32_TYPE: "0",
793+
api.SFIXED64_TYPE: "0",
794+
api.SINT32_TYPE: "0",
795+
api.SINT64_TYPE: "0",
796+
api.STRING_TYPE: "''",
797+
api.UINT32_TYPE: "0",
798+
api.UINT64_TYPE: "0",
799+
}
800+
bang = fmt.Sprintf(" ?? %s", defaultValues[field.Typez])
801+
}
715802
}
716803

717804
switch {
718-
case isList:
805+
case field.Repeated:
719806
switch field.Typez {
720807
case api.BYTES_TYPE:
721808
return fmt.Sprintf("decodeListBytes(%s)%s", data, bang)
@@ -733,7 +820,7 @@ func (annotate *annotateModel) createFromJsonLine(field *api.Field, state *api.A
733820
default:
734821
return fmt.Sprintf("decodeList(%s)%s", data, bang)
735822
}
736-
case isMap:
823+
case field.Map:
737824
valueField := message.Fields[1]
738825

739826
switch valueField.Typez {
@@ -759,6 +846,12 @@ func (annotate *annotateModel) createFromJsonLine(field *api.Field, state *api.A
759846
return fmt.Sprintf("decodeInt64(%s)%s", data, bang)
760847
case field.Typez == api.FLOAT_TYPE || field.Typez == api.DOUBLE_TYPE:
761848
return fmt.Sprintf("decodeDouble(%s)%s", data, bang)
849+
case field.Typez == api.INT32_TYPE || field.Typez == api.FIXED32_TYPE ||
850+
field.Typez == api.SFIXED32_TYPE || field.Typez == api.SINT32_TYPE ||
851+
field.Typez == api.UINT32_TYPE ||
852+
field.Typez == api.BOOL_TYPE ||
853+
field.Typez == api.STRING_TYPE:
854+
return fmt.Sprintf("%s%s", data, bang)
762855
case field.Typez == api.BYTES_TYPE:
763856
return fmt.Sprintf("decodeBytes(%s)%s", data, bang)
764857
case field.Typez == api.ENUM_TYPE:
@@ -768,9 +861,9 @@ func (annotate *annotateModel) createFromJsonLine(field *api.Field, state *api.A
768861
_, hasCustomEncoding := usesCustomEncoding[field.TypezID]
769862
typeName := annotate.resolveTypeName(state.MessageByID[field.TypezID], true)
770863
if hasCustomEncoding {
771-
return fmt.Sprintf("decodeCustom(%s, %s.fromJson)%s", data, typeName, bang)
864+
return fmt.Sprintf("decodeCustom(%s, %s.fromJson)", data, typeName)
772865
} else {
773-
return fmt.Sprintf("decode(%s, %s.fromJson)%s", data, typeName, bang)
866+
return fmt.Sprintf("decode(%s, %s.fromJson)", data, typeName)
774867
}
775868
}
776869

@@ -839,23 +932,34 @@ func createToJsonLine(field *api.Field, state *api.APIState, required bool) stri
839932
// - primitives, lists of primitives and enums are supported
840933
// - repeated fields are passed as lists
841934
// - messages need to be unrolled and fields passed individually
842-
func buildQueryLines(
935+
func (annotate *annotateModel) buildQueryLines(
843936
result []string, refPrefix string, paramPrefix string,
844937
field *api.Field, state *api.APIState,
845938
) []string {
846939
message := state.MessageByID[field.TypezID]
847940
isMap := message != nil && message.IsMap
848941

942+
if field.Codec == nil {
943+
annotate.annotateField(field)
944+
}
945+
codec := field.Codec.(*fieldAnnotation)
946+
849947
ref := fmt.Sprintf("%s%s", refPrefix, fieldName(field))
850948
param := fmt.Sprintf("%s%s", paramPrefix, field.JSONName)
851-
preable := fmt.Sprintf("if (%s != null) '%s'", ref, param)
949+
950+
var preable string
951+
if codec.Nullable {
952+
preable = fmt.Sprintf("if (%s != null) '%s'", ref, param)
953+
} else {
954+
preable = fmt.Sprintf("if (%s.isNotDefault) '%s'", ref, param)
955+
}
852956

853957
switch {
854958
case field.Repeated:
855959
// Handle lists; these should be lists of strings or other primitives.
856960
switch field.Typez {
857961
case api.STRING_TYPE:
858-
return append(result, fmt.Sprintf("%s: %s!", preable, ref))
962+
return append(result, fmt.Sprintf("%s: %s", preable, ref))
859963
case api.ENUM_TYPE:
860964
return append(result, fmt.Sprintf("%s: %s!.map((e) => e.value)", preable, ref))
861965
case api.BOOL_TYPE, api.INT32_TYPE, api.UINT32_TYPE, api.SINT32_TYPE,
@@ -876,16 +980,25 @@ func buildQueryLines(
876980
return append(result, fmt.Sprintf("/* unhandled query param type: %d */", field.Typez))
877981

878982
case field.Typez == api.MESSAGE_TYPE:
983+
deref := "."
984+
if codec.Nullable {
985+
deref = "!."
986+
}
987+
879988
// Unroll the fields for messages.
880989
for _, field := range message.Fields {
881-
result = buildQueryLines(result, ref+"?.", param+".", field, state)
990+
result = annotate.buildQueryLines(result, ref+deref, param+".", field, state)
882991
}
883992
return result
884993

885994
case field.Typez == api.STRING_TYPE:
886-
return append(result, fmt.Sprintf("%s: %s!", preable, ref))
995+
deref := ""
996+
if codec.Nullable {
997+
deref = "!"
998+
}
999+
return append(result, fmt.Sprintf("%s: %s%s", preable, ref, deref))
8871000
case field.Typez == api.ENUM_TYPE:
888-
return append(result, fmt.Sprintf("%s: %s!.value", preable, ref))
1001+
return append(result, fmt.Sprintf("%s: %s.value", preable, ref))
8891002
case field.Typez == api.BOOL_TYPE ||
8901003
field.Typez == api.INT32_TYPE ||
8911004
field.Typez == api.UINT32_TYPE || field.Typez == api.SINT32_TYPE ||
@@ -907,10 +1020,17 @@ func (annotate *annotateModel) annotateEnum(enum *api.Enum) {
9071020
for _, ev := range enum.Values {
9081021
annotate.annotateEnumValue(ev)
9091022
}
1023+
1024+
defaultValue := ""
1025+
if len(enum.Values) > 0 {
1026+
defaultValue = enumValueName(enum.Values[0])
1027+
}
1028+
9101029
enum.Codec = &enumAnnotation{
911-
Name: enumName(enum),
912-
DocLines: formatDocComments(enum.Documentation, annotate.state),
913-
Model: annotate.model,
1030+
Name: enumName(enum),
1031+
DocLines: formatDocComments(enum.Documentation, annotate.state),
1032+
DefaultValue: defaultValue,
1033+
Model: annotate.model,
9141034
}
9151035
}
9161036

0 commit comments

Comments
 (0)