From 301e791d9d8418ffaa824eb31f51fe05dd7c4b96 Mon Sep 17 00:00:00 2001 From: James Frowen Date: Sun, 22 Nov 2020 02:42:53 +0000 Subject: [PATCH] fix(weaver): Warning for multiple write/read functions for the same type (#2453) * fix(weaver): Warning for multiple write/read functions for the same type Adding warning message when trying to register a write or read function for a type that is already in dictionary. Previously weaver would be silent about this making it hard to know when an extension method overrides another one. * test for warnings --- Assets/Mirror/Editor/Weaver/Readers.cs | 6 ++++ Assets/Mirror/Editor/Weaver/Writers.cs | 8 ++++- .../Tests/Editor/Weaver/.WeaverTests.csproj | 1 + .../WeaverGeneratedReaderWriterTests.cs | 11 +++++++ ...gWhenRegisteringExistingExtensionMethod.cs | 32 +++++++++++++++++++ 5 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 Assets/Mirror/Tests/Editor/Weaver/WeaverGeneratedReaderWriterTests~/GivesWarningWhenRegisteringExistingExtensionMethod.cs diff --git a/Assets/Mirror/Editor/Weaver/Readers.cs b/Assets/Mirror/Editor/Weaver/Readers.cs index 91bdc7056..bc3954efb 100644 --- a/Assets/Mirror/Editor/Weaver/Readers.cs +++ b/Assets/Mirror/Editor/Weaver/Readers.cs @@ -17,6 +17,12 @@ public static void Init() internal static void Register(TypeReference dataType, MethodReference methodReference) { + string typeName = dataType.FullName; + if (readFuncs.ContainsKey(typeName)) + { + Weaver.Warning($"Registering a Read method for {typeName} when one already exists", methodReference); + } + readFuncs[dataType.FullName] = methodReference; } diff --git a/Assets/Mirror/Editor/Weaver/Writers.cs b/Assets/Mirror/Editor/Weaver/Writers.cs index 158dc9488..1bc27e754 100644 --- a/Assets/Mirror/Editor/Weaver/Writers.cs +++ b/Assets/Mirror/Editor/Weaver/Writers.cs @@ -17,7 +17,13 @@ public static void Init() public static void Register(TypeReference dataType, MethodReference methodReference) { - writeFuncs[dataType.FullName] = methodReference; + string typeName = dataType.FullName; + if (writeFuncs.ContainsKey(typeName)) + { + Weaver.Warning($"Registering a Write method for {typeName} when one already exists", methodReference); + } + + writeFuncs[typeName] = methodReference; } static void RegisterWriteFunc(TypeReference typeReference, MethodDefinition newWriterFunc) diff --git a/Assets/Mirror/Tests/Editor/Weaver/.WeaverTests.csproj b/Assets/Mirror/Tests/Editor/Weaver/.WeaverTests.csproj index 1eb7f91ce..f86b1a288 100644 --- a/Assets/Mirror/Tests/Editor/Weaver/.WeaverTests.csproj +++ b/Assets/Mirror/Tests/Editor/Weaver/.WeaverTests.csproj @@ -133,6 +133,7 @@ + diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverGeneratedReaderWriterTests.cs b/Assets/Mirror/Tests/Editor/Weaver/WeaverGeneratedReaderWriterTests.cs index 3c67ffda0..01bcd80c4 100644 --- a/Assets/Mirror/Tests/Editor/Weaver/WeaverGeneratedReaderWriterTests.cs +++ b/Assets/Mirror/Tests/Editor/Weaver/WeaverGeneratedReaderWriterTests.cs @@ -203,5 +203,16 @@ public void GivesErrorForInvalidListType() //HasError("Cannot generate reader for List because element MonoBehaviour does not have a reader. Use a supported type or provide a custom reader", // "System.Collections.Generic.List`1"); } + + [Test] + public void GivesWarningWhenRegisteringExistingExtensionMethod() + { + const string typeName = "GeneratedReaderWriter.GivesWarningWhenRegisteringExistingExtensionMethod.MyType"; + HasNoErrors(); + HasWarning($"Registering a Write method for {typeName} when one already exists", + "System.Void GeneratedReaderWriter.GivesWarningWhenRegisteringExistingExtensionMethod.ReadWriteExtension::WriteMyType2(Mirror.NetworkWriter,GeneratedReaderWriter.GivesWarningWhenRegisteringExistingExtensionMethod.MyType)"); + HasWarning($"Registering a Read method for {typeName} when one already exists", + "GeneratedReaderWriter.GivesWarningWhenRegisteringExistingExtensionMethod.MyType GeneratedReaderWriter.GivesWarningWhenRegisteringExistingExtensionMethod.ReadWriteExtension::ReadMyType2(Mirror.NetworkReader)"); + } } } diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverGeneratedReaderWriterTests~/GivesWarningWhenRegisteringExistingExtensionMethod.cs b/Assets/Mirror/Tests/Editor/Weaver/WeaverGeneratedReaderWriterTests~/GivesWarningWhenRegisteringExistingExtensionMethod.cs new file mode 100644 index 000000000..5ea2a4cb6 --- /dev/null +++ b/Assets/Mirror/Tests/Editor/Weaver/WeaverGeneratedReaderWriterTests~/GivesWarningWhenRegisteringExistingExtensionMethod.cs @@ -0,0 +1,32 @@ +using Mirror; + +namespace GeneratedReaderWriter.GivesWarningWhenRegisteringExistingExtensionMethod +{ + public struct MyType + { + public int number; + } + + public static class ReadWriteExtension + { + public static void WriteMyType(this NetworkWriter writer, MyType data) + { + writer.WriteInt32(data.number); + } + + public static void WriteMyType2(this NetworkWriter writer, MyType data) + { + writer.WriteInt32(data.number); + } + + public static MyType ReadMyType(this NetworkReader reader) + { + return new MyType { number = reader.ReadInt32() }; + } + + public static MyType ReadMyType2(this NetworkReader reader) + { + return new MyType { number = reader.ReadInt32() }; + } + } +}