protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

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

Open vrelk-net opened this issue 1 year ago • 4 comments

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

  • Google.Protobuf: 3.27.1
  • Protoc: 27.1
  • .NET 6.0 and .NET 7.0
  • Language: C#

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

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

  • Visual Studio 2022: 17.7.6
    • MSBuild: 17.7.2
    • C# Tools: 4.7.0

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

vrelk-net avatar Jun 24 '24 19:06 vrelk-net

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.

jskeet avatar Jul 01 '24 15:07 jskeet

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

vrelk-net avatar Jul 02 '24 13:07 vrelk-net

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.

jskeet avatar Jul 02 '24 14:07 jskeet

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago. This issue will be closed and archived after 14 additional days without activity.

github-actions[bot] avatar Oct 14 '24 10:10 github-actions[bot]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please reopen it.

This issue was closed and archived because there has been no new activity in the 14 days since the inactive label was added.

github-actions[bot] avatar Oct 29 '24 10:10 github-actions[bot]