Generated C# files don't contain numeric constants for FieldOptions,MessageOptions,etc

[jswigart]

I am generating files for C++ and C# using protoc 3.5.1 and while the C++ files have named constants generated for the various options I am defining in the proto file, there are no such named constants in the C# source file, even though the options appear to still function, if I manually use the numeric values in the CustomOptions.TryGet functions.

Any reason why there are no named constants generated in the C# file to avoid hard coding numbers(and make the code more readable, etc)

[jskeet]

Sorry, I'm incredibly busy at the moment - I can't take on any extra work.

[jswigart]

Willing to take a PR for this?

[ObsidianMinor]

You can also encourage the maintainers to accept the proto2 in C# proposal which has support for extensions.

[jswigart]

Proto3 has support for options already. In C# it's via the CustomOptions class, which has typed TryGet values that take the numeric field ids of the options. All I'm wanting is for the generator to also build named constants for those numbers, so instead of this

field.CustomOptions.TryGetString(4001, out myFieldOption)

it would be a cleaner and more readible like the C++ generated code, and look more like this

field.CustomOptions.TryGetString(MyNamespace.OptionFoo, out myFieldOption)

where

MyMessageNamespace.FieldOptionFoo = 4001 <== this is the missing generated bit, that the C++ generator already generates.

Seems like a simple thing to add to the csharp generator that already supports options. Is there some hidden complexity here?

[xfxyjwf]

@jswigart Feel free to send me a PR to add that. It was not added because that's not the right way to support custom options, but we don't see harm adding it though. Note that the generated constant should be named consistently with other existing constants, i.e., use "public const int FooFieldNumber = ...".

[jswigart]

@xfxyjwf

what do you think if I wanted to go a slight step forward, and make the generated identifiers type safe, such that they can only be passed to the CustomOptions that they are matched with?

For example, this commit in my fork

jswigart@7e968ac

generates a block in the csharp file that looks like this. this helper identifier is therefore type safe in that you can only pass a FileOptionFieldId to the CustomOptions on a FileDescriptor

  public static class Extensions {
    public static readonly pbr.FileOptionFieldId testFileOptions = new pbr.FileOptionFieldId(5000);
    public static readonly pbr.MessageOptionFieldId testMessageOptions = new pbr.MessageOptionFieldId(5001);
    public static readonly pbr.FieldOptionFieldId testFieldOptions = new pbr.FieldOptionFieldId(5002);
    public static readonly pbr.OneOfOptionFieldId testOneOfOptions = new pbr.OneOfOptionFieldId(5003);
    public static readonly pbr.EnumOptionFieldId testEnumOptions = new pbr.EnumOptionFieldId(5004);
    public static readonly pbr.EnumValueOptionFieldId testEnumValueOptions = new pbr.EnumValueOptionFieldId(5005);
    public static readonly pbr.ServiceOptionFieldId testServiceOptions = new pbr.ServiceOptionFieldId(5006);
    public static readonly pbr.MethodOptionFieldId testMethodOptions = new pbr.MethodOptionFieldId(5007);
  }

So then in your code you could query with a friendly identifier

string sout;
            if (TestMessage.Descriptor.CustomOptions.TryGetString(Extensions.testMessageOptions, out sout))
            {
            }

And you will get a compiler error if you mistakingly try to pass the wrong type as the first argument.

Thoughts?

If you'd prefer just a bunch of naked integer constants I can do that as well, but I thought it would be useful to make it safer within the option types.

[jswigart]

Nevermind that type safe approach, that would break existing users, so I rolled it back to the simpler version

#5246

[jskeet]

@ObsidianMinor: I believe this issue is now deprecated - the options API has changed a few times since this. Do you think we can just close this?

[ObsidianMinor]

Yeah that makes sense