Skip to content

Commit 85f7572

Browse files
authored
Treat an unchanged map/list component as wire-compatible in diff (#119)
wireCompatibleScalars recursed into map and list types requiring both components to be wire-compatible, but had no equality short-circuit. An unchanged component that is not a member of a wire-compatible scalar group (e.g. a string map key) returned false, dragging the whole verdict to incompatible. So a genuinely wire-safe map value widening such as map<string,int32> -> map<string,int64> was reported as an Error in Wire mode, failing CI on a safe change. Short-circuit identical components to wire-compatible.
1 parent 2e9775c commit 85f7572

2 files changed

Lines changed: 15 additions & 0 deletions

File tree

tools/src/main/scala/proteus/ProtoDiff.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ object ProtoDiff {
261261
)
262262

263263
private def wireCompatibleScalars(oldType: Type, newType: Type): Boolean = (oldType, newType) match {
264+
case (a, b) if a == b => true
264265
case (Type.ListType(x), Type.ListType(y)) => wireCompatibleScalars(x, y)
265266
case (Type.MapType(k1, v1), Type.MapType(k2, v2)) => wireCompatibleScalars(k1, k2) && wireCompatibleScalars(v1, v2)
266267
case (Type.String, Type.Bytes) => true

tools/src/test/scala/proteus/ProtoDiffSpec.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,20 @@ object ProtoDiffSpec extends ZIOSpecDefault {
154154
}
155155
assertTrue(results.forall(identity))
156156
},
157+
test("wire-compatible map key/value widening is not breaking in Wire mode") {
158+
val cases = List(
159+
("map<string, int32>", "map<string, int64>"),
160+
("map<int32, string>", "map<int64, string>")
161+
)
162+
val results = cases.map { case (from, to) =>
163+
val old = parse(s"""syntax = "proto3"; message Foo { $from counts = 1; }""")
164+
val nw = parse(s"""syntax = "proto3"; message Foo { $to counts = 1; }""")
165+
val changes = ProtoDiff.diff(old, nw)
166+
changes.nonEmpty &&
167+
changes.forall(c => ProtoDiff.severity(c, CompatMode.Wire) == Severity.Info)
168+
}
169+
assertTrue(results.forall(identity))
170+
},
157171
test("field optionality changed") {
158172
val old = parse("""syntax = "proto3"; message Foo { string name = 1; }""")
159173
val nw = parse("""syntax = "proto3"; message Foo { optional string name = 1; }""")

0 commit comments

Comments
 (0)