Skip to content

Commit 117d767

Browse files
fix(sidekick/rust): Fix generated setter samples corner cases (#3179)
1 parent 989869d commit 117d767

16 files changed

+777
-81
lines changed

internal/sidekick/api/documentation.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func patchMethodDocs(svc *Service, name string, override *config.DocumentationOv
113113
}
114114

115115
func patchElementDocs(documentation *string, override *config.DocumentationOverride) error {
116-
new := strings.ReplaceAll(*documentation, override.Match, override.Replace)
116+
new := strings.ReplaceAll(*documentation, strings.ReplaceAll(override.Match, "\r\n", "\n"), override.Replace)
117117
if *documentation == new {
118118
slog.Error("comment override mismatch", "id", override.ID, "want", override.Match, "text", *documentation)
119119
return fmt.Errorf("comment override for %s did not match", override.ID)

internal/sidekick/api/model.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -835,9 +835,23 @@ func (f *Field) IsBool() bool {
835835
// in the broad category of field type involved.
836836
func (f *Field) IsLikeInt() bool {
837837
switch f.Typez {
838-
case UINT32_TYPE, UINT64_TYPE, INT32_TYPE, INT64_TYPE, SINT32_TYPE, SINT64_TYPE:
838+
case INT32_TYPE, INT64_TYPE, SINT32_TYPE, SINT64_TYPE:
839839
return true
840-
case FIXED32_TYPE, FIXED64_TYPE, SFIXED32_TYPE, SFIXED64_TYPE:
840+
case SFIXED32_TYPE, SFIXED64_TYPE:
841+
return true
842+
default:
843+
return false
844+
}
845+
}
846+
847+
// IsLikeUInt returns true if the primitive type of a field is one of the
848+
// unsigned integer types.
849+
//
850+
// This is useful for mustache templates that differ only
851+
// in the broad category of field type involved.
852+
func (f *Field) IsLikeUInt() bool {
853+
switch f.Typez {
854+
case UINT32_TYPE, UINT64_TYPE, FIXED32_TYPE, FIXED64_TYPE:
841855
return true
842856
default:
843857
return false

internal/sidekick/api/model_test.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ func TestFieldTypePredicates(t *testing.T) {
239239
isBytes bool
240240
isBool bool
241241
isInt bool
242+
isUInt bool
242243
isFloat bool
243244
isEnum bool
244245
isObject bool
@@ -249,14 +250,14 @@ func TestFieldTypePredicates(t *testing.T) {
249250
{field: &Field{Typez: BOOL_TYPE}, isBool: true},
250251
{field: &Field{Typez: INT32_TYPE}, isInt: true},
251252
{field: &Field{Typez: INT64_TYPE}, isInt: true},
252-
{field: &Field{Typez: UINT32_TYPE}, isInt: true},
253-
{field: &Field{Typez: UINT64_TYPE}, isInt: true},
254253
{field: &Field{Typez: SINT32_TYPE}, isInt: true},
255254
{field: &Field{Typez: SINT64_TYPE}, isInt: true},
256-
{field: &Field{Typez: FIXED32_TYPE}, isInt: true},
257-
{field: &Field{Typez: FIXED64_TYPE}, isInt: true},
258255
{field: &Field{Typez: SFIXED32_TYPE}, isInt: true},
259256
{field: &Field{Typez: SFIXED64_TYPE}, isInt: true},
257+
{field: &Field{Typez: UINT32_TYPE}, isUInt: true},
258+
{field: &Field{Typez: UINT64_TYPE}, isUInt: true},
259+
{field: &Field{Typez: FIXED32_TYPE}, isUInt: true},
260+
{field: &Field{Typez: FIXED64_TYPE}, isUInt: true},
260261
{field: &Field{Typez: FLOAT_TYPE}, isFloat: true},
261262
{field: &Field{Typez: DOUBLE_TYPE}, isFloat: true},
262263
{field: &Field{Typez: ENUM_TYPE}, isEnum: true},
@@ -275,6 +276,9 @@ func TestFieldTypePredicates(t *testing.T) {
275276
if tc.field.IsLikeInt() != tc.isInt {
276277
t.Errorf("IsLikeInt() for %v should be %v", tc.field.Typez, tc.isInt)
277278
}
279+
if tc.field.IsLikeUInt() != tc.isUInt {
280+
t.Errorf("IsLikeUInt() for %v should be %v", tc.field.Typez, tc.isUInt)
281+
}
278282
if tc.field.IsLikeFloat() != tc.isFloat {
279283
t.Errorf("IsLikeFloat() for %v should be %v", tc.field.Typez, tc.isFloat)
280284
}

internal/sidekick/rust/annotate.go

Lines changed: 78 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -407,16 +407,20 @@ type oneOfAnnotation struct {
407407
QualifiedName string
408408
// The fully qualified name, relative to `codec.modulePath`. Typically this
409409
// is the `QualifiedName` with the `crate::model::` prefix removed.
410-
// This is always relative to `ScopeInExamples`.
411410
RelativeName string
412411
// The Rust `struct` that contains this oneof, fully qualified
413412
StructQualifiedName string
414-
// The scope to show in examples. For messages in external packages
413+
// The fully qualified name for examples. For messages in external packages
415414
// this is basically `QualifiedName`. For messages in the current package
416415
// this includes `modelAnnotations.PackageName`.
417-
ScopeInExamples string
418-
FieldType string
419-
DocLines []string
416+
NameInExamples string
417+
// The unqualified oneof name may be the same as the unqualified name of the
418+
// containing type. If that happens we need to alias one of them.
419+
AliasInExamples string
420+
// This is AliasInExamples if there's one, EnumName otherwise.
421+
EnumNameInExamples string
422+
FieldType string
423+
DocLines []string
420424
// The best field to show in a oneof related samples.
421425
// Non deprecated fields are preferred, then scalar, repeated, map fields
422426
// in that order.
@@ -443,6 +447,9 @@ type fieldAnnotations struct {
443447
FieldType string
444448
PrimitiveFieldType string
445449
AddQueryParameter string
450+
// For fields that are singular mesaage or list of messages, this is the
451+
// message type.
452+
MessageType *api.Message
446453
// For fields that are maps, these are the type of the key and value,
447454
// respectively.
448455
KeyType string
@@ -461,6 +468,15 @@ type fieldAnnotations struct {
461468
// If true, this is a `wkt::NullValue` field, and also requires super-extra
462469
// custom deserialization.
463470
IsWktNullValue bool
471+
// Some fields may be the type of the message they are defined in.
472+
// We need to know this in sample generation to avoid importing
473+
// the parent type twice.
474+
// This applies to single value, repeated and map fields.
475+
FieldTypeIsParentType bool
476+
// In some cases, for instance, for OpenApi and Discovery synthetic requests,
477+
// types in different namespaces have the same unqualified name. When the field type and the
478+
// containing type have the same unqualified name, we need to alias one of those.
479+
AliasInExamples string
464480
// If this field is part of a oneof group, this will contain the other fields
465481
// in the group.
466482
OtherFieldsInGroup []*api.Field
@@ -476,6 +492,24 @@ func (a *fieldAnnotations) RequiresSerdeAs() bool {
476492
return a.SerdeAs != ""
477493
}
478494

495+
// MessageNameInExamples is the type name as used in examples.
496+
// This will be AliasInExamples if there's an alias,
497+
// otherwise it will be the message type or value type name.
498+
func (a *fieldAnnotations) MessageNameInExamples() string {
499+
if a.AliasInExamples != "" {
500+
return a.AliasInExamples
501+
}
502+
if a.MessageType != nil {
503+
ma, _ := a.MessageType.Codec.(*messageAnnotation)
504+
return ma.Name
505+
}
506+
if a.ValueField != nil && a.ValueField.MessageType != nil {
507+
ma, _ := a.ValueField.MessageType.Codec.(*messageAnnotation)
508+
return ma.Name
509+
}
510+
return ""
511+
}
512+
479513
type enumAnnotation struct {
480514
Name string
481515
ModuleName string
@@ -492,6 +526,9 @@ type enumAnnotation struct {
492526
// this is basically `QualifiedName`. For messages in the current package
493527
// this includes `modelAnnotations.PackageName`.
494528
NameInExamples string
529+
// There's a missmatch between the sidekick model representation of wkt::NullValue
530+
// and the representation in Rust. We us this for sample generation.
531+
IsWktNullValue bool
495532
// These are some of the enum values that can be used in examples,
496533
// accompanied by an index to facilitate generating code that can
497534
// distinctly reference each value. These attempt to avoid deprecated
@@ -888,10 +925,7 @@ func (c *codec) annotateService(s *api.Service) {
888925
func (c *codec) annotateMessage(m *api.Message, model *api.API, full bool) {
889926
qualifiedName := fullyQualifiedMessageName(m, c.modulePath, model.PackageName, c.packageMapping)
890927
relativeName := strings.TrimPrefix(qualifiedName, c.modulePath+"::")
891-
nameInExamples := qualifiedName
892-
if strings.HasPrefix(qualifiedName, c.modulePath+"::") {
893-
nameInExamples = fmt.Sprintf("%s::model::%s", c.packageNamespace(model), relativeName)
894-
}
928+
nameInExamples := c.nameInExamplesFromQualifiedName(qualifiedName, model)
895929
annotations := &messageAnnotation{
896930
Name: toPascal(m.Name),
897931
ModuleName: toSnake(m.Name),
@@ -1143,10 +1177,7 @@ func (c *codec) annotateOneOf(oneof *api.OneOf, message *api.Message, model *api
11431177
qualifiedName := fmt.Sprintf("%s::%s", scope, enumName)
11441178
relativeEnumName := strings.TrimPrefix(qualifiedName, c.modulePath+"::")
11451179
structQualifiedName := fullyQualifiedMessageName(message, c.modulePath, model.PackageName, c.packageMapping)
1146-
scopeInExamples := scope
1147-
if strings.HasPrefix(scope, c.modulePath+"::") {
1148-
scopeInExamples = strings.Replace(scope, c.modulePath, fmt.Sprintf("%s::model", c.packageNamespace(model)), 1)
1149-
}
1180+
nameInExamples := c.nameInExamplesFromQualifiedName(qualifiedName, model)
11501181

11511182
bestField := slices.MaxFunc(oneof.Fields, func(f1 *api.Field, f2 *api.Field) int {
11521183
if f1.Deprecated == f2.Deprecated {
@@ -1176,18 +1207,33 @@ func (c *codec) annotateOneOf(oneof *api.OneOf, message *api.Message, model *api
11761207
}
11771208
})
11781209

1179-
oneof.Codec = &oneOfAnnotation{
1210+
ann := &oneOfAnnotation{
11801211
FieldName: toSnake(oneof.Name),
11811212
SetterName: toSnakeNoMangling(oneof.Name),
11821213
EnumName: enumName,
11831214
QualifiedName: qualifiedName,
11841215
RelativeName: relativeEnumName,
11851216
StructQualifiedName: structQualifiedName,
1186-
ScopeInExamples: scopeInExamples,
1217+
NameInExamples: nameInExamples,
11871218
FieldType: fmt.Sprintf("%s::%s", scope, enumName),
11881219
DocLines: c.formatDocComments(oneof.Documentation, oneof.ID, model.State, message.Scopes()),
11891220
ExampleField: bestField,
11901221
}
1222+
// Note that this is different from OneOf name-overrides
1223+
// as those solve for fully qualified name clashes where a oneof
1224+
// and a child message have the same name.
1225+
// This is solving for unqualified name clashes that affect samples
1226+
// because we show usings for all types involved.
1227+
if ann.EnumName == message.Name {
1228+
ann.AliasInExamples = fmt.Sprintf("%sOneOf", ann.EnumName)
1229+
}
1230+
if ann.AliasInExamples == "" {
1231+
ann.EnumNameInExamples = ann.EnumName
1232+
} else {
1233+
ann.EnumNameInExamples = ann.AliasInExamples
1234+
}
1235+
1236+
oneof.Codec = ann
11911237
}
11921238

11931239
func (c *codec) primitiveSerdeAs(field *api.Field) string {
@@ -1295,11 +1341,26 @@ func (c *codec) annotateField(field *api.Field, message *api.Message, model *api
12951341
}
12961342
} else {
12971343
ann.SerdeAs = c.messageFieldSerdeAs(field)
1344+
ann.MessageType = field.MessageType
12981345
}
12991346
}
13001347
if field.Group != nil {
13011348
ann.OtherFieldsInGroup = language.FilterSlice(field.Group.Fields, func(f *api.Field) bool { return field != f })
13021349
}
1350+
ann.FieldTypeIsParentType = (field.MessageType == message || // Single or repeated field whose type is the same as the containing type.
1351+
// Map field whose value type is the same as the conaining type.
1352+
(ann.ValueField != nil && ann.ValueField.MessageType == message))
1353+
if !ann.FieldTypeIsParentType && // When the type of the field is the same as the containing type we don't import twice. No alias needed.
1354+
// Single or repeated field whose type's unqualified name is the same as the containing message's.
1355+
((field.MessageType != nil && field.MessageType.Name == message.Name) ||
1356+
// Map field whose type's unqualified name is the same as the containing message's.
1357+
(ann.ValueField != nil && ann.ValueField.MessageType != nil && ann.ValueField.MessageType.Name == message.Name)) {
1358+
ann.AliasInExamples = toPascal(field.Name)
1359+
if ann.AliasInExamples == toPascal(message.Name) {
1360+
// The field name was the same as the type name so we still have to disambiguate.
1361+
ann.AliasInExamples = fmt.Sprintf("%sField", ann.AliasInExamples)
1362+
}
1363+
}
13031364
}
13041365

13051366
func (c *codec) annotateEnum(e *api.Enum, model *api.API, full bool) {
@@ -1309,10 +1370,7 @@ func (c *codec) annotateEnum(e *api.Enum, model *api.API, full bool) {
13091370

13101371
qualifiedName := fullyQualifiedEnumName(e, c.modulePath, model.PackageName, c.packageMapping)
13111372
relativeName := strings.TrimPrefix(qualifiedName, c.modulePath+"::")
1312-
nameInExamples := qualifiedName
1313-
if strings.HasPrefix(qualifiedName, c.modulePath+"::") {
1314-
nameInExamples = fmt.Sprintf("%s::model::%s", c.packageNamespace(model), relativeName)
1315-
}
1373+
nameInExamples := c.nameInExamplesFromQualifiedName(qualifiedName, model)
13161374

13171375
// For BigQuery (and so far only BigQuery), the enum values conflict when
13181376
// converted to the Rust style [1]. Basically, there are several enum values
@@ -1366,6 +1424,7 @@ func (c *codec) annotateEnum(e *api.Enum, model *api.API, full bool) {
13661424
QualifiedName: qualifiedName,
13671425
RelativeName: relativeName,
13681426
NameInExamples: nameInExamples,
1427+
IsWktNullValue: nameInExamples == "wkt::NullValue",
13691428
ValuesForExamples: forExamples,
13701429
}
13711430
e.Codec = annotations

0 commit comments

Comments
 (0)