Skip to content

Commit 4e417c9

Browse files
emcfarlaneemcfarlane
authored
Remove unused options for excluded types (#3758)
1 parent 47f7e78 commit 4e417c9

File tree

7 files changed

+166
-217
lines changed

7 files changed

+166
-217
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
## [Unreleased]
44

5-
- No changes yet.
5+
- Update exclude types to remove unused options reducing the size of generated code.
66

77
## [v1.56.0] - 2025-07-31
88

‎private/bufpkg/bufimage/bufimageutil/image_filter.go‎

Lines changed: 165 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,11 @@ func (b *sourcePathsBuilder) remapFileDescriptor(
220220
return nil, false, err
221221
}
222222
isDirty = isDirty || changed
223+
newOptions, changed, err := remapMessage(sourcePathsRemap, append(sourcePath, fileOptionsTag), fileDescriptor.Options, b.remapOptions)
224+
if err != nil {
225+
return nil, false, err
226+
}
227+
isDirty = isDirty || changed
223228
newDependencies, newPublicDependencies, newWeakDependencies, changed, err := b.remapDependencies(sourcePathsRemap, sourcePath, fileDescriptor)
224229
if err != nil {
225230
return nil, false, err
@@ -234,6 +239,7 @@ func (b *sourcePathsBuilder) remapFileDescriptor(
234239
newFileDescriptor.EnumType = newEnums
235240
newFileDescriptor.Service = newServices
236241
newFileDescriptor.Extension = newExtensions
242+
newFileDescriptor.Options = newOptions
237243
newFileDescriptor.Dependency = newDependencies
238244
newFileDescriptor.PublicDependency = newPublicDependencies
239245
newFileDescriptor.WeakDependency = newWeakDependencies
@@ -367,10 +373,16 @@ func (b *sourcePathsBuilder) remapDescriptor(
367373
return nil, false, err
368374
}
369375
isDirty = isDirty || changed
376+
newExtensionRange, changed, err := remapSlice(sourcePathsRemap, append(sourcePath, messageExtensionRangesTag), descriptor.ExtensionRange, b.remapExtensionRange, b.options)
377+
if err != nil {
378+
return nil, false, err
379+
}
380+
isDirty = isDirty || changed
370381
if isDirty {
371382
newDescriptor = maybeClone(descriptor, b.options)
372383
newDescriptor.Field = newFields
373384
newDescriptor.OneofDecl = newOneofs
385+
newDescriptor.ExtensionRange = newExtensionRange
374386
}
375387
}
376388
newExtensions, changed, err := remapSlice(sourcePathsRemap, append(sourcePath, messageExtensionsTag), descriptor.GetExtension(), b.remapField, b.options)
@@ -388,6 +400,11 @@ func (b *sourcePathsBuilder) remapDescriptor(
388400
return nil, false, err
389401
}
390402
isDirty = isDirty || changed
403+
newOptions, changed, err := remapMessage(sourcePathsRemap, append(sourcePath, messageOptionsTag), descriptor.GetOptions(), b.remapOptions)
404+
if err != nil {
405+
return nil, false, err
406+
}
407+
isDirty = isDirty || changed
391408

392409
if !isDirty {
393410
return descriptor, false, nil
@@ -398,9 +415,27 @@ func (b *sourcePathsBuilder) remapDescriptor(
398415
newDescriptor.Extension = newExtensions
399416
newDescriptor.NestedType = newDescriptors
400417
newDescriptor.EnumType = newEnums
418+
newDescriptor.Options = newOptions
401419
return newDescriptor, true, nil
402420
}
403421

422+
func (b *sourcePathsBuilder) remapExtensionRange(
423+
sourcePathsRemap *sourcePathsRemapTrie,
424+
sourcePath protoreflect.SourcePath,
425+
extensionRange *descriptorpb.DescriptorProto_ExtensionRange,
426+
) (*descriptorpb.DescriptorProto_ExtensionRange, bool, error) {
427+
newOptions, changed, err := remapMessage(sourcePathsRemap, append(sourcePath, extensionRangeOptionsTag), extensionRange.GetOptions(), b.remapOptions)
428+
if err != nil {
429+
return nil, false, err
430+
}
431+
if !changed {
432+
return extensionRange, false, nil
433+
}
434+
newExtensionRange := maybeClone(extensionRange, b.options)
435+
newExtensionRange.Options = newOptions
436+
return newExtensionRange, true, nil
437+
}
438+
404439
func (b *sourcePathsBuilder) remapEnum(
405440
sourcePathsRemap *sourcePathsRemapTrie,
406441
sourcePath protoreflect.SourcePath,
@@ -410,7 +445,43 @@ func (b *sourcePathsBuilder) remapEnum(
410445
// The type is excluded, enum values cannot be excluded individually.
411446
return nil, true, nil
412447
}
413-
return enum, false, nil
448+
var isDirty bool
449+
newOptions, changed, err := remapMessage(sourcePathsRemap, append(sourcePath, enumOptionsTag), enum.GetOptions(), b.remapOptions)
450+
if err != nil {
451+
return nil, false, err
452+
}
453+
isDirty = changed
454+
455+
// Walk the enum values.
456+
newEnumValues, changed, err := remapSlice(sourcePathsRemap, append(sourcePath, enumValuesTag), enum.Value, b.remapEnumValue, b.options)
457+
if err != nil {
458+
return nil, false, err
459+
}
460+
isDirty = isDirty || changed
461+
if !isDirty {
462+
return enum, true, nil
463+
}
464+
newEnum := maybeClone(enum, b.options)
465+
newEnum.Options = newOptions
466+
newEnum.Value = newEnumValues
467+
return newEnum, true, nil
468+
}
469+
470+
func (b *sourcePathsBuilder) remapEnumValue(
471+
sourcePathsRemap *sourcePathsRemapTrie,
472+
sourcePath protoreflect.SourcePath,
473+
enumValue *descriptorpb.EnumValueDescriptorProto,
474+
) (*descriptorpb.EnumValueDescriptorProto, bool, error) {
475+
newOptions, changed, err := remapMessage(sourcePathsRemap, append(sourcePath, enumValueOptionsTag), enumValue.GetOptions(), b.remapOptions)
476+
if err != nil {
477+
return nil, false, err
478+
}
479+
if !changed {
480+
return enumValue, false, nil
481+
}
482+
newEnumValue := maybeClone(enumValue, b.options)
483+
newEnumValue.Options = newOptions
484+
return newEnumValue, true, nil
414485
}
415486

416487
func (b *sourcePathsBuilder) remapOneof(
@@ -422,7 +493,16 @@ func (b *sourcePathsBuilder) remapOneof(
422493
// Oneofs are implicitly excluded when all of its fields types are excluded.
423494
return nil, true, nil
424495
}
425-
return oneof, false, nil
496+
options, changed, err := remapMessage(sourcePathsRemap, append(sourcePath, oneofOptionsTag), oneof.GetOptions(), b.remapOptions)
497+
if err != nil {
498+
return nil, false, err
499+
}
500+
if !changed {
501+
return oneof, false, nil
502+
}
503+
newOneof := maybeClone(oneof, b.options)
504+
newOneof.Options = options
505+
return newOneof, true, nil
426506
}
427507

428508
func (b *sourcePathsBuilder) remapService(
@@ -440,11 +520,17 @@ func (b *sourcePathsBuilder) remapService(
440520
return nil, false, err
441521
}
442522
isDirty = isDirty || changed
523+
newOptions, changed, err := remapMessage(sourcePathsRemap, append(sourcePath, serviceOptionsTag), service.GetOptions(), b.remapOptions)
524+
if err != nil {
525+
return nil, false, err
526+
}
527+
isDirty = isDirty || changed
443528
if !isDirty {
444529
return service, false, nil
445530
}
446531
newService := maybeClone(service, b.options)
447532
newService.Method = newMethods
533+
newService.Options = newOptions
448534
return newService, true, nil
449535
}
450536

@@ -456,7 +542,16 @@ func (b *sourcePathsBuilder) remapMethod(
456542
if !b.closure.hasType(method, b.options) {
457543
return nil, true, nil
458544
}
459-
return method, false, nil
545+
newOptions, changed, err := remapMessage(sourcePathsRemap, append(sourcePath, methodOptionsTag), method.GetOptions(), b.remapOptions)
546+
if err != nil {
547+
return nil, false, err
548+
}
549+
if !changed {
550+
return method, false, nil
551+
}
552+
newMethod := maybeClone(method, b.options)
553+
newMethod.Options = newOptions
554+
return newMethod, true, nil
460555
}
461556

462557
func (b *sourcePathsBuilder) remapField(
@@ -497,7 +592,73 @@ func (b *sourcePathsBuilder) remapField(
497592
default:
498593
return nil, false, fmt.Errorf("unknown field type %d", field.GetType())
499594
}
500-
return field, false, nil
595+
newOptions, changed, err := remapMessage(sourcePathsRemap, append(sourcePath, fieldOptionsTag), field.GetOptions(), b.remapOptions)
596+
if err != nil {
597+
return nil, false, err
598+
}
599+
if !changed {
600+
return field, false, nil
601+
}
602+
newField := maybeClone(field, b.options)
603+
newField.Options = newOptions
604+
return newField, true, nil
605+
}
606+
607+
func (b *sourcePathsBuilder) remapOptions(
608+
sourcePathsRemap *sourcePathsRemapTrie,
609+
optionsPath protoreflect.SourcePath,
610+
optionsMessage proto.Message,
611+
) (proto.Message, bool, error) {
612+
if optionsMessage == nil {
613+
return nil, false, nil
614+
}
615+
var newOptions protoreflect.Message
616+
options := optionsMessage.ProtoReflect()
617+
numFieldsToKeep := 0
618+
options.Range(func(fd protoreflect.FieldDescriptor, val protoreflect.Value) bool {
619+
if !b.closure.hasOption(fd, b.imageIndex, b.options) {
620+
// Remove this option.
621+
optionPath := append(optionsPath, int32(fd.Number()))
622+
sourcePathsRemap.markDeleted(optionPath)
623+
if newOptions == nil {
624+
newOptions = maybeClone(optionsMessage, b.options).ProtoReflect()
625+
}
626+
newOptions.Clear(fd)
627+
return true
628+
}
629+
numFieldsToKeep++
630+
return true
631+
})
632+
if numFieldsToKeep == 0 {
633+
// No options to keep.
634+
sourcePathsRemap.markDeleted(optionsPath)
635+
return nil, true, nil
636+
}
637+
if newOptions == nil {
638+
return optionsMessage, false, nil
639+
}
640+
return newOptions.Interface(), true, nil
641+
}
642+
643+
func remapMessage[T proto.Message](
644+
sourcePathsRemap *sourcePathsRemapTrie,
645+
sourcePath protoreflect.SourcePath,
646+
message T,
647+
remapMessage func(*sourcePathsRemapTrie, protoreflect.SourcePath, proto.Message) (proto.Message, bool, error),
648+
) (T, bool, error) {
649+
var zeroValue T
650+
newMessageOpaque, changed, err := remapMessage(sourcePathsRemap, sourcePath, message)
651+
if err != nil {
652+
return zeroValue, false, err
653+
}
654+
if newMessageOpaque == nil {
655+
return zeroValue, true, nil
656+
}
657+
if !changed {
658+
return message, false, nil
659+
}
660+
newMessage, _ := newMessageOpaque.(T) // Safe to assert.
661+
return newMessage, true, nil
501662
}
502663

503664
func remapSlice[T any](

‎private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Bar.txtar‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -92,32 +92,6 @@
9292
11
9393
]
9494
},
95-
{
96-
"path": [
97-
4,
98-
0,
99-
7
100-
],
101-
"span": [
102-
46,
103-
2,
104-
26
105-
]
106-
},
107-
{
108-
"path": [
109-
4,
110-
0,
111-
7,
112-
10101
113-
],
114-
"span": [
115-
46,
116-
2,
117-
26
118-
],
119-
"leadingComments": " Keep if Foo | NestedFoo: comment on option\n"
120-
},
12195
{
12296
"path": [
12397
4,

‎private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Do.txtar‎

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -92,32 +92,6 @@
9292
11
9393
]
9494
},
95-
{
96-
"path": [
97-
4,
98-
0,
99-
7
100-
],
101-
"span": [
102-
46,
103-
2,
104-
26
105-
]
106-
},
107-
{
108-
"path": [
109-
4,
110-
0,
111-
7,
112-
10101
113-
],
114-
"span": [
115-
46,
116-
2,
117-
26
118-
],
119-
"leadingComments": " Keep if Foo | NestedFoo: comment on option\n"
120-
},
12195
{
12296
"path": [
12397
4,
@@ -819,32 +793,6 @@
819793
11
820794
]
821795
},
822-
{
823-
"path": [
824-
6,
825-
0,
826-
3
827-
],
828-
"span": [
829-
143,
830-
2,
831-
29
832-
]
833-
},
834-
{
835-
"path": [
836-
6,
837-
0,
838-
3,
839-
10101
840-
],
841-
"span": [
842-
143,
843-
2,
844-
29
845-
],
846-
"leadingComments": " Keep if Svc: comment on option\n"
847-
},
848796
{
849797
"path": [
850798
6,

‎private/bufpkg/bufimage/bufimageutil/testdata/sourcecodeinfo/Foo.txtar‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -92,32 +92,6 @@
9292
11
9393
]
9494
},
95-
{
96-
"path": [
97-
4,
98-
0,
99-
7
100-
],
101-
"span": [
102-
46,
103-
2,
104-
26
105-
]
106-
},
107-
{
108-
"path": [
109-
4,
110-
0,
111-
7,
112-
10101
113-
],
114-
"span": [
115-
46,
116-
2,
117-
26
118-
],
119-
"leadingComments": " Keep if Foo | NestedFoo: comment on option\n"
120-
},
12195
{
12296
"path": [
12397
4,

0 commit comments

Comments
 (0)