Skip to content

Commit

Permalink
feat: Improve weaver error messages (#1779)
Browse files Browse the repository at this point in the history 
feat: Improve weaver error messages

Weaver error messages are easier to read and always display
location of error
  • Loading branch information
@paulpach
paulpach committed Apr 26, 2020
1 parent d6d702a commit bcd76c5
Show file tree
Hide file tree
Showing 31 changed files with 228 additions and 195 deletions.
4 changes: 2 additions & 2 deletions Assets/Mirror/Editor/Weaver/Processors/CommandProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,13 @@ public static bool ProcessMethodsValidateCommand(MethodDefinition md, CustomAttr
{
if (!md.Name.StartsWith("Cmd"))
{
Weaver.Error($"{md} must start with Cmd. Consider renaming it to Cmd{md.Name}");
Weaver.Error($"{md.Name} must start with Cmd. Consider renaming it to Cmd{md.Name}", md);
return false;
}

if (md.IsStatic)
{
Weaver.Error($"{md} cannot be static");
Weaver.Error($"{md.Name} cannot be static", md);
return false;
}

Expand Down
6 changes: 3 additions & 3 deletions Assets/Mirror/Editor/Weaver/Processors/GenericArgumentResolver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public bool GetGenericFromBaseClass(TypeDefinition td, int genericArgument, Type
TypeReference arg = parent.GenericArguments[genericArgument];
if (arg.IsGenericParameter)
{
itemType = FindParameterInStack(genericArgument);
itemType = FindParameterInStack(td, genericArgument);
}
else
{
Expand All @@ -32,7 +32,7 @@ public bool GetGenericFromBaseClass(TypeDefinition td, int genericArgument, Type
return itemType != null;
}

TypeReference FindParameterInStack(int genericArgument)
TypeReference FindParameterInStack(TypeDefinition td, int genericArgument)
{
while (stack.Count > 0)
{
Expand All @@ -54,7 +54,7 @@ TypeReference FindParameterInStack(int genericArgument)
{
// if greater than `genericArgument` it is hard to know which generic arg we want
// See SyncListGenericInheritanceWithMultipleGeneric test
Weaver.Error($"Too many generic argument for {next}");
Weaver.Error($"Type {td.Name} has too many generic arguments in base class {next}", td);
return null;
}

Expand Down
6 changes: 3 additions & 3 deletions Assets/Mirror/Editor/Weaver/Processors/MessageClassProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ static void GenerateSerialization(TypeDefinition td)
{
if (field.FieldType.FullName == td.FullName)
{
Weaver.Error($"{td} has field ${field} that references itself");
Weaver.Error($"{td.Name} has field {field.Name} that references itself", field);
return;
}
}
Expand Down Expand Up @@ -103,7 +103,7 @@ private static void CallWriter(ILProcessor serWorker, FieldDefinition field)
}
else
{
Weaver.Error($"{field} has unsupported type");
Weaver.Error($"{field.Name} has unsupported type", field);
}
}

Expand Down Expand Up @@ -184,7 +184,7 @@ private static void CallReader(ILProcessor serWorker, FieldDefinition field)
}
else
{
Weaver.Error($"{field} has unsupported type");
Weaver.Error($"{field.Name} has unsupported type", field);
}
}
}
Expand Down
18 changes: 9 additions & 9 deletions Assets/Mirror/Editor/Weaver/Processors/MonoBehaviourProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ static void ProcessSyncVars(TypeDefinition td)
foreach (FieldDefinition fd in td.Fields)
{
if (fd.HasCustomAttribute(Weaver.SyncVarType))
Weaver.Error($"[SyncVar] {fd} must be inside a NetworkBehaviour. {td} is not a NetworkBehaviour");
Weaver.Error($"SyncVar {fd.Name} must be inside a NetworkBehaviour. {td.Name} is not a NetworkBehaviour", fd);

if (SyncObjectInitializer.ImplementsSyncObject(fd.FieldType))
{
Weaver.Error($"{fd} is a SyncObject and must be inside a NetworkBehaviour. {td} is not a NetworkBehaviour");
Weaver.Error($"{fd.Name} is a SyncObject and must be inside a NetworkBehaviour. {td.Name} is not a NetworkBehaviour", fd);
}
}
}
Expand All @@ -35,34 +35,34 @@ static void ProcessMethods(TypeDefinition td)
{
if (ca.AttributeType.FullName == Weaver.CommandType.FullName)
{
Weaver.Error($"[Command] {md} must be declared inside a NetworkBehaviour");
Weaver.Error($"Command {md.Name} must be declared inside a NetworkBehaviour", md);
}

if (ca.AttributeType.FullName == Weaver.ClientRpcType.FullName)
{
Weaver.Error($"[ClientRpc] {md} must be declared inside a NetworkBehaviour");
Weaver.Error($"ClientRpc {md.Name} must be declared inside a NetworkBehaviour", md);
}

if (ca.AttributeType.FullName == Weaver.TargetRpcType.FullName)
{
Weaver.Error($"[TargetRpc] {md} must be declared inside a NetworkBehaviour");
Weaver.Error($"TargetRpc {md.Name} must be declared inside a NetworkBehaviour", md);
}

string attributeName = ca.Constructor.DeclaringType.ToString();

switch (attributeName)
{
case "Mirror.ServerAttribute":
Weaver.Error($"[Server] {md} must be declared inside a NetworkBehaviour");
Weaver.Error($"Server method {md.Name} must be declared inside a NetworkBehaviour", md);
break;
case "Mirror.ServerCallbackAttribute":
Weaver.Error($"[ServerCallback] {md} must be declared inside a NetworkBehaviour");
Weaver.Error($"ServerCallback method {md.Name} must be declared inside a NetworkBehaviour", md);
break;
case "Mirror.ClientAttribute":
Weaver.Error($"[Client] {md} must be declared inside a NetworkBehaviour");
Weaver.Error($"Client method {md.Name} must be declared inside a NetworkBehaviour", md);
break;
case "Mirror.ClientCallbackAttribute":
Weaver.Error($"[ClientCallback] {md} must be declared inside a NetworkBehaviour");
Weaver.Error($"ClientCallback method {md.Name} must be declared inside a NetworkBehaviour", md);
break;
}
}
Expand Down
36 changes: 18 additions & 18 deletions Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public void Process()
{
if (netBehaviourSubclass.HasGenericParameters)
{
Weaver.Error($"{netBehaviourSubclass} cannot have generic parameters");
Weaver.Error($"{netBehaviourSubclass.Name} cannot have generic parameters", netBehaviourSubclass);
return;
}
Weaver.DLog(netBehaviourSubclass, "Process Start");
Expand Down Expand Up @@ -128,7 +128,7 @@ public static bool WriteArguments(ILProcessor worker, MethodDefinition md, bool
MethodReference writeFunc = Writers.GetWriteFunc(pd.ParameterType);
if (writeFunc == null)
{
Weaver.Error($"{md} has invalid parameter {pd}");
Weaver.Error($"{md.Name} has invalid parameter {pd}", md);
return false;
}
// use built-in writer func on writer object
Expand Down Expand Up @@ -184,7 +184,7 @@ void GenerateConstants()
}
else
{
Weaver.Error($"{netBehaviourSubclass} has invalid class constructor");
Weaver.Error($"{netBehaviourSubclass.Name} has invalid class constructor", cctor);
return;
}
}
Expand All @@ -205,7 +205,7 @@ void GenerateConstants()

if (ctor == null)
{
Weaver.Error($"{netBehaviourSubclass} has invalid constructor");
Weaver.Error($"{netBehaviourSubclass.Name} has invalid constructor", netBehaviourSubclass);
return;
}

Expand All @@ -216,7 +216,7 @@ void GenerateConstants()
}
else
{
Weaver.Error($"{netBehaviourSubclass} has invalid constructor");
Weaver.Error($"{netBehaviourSubclass.Name} has invalid constructor", ctor);
return;
}

Expand Down Expand Up @@ -340,7 +340,7 @@ void GenerateSerialization()
}
else
{
Weaver.Error($"{syncVar} has unsupported type. Use a supported Mirror type instead");
Weaver.Error($"{syncVar.Name} has unsupported type. Use a supported Mirror type instead", syncVar);
return;
}
}
Expand Down Expand Up @@ -394,7 +394,7 @@ void GenerateSerialization()
}
else
{
Weaver.Error($"{syncVar} has unsupported type. Use a supported Mirror type instead");
Weaver.Error($"{syncVar.Name} has unsupported type. Use a supported Mirror type instead", syncVar);
return;
}

Expand Down Expand Up @@ -546,7 +546,7 @@ void DeserializeField(FieldDefinition syncVar, ILProcessor serWorker, MethodDefi
MethodReference readFunc = Readers.GetReadFunc(syncVar.FieldType);
if (readFunc == null)
{
Weaver.Error($"{syncVar} has unsupported type. Use a supported Mirror type instead");
Weaver.Error($"{syncVar.Name} has unsupported type. Use a supported Mirror type instead", syncVar);
return;
}

Expand Down Expand Up @@ -735,7 +735,7 @@ public static bool ProcessNetworkReaderParameters(MethodDefinition md, ILProcess
}
else
{
Weaver.Error($"{md} has invalid parameter {arg}. Unsupported type {arg.ParameterType}, use a supported Mirror type instead");
Weaver.Error($"{md.Name} has invalid parameter {arg}. Unsupported type {arg.ParameterType}, use a supported Mirror type instead", md);
return false;
}
}
Expand All @@ -752,17 +752,17 @@ public static bool ProcessMethodsValidateFunction(MethodReference md)
{
if (md.ReturnType.FullName == Weaver.IEnumeratorType.FullName)
{
Weaver.Error($"{md} cannot be a coroutine");
Weaver.Error($"{md.Name} cannot be a coroutine", md);
return false;
}
if (md.ReturnType.FullName != Weaver.voidType.FullName)
{
Weaver.Error($"{md} cannot return a value. Make it void instead");
Weaver.Error($"{md.Name} cannot return a value. Make it void instead", md);
return false;
}
if (md.HasGenericParameters)
{
Weaver.Error($"{md} cannot have generic parameters");
Weaver.Error($"{md.Name} cannot have generic parameters", md);
return false;
}
return true;
Expand All @@ -775,19 +775,19 @@ public static bool ProcessMethodsValidateParameters(MethodReference md, CustomAt
ParameterDefinition p = md.Parameters[i];
if (p.IsOut)
{
Weaver.Error($"{md} cannot have out parameters");
Weaver.Error($"{md.Name} cannot have out parameters", md);
return false;
}
if (p.IsOptional)
{
Weaver.Error($"{md} cannot have optional parameters");
Weaver.Error($"{md.Name} cannot have optional parameters", md);
return false;
}
// TargetRPC is an exception to this rule and can have a NetworkConnection as first parameter
if (p.ParameterType.FullName == Weaver.NetworkConnectionType.FullName &&
!(ca.AttributeType.FullName == Weaver.TargetRpcType.FullName && i == 0))
{
Weaver.Error($"{md} has invalid parameer {p}. Cannot pass NeworkConnections");
Weaver.Error($"{md.Name} has invalid parameer {p}. Cannot pass NeworkConnections", md);
return false;
}
}
Expand Down Expand Up @@ -835,7 +835,7 @@ void ProcessClientRpc(HashSet<string> names, MethodDefinition md, CustomAttribut

if (names.Contains(md.Name))
{
Weaver.Error("Duplicate ClientRpc name [" + netBehaviourSubclass.FullName + ":" + md.Name + "]");
Weaver.Error($"Duplicate ClientRpc name {md.Name}", md);
return;
}
names.Add(md.Name);
Expand All @@ -857,7 +857,7 @@ void ProcessTargetRpc(HashSet<string> names, MethodDefinition md, CustomAttribut

if (names.Contains(md.Name))
{
Weaver.Error("Duplicate Target Rpc name [" + netBehaviourSubclass.FullName + ":" + md.Name + "]");
Weaver.Error($"Duplicate Target Rpc name {md.Name}", md);
return;
}
names.Add(md.Name);
Expand All @@ -879,7 +879,7 @@ void ProcessCommand(HashSet<string> names, MethodDefinition md, CustomAttribute

if (names.Contains(md.Name))
{
Weaver.Error("Duplicate Command name [" + netBehaviourSubclass.FullName + ":" + md.Name + "]");
Weaver.Error($"Duplicate Command name {md.Name}", md);
return;
}

Expand Down
4 changes: 2 additions & 2 deletions Assets/Mirror/Editor/Weaver/Processors/RpcProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,13 @@ public static bool ProcessMethodsValidateRpc(MethodDefinition md, CustomAttribut
{
if (!md.Name.StartsWith("Rpc"))
{
Weaver.Error($"{md} must start with Rpc. Consider renaming it to Rpc{md.Name}");
Weaver.Error($"{md.Name} must start with Rpc. Consider renaming it to Rpc{md.Name}", md);
return false;
}

if (md.IsStatic)
{
Weaver.Error($"{md} must not be static");
Weaver.Error($"{md.Name} must not be static", md);
return false;
}

Expand Down
4 changes: 2 additions & 2 deletions Assets/Mirror/Editor/Weaver/Processors/ServerClientAttributeProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ static void InjectServerGuard(TypeDefinition td, MethodDefinition md, bool logWa
{
if (!Weaver.IsNetworkBehaviour(td))
{
Weaver.Error($"[Server] {md} must be declared in a NetworkBehaviour");
Weaver.Error($"Server method {md.Name} must be declared in a NetworkBehaviour", md);
return;
}
ILProcessor worker = md.Body.GetILProcessor();
Expand All @@ -57,7 +57,7 @@ static void InjectClientGuard(TypeDefinition td, MethodDefinition md, bool logWa
{
if (!Weaver.IsNetworkBehaviour(td))
{
Weaver.Error($"[Client] {md} must be declared in a NetworkBehaviour");
Weaver.Error($"Client method {md.Name} must be declared in a NetworkBehaviour", md);
return;
}
ILProcessor worker = md.Body.GetILProcessor();
Expand Down
4 changes: 2 additions & 2 deletions Assets/Mirror/Editor/Weaver/Processors/SyncDictionaryProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public static void Process(TypeDefinition td)
}
else
{
Weaver.Error($"Could not find generic arguments for {Weaver.SyncDictionaryType} using {td}");
Weaver.Error($"Could not find generic arguments for SyncDictionary in {td.Name}", td);
return;
}

Expand All @@ -29,7 +29,7 @@ public static void Process(TypeDefinition td)
}
else
{
Weaver.Error($"Could not find generic arguments for {Weaver.SyncDictionaryType} using {td}");
Weaver.Error($"Could not find generic arguments for SyncDictionary in {td.Name}", td);
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions Assets/Mirror/Editor/Weaver/Processors/SyncEventProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public static MethodDefinition ProcessEventInvoke(TypeDefinition td, EventDefini
}
if (eventField == null)
{
Weaver.Error($"{td} not found. Did you declare the event?");
Weaver.Error($"event field not found for {ed.Name}. Did you declare it as an event?", ed);
return null;
}

Expand Down Expand Up @@ -118,13 +118,13 @@ public static void ProcessEvents(TypeDefinition td, List<EventDefinition> events
{
if (!ed.Name.StartsWith("Event"))
{
Weaver.Error($"{ed} must start with Event. Consider renaming it to Event{ed.Name}");
Weaver.Error($"{ed.Name} must start with Event. Consider renaming it to Event{ed.Name}", ed);
return;
}

if (ed.EventType.Resolve().HasGenericParameters)
{
Weaver.Error($"{ed} must not have generic parameters. Consider creating a new class that inherits from {ed.EventType} instead");
Weaver.Error($"{ed.Name} must not have generic parameters. Consider creating a new class that inherits from {ed.EventType} instead", ed);
return;
}

Expand Down
2 changes: 1 addition & 1 deletion Assets/Mirror/Editor/Weaver/Processors/SyncListProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public static void Process(TypeDefinition td, TypeReference mirrorBaseType)
}
else
{
Weaver.Error($"Could not find generic arguments for {mirrorBaseType} using {td}");
Weaver.Error($"Could not find generic arguments for {mirrorBaseType.Name} in {td}", td);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion Assets/Mirror/Editor/Weaver/Processors/SyncObjectInitializer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ static void GenerateSyncObjectInstanceInitializer(ILProcessor ctorWorker, FieldD
MethodDefinition ctor = fieldType.Methods.FirstOrDefault(x => x.Name == ".ctor" && !x.HasParameters);
if (ctor == null)
{
Weaver.Error($"{fd} Can not intialize field because no default constructor was found. Manually intialize the field (call the constructor) or add constructor without Parameter");
Weaver.Error($"Can not intialize field {fd.Name} because no default constructor was found. Manually intialize the field (call the constructor) or add constructor without Parameter", fd);
return;
}
MethodReference objectConstructor = Weaver.CurrentAssembly.MainModule.ImportReference(ctor);
Expand Down

0 comments on commit bcd76c5

Please sign in to comment.