Skip to content

Commit 58336ac

Browse files
refactor(sidekick): Avoid passing many parameters when annotating. (#2235)
Also skip a test in Windows because it relies on setting file permissions. Note this change generates no diff on Rust generated code.
1 parent 2d22814 commit 58336ac

File tree

4 files changed

+92
-66
lines changed

4 files changed

+92
-66
lines changed

internal/sidekick/internal/rust/annotate.go

Lines changed: 62 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -452,16 +452,14 @@ func annotateModel(model *api.API, codec *codec) *modelAnnotations {
452452

453453
loadWellKnownTypes(model.State)
454454
resolveUsedPackages(model, codec.extraPackages)
455-
packageName := PackageName(model, codec.packageNameOverride)
456-
packageNamespace := strings.ReplaceAll(packageName, "-", "_")
457455
// Only annotate enums and messages that we intend to generate. In the
458456
// process we discover the external dependencies and trim the list of
459457
// packages used by this API.
460458
for _, e := range model.Enums {
461-
codec.annotateEnum(e, model.State, model.PackageName)
459+
codec.annotateEnum(e, model)
462460
}
463461
for _, m := range model.Messages {
464-
codec.annotateMessage(m, model.State, model.PackageName)
462+
codec.annotateMessage(m, model)
465463
}
466464
hasLROs := false
467465
for _, s := range model.Services {
@@ -472,15 +470,15 @@ func annotateModel(model *api.API, codec *codec) *modelAnnotations {
472470
if !codec.generateMethod(m) {
473471
continue
474472
}
475-
codec.annotateMethod(m, s, model.State, model.PackageName, packageNamespace)
473+
codec.annotateMethod(m)
476474
if m := m.InputType; m != nil {
477-
codec.annotateMessage(m, model.State, model.PackageName)
475+
codec.annotateMessage(m, model)
478476
}
479477
if m := m.OutputType; m != nil {
480-
codec.annotateMessage(m, model.State, model.PackageName)
478+
codec.annotateMessage(m, model)
481479
}
482480
}
483-
codec.annotateService(s, model)
481+
codec.annotateService(s)
484482
}
485483

486484
servicesSubset := language.FilterSlice(model.Services, func(s *api.Service) bool {
@@ -489,7 +487,7 @@ func annotateModel(model *api.API, codec *codec) *modelAnnotations {
489487
// The maximum (15) was chosen more or less arbitrarily circa 2025-06. At
490488
// the time, only a handful of services exceeded this number of services.
491489
if len(servicesSubset) > 15 && !codec.perServiceFeatures {
492-
slog.Warn("package has more than 15 services, consider enabling per-service features", "package", packageName, "count", len(servicesSubset))
490+
slog.Warn("package has more than 15 services, consider enabling per-service features", "package", codec.packageName(model), "count", len(servicesSubset))
493491
}
494492

495493
// Delay this until the Codec had a chance to compute what packages are
@@ -509,8 +507,8 @@ func annotateModel(model *api.API, codec *codec) *modelAnnotations {
509507
return defaultHost[:idx]
510508
}()
511509
ann := &modelAnnotations{
512-
PackageName: packageName,
513-
PackageNamespace: packageNamespace,
510+
PackageName: codec.packageName(model),
511+
PackageNamespace: codec.packageNamespace(model),
514512
PackageVersion: codec.version,
515513
ReleaseLevel: codec.releaseLevel,
516514
RequiredPackages: requiredPackages(codec.extraPackages),
@@ -616,7 +614,7 @@ func packageToModuleName(p string) string {
616614
return strings.Join(components, "::")
617615
}
618616

619-
func (c *codec) annotateService(s *api.Service, model *api.API) {
617+
func (c *codec) annotateService(s *api.Service) {
620618
// Some codecs skip some methods.
621619
methods := language.FilterSlice(s.Methods, func(m *api.Method) bool {
622620
return c.generateMethod(m)
@@ -627,11 +625,11 @@ func (c *codec) annotateService(s *api.Service, model *api.API) {
627625
if m.OperationInfo != nil {
628626
if _, ok := seenLROTypes[m.OperationInfo.MetadataTypeID]; !ok {
629627
seenLROTypes[m.OperationInfo.MetadataTypeID] = true
630-
lroTypes = append(lroTypes, model.State.MessageByID[m.OperationInfo.MetadataTypeID])
628+
lroTypes = append(lroTypes, s.Model.State.MessageByID[m.OperationInfo.MetadataTypeID])
631629
}
632630
if _, ok := seenLROTypes[m.OperationInfo.ResponseTypeID]; !ok {
633631
seenLROTypes[m.OperationInfo.ResponseTypeID] = true
634-
lroTypes = append(lroTypes, model.State.MessageByID[m.OperationInfo.ResponseTypeID])
632+
lroTypes = append(lroTypes, s.Model.State.MessageByID[m.OperationInfo.ResponseTypeID])
635633
}
636634
}
637635
}
@@ -642,11 +640,11 @@ func (c *codec) annotateService(s *api.Service, model *api.API) {
642640
PackageModuleName: packageToModuleName(s.Package),
643641
ModuleName: moduleName,
644642
DocLines: c.formatDocComments(
645-
s.Documentation, s.ID, model.State, []string{s.ID, s.Package}),
643+
s.Documentation, s.ID, s.Model.State, []string{s.ID, s.Package}),
646644
Methods: methods,
647645
DefaultHost: s.DefaultHost,
648646
LROTypes: lroTypes,
649-
APITitle: model.Title,
647+
APITitle: s.Model.Title,
650648
PerServiceFeatures: c.perServiceFeatures,
651649
HasVeneer: c.hasVeneer,
652650
Incomplete: slices.ContainsFunc(s.Methods, func(m *api.Method) bool { return !c.generateMethod(m) }),
@@ -656,18 +654,18 @@ func (c *codec) annotateService(s *api.Service, model *api.API) {
656654

657655
// annotateMessage annotates the message, its fields, its nested
658656
// messages, and its nested enums.
659-
func (c *codec) annotateMessage(m *api.Message, state *api.APIState, sourceSpecificationPackageName string) {
657+
func (c *codec) annotateMessage(m *api.Message, model *api.API) {
660658
for _, f := range m.Fields {
661-
c.annotateField(f, m, state, sourceSpecificationPackageName)
659+
c.annotateField(f, m, model)
662660
}
663661
for _, o := range m.OneOfs {
664-
c.annotateOneOf(o, m, state, sourceSpecificationPackageName)
662+
c.annotateOneOf(o, m, model)
665663
}
666664
for _, e := range m.Enums {
667-
c.annotateEnum(e, state, sourceSpecificationPackageName)
665+
c.annotateEnum(e, model)
668666
}
669667
for _, child := range m.Messages {
670-
c.annotateMessage(child, state, sourceSpecificationPackageName)
668+
c.annotateMessage(child, model)
671669
}
672670
hasSyntheticFields := false
673671
for _, f := range m.Fields {
@@ -679,7 +677,7 @@ func (c *codec) annotateMessage(m *api.Message, state *api.APIState, sourceSpeci
679677
basicFields := language.FilterSlice(m.Fields, func(f *api.Field) bool {
680678
return !f.IsOneOf
681679
})
682-
qualifiedName := fullyQualifiedMessageName(m, c.modulePath, sourceSpecificationPackageName, c.packageMapping)
680+
qualifiedName := fullyQualifiedMessageName(m, c.modulePath, model.PackageName, c.packageMapping)
683681
relativeName := strings.TrimPrefix(qualifiedName, c.modulePath+"::")
684682
m.Codec = &messageAnnotation{
685683
Name: toPascal(m.Name),
@@ -688,37 +686,37 @@ func (c *codec) annotateMessage(m *api.Message, state *api.APIState, sourceSpeci
688686
RelativeName: relativeName,
689687
PackageModuleName: packageToModuleName(m.Package),
690688
SourceFQN: strings.TrimPrefix(m.ID, "."),
691-
DocLines: c.formatDocComments(m.Documentation, m.ID, state, m.Scopes()),
689+
DocLines: c.formatDocComments(m.Documentation, m.ID, model.State, m.Scopes()),
692690
HasNestedTypes: language.HasNestedTypes(m),
693691
BasicFields: basicFields,
694692
HasSyntheticFields: hasSyntheticFields,
695693
Internal: slices.Contains(c.internalTypes, m.ID),
696694
}
697695
}
698696

699-
func (c *codec) annotateMethod(m *api.Method, s *api.Service, state *api.APIState, sourceSpecificationPackageName string, packageNamespace string) {
700-
annotatePathInfo(m.PathInfo, m, state)
697+
func (c *codec) annotateMethod(m *api.Method) {
698+
annotatePathInfo(m)
701699
for _, routing := range m.Routing {
702700
for _, variant := range routing.Variants {
703701
routingVariantAnnotations := &routingVariantAnnotations{
704-
FieldAccessors: c.annotateRoutingAccessors(variant, m, state),
702+
FieldAccessors: c.annotateRoutingAccessors(variant, m),
705703
PrefixSegments: annotateSegments(variant.Prefix.Segments),
706704
MatchingSegments: annotateSegments(variant.Matching.Segments),
707705
SuffixSegments: annotateSegments(variant.Suffix.Segments),
708706
}
709707
variant.Codec = routingVariantAnnotations
710708
}
711709
}
712-
returnType := c.methodInOutTypeName(m.OutputTypeID, state, sourceSpecificationPackageName)
710+
returnType := c.methodInOutTypeName(m.OutputTypeID, m.Model.State, m.Model.PackageName)
713711
if m.ReturnsEmpty {
714712
returnType = "()"
715713
}
716-
serviceName := c.ServiceName(s)
714+
serviceName := c.ServiceName(m.Service)
717715
annotation := &methodAnnotation{
718716
Name: strcase.ToSnake(m.Name),
719717
BuilderName: toPascal(m.Name),
720718
Body: bodyAccessor(m),
721-
DocLines: c.formatDocComments(m.Documentation, m.ID, state, s.Scopes()),
719+
DocLines: c.formatDocComments(m.Documentation, m.ID, m.Model.State, m.Service.Scopes()),
722720
PathInfo: m.PathInfo,
723721
ServiceNameToPascal: toPascal(serviceName),
724722
ServiceNameToCamel: toCamel(serviceName),
@@ -734,22 +732,22 @@ func (c *codec) annotateMethod(m *api.Method, s *api.Service, state *api.APIStat
734732
annotation.Attributes = []string{"#[allow(clippy::should_implement_trait)]"}
735733
}
736734
if m.OperationInfo != nil {
737-
metadataType := c.methodInOutTypeName(m.OperationInfo.MetadataTypeID, state, sourceSpecificationPackageName)
738-
responseType := c.methodInOutTypeName(m.OperationInfo.ResponseTypeID, state, sourceSpecificationPackageName)
735+
metadataType := c.methodInOutTypeName(m.OperationInfo.MetadataTypeID, m.Model.State, m.Model.PackageName)
736+
responseType := c.methodInOutTypeName(m.OperationInfo.ResponseTypeID, m.Model.State, m.Model.PackageName)
739737
m.OperationInfo.Codec = &operationInfo{
740738
MetadataType: metadataType,
741739
ResponseType: responseType,
742-
PackageNamespace: packageNamespace,
740+
PackageNamespace: c.packageNamespace(m.Model),
743741
}
744742
}
745743
m.Codec = annotation
746744
}
747745

748-
func (c *codec) annotateRoutingAccessors(variant *api.RoutingInfoVariant, m *api.Method, state *api.APIState) []string {
749-
return makeAccessors(variant.FieldPath, m, state)
746+
func (c *codec) annotateRoutingAccessors(variant *api.RoutingInfoVariant, m *api.Method) []string {
747+
return makeAccessors(variant.FieldPath, m)
750748
}
751749

752-
func makeAccessors(fields []string, m *api.Method, state *api.APIState) []string {
750+
func makeAccessors(fields []string, m *api.Method) []string {
753751
findField := func(name string, message *api.Message) *api.Field {
754752
for _, f := range message.Fields {
755753
if f.Name == name {
@@ -775,7 +773,7 @@ func makeAccessors(fields []string, m *api.Method, state *api.APIState) []string
775773
accessors = append(accessors, ".map(|s| s.as_str())")
776774
}
777775
if field.Typez == api.MESSAGE_TYPE {
778-
if fieldMessage, ok := state.MessageByID[field.TypezID]; ok {
776+
if fieldMessage, ok := m.Model.State.MessageByID[field.TypezID]; ok {
779777
message = fieldMessage
780778
}
781779
}
@@ -822,9 +820,9 @@ func annotateSegments(segments []string) []string {
822820
return ann
823821
}
824822

825-
func makeBindingSubstitution(v *api.PathVariable, m *api.Method, state *api.APIState) bindingSubstitution {
823+
func makeBindingSubstitution(v *api.PathVariable, m *api.Method) bindingSubstitution {
826824
fieldAccessor := "Some(&req)"
827-
for _, a := range makeAccessors(v.FieldPath, m, state) {
825+
for _, a := range makeAccessors(v.FieldPath, m) {
828826
fieldAccessor += a
829827
}
830828
return bindingSubstitution{
@@ -834,11 +832,11 @@ func makeBindingSubstitution(v *api.PathVariable, m *api.Method, state *api.APIS
834832
}
835833
}
836834

837-
func annotatePathBinding(b *api.PathBinding, m *api.Method, state *api.APIState) *pathBindingAnnotation {
835+
func annotatePathBinding(b *api.PathBinding, m *api.Method) *pathBindingAnnotation {
838836
var subs []*bindingSubstitution
839837
for _, s := range b.PathTemplate.Segments {
840838
if s.Variable != nil {
841-
sub := makeBindingSubstitution(s.Variable, m, state)
839+
sub := makeBindingSubstitution(s.Variable, m)
842840
subs = append(subs, &sub)
843841
}
844842
}
@@ -850,12 +848,12 @@ func annotatePathBinding(b *api.PathBinding, m *api.Method, state *api.APIState)
850848
}
851849

852850
// annotatePathInfo annotates the `PathInfo` and all of its `PathBinding`s.
853-
func annotatePathInfo(p *api.PathInfo, m *api.Method, state *api.APIState) {
851+
func annotatePathInfo(m *api.Method) {
854852
seen := make(map[string]bool)
855853
var uniqueParameters []*bindingSubstitution
856854

857-
for _, b := range p.Bindings {
858-
ann := annotatePathBinding(b, m, state)
855+
for _, b := range m.PathInfo.Bindings {
856+
ann := annotatePathBinding(b, m)
859857

860858
// We need to keep track of unique path parameters to support
861859
// implicit routing over gRPC. This is go/aip/4222.
@@ -871,19 +869,19 @@ func annotatePathInfo(p *api.PathInfo, m *api.Method, state *api.APIState) {
871869
}
872870

873871
// Annotate the `PathInfo`
874-
p.Codec = &pathInfoAnnotation{
872+
m.PathInfo.Codec = &pathInfoAnnotation{
875873
HasBody: m.PathInfo.BodyFieldPath != "",
876874
UniqueParameters: uniqueParameters,
877-
IsIdempotent: isIdempotent(p),
875+
IsIdempotent: isIdempotent(m.PathInfo),
878876
}
879877
}
880878

881-
func (c *codec) annotateOneOf(oneof *api.OneOf, message *api.Message, state *api.APIState, sourceSpecificationPackageName string) {
882-
scope := messageScopeName(message, "", c.modulePath, sourceSpecificationPackageName, c.packageMapping)
879+
func (c *codec) annotateOneOf(oneof *api.OneOf, message *api.Message, model *api.API) {
880+
scope := messageScopeName(message, "", c.modulePath, model.PackageName, c.packageMapping)
883881
enumName := c.OneOfEnumName(oneof)
884882
qualifiedName := fmt.Sprintf("%s::%s", scope, enumName)
885883
relativeEnumName := strings.TrimPrefix(qualifiedName, c.modulePath+"::")
886-
structQualifiedName := fullyQualifiedMessageName(message, c.modulePath, sourceSpecificationPackageName, c.packageMapping)
884+
structQualifiedName := fullyQualifiedMessageName(message, c.modulePath, model.PackageName, c.packageMapping)
887885
oneof.Codec = &oneOfAnnotation{
888886
FieldName: toSnake(oneof.Name),
889887
SetterName: toSnakeNoMangling(oneof.Name),
@@ -892,7 +890,7 @@ func (c *codec) annotateOneOf(oneof *api.OneOf, message *api.Message, state *api
892890
RelativeName: relativeEnumName,
893891
StructQualifiedName: structQualifiedName,
894892
FieldType: fmt.Sprintf("%s::%s", scope, enumName),
895-
DocLines: c.formatDocComments(oneof.Documentation, oneof.ID, state, message.Scopes()),
893+
DocLines: c.formatDocComments(oneof.Documentation, oneof.ID, model.State, message.Scopes()),
896894
}
897895
}
898896

@@ -954,15 +952,15 @@ func (c *codec) messageFieldSerdeAs(field *api.Field) string {
954952
}
955953
}
956954

957-
func (c *codec) annotateField(field *api.Field, message *api.Message, state *api.APIState, sourceSpecificationPackageName string) {
955+
func (c *codec) annotateField(field *api.Field, message *api.Message, model *api.API) {
958956
ann := &fieldAnnotations{
959957
FieldName: toSnake(field.Name),
960958
SetterName: toSnakeNoMangling(field.Name),
961-
FQMessageName: fullyQualifiedMessageName(message, c.modulePath, sourceSpecificationPackageName, c.packageMapping),
959+
FQMessageName: fullyQualifiedMessageName(message, c.modulePath, model.PackageName, c.packageMapping),
962960
BranchName: toPascal(field.Name),
963-
DocLines: c.formatDocComments(field.Documentation, field.ID, state, message.Scopes()),
964-
FieldType: fieldType(field, state, false, c.modulePath, sourceSpecificationPackageName, c.packageMapping),
965-
PrimitiveFieldType: fieldType(field, state, true, c.modulePath, sourceSpecificationPackageName, c.packageMapping),
961+
DocLines: c.formatDocComments(field.Documentation, field.ID, model.State, message.Scopes()),
962+
FieldType: fieldType(field, model.State, false, c.modulePath, model.PackageName, c.packageMapping),
963+
PrimitiveFieldType: fieldType(field, model.State, true, c.modulePath, model.PackageName, c.packageMapping),
966964
AddQueryParameter: addQueryParameter(field),
967965
SerdeAs: c.primitiveSerdeAs(field),
968966
SkipIfIsDefault: field.Typez != api.STRING_TYPE && field.Typez != api.BYTES_TYPE,
@@ -974,14 +972,14 @@ func (c *codec) annotateField(field *api.Field, message *api.Message, state *api
974972
}
975973
field.Codec = ann
976974
if field.Typez == api.MESSAGE_TYPE {
977-
if msg, ok := state.MessageByID[field.TypezID]; ok && msg.IsMap {
975+
if msg, ok := model.State.MessageByID[field.TypezID]; ok && msg.IsMap {
978976
if len(msg.Fields) != 2 {
979977
slog.Error("expected exactly two fields for map message", "field ID", field.ID, "map ID", field.TypezID)
980978
}
981979
ann.KeyField = msg.Fields[0]
982-
ann.KeyType = mapType(msg.Fields[0], state, c.modulePath, sourceSpecificationPackageName, c.packageMapping)
980+
ann.KeyType = mapType(msg.Fields[0], model.State, c.modulePath, model.PackageName, c.packageMapping)
983981
ann.ValueField = msg.Fields[1]
984-
ann.ValueType = mapType(msg.Fields[1], state, c.modulePath, sourceSpecificationPackageName, c.packageMapping)
982+
ann.ValueType = mapType(msg.Fields[1], model.State, c.modulePath, model.PackageName, c.packageMapping)
985983
key := c.mapKeySerdeAs(msg.Fields[0])
986984
value := c.mapValueSerdeAs(msg.Fields[1])
987985
if key != "" || value != "" {
@@ -999,9 +997,9 @@ func (c *codec) annotateField(field *api.Field, message *api.Message, state *api
999997
}
1000998
}
1001999

1002-
func (c *codec) annotateEnum(e *api.Enum, state *api.APIState, sourceSpecificationPackageName string) {
1000+
func (c *codec) annotateEnum(e *api.Enum, model *api.API) {
10031001
for _, ev := range e.Values {
1004-
c.annotateEnumValue(ev, e, state)
1002+
c.annotateEnumValue(ev, model)
10051003
}
10061004
// For BigQuery (and so far only BigQuery), the enum values conflict when
10071005
// converted to the Rust style [1]. Basically, there are several enum values
@@ -1026,23 +1024,23 @@ func (c *codec) annotateEnum(e *api.Enum, state *api.APIState, sourceSpecificati
10261024
}
10271025
}
10281026

1029-
qualifiedName := fullyQualifiedEnumName(e, c.modulePath, sourceSpecificationPackageName, c.packageMapping)
1027+
qualifiedName := fullyQualifiedEnumName(e, c.modulePath, model.PackageName, c.packageMapping)
10301028
relativeName := strings.TrimPrefix(qualifiedName, c.modulePath+"::")
10311029
e.Codec = &enumAnnotation{
10321030
Name: enumName(e),
10331031
ModuleName: toSnake(enumName(e)),
1034-
DocLines: c.formatDocComments(e.Documentation, e.ID, state, e.Scopes()),
1032+
DocLines: c.formatDocComments(e.Documentation, e.ID, model.State, e.Scopes()),
10351033
UniqueNames: unique,
10361034
QualifiedName: qualifiedName,
10371035
RelativeName: relativeName,
10381036
}
10391037
}
10401038

1041-
func (c *codec) annotateEnumValue(ev *api.EnumValue, e *api.Enum, state *api.APIState) {
1039+
func (c *codec) annotateEnumValue(ev *api.EnumValue, model *api.API) {
10421040
ev.Codec = &enumValueAnnotation{
1043-
DocLines: c.formatDocComments(ev.Documentation, ev.ID, state, ev.Scopes()),
1041+
DocLines: c.formatDocComments(ev.Documentation, ev.ID, model.State, ev.Scopes()),
10441042
Name: enumValueName(ev),
1045-
EnumType: enumName(e),
1043+
EnumType: enumName(ev.Parent),
10461044
VariantName: enumValueVariantName(ev),
10471045
}
10481046
}

0 commit comments

Comments
 (0)