Revise code as per feedback. Add additional test cases and revise rul…

…e code.

gendarme/rules/Gendarme.Rules.Performance/AvoidUnnecessaryOverridesRule.cs

@@ -42,25 +42,34 @@ namespace Gendarme.Rules.Performance {
 	/// <example>
 	/// Bad example:
 	/// <code>
-	/// public override string ToString()
+	/// public override string ToString ()
 	/// {
-	///		return base.ToString();
+	///		return base.ToString ();
 	///	}
 	/// </code>
 	/// </example>
 	/// <example>
-	/// Good example:
+	/// Good example (1):
 	/// <code>
 	/// [FileIOPermission (SecurityAction.Demand, @"c:\dir\file")]
-	/// public override string ToString()
+	/// public override string ToString ()
 	/// {
-	///		return base.ToString();
+	///		return base.ToString ();
 	///	}
 	/// </code>
 	/// </example>
+	/// <example>
+	/// Good example (2):
+	/// <code>
+	/// /*public override string ToString ()
+	/// {
+	///		return base.ToString ();
+	///	}*/
+	/// </code>
+	/// </example>
 
 	[Problem ("This override of a base class method is unnecessary.")]
-	[Solution ("Remove the override method.")]
+	[Solution ("Remove the override method or extend the functionality of the method.")]
 	public class AvoidUnnecessaryOverridesRule : Rule, ITypeRule {
 
 		public RuleResult CheckType(TypeDefinition type)
@@ -88,9 +97,8 @@ public RuleResult CheckType(TypeDefinition type)
 				while (i < instrs.Count && (instrs[i].Is (Code.Nop) || instrs[i].IsLoadArgument ()))
 					i++;
 
-				// If the next instruction is not a call we are good. (Relatively certain
-				// calls to the base class cannot be virtual.)
-				if (!instrs[i].Is (Code.Call))
+				// If the next instruction is not a call we are good.
+				if (!instrs[i].Is (Code.Call) && !instrs[i].Is(Code.Call))
 					continue;
 				// Check to make sure the call is to the base class, and the same method name...
 				MethodReference mr = instrs[i].Operand as MethodReference;
@@ -102,6 +110,26 @@ public RuleResult CheckType(TypeDefinition type)
 					continue;
 				if (mr.Name != method.Name)
 					continue;
+				// Account for calling overrides.
+				if (mr.HasParameters != method.HasParameters)
+					continue;
+				// Check the parameter count and type.
+				if (mr.Parameters.Count != method.Parameters.Count)
+					continue;
+				// Shallow check for equality.
+				bool sameParam = true;
+				for (int o = 0; o < mr.Parameters.Count; o++) {
+					var p1 = mr.Parameters[o];
+					var p2 = method.Parameters[o];
+					sameParam = true;
+					sameParam &= p1.HasDefault == p2.HasDefault;
+					sameParam &= p1.HasFieldMarshal == p2.HasFieldMarshal;
+					sameParam &= p1.IsOptional == p2.IsOptional;
+					sameParam &= p1.IsOut == p2.IsOut;
+					sameParam &= p1.ParameterType == p2.ParameterType;
+					if (!sameParam) break;
+				}
+				if (!sameParam) continue;
 
 				i++;
 				// If the return type is void, all we should have is nop and return.

gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUnnecessaryOverridesTest.cs

@@ -35,74 +35,109 @@
 
 using Test.Rules.Definitions;
 using Test.Rules.Fixtures;
+using Test.Rules.Helpers;
 
 namespace Tests.Rules.Performance {
 
 	[TestFixture]
 	public class AvoidUnnecessaryOverridesTest : TypeRuleTestFixture <AvoidUnnecessaryOverridesRule> {
 
 		private class TestBaseClass {
-			public virtual string DoSomething(string s)
+			public virtual string DoSomething (string s)
 			{
 				return s;
 			}
+			public virtual string DoSomething ()
+			{
+				return ":D";
+			}
 		}
 
 		private class TestClassGood : TestBaseClass {
 			[STAThread]
-			public override string DoSomething(string s)
+			public override string DoSomething (string s)
 			{
 				return base.DoSomething (s);
 			}
+			public override string DoSomething ()
+			{
+				return base.DoSomething (":P");
+			}
 			[FileIOPermission (SecurityAction.Demand)]
-			public override string ToString()
+			public override string ToString ()
 			{
 				return base.ToString ();
 			}
-			public override bool Equals(object obj)
+			public override bool Equals (object obj)
 			{
 				if (obj == null)
 					return false;
 				else
 					return base.Equals (obj);
+			}	
+		}
+
+		private class TestClassAlsoGood : ApplicationException {
+			public override bool Equals (object obj)
+			{
+				if (obj.GetType () != typeof (TestClassAlsoGood))
+					return false;
+
+				return base.Equals (obj);
 			}
 		}
 
 		private class TestClassBad : TestBaseClass {
-			public override string ToString()
+			public override string ToString ()
 			{
 				return base.ToString ();
 			}
-			public override string DoSomething(string s)
+			public override string DoSomething (string s)
 			{
 				return base.DoSomething (s);
 			}
+			public override string DoSomething ()
+			{
+				return base.DoSomething ();
+			}
+		}
+
+		private class TestClassAlsoBad : ApplicationException {
+			public override Exception GetBaseException ()
+			{
+				return base.GetBaseException ();
+			}
+		}
+
+		private class MySimpleClass {
 		}
 
 		private Mono.Cecil.TypeDefinition SimpleClassNoMethods;
 
 		[SetUp]
-		public void SetUp()
+		public void SetUp ()
 		{
 			// Classes always have a constuctor so we hack our own methodless class.
-			SimpleClassNoMethods = SimpleTypes.Class;
+			SimpleClassNoMethods = DefinitionLoader.GetTypeDefinition<MySimpleClass> ();
 			SimpleClassNoMethods.Methods.Clear ();
 		}
 
 		[Test]
-		public void Good()
+		public void Good ()
 		{
 			AssertRuleSuccess<TestClassGood> ();
+			AssertRuleSuccess<TestClassAlsoGood> ();
 		}
 
 		[Test]
-		public void Bad()
+		public void Bad ()
 		{
-			AssertRuleFailure<TestClassBad> (2);
+			AssertRuleFailure<TestClassBad> (3);
+			AssertRuleFailure<TestClassAlsoBad> (1);
 		}
 
 		[Test]
-		public void DoesNotApply()
+		public void DoesNotApply ()
 		{
 			AssertRuleDoesNotApply (SimpleClassNoMethods);
 		}