Check for wrapper types in C# code gen should be case insensitive

[JamesNK]

What version of protobuf and what language are you using? Version: Gprc.Tools version 2.71.0 Language: C#

What operating system (Linux, Windows, ...) and version?

Windows

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

What did you do?

Have a proto file like this:

syntax = "proto3";

option csharp_namespace = "Microsoft.AspNetCore.Grpc.JsonTranscoding.Tests.TestObjects.ProtobutMessages";

import "Google/protobuf/wrappers.proto"; 
import "Google/api/annotations.proto"; 

package test;

message WrappersMessage {
    google.protobuf.StringValue string_value = 1;
    google.protobuf.Int32Value int32_value = 2;
    google.protobuf.Int64Value int64_value = 3;
    google.protobuf.FloatValue float_value = 4;
    google.protobuf.DoubleValue double_value = 5;
    google.protobuf.BoolValue bool_value = 6;
    google.protobuf.UInt32Value uint32_value = 7;
    google.protobuf.UInt64Value uint64_value = 8;
    google.protobuf.BytesValue bytes_value = 9;
}

It successfully compiles, but the properties generated on the C# type are StringValue rather than string.

This is because the import isn't completely lower case: import "Google/protobuf/wrappers.proto";. The code to detect if a field is a wrapper then checks the file name without ignoring case:

https://github.com/protocolbuffers/protobuf/blob/8228ee42b512cc330971e61bc9b86935a59f3477/src/google/protobuf/compiler/csharp/csharp_helpers.h#L112-L115

What did you expect to see

string field:

/// <summary>Field number for the "string_value" field.</summary>
public const int StringValueFieldNumber = 1;
private static readonly pb::FieldCodec<string> _single_stringValue_codec = pb::FieldCodec.ForClassWrapper<string>(10);
private string stringValue_;
[global::System.Diagnostics.DebuggerNonUserCodeAttribute]
[global::System.CodeDom.Compiler.GeneratedCode("protoc", null)]
public string StringValue {
  get { return stringValue_; }
  set {
    stringValue_ = value;
  }
}

What did you see instead?

StringValue field:

/// <summary>Field number for the "string_value" field.</summary>
public const int StringValueFieldNumber = 1;
private global::Google.Protobuf.WellKnownTypes.StringValue stringValue_;
[global::System.Diagnostics.DebuggerNonUserCodeAttribute]
[global::System.CodeDom.Compiler.GeneratedCode("protoc", null)]
public global::Google.Protobuf.WellKnownTypes.StringValue StringValue {
  get { return stringValue_; }
  set {
    stringValue_ = value;
  }
}

Anything else we should know about your project / environment

[jskeet]

@anandolee: I don't have time to maintain protobuf at the moment; I'll unassign myself.

I think this is user error which would be better caught elsewhere, to be honest. It would probably be good for protoc to check that the import is actually case-sensitively-correct, but that in itself would be a breaking change. Note that the proposed case-insensitive behavior could end up with weirdness when on a case-sensitive file-system where both Google/protobuf and google/protobuf exist. (Either way, I'm not personally going to do anything about this, but that's just my initial thoughts.)

[esrauchg]

If this is a 'bug' then I think right fix for this would be for IsWrapperType to check the message name against the complete list of them, rather than looking at the import at all. If someone wants to send a PR that does that I can review it.

It's somewhat gray area though, the reality is that the listed input file should probably be understood as outright malformed from a Protobuf point of view even when used on a case-insensitive file system, but in a way that is very hard for us to catch to error on.