From 2e861c6d65880b5f8b1920e9f2985ae59561b98d Mon Sep 17 00:00:00 2001 From: Taryn Hill Date: Sat, 28 Nov 2015 18:49:43 -0600 Subject: [PATCH 1/3] Implement a utility command to check for explicit interface implementation violations --- OpenRA.Game/Traits/TraitsInterfaces.cs | 4 + OpenRA.Mods.Common/OpenRA.Mods.Common.csproj | 1 + .../CheckExplicitInterfacesCommand.cs | 122 ++++++++++++++++++ 3 files changed, 127 insertions(+) create mode 100644 OpenRA.Mods.Common/UtilityCommands/CheckExplicitInterfacesCommand.cs diff --git a/OpenRA.Game/Traits/TraitsInterfaces.cs b/OpenRA.Game/Traits/TraitsInterfaces.cs index ffa931225a..04e860e609 100644 --- a/OpenRA.Game/Traits/TraitsInterfaces.cs +++ b/OpenRA.Game/Traits/TraitsInterfaces.cs @@ -19,6 +19,8 @@ using OpenRA.Primitives; namespace OpenRA.Traits { + public sealed class RequireExplicitImplementationAttribute : Attribute { } + public enum DamageState { Undamaged, Light, Medium, Heavy, Critical, Dead } public interface IHealth @@ -249,6 +251,8 @@ namespace OpenRA.Traits public interface ILoadsPlayerPalettes { void LoadPlayerPalettes(WorldRenderer wr, string playerName, HSLColor playerColor, bool replaceExisting); } public interface IPaletteModifier { void AdjustPalette(IReadOnlyDictionary b); } public interface IPips { IEnumerable GetPips(Actor self); } + + [RequireExplicitImplementation] public interface ISelectionBar { float GetValue(); Color GetColor(); } public interface IPositionableInfo : ITraitInfoInterface { } diff --git a/OpenRA.Mods.Common/OpenRA.Mods.Common.csproj b/OpenRA.Mods.Common/OpenRA.Mods.Common.csproj index 96efe75e0a..d79458f8e0 100644 --- a/OpenRA.Mods.Common/OpenRA.Mods.Common.csproj +++ b/OpenRA.Mods.Common/OpenRA.Mods.Common.csproj @@ -726,6 +726,7 @@ + diff --git a/OpenRA.Mods.Common/UtilityCommands/CheckExplicitInterfacesCommand.cs b/OpenRA.Mods.Common/UtilityCommands/CheckExplicitInterfacesCommand.cs new file mode 100644 index 0000000000..06af9d8350 --- /dev/null +++ b/OpenRA.Mods.Common/UtilityCommands/CheckExplicitInterfacesCommand.cs @@ -0,0 +1,122 @@ +#region Copyright & License Information +/* + * Copyright 2007-2015 The OpenRA Developers (see AUTHORS) + * This file is part of OpenRA, which is free software. It is made + * available to you under the terms of the GNU General Public License + * as published by the Free Software Foundation. For more information, + * see COPYING. + */ +#endregion + +using System; +using System.Linq; +using System.Reflection; +using OpenRA.Traits; + +namespace OpenRA.Mods.Common.UtilityCommands +{ + public class CheckExplicitInterfacesCommand : IUtilityCommand + { + string IUtilityCommand.Name { get { return "--check-explicit-interfaces"; } } + + bool IUtilityCommand.ValidateArguments(string[] args) + { + return args.Length == 1; + } + + int violationCount; + + [Desc("Check for explicit interface implementation violations in all assemblies referenced by the specified mod.")] + void IUtilityCommand.Run(ModData modData, string[] args) + { + var types = modData.ObjectCreator.GetTypes(); + + foreach (var implementingType in types.Where(t => !t.IsInterface)) + { + if (implementingType.IsEnum) + continue; + + var interfaces = implementingType.GetInterfaces(); + foreach (var interfaceType in interfaces) + { + if (!interfaceType.HasAttribute()) + continue; + + var interfaceMembers = interfaceType.GetMembers(); + foreach (var interfaceMember in interfaceMembers) + { + if (interfaceMember.Name.StartsWith("get_") || interfaceMember.Name.StartsWith("set_")) + continue; + + var interfaceMethod = interfaceMember as MethodInfo; + if (interfaceMethod != null) + { + var interfaceMethodParams = interfaceMethod.GetParameters(); + foreach (var implementingMethod in implementingType.GetMethods()) + { + if (implementingMethod.Name != interfaceMethod.Name + || implementingMethod.ReturnType != interfaceMethod.ReturnType) + continue; + + var implementingMethodParams = implementingMethod.GetParameters(); + var lenImpl = implementingMethodParams.Length; + if (lenImpl != interfaceMethodParams.Length) + continue; + + var allMatch = true; + for (var i = 0; i < lenImpl; i++) + { + var implementingParam = implementingMethodParams[i]; + var interfaceParam = interfaceMethodParams[i]; + if (implementingParam.ParameterType != interfaceParam.ParameterType + || implementingParam.Name != interfaceParam.Name + || implementingParam.IsOut != interfaceParam.IsOut) + { + allMatch = false; + break; + } + } + + // Explicitly implemented methods are never public in C#. + if (allMatch && implementingMethod.IsPublic) + OnViolation(implementingType, interfaceType, implementingMethod); + } + } + + var interfaceProperty = interfaceMember as PropertyInfo; + if (interfaceProperty != null) + { + var implementingProperties = implementingType.GetProperties(); + foreach (var implementingProperty in implementingProperties) + { + if (implementingProperty.PropertyType != interfaceProperty.PropertyType + || implementingProperty.Name != interfaceProperty.Name) + continue; + + if (!IsExplicitInterfaceProperty(implementingProperty)) + OnViolation(implementingType, interfaceType, implementingProperty); + } + } + } + } + } + + if (violationCount > 0) + { + Console.WriteLine("Explicit interface violations: {0}", violationCount); + Environment.Exit(1); + } + } + + static bool IsExplicitInterfaceProperty(PropertyInfo pi) + { + return pi.Name.Contains("."); + } + + void OnViolation(Type implementor, Type interfaceType, MemberInfo violator) + { + Console.WriteLine("{0} must explicitly implement the interface member {1}.{2}", implementor.Name, interfaceType.Name, violator.Name); + violationCount++; + } + } +} From e3229f4cd8ff1193ecf32f28b0adcf31d87f701e Mon Sep 17 00:00:00 2001 From: Taryn Hill Date: Wed, 30 Dec 2015 08:03:39 -0600 Subject: [PATCH 2/3] Add explicit interface checking to 'make check' --- Makefile | 3 +++ make.ps1 | 2 ++ 2 files changed, 5 insertions(+) diff --git a/Makefile b/Makefile index f38b784641..677bc0f335 100644 --- a/Makefile +++ b/Makefile @@ -227,6 +227,9 @@ check: utility mods @echo @echo "Checking for code style violations in OpenRA.Test..." @mono --debug OpenRA.Utility.exe ra --check-code-style OpenRA.Test + @echo + @echo "Checking for explicit interface violations..." + @mono --debug OpenRA.Utility.exe all --check-explicit-interfaces NUNIT_CONSOLE := $(shell test -f thirdparty/download/nunit3-console.exe && echo mono thirdparty/download/nunit3-console.exe || \ which nunit3-console 2>/dev/null || which nunit2-console 2>/dev/null || which nunit-console 2>/dev/null) diff --git a/make.ps1 b/make.ps1 index eccb8b91ee..1df06d5a34 100644 --- a/make.ps1 +++ b/make.ps1 @@ -174,6 +174,8 @@ elseif ($command -eq "check") ./OpenRA.Utility.exe cnc --check-code-style OpenRA.Utility echo "Checking for code style violations in OpenRA.Test..." ./OpenRA.Utility.exe cnc --check-code-style OpenRA.Test + echo "Checking for explicit interface violations..." + ./OpenRA.Utility.exe all --check-explicit-interfaces } else { From 25eddb9567f38a26ab11eb5624cba924c7eea56f Mon Sep 17 00:00:00 2001 From: Taryn Hill Date: Wed, 30 Dec 2015 08:19:50 -0600 Subject: [PATCH 3/3] Explicitly implement ISelectionBar --- OpenRA.Mods.Common/Traits/Buildings/BaseProvider.cs | 5 ++--- OpenRA.Mods.Common/Traits/ExternalCapturableBar.cs | 4 ++-- OpenRA.Mods.Common/Traits/Power/AffectedByPowerOutage.cs | 4 ++-- OpenRA.Mods.Common/Traits/Render/ProductionBar.cs | 4 ++-- OpenRA.Mods.Common/Traits/Render/SupportPowerChargeBar.cs | 4 ++-- OpenRA.Mods.Common/Traits/Render/TimedUpgradeBar.cs | 4 ++-- OpenRA.Mods.D2k/Traits/TemporaryOwnerManager.cs | 4 ++-- OpenRA.Mods.RA/Traits/Chronoshiftable.cs | 4 ++-- OpenRA.Mods.RA/Traits/PortableChrono.cs | 4 ++-- 9 files changed, 18 insertions(+), 19 deletions(-) diff --git a/OpenRA.Mods.Common/Traits/Buildings/BaseProvider.cs b/OpenRA.Mods.Common/Traits/Buildings/BaseProvider.cs index cf491915c2..0facf33c22 100644 --- a/OpenRA.Mods.Common/Traits/Buildings/BaseProvider.cs +++ b/OpenRA.Mods.Common/Traits/Buildings/BaseProvider.cs @@ -78,8 +78,7 @@ namespace OpenRA.Mods.Common.Traits Color.FromArgb(96, Color.Black)); } - // Selection bar - public float GetValue() + float ISelectionBar.GetValue() { // Visible to player and allies if (!ValidRenderPlayer()) @@ -92,6 +91,6 @@ namespace OpenRA.Mods.Common.Traits return (float)progress / total; } - public Color GetColor() { return Color.Purple; } + Color ISelectionBar.GetColor() { return Color.Purple; } } } diff --git a/OpenRA.Mods.Common/Traits/ExternalCapturableBar.cs b/OpenRA.Mods.Common/Traits/ExternalCapturableBar.cs index 2634392f15..57837084c7 100644 --- a/OpenRA.Mods.Common/Traits/ExternalCapturableBar.cs +++ b/OpenRA.Mods.Common/Traits/ExternalCapturableBar.cs @@ -28,7 +28,7 @@ namespace OpenRA.Mods.Common.Traits capturable = self.Trait(); } - public float GetValue() + float ISelectionBar.GetValue() { // only show when building is being captured if (!capturable.CaptureInProgress) @@ -37,6 +37,6 @@ namespace OpenRA.Mods.Common.Traits return (float)capturable.CaptureProgressTime / (capturable.Info.CaptureCompleteTime * 25); } - public Color GetColor() { return Color.Orange; } + Color ISelectionBar.GetColor() { return Color.Orange; } } } diff --git a/OpenRA.Mods.Common/Traits/Power/AffectedByPowerOutage.cs b/OpenRA.Mods.Common/Traits/Power/AffectedByPowerOutage.cs index 8355be4f28..105b2daeda 100644 --- a/OpenRA.Mods.Common/Traits/Power/AffectedByPowerOutage.cs +++ b/OpenRA.Mods.Common/Traits/Power/AffectedByPowerOutage.cs @@ -28,7 +28,7 @@ namespace OpenRA.Mods.Common.Traits playerPower = self.Owner.PlayerActor.Trait(); } - public float GetValue() + float ISelectionBar.GetValue() { if (playerPower.PowerOutageRemainingTicks <= 0) return 0; @@ -36,7 +36,7 @@ namespace OpenRA.Mods.Common.Traits return (float)playerPower.PowerOutageRemainingTicks / playerPower.PowerOutageTotalTicks; } - public Color GetColor() + Color ISelectionBar.GetColor() { return Color.Yellow; } diff --git a/OpenRA.Mods.Common/Traits/Render/ProductionBar.cs b/OpenRA.Mods.Common/Traits/Render/ProductionBar.cs index 7e4ecea686..20492f46ee 100644 --- a/OpenRA.Mods.Common/Traits/Render/ProductionBar.cs +++ b/OpenRA.Mods.Common/Traits/Render/ProductionBar.cs @@ -70,7 +70,7 @@ namespace OpenRA.Mods.Common.Traits value = current != null ? 1 - (float)current.RemainingCost / current.TotalCost : 0; } - public float GetValue() + float ISelectionBar.GetValue() { // only people we like should see our production status. if (!self.Owner.IsAlliedWith(self.World.RenderPlayer)) @@ -79,7 +79,7 @@ namespace OpenRA.Mods.Common.Traits return value; } - public Color GetColor() { return info.Color; } + Color ISelectionBar.GetColor() { return info.Color; } public void OnOwnerChanged(Actor self, Player oldOwner, Player newOwner) { diff --git a/OpenRA.Mods.Common/Traits/Render/SupportPowerChargeBar.cs b/OpenRA.Mods.Common/Traits/Render/SupportPowerChargeBar.cs index 9ef7495053..6320160eb4 100644 --- a/OpenRA.Mods.Common/Traits/Render/SupportPowerChargeBar.cs +++ b/OpenRA.Mods.Common/Traits/Render/SupportPowerChargeBar.cs @@ -33,7 +33,7 @@ namespace OpenRA.Mods.Common.Traits this.info = info; } - public float GetValue() + float ISelectionBar.GetValue() { if (!self.Owner.IsAlliedWith(self.World.RenderPlayer)) return 0; @@ -46,6 +46,6 @@ namespace OpenRA.Mods.Common.Traits return 1 - (float)power.RemainingTime / power.TotalTime; } - public Color GetColor() { return info.Color; } + Color ISelectionBar.GetColor() { return info.Color; } } } diff --git a/OpenRA.Mods.Common/Traits/Render/TimedUpgradeBar.cs b/OpenRA.Mods.Common/Traits/Render/TimedUpgradeBar.cs index 58ae1669a9..1440484ad3 100644 --- a/OpenRA.Mods.Common/Traits/Render/TimedUpgradeBar.cs +++ b/OpenRA.Mods.Common/Traits/Render/TimedUpgradeBar.cs @@ -49,7 +49,7 @@ namespace OpenRA.Mods.Common.Traits value = remaining * 1f / duration; } - public float GetValue() + float ISelectionBar.GetValue() { if (!self.Owner.IsAlliedWith(self.World.RenderPlayer)) return 0; @@ -57,6 +57,6 @@ namespace OpenRA.Mods.Common.Traits return value; } - public Color GetColor() { return info.Color; } + Color ISelectionBar.GetColor() { return info.Color; } } } diff --git a/OpenRA.Mods.D2k/Traits/TemporaryOwnerManager.cs b/OpenRA.Mods.D2k/Traits/TemporaryOwnerManager.cs index 5a22a0f9e2..53fad7e016 100644 --- a/OpenRA.Mods.D2k/Traits/TemporaryOwnerManager.cs +++ b/OpenRA.Mods.D2k/Traits/TemporaryOwnerManager.cs @@ -66,7 +66,7 @@ namespace OpenRA.Mods.D2k.Traits changingOwner = null; // It was triggered by this trait: reset } - public float GetValue() + float ISelectionBar.GetValue() { if (remaining <= 0) return 0; @@ -74,7 +74,7 @@ namespace OpenRA.Mods.D2k.Traits return (float)remaining / duration; } - public Color GetColor() + Color ISelectionBar.GetColor() { return info.BarColor; } diff --git a/OpenRA.Mods.RA/Traits/Chronoshiftable.cs b/OpenRA.Mods.RA/Traits/Chronoshiftable.cs index 613f74812b..4f038e686b 100644 --- a/OpenRA.Mods.RA/Traits/Chronoshiftable.cs +++ b/OpenRA.Mods.RA/Traits/Chronoshiftable.cs @@ -114,7 +114,7 @@ namespace OpenRA.Mods.RA.Traits } // Show the remaining time as a bar - public float GetValue() + float ISelectionBar.GetValue() { if (!info.ReturnToOrigin) return 0f; @@ -126,7 +126,7 @@ namespace OpenRA.Mods.RA.Traits return (float)ReturnTicks / duration; } - public Color GetColor() { return info.TimeBarColor; } + Color ISelectionBar.GetColor() { return info.TimeBarColor; } public void ModifyDeathActorInit(Actor self, TypeDictionary init) { diff --git a/OpenRA.Mods.RA/Traits/PortableChrono.cs b/OpenRA.Mods.RA/Traits/PortableChrono.cs index 67a4fd7161..9e4fb26ead 100644 --- a/OpenRA.Mods.RA/Traits/PortableChrono.cs +++ b/OpenRA.Mods.RA/Traits/PortableChrono.cs @@ -105,12 +105,12 @@ namespace OpenRA.Mods.RA.Traits get { return chargeTick <= 0; } } - public float GetValue() + float ISelectionBar.GetValue() { return (float)(Info.ChargeDelay - chargeTick) / Info.ChargeDelay; } - public Color GetColor() { return Color.Magenta; } + Color ISelectionBar.GetColor() { return Color.Magenta; } } class PortableChronoOrderTargeter : IOrderTargeter