Skip to content

Commit 7217580

Browse files
resolve oneof field name conflicts with imported packages (#1947)
Include oneof field names in conflict detection to properly generate _root_ prefixes when oneof fields conflict with imported package names. This fixes issue #1911 where ScalaPB would generate incorrect import paths for oneof fields that conflict with imported package names. The fix includes: - Enhanced conflict detection in DescriptorImplicits.scala to include oneof field names - Updated ProtobufGenerator.scala to properly handle conflicts when generating imports - Added comprehensive tests to verify the fix works correctly - Fixed test file to use correct generated class name (OneofImportConflictTest) Closes #1911
1 parent 07b7799 commit 7217580

File tree

5 files changed

+110
-10
lines changed

5 files changed

+110
-10
lines changed

compiler-plugin/src/main/scala/scalapb/compiler/DescriptorImplicits.scala

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -387,10 +387,13 @@ class DescriptorImplicits private[compiler] (
387387
case FieldDescriptor.JavaType.BYTE_STRING => "_root_.com.google.protobuf.ByteString"
388388
case FieldDescriptor.JavaType.STRING => "_root_.scala.Predef.String"
389389
case FieldDescriptor.JavaType.MESSAGE =>
390-
fd.getMessageType.scalaType
391-
.fullNameWithMaybeRoot(fd.getContainingType.fields.map(_.scalaName))
390+
val contextNames = fd.getContainingType.fields.map(_.scalaName) ++
391+
fd.getContainingType.getRealOneofs.asScala.map(_.scalaName.nameSymbol)
392+
fd.getMessageType.scalaType.fullNameWithMaybeRoot(contextNames)
392393
case FieldDescriptor.JavaType.ENUM =>
393-
fd.getEnumType.scalaType.fullNameWithMaybeRoot(fd.getContainingType.fields.map(_.scalaName))
394+
val contextNames = fd.getContainingType.fields.map(_.scalaName) ++
395+
fd.getContainingType.getRealOneofs.asScala.map(_.scalaName.nameSymbol)
396+
fd.getEnumType.scalaType.fullNameWithMaybeRoot(contextNames)
394397
}
395398

396399
def singleScalaTypeName = customSingleScalaTypeName.getOrElse(baseSingleScalaTypeName)

compiler-plugin/src/main/scala/scalapb/compiler/ProtobufGenerator.scala

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -188,11 +188,15 @@ class ProtobufGenerator(
188188
.mkString("_root_.com.google.protobuf.ByteString.copyFrom(Array[Byte](", ", ", "))")
189189
case FieldDescriptor.JavaType.STRING => escapeScalaString(defaultValue.asInstanceOf[String])
190190
case FieldDescriptor.JavaType.MESSAGE =>
191+
val contextNames = field.getContainingType.fields.map(_.scalaName) ++
192+
field.getContainingType.getRealOneofs.asScala.map(_.scalaName.nameSymbol)
191193
field.getMessageType.scalaType
192-
.fullNameWithMaybeRoot(field.getContainingType) + ".defaultInstance"
194+
.fullNameWithMaybeRoot(contextNames) + ".defaultInstance"
193195
case FieldDescriptor.JavaType.ENUM =>
196+
val contextNames = field.getContainingType.fields.map(_.scalaName) ++
197+
field.getContainingType.getRealOneofs.asScala.map(_.scalaName.nameSymbol)
194198
field.getEnumType.scalaType
195-
.fullNameWithMaybeRoot(field.getContainingType) + "." + defaultValue
199+
.fullNameWithMaybeRoot(contextNames) + "." + defaultValue
196200
.asInstanceOf[EnumValueDescriptor]
197201
.scalaName
198202
.asSymbol
@@ -217,13 +221,22 @@ class ProtobufGenerator(
217221
case FieldDescriptor.JavaType.BYTE_STRING => Identity
218222
case FieldDescriptor.JavaType.STRING => Identity
219223
case FieldDescriptor.JavaType.MESSAGE =>
220-
FunctionApplication(field.getMessageType.scalaType.fullName + ".fromJavaProto")
224+
val contextNames = field.getContainingType.fields.map(_.scalaName) ++
225+
field.getContainingType.getRealOneofs.asScala.map(_.scalaName.nameSymbol)
226+
FunctionApplication(
227+
field.getMessageType.scalaType.fullNameWithMaybeRoot(contextNames) + ".fromJavaProto"
228+
)
221229
case FieldDescriptor.JavaType.ENUM =>
230+
val contextNames = field.getContainingType.fields.map(_.scalaName) ++
231+
field.getContainingType.getRealOneofs.asScala.map(_.scalaName.nameSymbol)
222232
if (!field.legacyEnumFieldTreatedAsClosed())
223233
MethodApplication("intValue") andThen FunctionApplication(
224-
field.getEnumType.scalaType.fullName + ".fromValue"
234+
field.getEnumType.scalaType.fullNameWithMaybeRoot(contextNames) + ".fromValue"
235+
)
236+
else
237+
FunctionApplication(
238+
field.getEnumType.scalaType.fullNameWithMaybeRoot(contextNames) + ".fromJavaValue"
225239
)
226-
else FunctionApplication(field.getEnumType.scalaType.fullName + ".fromJavaValue")
227240
}
228241
baseValueConversion andThen toCustomTypeExpr(field)
229242
}
@@ -289,12 +302,20 @@ class ProtobufGenerator(
289302
case FieldDescriptor.JavaType.BYTE_STRING => Identity
290303
case FieldDescriptor.JavaType.STRING => Identity
291304
case FieldDescriptor.JavaType.MESSAGE =>
292-
FunctionApplication(field.getMessageType.scalaType.fullName + ".toJavaProto")
305+
val contextNames = field.getContainingType.fields.map(_.scalaName) ++
306+
field.getContainingType.getRealOneofs.asScala.map(_.scalaName.nameSymbol)
307+
FunctionApplication(
308+
field.getMessageType.scalaType.fullNameWithMaybeRoot(contextNames) + ".toJavaProto"
309+
)
293310
case FieldDescriptor.JavaType.ENUM =>
311+
val contextNames = field.getContainingType.fields.map(_.scalaName) ++
312+
field.getContainingType.getRealOneofs.asScala.map(_.scalaName.nameSymbol)
294313
if (!field.legacyEnumFieldTreatedAsClosed())
295314
(MethodApplication("value") andThen maybeBox("_root_.scala.Int.box"))
296315
else
297-
FunctionApplication(field.getEnumType.scalaType.fullName + ".toJavaValue")
316+
FunctionApplication(
317+
field.getEnumType.scalaType.fullNameWithMaybeRoot(contextNames) + ".toJavaValue"
318+
)
298319
}
299320
}
300321

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
syntax = "proto3";
2+
3+
package location.v1;
4+
5+
message Coordinate {
6+
double latitude = 1;
7+
double longitude = 2;
8+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
syntax = "proto3";
2+
3+
package com.thesamet.proto.e2e;
4+
5+
import "location/v1/location.proto";
6+
7+
// Test case for oneof field name conflicting with imported package name.
8+
// This ensures ScalaPB correctly adds _root_ prefixes to avoid naming conflicts.
9+
message OneofImportConflictTest {
10+
oneof location { // This field name conflicts with the imported package "location"
11+
location.v1.Coordinate location_coordinate = 2;
12+
}
13+
}
14+
15+
// Additional test with multiple conflicts
16+
message MultipleConflictTest {
17+
oneof location {
18+
location.v1.Coordinate coord = 1;
19+
}
20+
21+
// Regular field with same type should also get _root_ prefix due to oneof conflict
22+
location.v1.Coordinate regular_coord = 2;
23+
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import org.scalatest.flatspec.AnyFlatSpec
2+
import org.scalatest.matchers.should.Matchers
3+
import com.thesamet.proto.e2e.oneof_import_conflict.{OneofImportConflictTest, MultipleConflictTest}
4+
5+
class OneofImportConflictSpec extends AnyFlatSpec with Matchers {
6+
7+
"oneof field conflicting with imported package name" should "generate correct code with _root_ prefixes" in {
8+
// Test basic oneof conflict - the fact this compiles means _root_ prefixes work
9+
val coord = _root_.location.v1.location.Coordinate(latitude = 1.0, longitude = 2.0)
10+
11+
val message = OneofImportConflictTest(
12+
location = OneofImportConflictTest.Location.LocationCoordinate(coord)
13+
)
14+
15+
// Test functionality works correctly
16+
message.getLocationCoordinate shouldBe coord
17+
message.location.isLocationCoordinate shouldBe true
18+
message.location.locationCoordinate shouldBe Some(coord)
19+
20+
// Test serialization round-trip
21+
val bytes = message.toByteArray
22+
val parsed = OneofImportConflictTest.parseFrom(bytes)
23+
parsed shouldBe message
24+
}
25+
26+
"multiple field types with import conflicts" should "all use _root_ prefixes consistently" in {
27+
val coord1 = _root_.location.v1.location.Coordinate(latitude = 3.0, longitude = 4.0)
28+
val coord2 = _root_.location.v1.location.Coordinate(latitude = 5.0, longitude = 6.0)
29+
30+
val message = MultipleConflictTest(
31+
location = MultipleConflictTest.Location.Coord(coord1),
32+
regularCoord = Some(coord2)
33+
)
34+
35+
// Test both oneof and regular fields work correctly
36+
message.location.isCoord shouldBe true
37+
message.location.coord shouldBe Some(coord1)
38+
message.regularCoord shouldBe Some(coord2)
39+
40+
// Test serialization round-trip
41+
val bytes = message.toByteArray
42+
val parsed = MultipleConflictTest.parseFrom(bytes)
43+
parsed shouldBe message
44+
}
45+
}

0 commit comments

Comments
 (0)