Skip to content

Commit ff9ed35

Browse files
committed
feat(generator/rust): skip duplicate enum values
After converting the enum value names to Rust style we may find some duplicates. With this change the generator can skip the dups. In all practical cases the dups are harmless, they differ in case but have the name numeric value. We warn if this assumption fails.
1 parent 58727a4 commit ff9ed35

File tree

3 files changed

+92
-25
lines changed

3 files changed

+92
-25
lines changed

generator/internal/rust/annotate.go

+30-14
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package rust
1616

1717
import (
1818
"fmt"
19+
"log/slog"
1920
"strings"
2021

2122
"github.com/googleapis/google-cloud-rust/generator/internal/api"
@@ -187,10 +188,10 @@ type fieldAnnotations struct {
187188
}
188189

189190
type enumAnnotation struct {
190-
Name string
191-
ModuleName string
192-
DocLines []string
193-
DefaultValueName string
191+
Name string
192+
ModuleName string
193+
DocLines []string
194+
UniqueNames []*api.EnumValue
194195
// The fully qualified name, including the `codec.modulePath`
195196
// (typically `crate::model::`) prefix. For external enums this is prefixed
196197
// by the external crate name.
@@ -491,22 +492,37 @@ func (c *codec) annotateEnum(e *api.Enum, state *api.APIState, sourceSpecificati
491492
for _, ev := range e.Values {
492493
c.annotateEnumValue(ev, e, state)
493494
}
494-
defaultValueName := ""
495+
// For BigQuery (and so far only BigQuery), the enum values conflict when
496+
// converted to the Rust style [1]. Basically, there are several enum values
497+
// in this service that differ only in case, such as `FULL` vs. `full`.
498+
//
499+
// We create a list with the duplicates removed to avoid conflicts in the
500+
// generated code.
501+
//
502+
// [1]: Both Rust and Protobuf use `SCREAMING_SNAKE_CASE` for these, but
503+
// some services do not follow the Protobuf convention.
504+
seen := map[string]*api.EnumValue{}
505+
var unique []*api.EnumValue
495506
for _, ev := range e.Values {
496-
if ev.Number == 0 {
497-
defaultValueName = enumValueName(ev)
498-
break
507+
name := enumValueName(ev)
508+
if existing, ok := seen[name]; ok {
509+
if existing.Number != ev.Number {
510+
slog.Warn("conflicting names for enum values", "enum.ID", e.ID)
511+
}
512+
} else {
513+
unique = append(unique, ev)
514+
seen[name] = ev
499515
}
500516
}
501517
qualifiedName := fullyQualifiedEnumName(e, c.modulePath, sourceSpecificationPackageName, c.packageMapping)
502518
relativeName := strings.TrimPrefix(qualifiedName, c.modulePath+"::")
503519
e.Codec = &enumAnnotation{
504-
Name: enumName(e),
505-
ModuleName: toSnake(enumName(e)),
506-
DocLines: formatDocComments(e.Documentation, e.ID, state, c.modulePath, e.Scopes(), c.packageMapping),
507-
DefaultValueName: defaultValueName,
508-
QualifiedName: qualifiedName,
509-
RelativeName: relativeName,
520+
Name: enumName(e),
521+
ModuleName: toSnake(enumName(e)),
522+
DocLines: formatDocComments(e.Documentation, e.ID, state, c.modulePath, e.Scopes(), c.packageMapping),
523+
UniqueNames: unique,
524+
QualifiedName: qualifiedName,
525+
RelativeName: relativeName,
510526
}
511527
}
512528

generator/internal/rust/annotate_test.go

+60-9
Original file line numberDiff line numberDiff line change
@@ -289,14 +289,15 @@ func TestEnumAnnotations(t *testing.T) {
289289
}
290290
annotateModel(model, codec, "")
291291

292-
if diff := cmp.Diff(&enumAnnotation{
293-
Name: "TestEnum",
294-
ModuleName: "test_enum",
295-
QualifiedName: "crate::model::TestEnum",
296-
RelativeName: "TestEnum",
297-
DocLines: []string{"/// The enum is documented."},
298-
DefaultValueName: "VALUE",
299-
}, enum.Codec); diff != "" {
292+
want := &enumAnnotation{
293+
Name: "TestEnum",
294+
ModuleName: "test_enum",
295+
QualifiedName: "crate::model::TestEnum",
296+
RelativeName: "TestEnum",
297+
DocLines: []string{"/// The enum is documented."},
298+
UniqueNames: []*api.EnumValue{v0, v1, v2},
299+
}
300+
if diff := cmp.Diff(want, enum.Codec, cmpopts.IgnoreFields(api.EnumValue{}, "Codec", "Parent")); diff != "" {
300301
t.Errorf("mismatch in enum annotations (-want, +got)\n:%s", diff)
301302
}
302303

@@ -315,7 +316,6 @@ func TestEnumAnnotations(t *testing.T) {
315316
}, v1.Codec); diff != "" {
316317
t.Errorf("mismatch in enum annotations (-want, +got)\n:%s", diff)
317318
}
318-
319319
if diff := cmp.Diff(&enumValueAnnotation{
320320
Name: "VALUE",
321321
EnumType: "TestEnum",
@@ -325,6 +325,57 @@ func TestEnumAnnotations(t *testing.T) {
325325
}
326326
}
327327

328+
func TestDuplicateEnumValueAnnotations(t *testing.T) {
329+
// Verify we can handle values that are not in SCREAMING_SNAKE_CASE style.
330+
v0 := &api.EnumValue{
331+
Name: "full",
332+
ID: ".test.v1.TestEnum.full",
333+
Number: 1,
334+
}
335+
v1 := &api.EnumValue{
336+
Name: "FULL",
337+
ID: ".test.v1.TestEnum.FULL",
338+
Number: 1,
339+
}
340+
v2 := &api.EnumValue{
341+
Name: "partial",
342+
ID: ".test.v1.TestEnum.partial",
343+
Number: 2,
344+
}
345+
// This does not happen in practice, but we want to verify the code can
346+
// handle it if it ever does.
347+
v3 := &api.EnumValue{
348+
Name: "PARTIAL",
349+
ID: ".test.v1.TestEnum.PARTIAL",
350+
Number: 3,
351+
}
352+
enum := &api.Enum{
353+
Name: "TestEnum",
354+
ID: ".test.v1.TestEnum",
355+
Values: []*api.EnumValue{v0, v1, v2, v3},
356+
}
357+
358+
model := api.NewTestAPI(
359+
[]*api.Message{}, []*api.Enum{enum}, []*api.Service{})
360+
codec, err := newCodec(true, map[string]string{})
361+
if err != nil {
362+
t.Fatal(err)
363+
}
364+
annotateModel(model, codec, "")
365+
366+
want := &enumAnnotation{
367+
Name: "TestEnum",
368+
ModuleName: "test_enum",
369+
QualifiedName: "crate::model::TestEnum",
370+
RelativeName: "TestEnum",
371+
UniqueNames: []*api.EnumValue{v0, v2},
372+
}
373+
374+
if diff := cmp.Diff(want, enum.Codec, cmpopts.IgnoreFields(api.EnumValue{}, "Codec", "Parent")); diff != "" {
375+
t.Errorf("mismatch in enum annotations (-want, +got)\n:%s", diff)
376+
}
377+
}
378+
328379
func TestJsonNameAnnotations(t *testing.T) {
329380
parent := &api.Field{
330381
Name: "parent",

generator/internal/rust/templates/common/enum.mustache

+2-2
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,13 @@ limitations under the License.
2121
pub struct {{Codec.Name}}(i32);
2222

2323
impl {{Codec.Name}} {
24-
{{#Values}}
24+
{{#Codec.UniqueNames}}
2525

2626
{{#Codec.DocLines}}
2727
{{{.}}}
2828
{{/Codec.DocLines}}
2929
pub const {{Codec.Name}}: {{Codec.EnumType}} = {{Codec.EnumType}}::new({{Number}});
30-
{{/Values}}
30+
{{/Codec.UniqueNames}}
3131

3232
/// Creates a new {{Codec.Name}} instance.
3333
pub(crate) const fn new(value: i32) -> Self {

0 commit comments

Comments
 (0)