search

protocolbuffers / protobuf

Protocol Buffers - Google's data interchange format
http://protobuf.dev
Other
65.37k stars 15.46k forks source link

Invalid generated C# code for the "record" class. #17220

Open vrelk-net opened 3 months ago

vrelk-net commented 3 months ago

What version of protobuf and what language are you using? Version:

OS: Ubuntu 22.04 (protoc), Windows 11 (Visual Studio)

What runtime / compiler are you using (e.g., python version or gcc version):

What did you do? Steps to reproduce the behavior:

  1. Generate the C# source using the command ./protoc --proto_path=../include --proto_path=central --csharp_out=out central/location.proto
  2. Copy the .cs file into VS (same result when copying the file, or just the contents)
  3. Build the project.
  4. See error

Compiler errors

1>------ Build started: Project: ArubaCentral.Protobuf, Configuration: Debug Any CPU ------
1>C:\Users\user\source\repos\ArubaCentral.Protobuf\Location.cs(398,7,398,13): error CS1519: Invalid token 'return' in class, record, struct, or interface member declaration
1>C:\Users\user\source\repos\ArubaCentral.Protobuf\Location.cs(398,29,398,30): error CS1031: Type expected
1>C:\Users\user\source\repos\ArubaCentral.Protobuf\Location.cs(398,29,398,30): error CS1001: Identifier expected
1>Done building project "ArubaCentral.Protobuf.csproj" -- FAILED.

Along with the following errors that Visual Studio spits out in addition to the ones above.

Error   CS0535  'record' does not implement interface member 'IDeepCloneable<record>.Clone()'   Location.cs - Line 352
Error   CS0106  The modifier 'new' is not valid for this item   Location.cs - Line 398
Error   CS1520  Method must have a return type  Location.cs - Line 398
Error   CS0027  Keyword 'this' is not available in the current context  Location.cs - Line 398

generated Location.cs source location.proto

jskeet commented 3 months ago

I strongly suspect this is due to record now being a contextual keyword in C#.

My personal recommendation is that you rename all your messages and enums to follow more common proto conventions (where they're PascalCased) - I strongly suspect that would fix the issue.

Obviously it would be good for us to handle this anyway, but it may well end up being tricky in terms of compatibility. I'm unlikely to get to this any time soon, I'm afraid - I have higher priority work.

vrelk-net commented 3 months ago

Yep. Changing record to Record in the protobuf file did fix it. Wouldn't this fall under the pascal case function? I'm not sure that's working properly (ex. public sealed partial class mac_address : pb::IMessage<mac_address>), unless it's only intended to capitalize some of them (ex. namespace Location and public static partial class LocationReflection. Location.cs.txt

jskeet commented 3 months ago

Wouldn't this fall under the pascal case function?

The generator doesn't apply that to message names (or RPC names, IIRC) - only field names, package elements, and file names. It potentially could do, but I'm not sure whether it does in any other languages, and it would be a very breaking change to make it start doing so now.