From 34378b49b5b7a2cac2305cd32a041b3ec29c66f8 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Mon, 4 Jun 2012 10:35:59 +0100 Subject: [PATCH] Test and fix for issue 45. When we fetch properties, explicitly state that we don't want any arguments, to avoid ambiguity with indexers. (Also, change a few "typeof (Foo)" to "typeof(Foo)". Fuller replacement coming.) --- protos/extest/unittest_issues.proto | 12 +- src/ProtocolBuffers.Test/IssuesTest.cs | 59 ++++ .../ProtocolBuffers.Test.csproj | 1 + .../UnitTestExtrasIssuesProtoFile.cs | 251 +++++++++++++++++- .../FieldAccess/ReflectionUtil.cs | 5 + .../FieldAccess/RepeatedPrimitiveAccessor.cs | 18 +- .../FieldAccess/SingleMessageAccessor.cs | 2 +- .../FieldAccess/SinglePrimitiveAccessor.cs | 16 +- 8 files changed, 338 insertions(+), 26 deletions(-) create mode 100644 src/ProtocolBuffers.Test/IssuesTest.cs diff --git a/protos/extest/unittest_issues.proto b/protos/extest/unittest_issues.proto index 459e58f865..6d2bcf3fcb 100644 --- a/protos/extest/unittest_issues.proto +++ b/protos/extest/unittest_issues.proto @@ -70,19 +70,23 @@ service TestGenericService { // Issue 13: http://code.google.com/p/protobuf-csharp-port/issues/detail?id=13 message A { - optional int32 _A = 1; + optional int32 _A = 1; } message B { - optional int32 B_ = 1; + optional int32 B_ = 1; } message AB { - optional int32 a_b = 1; + optional int32 a_b = 1; } // Similar issue with numeric names message NumberField { - optional int32 _01 = 1; + optional int32 _01 = 1; } +// Issue 45: http://code.google.com/p/protobuf-csharp-port/issues/detail?id=45 +message ItemField { + optional int32 item = 1; +} diff --git a/src/ProtocolBuffers.Test/IssuesTest.cs b/src/ProtocolBuffers.Test/IssuesTest.cs new file mode 100644 index 0000000000..7096a52dfa --- /dev/null +++ b/src/ProtocolBuffers.Test/IssuesTest.cs @@ -0,0 +1,59 @@ +#region Copyright notice and license + +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// http://github.com/jskeet/dotnet-protobufs/ +// Original C++/Java/Python code: +// http://code.google.com/p/protobuf/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#endregion + +using Google.ProtocolBuffers.Descriptors; +using NUnit.Framework; +using UnitTest.Issues.TestProtos; + +namespace Google.ProtocolBuffers +{ + /// + /// Tests for issues which aren't easily compartmentalized into other unit tests. + /// + [TestFixture] + public class IssuesTest + { + // Issue 45 + [Test] + public void FieldCalledItem() + { + ItemField message = new ItemField.Builder { Item = 3 }.Build(); + FieldDescriptor field = ItemField.Descriptor.FindFieldByName("item"); + Assert.IsNotNull(field); + Assert.AreEqual(3, message[field]); + } + } +} diff --git a/src/ProtocolBuffers.Test/ProtocolBuffers.Test.csproj b/src/ProtocolBuffers.Test/ProtocolBuffers.Test.csproj index 95ab0b9c63..b2fe91e02e 100644 --- a/src/ProtocolBuffers.Test/ProtocolBuffers.Test.csproj +++ b/src/ProtocolBuffers.Test/ProtocolBuffers.Test.csproj @@ -98,6 +98,7 @@ + diff --git a/src/ProtocolBuffers.Test/TestProtos/UnitTestExtrasIssuesProtoFile.cs b/src/ProtocolBuffers.Test/TestProtos/UnitTestExtrasIssuesProtoFile.cs index db56e8b919..951691d59c 100644 --- a/src/ProtocolBuffers.Test/TestProtos/UnitTestExtrasIssuesProtoFile.cs +++ b/src/ProtocolBuffers.Test/TestProtos/UnitTestExtrasIssuesProtoFile.cs @@ -26,6 +26,8 @@ namespace UnitTest.Issues.TestProtos { internal static pb::FieldAccess.FieldAccessorTable internal__static_unittest_issues_AB__FieldAccessorTable; internal static pbd::MessageDescriptor internal__static_unittest_issues_NumberField__Descriptor; internal static pb::FieldAccess.FieldAccessorTable internal__static_unittest_issues_NumberField__FieldAccessorTable; + internal static pbd::MessageDescriptor internal__static_unittest_issues_ItemField__Descriptor; + internal static pb::FieldAccess.FieldAccessorTable internal__static_unittest_issues_ItemField__FieldAccessorTable; #endregion #region Descriptor public static pbd::FileDescriptor Descriptor { @@ -38,9 +40,9 @@ namespace UnitTest.Issues.TestProtos { "ChxleHRlc3QvdW5pdHRlc3RfaXNzdWVzLnByb3RvEg91bml0dGVzdF9pc3N1" + "ZXMaJGdvb2dsZS9wcm90b2J1Zi9jc2hhcnBfb3B0aW9ucy5wcm90byIPCgFB" + "EgoKAl9BGAEgASgFIg8KAUISCgoCQl8YASABKAUiEQoCQUISCwoDYV9iGAEg" + - "ASgFIhoKC051bWJlckZpZWxkEgsKA18wMRgBIAEoBUJASAHCPjsKGlVuaXRU" + - "ZXN0Lklzc3Vlcy5UZXN0UHJvdG9zEh1Vbml0VGVzdEV4dHJhc0lzc3Vlc1By" + - "b3RvRmlsZQ=="); + "ASgFIhoKC051bWJlckZpZWxkEgsKA18wMRgBIAEoBSIZCglJdGVtRmllbGQS" + + "DAoEaXRlbRgBIAEoBUJASAHCPjsKGlVuaXRUZXN0Lklzc3Vlcy5UZXN0UHJv" + + "dG9zEh1Vbml0VGVzdEV4dHJhc0lzc3Vlc1Byb3RvRmlsZQ=="); pbd::FileDescriptor.InternalDescriptorAssigner assigner = delegate(pbd::FileDescriptor root) { descriptor = root; internal__static_unittest_issues_A__Descriptor = Descriptor.MessageTypes[0]; @@ -59,6 +61,10 @@ namespace UnitTest.Issues.TestProtos { internal__static_unittest_issues_NumberField__FieldAccessorTable = new pb::FieldAccess.FieldAccessorTable(internal__static_unittest_issues_NumberField__Descriptor, new string[] { "_01", }); + internal__static_unittest_issues_ItemField__Descriptor = Descriptor.MessageTypes[4]; + internal__static_unittest_issues_ItemField__FieldAccessorTable = + new pb::FieldAccess.FieldAccessorTable(internal__static_unittest_issues_ItemField__Descriptor, + new string[] { "Item", }); pb::ExtensionRegistry registry = pb::ExtensionRegistry.CreateInstance(); RegisterAllExtensions(registry); global::Google.ProtocolBuffers.DescriptorProtos.CSharpOptions.RegisterAllExtensions(registry); @@ -1033,6 +1039,245 @@ namespace UnitTest.Issues.TestProtos { } } + [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] + [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("ProtoGen", "2.3.0.277")] + public sealed partial class ItemField : pb::GeneratedMessage { + private static readonly ItemField defaultInstance = new Builder().BuildPartial(); + private static readonly string[] _itemFieldFieldNames = new string[] { "item" }; + private static readonly uint[] _itemFieldFieldTags = new uint[] { 8 }; + public static ItemField DefaultInstance { + get { return defaultInstance; } + } + + public override ItemField DefaultInstanceForType { + get { return defaultInstance; } + } + + protected override ItemField ThisMessage { + get { return this; } + } + + public static pbd::MessageDescriptor Descriptor { + get { return global::UnitTest.Issues.TestProtos.UnitTestExtrasIssuesProtoFile.internal__static_unittest_issues_ItemField__Descriptor; } + } + + protected override pb::FieldAccess.FieldAccessorTable InternalFieldAccessors { + get { return global::UnitTest.Issues.TestProtos.UnitTestExtrasIssuesProtoFile.internal__static_unittest_issues_ItemField__FieldAccessorTable; } + } + + public const int ItemFieldNumber = 1; + private bool hasItem; + private int item_; + public bool HasItem { + get { return hasItem; } + } + public int Item { + get { return item_; } + } + + public override bool IsInitialized { + get { + return true; + } + } + + public override void WriteTo(pb::ICodedOutputStream output) { + int size = SerializedSize; + string[] field_names = _itemFieldFieldNames; + if (hasItem) { + output.WriteInt32(1, field_names[0], Item); + } + UnknownFields.WriteTo(output); + } + + private int memoizedSerializedSize = -1; + public override int SerializedSize { + get { + int size = memoizedSerializedSize; + if (size != -1) return size; + + size = 0; + if (hasItem) { + size += pb::CodedOutputStream.ComputeInt32Size(1, Item); + } + size += UnknownFields.SerializedSize; + memoizedSerializedSize = size; + return size; + } + } + + public static ItemField ParseFrom(pb::ByteString data) { + return ((Builder) CreateBuilder().MergeFrom(data)).BuildParsed(); + } + public static ItemField ParseFrom(pb::ByteString data, pb::ExtensionRegistry extensionRegistry) { + return ((Builder) CreateBuilder().MergeFrom(data, extensionRegistry)).BuildParsed(); + } + public static ItemField ParseFrom(byte[] data) { + return ((Builder) CreateBuilder().MergeFrom(data)).BuildParsed(); + } + public static ItemField ParseFrom(byte[] data, pb::ExtensionRegistry extensionRegistry) { + return ((Builder) CreateBuilder().MergeFrom(data, extensionRegistry)).BuildParsed(); + } + public static ItemField ParseFrom(global::System.IO.Stream input) { + return ((Builder) CreateBuilder().MergeFrom(input)).BuildParsed(); + } + public static ItemField ParseFrom(global::System.IO.Stream input, pb::ExtensionRegistry extensionRegistry) { + return ((Builder) CreateBuilder().MergeFrom(input, extensionRegistry)).BuildParsed(); + } + public static ItemField ParseDelimitedFrom(global::System.IO.Stream input) { + return CreateBuilder().MergeDelimitedFrom(input).BuildParsed(); + } + public static ItemField ParseDelimitedFrom(global::System.IO.Stream input, pb::ExtensionRegistry extensionRegistry) { + return CreateBuilder().MergeDelimitedFrom(input, extensionRegistry).BuildParsed(); + } + public static ItemField ParseFrom(pb::ICodedInputStream input) { + return ((Builder) CreateBuilder().MergeFrom(input)).BuildParsed(); + } + public static ItemField ParseFrom(pb::ICodedInputStream input, pb::ExtensionRegistry extensionRegistry) { + return ((Builder) CreateBuilder().MergeFrom(input, extensionRegistry)).BuildParsed(); + } + public static Builder CreateBuilder() { return new Builder(); } + public override Builder ToBuilder() { return CreateBuilder(this); } + public override Builder CreateBuilderForType() { return new Builder(); } + public static Builder CreateBuilder(ItemField prototype) { + return (Builder) new Builder().MergeFrom(prototype); + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] + [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("ProtoGen", "2.3.0.277")] + public sealed partial class Builder : pb::GeneratedBuilder { + protected override Builder ThisBuilder { + get { return this; } + } + public Builder() {} + + ItemField result = new ItemField(); + + protected override ItemField MessageBeingBuilt { + get { return result; } + } + + public override Builder Clear() { + result = new ItemField(); + return this; + } + + public override Builder Clone() { + return new Builder().MergeFrom(result); + } + + public override pbd::MessageDescriptor DescriptorForType { + get { return global::UnitTest.Issues.TestProtos.ItemField.Descriptor; } + } + + public override ItemField DefaultInstanceForType { + get { return global::UnitTest.Issues.TestProtos.ItemField.DefaultInstance; } + } + + public override ItemField BuildPartial() { + if (result == null) { + throw new global::System.InvalidOperationException("build() has already been called on this Builder"); + } + ItemField returnMe = result; + result = null; + return returnMe; + } + + public override Builder MergeFrom(pb::IMessage other) { + if (other is ItemField) { + return MergeFrom((ItemField) other); + } else { + base.MergeFrom(other); + return this; + } + } + + public override Builder MergeFrom(ItemField other) { + if (other == global::UnitTest.Issues.TestProtos.ItemField.DefaultInstance) return this; + if (other.HasItem) { + Item = other.Item; + } + this.MergeUnknownFields(other.UnknownFields); + return this; + } + + public override Builder MergeFrom(pb::ICodedInputStream input) { + return MergeFrom(input, pb::ExtensionRegistry.Empty); + } + + public override Builder MergeFrom(pb::ICodedInputStream input, pb::ExtensionRegistry extensionRegistry) { + pb::UnknownFieldSet.Builder unknownFields = null; + uint tag; + string field_name; + while (input.ReadTag(out tag, out field_name)) { + if(tag == 0 && field_name != null) { + int field_ordinal = global::System.Array.BinarySearch(_itemFieldFieldNames, field_name, global::System.StringComparer.Ordinal); + if(field_ordinal >= 0) + tag = _itemFieldFieldTags[field_ordinal]; + else { + if (unknownFields == null) { + unknownFields = pb::UnknownFieldSet.CreateBuilder(this.UnknownFields); + } + ParseUnknownField(input, unknownFields, extensionRegistry, tag, field_name); + continue; + } + } + switch (tag) { + case 0: { + throw pb::InvalidProtocolBufferException.InvalidTag(); + } + default: { + if (pb::WireFormat.IsEndGroupTag(tag)) { + if (unknownFields != null) { + this.UnknownFields = unknownFields.Build(); + } + return this; + } + if (unknownFields == null) { + unknownFields = pb::UnknownFieldSet.CreateBuilder(this.UnknownFields); + } + ParseUnknownField(input, unknownFields, extensionRegistry, tag, field_name); + break; + } + case 8: { + result.hasItem = input.ReadInt32(ref result.item_); + break; + } + } + } + + if (unknownFields != null) { + this.UnknownFields = unknownFields.Build(); + } + return this; + } + + + public bool HasItem { + get { return result.hasItem; } + } + public int Item { + get { return result.Item; } + set { SetItem(value); } + } + public Builder SetItem(int value) { + result.hasItem = true; + result.item_ = value; + return this; + } + public Builder ClearItem() { + result.hasItem = false; + result.item_ = 0; + return this; + } + } + static ItemField() { + object.ReferenceEquals(global::UnitTest.Issues.TestProtos.UnitTestExtrasIssuesProtoFile.Descriptor, null); + } + } + #endregion } diff --git a/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs b/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs index 31ebde067c..e876d9ceb0 100644 --- a/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs +++ b/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs @@ -44,6 +44,11 @@ namespace Google.ProtocolBuffers.FieldAccess /// internal static class ReflectionUtil { + /// + /// Empty Type[] used when calling GetProperty to force property instead of indexer fetching. + /// + internal static readonly Type[] EmptyTypes = new Type[0]; + /// /// Creates a delegate which will execute the given method and then return /// the result as an object. diff --git a/src/ProtocolBuffers/FieldAccess/RepeatedPrimitiveAccessor.cs b/src/ProtocolBuffers/FieldAccess/RepeatedPrimitiveAccessor.cs index 0c2cdfcf6d..612e491954 100644 --- a/src/ProtocolBuffers/FieldAccess/RepeatedPrimitiveAccessor.cs +++ b/src/ProtocolBuffers/FieldAccess/RepeatedPrimitiveAccessor.cs @@ -66,14 +66,14 @@ namespace Google.ProtocolBuffers.FieldAccess internal RepeatedPrimitiveAccessor(string name) { - PropertyInfo messageProperty = typeof (TMessage).GetProperty(name + "List"); - PropertyInfo builderProperty = typeof (TBuilder).GetProperty(name + "List"); - PropertyInfo countProperty = typeof (TMessage).GetProperty(name + "Count"); - MethodInfo clearMethod = typeof (TBuilder).GetMethod("Clear" + name, EmptyTypes); - getElementMethod = typeof (TMessage).GetMethod("Get" + name, new Type[] {typeof (int)}); + PropertyInfo messageProperty = typeof(TMessage).GetProperty(name + "List", ReflectionUtil.EmptyTypes); + PropertyInfo builderProperty = typeof(TBuilder).GetProperty(name + "List", ReflectionUtil.EmptyTypes); + PropertyInfo countProperty = typeof(TMessage).GetProperty(name + "Count", ReflectionUtil.EmptyTypes); + MethodInfo clearMethod = typeof(TBuilder).GetMethod("Clear" + name, EmptyTypes); + getElementMethod = typeof(TMessage).GetMethod("Get" + name, new Type[] {typeof(int)}); clrType = getElementMethod.ReturnType; - MethodInfo addMethod = typeof (TBuilder).GetMethod("Add" + name, new Type[] {ClrType}); - setElementMethod = typeof (TBuilder).GetMethod("Set" + name, new Type[] {typeof (int), ClrType}); + MethodInfo addMethod = typeof(TBuilder).GetMethod("Add" + name, new Type[] {ClrType}); + setElementMethod = typeof(TBuilder).GetMethod("Set" + name, new Type[] {typeof(int), ClrType}); if (messageProperty == null || builderProperty == null || countProperty == null @@ -85,9 +85,9 @@ namespace Google.ProtocolBuffers.FieldAccess throw new ArgumentException("Not all required properties/methods available"); } clearDelegate = - (Func) Delegate.CreateDelegate(typeof (Func), null, clearMethod); + (Func) Delegate.CreateDelegate(typeof(Func), null, clearMethod); countDelegate = (Func) Delegate.CreateDelegate - (typeof (Func), null, countProperty.GetGetMethod()); + (typeof(Func), null, countProperty.GetGetMethod()); getValueDelegate = ReflectionUtil.CreateUpcastDelegate(messageProperty.GetGetMethod()); addValueDelegate = ReflectionUtil.CreateDowncastDelegateIgnoringReturn(addMethod); getRepeatedWrapperDelegate = ReflectionUtil.CreateUpcastDelegate(builderProperty.GetGetMethod()); diff --git a/src/ProtocolBuffers/FieldAccess/SingleMessageAccessor.cs b/src/ProtocolBuffers/FieldAccess/SingleMessageAccessor.cs index 54b99c2b2a..6bf48a0ca9 100644 --- a/src/ProtocolBuffers/FieldAccess/SingleMessageAccessor.cs +++ b/src/ProtocolBuffers/FieldAccess/SingleMessageAccessor.cs @@ -50,7 +50,7 @@ namespace Google.ProtocolBuffers.FieldAccess internal SingleMessageAccessor(string name) : base(name) { - MethodInfo createBuilderMethod = ClrType.GetMethod("CreateBuilder", EmptyTypes); + MethodInfo createBuilderMethod = ClrType.GetMethod("CreateBuilder", ReflectionUtil.EmptyTypes); if (createBuilderMethod == null) { throw new ArgumentException("No public static CreateBuilder method declared in " + ClrType.Name); diff --git a/src/ProtocolBuffers/FieldAccess/SinglePrimitiveAccessor.cs b/src/ProtocolBuffers/FieldAccess/SinglePrimitiveAccessor.cs index 26d4a5b30b..088b93fcb7 100644 --- a/src/ProtocolBuffers/FieldAccess/SinglePrimitiveAccessor.cs +++ b/src/ProtocolBuffers/FieldAccess/SinglePrimitiveAccessor.cs @@ -47,8 +47,6 @@ namespace Google.ProtocolBuffers.FieldAccess private readonly Func hasDelegate; private readonly Func clearDelegate; - internal static readonly Type[] EmptyTypes = new Type[0]; - /// /// The CLR type of the field (int, the enum type, ByteString, the message etc). /// As declared by the property. @@ -60,14 +58,14 @@ namespace Google.ProtocolBuffers.FieldAccess internal SinglePrimitiveAccessor(string name) { - PropertyInfo messageProperty = typeof (TMessage).GetProperty(name); - PropertyInfo builderProperty = typeof (TBuilder).GetProperty(name); + PropertyInfo messageProperty = typeof(TMessage).GetProperty(name, ReflectionUtil.EmptyTypes); + PropertyInfo builderProperty = typeof(TBuilder).GetProperty(name, ReflectionUtil.EmptyTypes); if (builderProperty == null) { - builderProperty = typeof (TBuilder).GetProperty(name); + builderProperty = typeof(TBuilder).GetProperty(name, ReflectionUtil.EmptyTypes); } - PropertyInfo hasProperty = typeof (TMessage).GetProperty("Has" + name); - MethodInfo clearMethod = typeof (TBuilder).GetMethod("Clear" + name, EmptyTypes); + PropertyInfo hasProperty = typeof(TMessage).GetProperty("Has" + name, ReflectionUtil.EmptyTypes); + MethodInfo clearMethod = typeof(TBuilder).GetMethod("Clear" + name, ReflectionUtil.EmptyTypes); if (messageProperty == null || builderProperty == null || hasProperty == null || clearMethod == null) { throw new ArgumentException("Not all required properties/methods available"); @@ -75,9 +73,9 @@ namespace Google.ProtocolBuffers.FieldAccess clrType = messageProperty.PropertyType; hasDelegate = (Func) - Delegate.CreateDelegate(typeof (Func), null, hasProperty.GetGetMethod()); + Delegate.CreateDelegate(typeof(Func), null, hasProperty.GetGetMethod()); clearDelegate = - (Func) Delegate.CreateDelegate(typeof (Func), null, clearMethod); + (Func) Delegate.CreateDelegate(typeof(Func), null, clearMethod); getValueDelegate = ReflectionUtil.CreateUpcastDelegate(messageProperty.GetGetMethod()); setValueDelegate = ReflectionUtil.CreateDowncastDelegate(builderProperty.GetSetMethod()); }