Skip to content

Commit

Permalink
Don't qualify oneof type name when it doesn't share a name with its e…
Browse files Browse the repository at this point in the history
…nclosing type (#82)
  • Loading branch information
andrewparmet authored Jun 26, 2020
1 parent 7457803 commit 8a30fc5
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ private constructor(
PClass.fromName(fieldType).let {
// If a wrapper type is specified and it shares a name with the
// oneof, it must be fully qualified.
// See testing/options/src/main/proto/com/toasttab/protokt/testing/options/oneof_exercises.proto
if (unqualifiedType.simpleName == it.simpleName) {
it.possiblyQualify(ctx.pkg).qualifiedName
} else {
Expand All @@ -109,21 +110,21 @@ private constructor(
val pClass = f.typePClass

// Cannot strip qualifiers for field type in a different package
// See testing/runtime-tests/src/main/proto/com/toasttab/protokt/testing/rt/oneof/oneof_packages.proto
val requiresQualifiedTypeName = pClass.ppackage != ctx.pkg

return if (requiresQualifiedTypeName) {
pClass.renderName(ctx.pkg)
} else {
if (oneofFieldTypeName == pClass.nestedName) {
// Oneof field name shares name of its type
if (oneofFieldTypeName == pClass.simpleName) {
// Oneof field name shares name of its enclosing type
// See testing/runtime-tests/src/main/proto/com/toasttab/protokt/testing/rt/oneof/oneof_exercises.proto
if (oneofFieldTypeName == pClass.simpleName) {
if (oneofFieldTypeName == pClass.nestedName) {
pClass.qualifiedName
} else {
pClass.simpleName
pClass.nestedName
}
} else {
pClass.nestedName
pClass.simpleName
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,27 +23,9 @@ import "protokt/protokt.proto";
// generated sealed class type names. If the code generator does
// not handle them properly then compilation will fail.

message OneofExerciseModel {
oneof oneof {
// Needs partial qualification: OneofExerciseModel.Model
Model model = 1;
}

message Model {
bytes id = 1;
}
}

message OneofExerciseModel2 {
oneof oneof {
// Needs full qualification: com.toasttab.model.OneofExerciseModel
OneofExerciseModel oneof_exercise_model = 1;
}
}

message OneofExerciseModelWithWrapper {
oneof oneof {
// Needs full qualification: com.toasttab.testing.options.CachingId
// Needs full qualification: com.toasttab.protokt.testing.options.CachingId
bytes caching_id = 1 [
(.protokt.property).wrap = "CachingId"
];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright (c) 2020 Toast Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

syntax = "proto3";

package com.toasttab.protokt.testing.rt.oneof;

// This file exercises oneofs whose names are the same as their
// generated sealed class type names. If the code generator does
// not handle them properly then compilation will fail.

message OneofExerciseModel {
oneof oneof {
// Needs partial qualification: OneofExerciseModel.Model
Model model = 1;
}

oneof oneof2 {
// Does not need partial qualification: Model
// This will still compile if qualified but may show a warning in IDEs
Model not_named_model = 2;
}

message Model {
bytes id = 1;
}
}

message OneofExerciseModel2 {
oneof oneof {
// Needs full qualification: com.toasttab.protokt.testing.rt.oneof.OneofExerciseModel
OneofExerciseModel oneof_exercise_model = 1;
}
}

0 comments on commit 8a30fc5

Please sign in to comment.