Skip to content

Commit

Permalink
Don't arbitrarily prune oneOf options
Browse files Browse the repository at this point in the history
  • Loading branch information
@oldergod
oldergod committed Oct 10, 2023
1 parent 126c5f4 commit 3d610dc
Show file tree
Hide file tree
Showing 4 changed files with 149 additions and 5 deletions.
1 change: 1 addition & 0 deletions wire-schema/api/wire-schema.api
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,7 @@ public final class com/squareup/wire/schema/MessageType : com/squareup/wire/sche
public final fun isDeprecated ()Z
public fun linkMembers (Lcom/squareup/wire/schema/Linker;)V
public fun linkOptions (Lcom/squareup/wire/schema/Linker;Lcom/squareup/wire/schema/SyntaxRules;Z)V
public final fun oneOf (Ljava/lang/String;)Lcom/squareup/wire/schema/OneOf;
public fun retainAll (Lcom/squareup/wire/schema/Schema;Lcom/squareup/wire/schema/MarkSet;)Lcom/squareup/wire/schema/Type;
public fun retainLinked (Ljava/util/Set;Ljava/util/Set;)Lcom/squareup/wire/schema/Type;
public final fun toElement ()Lcom/squareup/wire/schema/internal/parser/MessageElement;
Expand Down
4 changes: 4 additions & 0 deletions wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/MessageType.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ data class MessageType(
fun extensionField(qualifiedName: String): Field? =
extensionFields.firstOrNull { it.qualifiedName == qualifiedName }

/** Returns the oneOf named [name], or null if this type has no such oneOf. */
fun oneOf(name: String): OneOf? =
oneOfs.firstOrNull { it.name == name }

/** Returns the field tagged [tag], or null if this type has no such field. */
fun field(tag: Int): Field? {
for (field in declaredFields) {
Expand Down
20 changes: 15 additions & 5 deletions wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/Pruner.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,12 @@ class Pruner(
val member = protoMember.member
return when (val type = schema.getType(protoMember.type)) {
is MessageType -> {
val field = type.field(member) ?: type.extensionField(member)!!
pruningRules.isFieldRetainedVersion(field.options)
val field = type.field(member) ?: type.extensionField(member)
if (field != null) {
pruningRules.isFieldRetainedVersion(field.options)
} else {
pruningRules.isFieldRetainedVersion(type.oneOf(member)!!.options)
}
}
is EnumType -> {
val enumConstant = type.constant(member)!!
Expand Down Expand Up @@ -177,9 +181,14 @@ class Pruner(

if (type is MessageType) {
val field = type.field(member) ?: type.extensionField(member)
checkNotNull(field) { "unexpected member: $member" }
result.add(field.type)
options = field.options
if (field != null) {
result.add(field.type)
options = field.options
} else {
val oneOf = type.oneOf(member)
checkNotNull(oneOf) { "unexpected member: $member" }
options = oneOf.options
}
} else if (type is EnumType) {
val constant = type.constant(member)
?: throw IllegalStateException("unexpected member: $member")
Expand Down Expand Up @@ -218,6 +227,7 @@ class Pruner(
result.add(get(root, field.qualifiedName))
}
for (oneOf in type.oneOfs) {
result.add(get(root, oneOf.name))
for (field in oneOf.fields) {
result.add(get(root, field.name))
}
Expand Down
129 changes: 129 additions & 0 deletions wire-schema/src/jvmTest/kotlin/com/squareup/wire/schema/PrunerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,135 @@ class PrunerTest {
assertThat(pruned.getType("MessageB")).isNull()
}

@Test
fun oneOfOptionsAreNotArbitrarilyPruned() {
val schema = buildSchema {
add(
"test_event.proto".toPath(),
"""
|syntax = "proto3";
|
|import "test_event_custom_option.proto";
|
|package test.oneOf.options.test;
|
|message TestMessage {
| oneof element {
| option (my_custom_oneOf_option) = true;
| string one = 1;
| string two = 2;
| }
|}
""".trimMargin(),
)
add(
"test_event_custom_option.proto".toPath(),
"""
|syntax = "proto3";
|
|import "google/protobuf/descriptor.proto";
|
|package test.oneOf.options;
|
|extend google.protobuf.OneofOptions {
| bool my_custom_oneOf_option = 101400;
|}
""".trimMargin(),
)
}
val pruned = schema.prune(
PruningRules.Builder()
.addRoot("test.oneOf.options.test.TestMessage")
.build(),
)
assertThat(pruned.protoFile("test_event.proto")!!.toSchema())
.isEqualTo(
// spotless:off because spotless will remove the indents (trailing spaces) in the oneof block.
"""
|// Proto schema formatted by Wire, do not edit.
|// Source: test_event.proto
|
|syntax = "proto3";
|
|package test.oneOf.options.test;
|
|import "test_event_custom_option.proto";
|
|message TestMessage {
| oneof element {
| option (my_custom_oneOf_option) = true;
|
| string one = 1;
| string two = 2;
| }
|}
|""".trimMargin(),
// spotless:on
)
}

@Test
fun oneOfOptionsArePruned() {
val schema = buildSchema {
add(
"test_event.proto".toPath(),
"""
|syntax = "proto3";
|
|import "test_event_custom_option.proto";
|
|package test.oneOf.options.test;
|
|message TestMessage {
| oneof element {
| option (my_custom_oneOf_option) = true;
| string one = 1;
| string two = 2;
| }
|}
""".trimMargin(),
)
add(
"test_event_custom_option.proto".toPath(),
"""
|syntax = "proto3";
|
|import "google/protobuf/descriptor.proto";
|
|package test.oneOf.options;
|
|extend google.protobuf.OneofOptions {
| bool my_custom_oneOf_option = 101400;
|}
""".trimMargin(),
)
}
val pruned = schema.prune(
PruningRules.Builder()
.prune("google.protobuf.OneofOptions#test.oneOf.options.my_custom_oneOf_option")
.build(),
)
assertThat(pruned.protoFile("test_event.proto")!!.toSchema())
.isEqualTo(
"""
|// Proto schema formatted by Wire, do not edit.
|// Source: test_event.proto
|
|syntax = "proto3";
|
|package test.oneOf.options.test;
|
|message TestMessage {
| oneof element {
| string one = 1;
| string two = 2;
| }
|}
|
""".trimMargin(),
)
}

@Test
fun retainMap() {
val schema = buildSchema {
Expand Down

0 comments on commit 3d610dc

Please sign in to comment.