Alternative solution for: Fix double nested naming

[adament]

This is an alternative solution to #17 than #18. This fixes the double naming issue, even if the prefix is already a prefix of the child name. All credit should go to @cetanu for identifying and tracking down the problem.

Test case:

syntax = "proto3";

message Root {
  message Parent {
    message RootParentChild {
      string a = 1;
    }
    enum EnumChild{
      A = 0;
      B = 1;
    }
    message Child {
      string foo = 1;
    }
    reserved 1;
    repeated Child child = 2;
    repeated EnumChild enum_child=3;
    repeated RootParentChild root_parent_child=4;
    bool bar = 5;
  }
  string name = 1;
  Parent parent = 4;
}

# Generated by the protocol buffer compiler.  DO NOT EDIT!
# sources: test.proto
# plugin: python-betterproto
from dataclasses import dataclass
from typing import List

import betterproto


class RootParentEnumChild(betterproto.Enum):
    A = 0
    B = 1


@dataclass
class Root(betterproto.Message):
    name: str = betterproto.string_field(1)
    parent: "RootParent" = betterproto.message_field(4)


@dataclass
class RootParent(betterproto.Message):
    child: List["RootParentChild"] = betterproto.message_field(2)
    enum_child: List["RootParentEnumChild"] = betterproto.enum_field(3)
    root_parent_child: List["RootParentRootParentChild"] = betterproto.message_field(4)
    bar: bool = betterproto.bool_field(5)


@dataclass
class RootParentRootParentChild(betterproto.Message):
    a: str = betterproto.string_field(1)


@dataclass
class RootParentChild(betterproto.Message):
    foo: str = betterproto.string_field(1)

[cetanu]

Trying to add a test for this in betterproto/tests

proto file as above (filename: nestedtwice.proto):

syntax = "proto3";

message Test {
  message Parent {
    message RootParentChild {
      string a = 1;
    }
    enum EnumChild{
      A = 0;
      B = 1;
    }
    message Child {
      string foo = 1;
    }
    reserved 1;
    repeated Child child = 2;
    repeated EnumChild enumChild=3;
    repeated RootParentChild rootParentChild=4;
    bool bar = 5;
  }
  string name = 1;
  Parent parent = 2;
}

json file (filename: nestedtwice.json)

{
  "name": "double-nested",
  "parent": {
    "child": [{"foo": "hello"}],
    "enumChild": ["A"],
    "rootParentChild": [{"a": "hello"}],
    "bar": true
  }
}

Can you add this to your branch?

[adament]

Yes I will, thank you very much for picking this up. Is it okay if I rename RootParentChild to TestParentChild? I added it originally because I wanted to test the case where the parent names were already a prefix of the child name.

[nat-n]

Is it okay if I rename RootParentChild to TestParentChild? I added it originally because I wanted to test the case where the parent names were already a prefix of the child name.

Might not be a good idea; if it results in a class being generated under the tests directory with a name that starts or ends with Test then pytest gets all exited about it and spams warnings.

[adament]

@nat-n Should we then not name the enclosing message Test at all? As the concatenation of nested message names will mean that all messages will be prefixed with Test?

[adament]

@cetanu I cherry picked commit 5abfeb6 onto my branch, I hope this is what you wanted?

[nat-n]

@adament indeed.

Seems kinda redundant to name test fixtures "Test*" coming to think of it. Root seems to be like a good name.

[adament]

Looking a bit closer at the test framework, the test generator in generate.py assumes that the test message is named Test. If this should be changed, I think it should be a separate changeset.

[nat-n]

@adament ah, so it does. I agree it's beyond the scope of this change to fix.

I guess it's not really a problem so long as none of the generated classes happen to include an __init__ method (which will cause a warning) or any methods starting with test_ which will cause them to be executed as tests.

[cetanu]

I think I may have caused some confusion, I apologize folks.
In the original PR you actually had a message Root at the very top, and somehow when I was making up a test I omitted this.

I did not intend to replace Root with Test, I actually should have put everything including Root inside the Test message, because the Test message is just a container that the test framework recognizes as a target