diff options
author | Néstor Salceda <nestor@mono-cvs.ximian.com> | 2008-02-15 21:14:10 +0300 |
---|---|---|
committer | Néstor Salceda <nestor@mono-cvs.ximian.com> | 2008-02-15 21:14:10 +0300 |
commit | 79bd04b6a96081018274baecf463892a7edf1d84 (patch) | |
tree | 19f1c9e69a450ba41fee2676be921e59c77ba65f /gendarme/rules | |
parent | 7779e3a21b8cd21fdde161be600b7133a41ef53f (diff) |
2008-02-15 Nestor Salceda <nestor.salceda@gmail.com>
* Test/*.cs: Refactor the tests for accept the new rule's format.
* *.cs: Adapt the rules to the new framework.
* Makefile.am: Removed the -langversion:linq option. You should use at
least the 1.9 gmcs version to compile.
svn path=/trunk/mono-tools/; revision=95777
Diffstat (limited to 'gendarme/rules')
28 files changed, 389 insertions, 490 deletions
diff --git a/gendarme/rules/Gendarme.Rules.Performance/AvoidReturningArraysOnPropertiesRule.cs b/gendarme/rules/Gendarme.Rules.Performance/AvoidReturningArraysOnPropertiesRule.cs index 401bda30..881df310 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/AvoidReturningArraysOnPropertiesRule.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/AvoidReturningArraysOnPropertiesRule.cs @@ -27,25 +27,23 @@ using System; using Mono.Cecil; - using Gendarme.Framework; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Performance { - public class AvoidReturningArraysOnPropertiesRule : IMethodRule { - - public MessageCollection CheckMethod (MethodDefinition method, Runner runner) + + [Problem ("By convention properties should not return Arrays.")] + [Solution ("Replace the property by a method.")] + public class AvoidReturningArraysOnPropertiesRule : Rule, IMethodRule { + + public RuleResult CheckMethod (MethodDefinition method) { - if (!method.IsGetter) - return runner.RuleSuccess; - - if (!method.ReturnType.ReturnType.IsArray ()) - return runner.RuleSuccess; + if (!method.IsGetter || !method.ReturnType.ReturnType.IsArray ()) + return RuleResult.Success; - Location loc = new Location (method); - Message msg = new Message ("Avoid returning arrays from properties.", loc, MessageType.Warning); - return new MessageCollection (msg); + Runner.Report (method, Severity.Medium, Confidence.Total, "Avoid returning arrays from properties"); + return RuleResult.Failure; } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/AvoidToStringOnStringsRule.cs b/gendarme/rules/Gendarme.Rules.Performance/AvoidToStringOnStringsRule.cs index 0c885705..12c36c79 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/AvoidToStringOnStringsRule.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/AvoidToStringOnStringsRule.cs @@ -37,17 +37,19 @@ using Gendarme.Framework; namespace Gendarme.Rules.Performance { - public class AvoidToStringOnStringsRule : IMethodRule { + [Problem ("You are calling ToString () in a string in the member '{0}', this is redundant and may produce some performance penalities.")] + [Solution ("You should remove the ToString () call.")] + public class AvoidToStringOnStringsRule : Rule, IMethodRule { private const string MessageString = "No need to call ToString on a System.String instance"; - public MessageCollection CheckMethod (MethodDefinition method, Runner runner) + public RuleResult CheckMethod (MethodDefinition method) { + bool containsAvoidToStringOnStrings = false; // rule apply only if the method has a body (e.g. p/invokes, icalls don't) if (!method.HasBody) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; - MessageCollection messageCollection = null; foreach (Instruction instruction in method.Body.Instructions) { switch (instruction.OpCode.Code) { case Code.Call: @@ -55,19 +57,15 @@ namespace Gendarme.Rules.Performance { case Code.Callvirt: if (IsToString (instruction.Operand as MethodReference)) { if (CheckStack (instruction.Previous, method)) { - if (messageCollection == null) - messageCollection = new MessageCollection (); - - Location location = new Location (method, instruction.Offset); - Message message = new Message (MessageString, location, MessageType.Error); - messageCollection.Add (message); + Runner.Report (method, instruction, Severity.Medium, Confidence.Normal, MessageString); + containsAvoidToStringOnStrings = true; } } break; } } - return messageCollection; + return containsAvoidToStringOnStrings? RuleResult.Failure : RuleResult.Success; } private static bool IsToString (MethodReference method) diff --git a/gendarme/rules/Gendarme.Rules.Performance/AvoidUncalledPrivateCodeRule.cs b/gendarme/rules/Gendarme.Rules.Performance/AvoidUncalledPrivateCodeRule.cs index f13aa219..d5093601 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/AvoidUncalledPrivateCodeRule.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/AvoidUncalledPrivateCodeRule.cs @@ -34,8 +34,10 @@ using Gendarme.Framework; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Performance { - - public class AvoidUncalledPrivateCodeRule: IMethodRule { + + [Problem ("A private or internal (assembly-level) member '{0}' does not have callers in the assembly, is not invoked by the common language runtime, and the member is not invoked by a delegate.")] + [Solution ("Remove the non-callable code or add the code that calls it.")] + public class AvoidUncalledPrivateCodeRule: Rule,IMethodRule { // we should move these methods into an helper class (e.g. MethodHelper) // and reuse them in other rules @@ -68,23 +70,14 @@ namespace Gendarme.Rules.Performance { // - public MessageCollection CheckMethod (MethodDefinition method, Runner runner) + public RuleResult CheckMethod (MethodDefinition method) { // #1 - rule doesn't apply to static ctor - if (method.IsStatic && method.IsConstructor) - return runner.RuleSuccess; - // #2 - rule doesn't apply if the method is the assembly entry point - if (method.IsEntryPoint ()) - return runner.RuleSuccess; - // #3 - rule doesn't apply to Main - if (method.IsMain ()) - return runner.RuleSuccess; - // #4 - rule doesn't apply if the method is generated by the compiler or by a tool - if (method.IsGeneratedCode ()) - return runner.RuleSuccess; + if ((method.IsStatic && method.IsConstructor) || method.IsEntryPoint () || method.IsMain () || method.IsGeneratedCode ()) + return RuleResult.DoesNotApply; // ok, the rule applies @@ -93,17 +86,16 @@ namespace Gendarme.Rules.Performance { // it's ok for have unused private ctor (and common before static class were introduced in 2.0) // this also covers private serialization constructors if (method.IsConstructor) - return runner.RuleSuccess; + return RuleResult.Success; // it's ok (used or not) if it's required to implement explicitely an interface if (IsExplicitImplementationOfInterface (method)) - return runner.RuleSuccess; + return RuleResult.Success; // then we must check if this type use the private method if (!CheckTypeForMethodUsage ((method.DeclaringType as TypeDefinition), method)) { - Location location = new Location (method); - Message message = new Message ("The private method code is not used in it's declaring type.", location, MessageType.Error); - return new MessageCollection (message); + Runner.Report (method, Severity.High, Confidence.Normal, "The private method code is not used in it's declaring type"); + return RuleResult.Failure; } } @@ -111,18 +103,17 @@ namespace Gendarme.Rules.Performance { if (method.IsAssembly) { // internal ctor for serialization are ok if (IsSerializationConstructor (method)) - return runner.RuleSuccess; + return RuleResult.Success; // then we must check if something in the assembly is using this method if (!CheckAssemblyForMethodUsage (method.DeclaringType.Module.Assembly, method)) { - Location location = new Location (method); - Message message = new Message ("The internal method code is not used in it's declaring assembly.", location, MessageType.Error); - return new MessageCollection (message); + Runner.Report (method, Severity.High, Confidence.Normal, "The internal method code is not used in it's declaring assembly"); + return RuleResult.Failure; } } // then method is accessible - return runner.RuleSuccess; + return RuleResult.Success; } private static bool CheckAssemblyForMethodUsage (AssemblyDefinition ad, MethodDefinition md) diff --git a/gendarme/rules/Gendarme.Rules.Performance/AvoidUninstantiatedInternalClassesRule.cs b/gendarme/rules/Gendarme.Rules.Performance/AvoidUninstantiatedInternalClassesRule.cs index 842c6197..657c1755 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/AvoidUninstantiatedInternalClassesRule.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/AvoidUninstantiatedInternalClassesRule.cs @@ -37,7 +37,9 @@ using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Performance { - public class AvoidUninstantiatedInternalClassesRule : ITypeRule { + [Problem ("The internal type is not instantiated by code within the assembly.")] + [Solution ("Remove the type or add the code that uses it. If the type contains only static methods then either add the static modifier to the type or add the private construtor to the type to prevent the compiler from emitting a default public instance constructor.")] + public class AvoidUninstantiatedInternalClassesRule : Rule, ITypeRule { private static bool CheckSpecialTypes (TypeDefinition type) { @@ -141,27 +143,19 @@ namespace Gendarme.Rules.Performance { } } - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // rule apply to internal (non-visible) types // note: IsAbstract also excludes static types (2.0) - if (type.IsVisible () || type.IsAbstract) - return runner.RuleSuccess; - - // some types are created by compilers and should be ignored - if (CheckSpecialTypes (type)) - return runner.RuleSuccess; - - // rule doesn't apply if the assembly open up itself to others using [InternalsVisibleTo] - if (type.Module.Assembly.HasAttribute ("System.Runtime.CompilerServices.InternalsVisibleToAttribute")) - return runner.RuleSuccess; + if (type.IsVisible () || type.IsAbstract || CheckSpecialTypes (type) || type.Module.Assembly.HasAttribute ("System.Runtime.CompilerServices.InternalsVisibleToAttribute")) + return RuleResult.DoesNotApply; // rule applies // if the type holds the Main entry point then it is considered useful MethodDefinition entry_point = type.Module.Assembly.EntryPoint; if ((entry_point != null) && (entry_point.DeclaringType == type)) - return runner.RuleSuccess; + return RuleResult.Success; // create a cache of all type instantiation inside this AssemblyDefinition assembly = type.Module.Assembly; @@ -173,12 +167,10 @@ namespace Gendarme.Rules.Performance { // if we can't find the non-public type being used in the assembly then the rule fails if (list == null || !list.Contains (type)) { - Location location = new Location (type); - Message message = new Message ("There is no call for any of the types constructor found", location, MessageType.Error); - return new MessageCollection (message); + Runner.Report (type, Severity.High, Confidence.Normal, "There is no call for any of the types constructor found"); + return RuleResult.Failure; } - - return runner.RuleSuccess; + return RuleResult.Success; } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/AvoidUnsealedConcreteAttributesRule.cs b/gendarme/rules/Gendarme.Rules.Performance/AvoidUnsealedConcreteAttributesRule.cs index 77eed715..393af8be 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/AvoidUnsealedConcreteAttributesRule.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/AvoidUnsealedConcreteAttributesRule.cs @@ -32,24 +32,25 @@ using Gendarme.Framework; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Performance { + + [Problem ("Due performance issues, concrete attributes should be sealed.")] + [Solution ("Unless you plan to inherit from this attribute you should consider to seal it's type.")] + public class AvoidUnsealedConcreteAttributesRule : Rule, ITypeRule { - public class AvoidUnsealedConcreteAttributesRule : ITypeRule { - - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // rule applies only to attributes if (!type.IsAttribute ()) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; if (type.IsAbstract) // it's ok - return runner.RuleSuccess; + return RuleResult.Success; if (type.IsSealed) // it's ok - return runner.RuleSuccess; + return RuleResult.Success; - Location loc = new Location (type); - Message msg = new Message ("Due to performance issues, attributes should all be declared sealed.", loc, MessageType.Error); - return new MessageCollection (msg); + Runner.Report (type, Severity.Medium, Confidence.High, "Due to performance issues, attributes should all be declared sealed"); + return RuleResult.Failure; } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/AvoidUnsealedUninheritedInternalClassesRule.cs b/gendarme/rules/Gendarme.Rules.Performance/AvoidUnsealedUninheritedInternalClassesRule.cs index 0c5166d5..b7143609 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/AvoidUnsealedUninheritedInternalClassesRule.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/AvoidUnsealedUninheritedInternalClassesRule.cs @@ -33,26 +33,24 @@ using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Performance { - public class AvoidUnsealedUninheritedInternalClassesRule : ITypeRule { + [Problem ("Due to performance issues, types which are not visible outside of the assembly and which have no inherited types within the assembly should be sealed.")] + [Solution ("You should seal this type, unless you plan to inherit from this type in the near-future.")] + public class AvoidUnsealedUninheritedInternalClassesRule : Rule, ITypeRule { - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { if (type.IsAbstract || type.IsSealed || type.IsVisible () || type.IsGeneratedCode ()) - return runner.RuleSuccess; + return RuleResult.Success; foreach (TypeDefinition type_definition in type.Module.Types) { // skip ourself if (type_definition.FullName == type.FullName) continue; if (type_definition.Inherits (type.FullName)) - return runner.RuleSuccess; + return RuleResult.Success; } - - Message msg = new Message ( - "Due to performance issues, types which are not visible outside of the " + - "assembly and which have no inherited types within the assembly should be sealed.", - new Location (type), MessageType.Error); - return new MessageCollection (msg); + Runner.Report (type, Severity.High, Confidence.High, "Types which are not visible outside the assembly and without inherited types within the assembly should be sealed"); + return RuleResult.Failure; } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/AvoidUnusedParametersRule.cs b/gendarme/rules/Gendarme.Rules.Performance/AvoidUnusedParametersRule.cs index 8237e698..b86d0bda 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/AvoidUnusedParametersRule.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/AvoidUnusedParametersRule.cs @@ -37,7 +37,9 @@ using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Performance { - public class AvoidUnusedParametersRule : IMethodRule { + [Problem ("The method contains unused parameters.")] + [Solution ("You should remove or use the unused parameters.")] + public class AvoidUnusedParametersRule : Rule, IMethodRule { private static bool UseParameter (MethodDefinition method, ParameterDefinition parameter) { @@ -112,32 +114,21 @@ namespace Gendarme.Rules.Performance { return unusedParameters; } - public MessageCollection CheckMethod (MethodDefinition method, Runner runner) + public RuleResult CheckMethod (MethodDefinition method) { // catch abstract, pinvoke and icalls - where rule does not apply - if (!method.HasBody) - return runner.RuleSuccess; - // rule doesn't apply to virtual, overrides or generated code - if (method.IsVirtual || method.Overrides.Count != 0 || method.IsGeneratedCode ()) - return runner.RuleSuccess; - // doesn't apply to code referenced by delegates (note: more complex check moved last) - if (IsReferencedByDelegate (method)) - return runner.RuleSuccess; + if (!method.HasBody || method.IsVirtual || method.Overrides.Count != 0 || + method.IsGeneratedCode () || IsReferencedByDelegate (method)) + return RuleResult.DoesNotApply; // rule applies - - MessageCollection mc = null; + bool containsUnusedParameters = false; foreach (ParameterDefinition parameter in GetUnusedParameters (method)) { - Location location = new Location (method); - Message message = new Message (String.Format ("The parameter {0} is never used.", parameter.Name), location, MessageType.Error); - if (mc == null) - mc = new MessageCollection (message); - else - mc.Add (message); + Runner.Report (parameter, Severity.Medium, Confidence.Normal, String.Format ("The parameter {0} is never used", parameter.Name)); } - return mc; + return Runner.CurrentRuleResult; } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/ChangeLog b/gendarme/rules/Gendarme.Rules.Performance/ChangeLog index b87aae6c..dfb760f7 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/ChangeLog +++ b/gendarme/rules/Gendarme.Rules.Performance/ChangeLog @@ -1,3 +1,10 @@ +2008-02-15 Nestor Salceda <nestor.salceda@gmail.com> + + * Test/*.cs: Refactor the tests for accept the new rule's format. + * *.cs: Adapt the rules to the new framework. + * Makefile.am: Removed the -langversion:linq option. You should use at + least the 1.9 gmcs version to compile. + 2008-02-07 Nestor Salceda <nestor.salceda@gmail.com> * DisposableTypesShouldHaveFinalizerRule.cs, diff --git a/gendarme/rules/Gendarme.Rules.Performance/DontIgnoreMethodResultRule.cs b/gendarme/rules/Gendarme.Rules.Performance/DontIgnoreMethodResultRule.cs index cddff6d7..e567e01f 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/DontIgnoreMethodResultRule.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/DontIgnoreMethodResultRule.cs @@ -38,31 +38,25 @@ using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Performance { - public class DontIgnoreMethodResultRule : IMethodRule { + [Problem ("The member ignores the result from the call.")] + [Solution ("You shouldn't ignore this result.")] + public class DontIgnoreMethodResultRule : Rule,IMethodRule { - public MessageCollection CheckMethod (MethodDefinition method, Runner runner) + public RuleResult CheckMethod (MethodDefinition method) { // rule only applies if the method has a body - if (!method.HasBody) - return runner.RuleSuccess; - // rule doesn't not apply to generated code (out of developer's control) - if (method.IsGeneratedCode ()) - return runner.RuleSuccess; + if (!method.HasBody || method.IsGeneratedCode ()) + return RuleResult.DoesNotApply; - MessageCollection mc = null; foreach (Instruction instruction in method.Body.Instructions) { if (instruction.OpCode.Code == Code.Pop) { - Message message = CheckForViolation (method, instruction.Previous); - if (message != null) { - if (mc == null) - mc = new MessageCollection (message); - else - mc.Add (message); - } + Defect defect = CheckForViolation (method, instruction.Previous); + if (defect != null) + Runner.Report (defect); } } - return mc; + return Runner.CurrentRuleResult; } // some method return stuff that isn't required in most cases @@ -111,14 +105,13 @@ namespace Gendarme.Rules.Performance { } } - private static Message CheckForViolation (MethodDefinition method, Instruction instruction) + private Defect CheckForViolation (MethodDefinition method, Instruction instruction) { if ((instruction.OpCode.Code == Code.Newobj || instruction.OpCode.Code == Code.Newarr)) { MemberReference member = (instruction.Operand as MemberReference); if ((member != null) && !IsNewException (member)) { string s = String.Format ("Unused object of type {0} created", member.ToString ()); - Location loc = new Location (method, instruction.Offset); - return new Message (s, null, MessageType.Warning); + return new Defect (this, method.DeclaringType, method, instruction, Severity.Medium, Confidence.Normal, s); } } @@ -127,11 +120,8 @@ namespace Gendarme.Rules.Performance { if (callee != null && !callee.ReturnType.ReturnType.IsValueType) { // check for some common exceptions (to reduce false positive) if (!IsCallException (callee)) { - Location loc = new Location (method, instruction.Offset); - // most common case is something like: s.ToLower (); - MessageType messageType = (method.DeclaringType.FullName == "System.String") ? MessageType.Error : MessageType.Warning; string s = String.Format ("Do not ignore method results from call to '{0}'.", callee.ToString ()); - return new Message (s, loc, messageType); + return new Defect (this, method.DeclaringType, method, instruction, Severity.Medium, Confidence.Normal, s); } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/EmptyDestructorRule.cs b/gendarme/rules/Gendarme.Rules.Performance/EmptyDestructorRule.cs index 02234876..1325363f 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/EmptyDestructorRule.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/EmptyDestructorRule.cs @@ -35,15 +35,17 @@ using Gendarme.Framework; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Performance { + + [Problem ("The type has an empty destructor (or Finalize method).")] + [Solution ("Remove the empty destructor (or Finalize method) from the class.")] + public class EmptyDestructorRule : Rule, ITypeRule { - public class EmptyDestructorRule : ITypeRule { - - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // rule applies only to type with a finalizer MethodDefinition finalizer = type.GetMethod (MethodSignatures.Finalize); if (finalizer == null) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; // rule applies @@ -57,7 +59,7 @@ namespace Gendarme.Rules.Performance { // it's empty if we're calling the base class destructor MethodReference mr = (ins.Operand as MethodReference); if ((mr == null) || !mr.IsFinalizer ()) - return runner.RuleSuccess; + return RuleResult.Success; break; case Code.Nop: case Code.Leave: @@ -69,14 +71,13 @@ namespace Gendarme.Rules.Performance { break; default: // destructor isn't empty (normal) - return runner.RuleSuccess; + return RuleResult.Success; } } // finalizer is empty (bad / useless) - Location loc = new Location (finalizer); - Message msg = new Message ("The type finalizer is empty and should be removed.", loc, MessageType.Warning); - return new MessageCollection (msg); + Runner.Report (type, Severity.Medium, Confidence.Normal, "The type finalizer is empty and should be removed."); + return RuleResult.Failure; } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/IDisposableWithDestructorWithoutSuppressFinalizeRule.cs b/gendarme/rules/Gendarme.Rules.Performance/IDisposableWithDestructorWithoutSuppressFinalizeRule.cs index 7fbd3677..0e52dd31 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/IDisposableWithDestructorWithoutSuppressFinalizeRule.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/IDisposableWithDestructorWithoutSuppressFinalizeRule.cs @@ -34,8 +34,10 @@ using Gendarme.Framework; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Performance { - - public class IDisposableWithDestructorWithoutSuppressFinalizeRule : ITypeRule { + + [Problem ("The type has a destructor and implements IDisposable. However it doesn't call System.GC.SuppressFinalize inside it's Dispose method.")] + [Solution ("Add a call to GC.SuppressFinalize inside your Dispose method.")] + public class IDisposableWithDestructorWithoutSuppressFinalizeRule : Rule, ITypeRule { private static bool MethodMatchNameVoidEmpty (MethodDefinition md, string methodName) { @@ -46,11 +48,11 @@ namespace Gendarme.Rules.Performance { return (md.ReturnType.ReturnType.ToString () == "System.Void"); } - private MessageCollection Recurse (MethodDefinition method, int level, Runner runner) + private RuleResult Recurse (MethodDefinition method, int level) { // some methods have no body (e.g. p/invokes, icalls) if (!method.HasBody) - return runner.RuleFailure; + return RuleResult.Failure; foreach (Instruction ins in method.Body.Instructions) { switch (ins.OpCode.Code) { @@ -59,25 +61,25 @@ namespace Gendarme.Rules.Performance { case Code.Callvirt: // are we calling GC.SuppressFinalize ? if (ins.Operand.ToString () == "System.Void System.GC::SuppressFinalize(System.Object)") - return runner.RuleSuccess; + return RuleResult.Success; else if (level < 3) { MethodDefinition callee = (ins.Operand as MethodDefinition); if (callee != null) { - if (Recurse (callee, level + 1, runner) == null) - return runner.RuleSuccess; + if (Recurse (callee, level + 1) == RuleResult.Success) + return RuleResult.Success; } } break; } } - return runner.RuleFailure; + return RuleResult.Failure; } - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // #1 - does the type implements System.IDisposable ? if (!type.Implements ("System.IDisposable")) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; // #2 - look for the Dispose method MethodDefinition dispose = null; @@ -90,15 +92,15 @@ namespace Gendarme.Rules.Performance { } } if (dispose == null) - return runner.RuleSuccess; + return RuleResult.Success; // #3 - look for a destructor if (type.GetMethod (MethodSignatures.Finalize) == null) - return runner.RuleSuccess; + return RuleResult.Success; // #4 - look if GC.SuppressFinalize is being called in the // Dispose method - or one of the method it calls - return Recurse (dispose, 0, runner); + return Recurse (dispose, 0); } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/Makefile.am b/gendarme/rules/Gendarme.Rules.Performance/Makefile.am index aec3d074..98c08c64 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/Makefile.am +++ b/gendarme/rules/Gendarme.Rules.Performance/Makefile.am @@ -10,24 +10,42 @@ DISTCLEANFILES = Makefile.in Gendarme.Rules.Performance.xml TestResult.xml performance_rules_sources_in = ../../AssemblyInfo.cs.in performance_rules_generated_sources = $(performance_rules_sources_in:.in=) -performance_rules_sources = EmptyDestructorRule.cs IDisposableWithDestructorWithoutSuppressFinalizeRule.cs \ - UseStringEmptyRule.cs AvoidUnusedParametersRule.cs AvoidUninstantiatedInternalClassesRule.cs \ - AvoidUncalledPrivateCodeRule.cs AvoidToStringOnStringsRule.cs DontIgnoreMethodResultRule.cs \ - UsingStringLengthInsteadOfCheckingEmptyStringRule.cs AvoidReturningArraysOnPropertiesRule.cs \ - AvoidUnsealedConcreteAttributesRule.cs UseIsOperatorRule.cs AvoidUnsealedUninheritedInternalClassesRule.cs +performance_rules_sources = \ + EmptyDestructorRule.cs \ + IDisposableWithDestructorWithoutSuppressFinalizeRule.cs \ + UseStringEmptyRule.cs \ + AvoidUnusedParametersRule.cs \ + AvoidUninstantiatedInternalClassesRule.cs \ + AvoidUncalledPrivateCodeRule.cs \ + AvoidToStringOnStringsRule.cs \ + DontIgnoreMethodResultRule.cs \ + UsingStringLengthInsteadOfCheckingEmptyStringRule.cs \ + AvoidReturningArraysOnPropertiesRule.cs \ + AvoidUnsealedConcreteAttributesRule.cs \ + UseIsOperatorRule.cs \ + AvoidUnsealedUninheritedInternalClassesRule.cs performance_rules_build_sources = $(addprefix $(srcdir)/, $(performance_rules_sources)) performance_rules_build_sources += $(performance_rules_generated_sources) ../../bin/Gendarme.Rules.Performance.dll: $(performance_rules_build_sources) - $(GMCS) -debug -target:library -langversion:linq -r:$(top_builddir)/gendarme/bin/Mono.Cecil.dll -r:../../bin/Gendarme.Framework.dll -out:$@ $(performance_rules_build_sources) + $(GMCS) -debug -target:library -r:$(top_builddir)/gendarme/bin/Mono.Cecil.dll -r:../../bin/Gendarme.Framework.dll -out:$@ $(performance_rules_build_sources) cp Gendarme.Rules.*.xml ../../bin/ -performance_test_sources = EmptyDestructorTest.cs IDisposableWithDestructorWithoutSuppressFinalizeTest.cs \ - UseStringEmptyTest.cs AvoidUnusedParametersTest.cs UsingStringLengthInsteadOfCheckingEmptyStringTest.cs \ - AvoidUncalledPrivateCodeTest.cs AvoidToStringOnStringsTest.cs AvoidUninstantiatedInternalClassesTest.cs \ - DontIgnoreMethodResultTest.cs AvoidReturningArraysOnPropertiesTest.cs AvoidUnsealedConcreteAttributesTest.cs \ - UseIsOperatorTest.cs AvoidUnsealedUninheritedInternalClassesTest.cs +performance_test_sources = \ + EmptyDestructorTest.cs \ + IDisposableWithDestructorWithoutSuppressFinalizeTest.cs \ + UseStringEmptyTest.cs \ + AvoidUnusedParametersTest.cs \ + UsingStringLengthInsteadOfCheckingEmptyStringTest.cs \ + AvoidUncalledPrivateCodeTest.cs \ + AvoidToStringOnStringsTest.cs \ + AvoidUninstantiatedInternalClassesTest.cs \ + DontIgnoreMethodResultTest.cs \ + AvoidReturningArraysOnPropertiesTest.cs \ + AvoidUnsealedConcreteAttributesTest.cs \ + UseIsOperatorTest.cs \ + AvoidUnsealedUninheritedInternalClassesTest.cs performance_test_build_sources = $(addprefix $(srcdir)/Test/, $(performance_test_sources)) diff --git a/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidReturningArraysOnPropertiesTest.cs b/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidReturningArraysOnPropertiesTest.cs index 9b7d98e2..20a758af 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidReturningArraysOnPropertiesTest.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidReturningArraysOnPropertiesTest.cs @@ -61,6 +61,7 @@ namespace Test.Rules.Performance { private IMethodRule rule; private AssemblyDefinition assembly; private ModuleDefinition module; + private TestRunner runner; [TestFixtureSetUp] public void FixtureSetUp () @@ -69,6 +70,7 @@ namespace Test.Rules.Performance { assembly = AssemblyFactory.GetAssembly (unit); module = assembly.MainModule; rule = new AvoidReturningArraysOnPropertiesRule (); + runner = new TestRunner (rule); } private TypeDefinition GetTest (string name) @@ -82,7 +84,7 @@ namespace Test.Rules.Performance { { TypeDefinition type = GetTest ("ShouldBeCaught"); foreach (MethodDefinition md in type.Methods) - Assert.IsNotNull (rule.CheckMethod (md, new MinimalRunner ())); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (md)); } [Test] @@ -90,7 +92,7 @@ namespace Test.Rules.Performance { { TypeDefinition type = GetTest ("ShouldBeIgnored"); foreach (MethodDefinition md in type.Methods) - Assert.IsNull (rule.CheckMethod (md, new MinimalRunner ())); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (md)); } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidToStringOnStringsTest.cs b/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidToStringOnStringsTest.cs index 42ccc06c..466729d8 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidToStringOnStringsTest.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidToStringOnStringsTest.cs @@ -96,6 +96,7 @@ namespace Test.Rules.Performance private AssemblyDefinition assembly; private TypeDefinition type; private ModuleDefinition module; + private TestRunner runner; [TestFixtureSetUp] public void FixtureSetUp() @@ -105,6 +106,7 @@ namespace Test.Rules.Performance module = assembly.MainModule; type = module.Types["Test.Rules.Performance.AvoidToStringOnStringsTest/Item"]; rule = new AvoidToStringOnStringsRule(); + runner = new TestRunner (rule); } MethodDefinition GetTest(string name) @@ -116,51 +118,46 @@ namespace Test.Rules.Performance return null; } - MessageCollection CheckMethod(MethodDefinition method) - { - return rule.CheckMethod(method, new MinimalRunner()); - } - [Test] public void TestLocalString() { MethodDefinition method = GetTest("ToStringOnLocalString"); - Assert.IsNotNull(CheckMethod(method)); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod(method)); } [Test] public void TestParameter() { MethodDefinition method = GetTest("ToStringOnParameter"); - Assert.IsNotNull(CheckMethod(method)); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod(method)); } [Test] public void TestStaticField() { MethodDefinition method = GetTest("ToStringOnStaticField"); - Assert.IsNotNull(CheckMethod(method)); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod(method)); } [Test] public void TestField() { MethodDefinition method = GetTest("ToStringOnField"); - Assert.IsNotNull(CheckMethod(method)); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod(method)); } [Test] public void TestMethodResult() { MethodDefinition method = GetTest("ToStringOnMethodResult"); - Assert.IsNotNull(CheckMethod(method)); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod(method)); } [Test] public void TestValidToString() { MethodDefinition method = GetTest("ValidToString"); - Assert.IsNull(CheckMethod(method)); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod(method)); } diff --git a/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUncalledPrivateCodeTest.cs b/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUncalledPrivateCodeTest.cs index 041a53f3..8baabd3e 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUncalledPrivateCodeTest.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUncalledPrivateCodeTest.cs @@ -334,7 +334,7 @@ namespace Test.Rules.Performance { private IMethodRule methodRule; private AssemblyDefinition assembly; private TypeDefinition type; - MessageCollection messageCollection; + private TestRunner runner; [TestFixtureSetUp] public void FixtureSetUp () @@ -342,12 +342,7 @@ namespace Test.Rules.Performance { string unit = Assembly.GetExecutingAssembly ().Location; assembly = AssemblyFactory.GetAssembly (unit); methodRule = new AvoidUncalledPrivateCodeRule (); - } - - [SetUp] - public void SetUp () - { - messageCollection = null; + runner = new TestRunner (methodRule); } private TypeDefinition GetTest (string name) @@ -357,71 +352,64 @@ namespace Test.Rules.Performance { } [Test] - public void uncalledPrivateMethodTest () + public void UncalledPrivateMethodTest () { type = GetTest ("UncalledPrivateMethod"); Assert.AreEqual (1, type.Methods.Count, "Methods.Count"); - messageCollection = methodRule.CheckMethod (type.Methods [0], new MinimalRunner ()); - Assert.IsNotNull (messageCollection, "UncalledPrivateMethod"); - Assert.AreEqual (1, messageCollection.Count, "Count"); + Assert.AreEqual (RuleResult.Failure , runner.CheckMethod (type.Methods [0])); + Assert.AreEqual (1, runner.Defects.Count, "Count"); } [Test] - public void calledPrivateMethodTest () + public void CalledPrivateMethodTest () { type = GetTest ("CalledPrivateMethod"); foreach (MethodDefinition md in type.Methods) { switch (md.Name) { case "Main": // rule does not apply to Main - messageCollection = methodRule.CheckMethod (md, new MinimalRunner ()); - Assert.IsNull (messageCollection, "Main"); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckMethod (md)); break; case "display": - messageCollection = methodRule.CheckMethod (md, new MinimalRunner ()); - Assert.IsNull (messageCollection, "display"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (md)); break; } } } [Test] - public void uncalledInternalMethodTest () + public void UncalledInternalMethodTest () { type = GetTest ("UncalledInternalMethod"); Assert.AreEqual (1, type.Methods.Count, "Methods.Count"); - messageCollection = methodRule.CheckMethod (type.Methods [0], new MinimalRunner ()); - Assert.IsNotNull (messageCollection, "UncalledInternalMethod"); - Assert.AreEqual (1, messageCollection.Count, "Count"); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (type.Methods [0])); + Assert.AreEqual (1, runner.Defects.Count, "Count"); } [Test] - public void calledInternalMethodTest () + public void CalledInternalMethodTest () { type = GetTest ("CalledInternalMethod"); Assert.AreEqual (1, type.Methods.Count, "Methods.Count"); - messageCollection = methodRule.CheckMethod (type.Methods [0], new MinimalRunner ()); - Assert.IsNull (messageCollection, "CalledInternalMethod"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (type.Methods [0])); } [Test] - public void checkingForMainMethodTest () - { + public void CheckingForMainMethodTest () { type = GetTest ("CalledInternalMethod"); foreach (MethodDefinition method in type.Methods) if (method.Name == "Main") - messageCollection = methodRule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckMethod (method)); } [Test] - public void publicMethodNotCalledInPrivateClassTest () + public void PublicMethodNotCalledInPrivateClassTest () { type = GetTest ("PublicMethodNotCalledInPrivateClass"); foreach (MethodDefinition method in type.Methods) { switch (method.Name) { case "publicMethod": - Assert.IsNull (methodRule.CheckMethod (method, new MinimalRunner ()), method.Name); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), method.Name); break; default: Assert.Fail ("Test case for method {0} is not handled", method.Name); @@ -431,44 +419,41 @@ namespace Test.Rules.Performance { } [Test] - public void publicMethodCalledInPrivateClassTest () + public void PublicMethodCalledInPrivateClassTest () { type = GetTest ("PublicMethodCalledInPrivateClass"); foreach (MethodDefinition method in type.Methods) if (method.Name == "publicCalledMethod") - messageCollection = methodRule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success ,runner.CheckMethod (method)); } [Test] - public void publicMethodCalledInInternalClassTest () + public void PublicMethodCalledInInternalClassTest () { type = GetTest ("PublicMethodCalledInInternalClass"); foreach (MethodDefinition method in type.Methods) if (method.Name == "publicMethodCalled") - messageCollection = methodRule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } [Test] - public void privateMethodInPrivateClassNotCalledTest () + public void PrivateMethodInPrivateClassNotCalledTest () { type = GetTest ("PrivateMethodInPrivateClassNotCalled"); foreach (MethodDefinition method in type.Methods) if (method.Name == "privateMethodNotCalled") - messageCollection = methodRule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNotNull (messageCollection); - Assert.AreEqual (1, messageCollection.Count); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method)); + Assert.AreEqual (1, runner.Defects.Count); } [Test] - public void publicMethodNotCalledInNestedInternalClassTest () + public void PublicMethodNotCalledInNestedInternalClassTest () { type = GetTest ("NestedClasses"); foreach (MethodDefinition method in type.Methods) { switch (method.Name) { case "publicMethodNotCalledInNestedInternalClass": - Assert.IsNull (methodRule.CheckMethod (method, new MinimalRunner ()), method.Name); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), method.Name); break; default: Assert.Fail ("Test case for method {0} is not handled", method.Name); @@ -478,7 +463,7 @@ namespace Test.Rules.Performance { } [Test] - public void implementingInterfacesMembersTest () + public void ImplementingInterfacesMembersTest () { type = GetTest ("ImplementingExplicitInterfacesMembers"); foreach (MethodDefinition method in type.Methods) { @@ -487,12 +472,12 @@ namespace Test.Rules.Performance { case "Test.Rules.Performance.AvoidUncalledPrivateCodeTest.Iface1.IfaceMethod1": // mono bug #343465 case "Test.Rules.Performance.AvoidUncalledPrivateCodeTest+Iface1.IfaceMethod1": - Assert.IsNull (methodRule.CheckMethod (method, new MinimalRunner ()), method.Name); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckMethod (method), method.Name); break; case "Test.Rules.Performance.AvoidUncalledPrivateCodeTest.Iface2.IfaceMethod2": // mono bug #343465 case "Test.Rules.Performance.AvoidUncalledPrivateCodeTest+Iface2.IfaceMethod2": - Assert.IsNotNull (methodRule.CheckMethod (method, new MinimalRunner ()), method.Name); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method), method.Name); break; default: Assert.Fail ("Test case for method {0} is not handled", method.Name); @@ -502,50 +487,48 @@ namespace Test.Rules.Performance { } [Test] - public void privateConstructorNotCalledTest () + public void PrivateConstructorNotCalledTest () { type = GetTest ("PrivateConstructorNotCalled"); foreach (MethodDefinition method in type.Constructors) { - Assert.IsNull (methodRule.CheckMethod (method, new MinimalRunner ()), method.Name); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), method.Name); } } [Test] - public void staticConstructorNotCalledTest () + public void StaticConstructorNotCalledTest () { type = GetTest ("StaticConstructorNotCalled"); foreach (MethodDefinition method in type.Constructors) if (method.Name == ".cctor") - messageCollection = methodRule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckMethod (method)); } [Test] public void SerializationConstructors () { - Runner runner = new MinimalRunner (); type = GetTest ("PublicSerializableConstructorNotCalled"); foreach (MethodDefinition ctor in type.Constructors) - Assert.IsNull (methodRule.CheckMethod (ctor, runner), ctor.ToString ()); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (ctor), ctor.ToString ()); type = GetTest ("PrivateSerializableConstructorNotCalled"); foreach (MethodDefinition ctor in type.Constructors) - Assert.IsNull (methodRule.CheckMethod (ctor, runner), ctor.ToString ()); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (ctor), ctor.ToString ()); type = GetTest ("ProtectedSerializableConstructorNotCalled"); foreach (MethodDefinition ctor in type.Constructors) - Assert.IsNull (methodRule.CheckMethod (ctor, runner), ctor.ToString ()); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (ctor), ctor.ToString ()); type = GetTest ("InternalSerializableConstructorNotCalled"); foreach (MethodDefinition ctor in type.Constructors) - Assert.IsNull (methodRule.CheckMethod (ctor, runner), ctor.ToString ()); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (ctor), ctor.ToString ()); } [Test] - public void uncalledOverriddenMethodTest () + public void UncalledOverriddenMethodTest () { type = GetTest ("UncalledOverriddenMethod"); foreach (MethodDefinition method in type.Methods) { switch (method.Name) { case "ToString": - Assert.IsNull (methodRule.CheckMethod (method, new MinimalRunner ()), method.Name); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), method.Name); break; default: Assert.Fail ("Test case not handled"); @@ -555,40 +538,37 @@ namespace Test.Rules.Performance { } [Test] - public void implementingComRegisterFunctionAttributeTest () + public void ImplementingComRegisterFunctionAttributeTest () { type = GetTest ("UsingComRegisterAndUnRegisterFunctionAttribute"); foreach (MethodDefinition method in type.Constructors) if (method.Name == "register") - messageCollection = methodRule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } [Test] - public void implementingComUnregisterFunctionAttributeTest () + public void ImplementingComUnregisterFunctionAttributeTest () { type = GetTest ("UsingComRegisterAndUnRegisterFunctionAttribute"); foreach (MethodDefinition method in type.Constructors) if (method.Name == "unregister") - messageCollection = methodRule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } [Test] - public void callingPrivateMethodsThroughDelegatesTest () + public void CallingPrivateMethodsThroughDelegatesTest () { type = GetTest ("CallingPrivateMethodsThroughDelegates"); foreach (MethodDefinition method in type.Constructors) if (method.Name == "privateMethod") - messageCollection = methodRule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } [Test] public void CheckClassWithFinalizer () { type = GetTest ("ClassWithFinalizer"); - Assert.IsNull (methodRule.CheckMethod (type.Methods [0], new MinimalRunner ())); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (type.Methods [0])); } [Test] @@ -596,7 +576,7 @@ namespace Test.Rules.Performance { { type = GetTest ("MyList"); foreach (MethodDefinition method in type.Methods) { - Assert.IsNull (methodRule.CheckMethod (method, new MinimalRunner ()), method.Name); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), method.Name); } } @@ -616,7 +596,7 @@ namespace Test.Rules.Performance { // this isn't part of the test (but included with CSC) break; default: - Assert.IsNull (methodRule.CheckMethod (method, new MinimalRunner ())); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); break; } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUninstantiatedInternalClassesTest.cs b/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUninstantiatedInternalClassesTest.cs index 107f38f8..8449938d 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUninstantiatedInternalClassesTest.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUninstantiatedInternalClassesTest.cs @@ -200,7 +200,7 @@ namespace Test.Rules.Performance { private ITypeRule typeRule; private AssemblyDefinition assembly; private TypeDefinition type; - MessageCollection messageCollection; + private TestRunner runner; [TestFixtureSetUp] public void FixtureSetUp () @@ -208,7 +208,7 @@ namespace Test.Rules.Performance { string unit = Assembly.GetExecutingAssembly ().Location; assembly = AssemblyFactory.GetAssembly (unit); typeRule = new AvoidUninstantiatedInternalClassesRule (); - messageCollection = null; + runner = new TestRunner (typeRule); } private TypeDefinition GetTest (string name) @@ -218,124 +218,110 @@ namespace Test.Rules.Performance { } [Test] - public void uninstantiatedInternalClassTest () + public void UninstantiatedInternalClassTest () { type = GetTest ("UninstantiatedInternalClass"); - messageCollection = typeRule.CheckType (type, new MinimalRunner ()); - Assert.IsNotNull (messageCollection); - Assert.AreEqual (1, messageCollection.Count); + Assert.AreEqual (RuleResult.Failure, runner.CheckType (type)); + Assert.AreEqual (1, runner.Defects.Count); } [Test] - public void instantiatedInternalClassTest () + public void InstantiatedInternalClassTest () { type = GetTest ("InstantiatedInternalClass"); - messageCollection = typeRule.CheckType (type, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckType (type)); } [Test] - public void nestedInternalUninstantiatedClassTest () + public void NestedInternalUninstantiatedClassTest () { type = GetTest ("NestedInternalUninstantiatedClass"); - messageCollection = typeRule.CheckType (type, new MinimalRunner ()); - Assert.IsNotNull (messageCollection); - Assert.AreEqual (1, messageCollection.Count); + Assert.AreEqual (RuleResult.Failure, runner.CheckType (type)); + Assert.AreEqual (1, runner.Defects.Count); } [Test] - public void nestedInternalInstantiatedClassTest () + public void NestedInternalInstantiatedClassTest () { type = GetTest ("NestedInternalInstantiatedClass"); - messageCollection = typeRule.CheckType (type, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckType (type)); } [Test] - public void nonInternalClassNotInstantiatedTest () + public void NonInternalClassNotInstantiatedTest () { type = GetTest ("NonInternalClassNotInstantiated"); - messageCollection = typeRule.CheckType (type, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckType (type)); } [Test] - public void staticClassTest () + public void StaticClassTest () { type = GetTest ("StaticClass"); - messageCollection = typeRule.CheckType (type, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckType (type)); } [Test] - public void methodContainingObjectCallIsNotCalledTest () + public void MethodContainingObjectCallIsNotCalledTest () { type = GetTest ("MethodContainingObjectCallIsNotCalled"); - Assert.IsNull (typeRule.CheckType (type, new MinimalRunner ())); + Assert.AreEqual (RuleResult.Success, runner.CheckType (type)); } [Test] - public void iFaceTest () + public void IFaceTest () { type = GetTest ("IFace"); - messageCollection = typeRule.CheckType (type, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckType (type)); } [Test] public void NestedEnumReturnInstantiated () { type = GetTest ("NestedEnumReturnInstantiated/PrivateEnum"); - messageCollection = typeRule.CheckType (type, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckType (type)); } [Test] public void NestedEnumUsingInstantiated () { type = GetTest ("NestedEnumUsingInstantiated/PrivateEnum"); - messageCollection = typeRule.CheckType (type, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckType (type)); } [Test] public void NestedEnumExternInstantiated () { type = GetTest ("NestedEnumExternInstantiated/PrivateEnum"); - messageCollection = typeRule.CheckType (type, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckType (type)); } [Test] public void NestedEnumExternOutInstantiated () { type = GetTest ("NestedEnumExternOutInstantiated/PrivateEnum"); - messageCollection = typeRule.CheckType (type, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckType (type)); } [Test] public void NestedEnumNotInstantiated () { type = GetTest ("NestedEnumNotInstantiated/PrivateEnum"); - messageCollection = typeRule.CheckType (type, new MinimalRunner ()); - Assert.IsNotNull (messageCollection); + Assert.AreEqual (RuleResult.Failure, runner.CheckType (type)); } [Test] public void NestedEnumUsedAsParameter () { type = GetTest ("NestedEnumUsedAsParameter/PrivateEnum"); - messageCollection = typeRule.CheckType (type, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckType (type)); } [Test] public void StructExistAsArrayOnly () { type = GetTest ("ClassWithArray/ExistAsArrayOnly"); - messageCollection = typeRule.CheckType (type, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckType (type)); } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUnsealedConcreteAttributesTest.cs b/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUnsealedConcreteAttributesTest.cs index b7d3f9f6..61d9e6de 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUnsealedConcreteAttributesTest.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUnsealedConcreteAttributesTest.cs @@ -55,7 +55,7 @@ namespace Test.Rules.Performance { private ITypeRule rule; private AssemblyDefinition assembly; - private Runner runner; + private TestRunner runner; [TestFixtureSetUp] @@ -64,7 +64,7 @@ namespace Test.Rules.Performance { string unit = System.Reflection.Assembly.GetExecutingAssembly ().Location; assembly = AssemblyFactory.GetAssembly (unit); rule = new AvoidUnsealedConcreteAttributesRule (); - runner = new MinimalRunner (); + runner = new TestRunner (rule); } private TypeDefinition GetTest<T> () @@ -75,37 +75,32 @@ namespace Test.Rules.Performance { [Test] public void TestAbstractAttribute () { - MessageCollection messages = rule.CheckType (GetTest<AbstractAttribute> (), runner); - Assert.IsNull (messages); + Assert.AreEqual (RuleResult.Success, runner.CheckType (GetTest<AbstractAttribute> ())); } [Test] public void TestAnAttribute () { - MessageCollection messages = rule.CheckType (GetTest<AnAttribute> (), runner); - Assert.IsNotNull (messages); - Assert.AreEqual (1, messages.Count); + Assert.AreEqual (RuleResult.Failure, runner.CheckType (GetTest<AnAttribute> ())); + Assert.AreEqual (1, runner.Defects.Count); } [Test] public void TestNotAttribute () { - MessageCollection messages = rule.CheckType (GetTest<NotAttribute> (), runner); - Assert.IsNull (messages); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckType (GetTest<NotAttribute> ())); } [Test] public void TestSealedAttribute () { - MessageCollection messages = rule.CheckType (GetTest<SealedAttribute> (), runner); - Assert.IsNull (messages); + Assert.AreEqual (RuleResult.Success, runner.CheckType (GetTest<SealedAttribute> ())); } [Test] public void TestSealedAttributeInheritsAnAttribute () { - MessageCollection messages = rule.CheckType (GetTest<SealedAttributeInheritsAnAttribute> (), runner); - Assert.IsNull (messages); + Assert.AreEqual (RuleResult.Success, runner.CheckType (GetTest<SealedAttributeInheritsAnAttribute> ())); } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUnsealedUninheritedInternalClassesTest.cs b/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUnsealedUninheritedInternalClassesTest.cs index a71a270c..12748e0f 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUnsealedUninheritedInternalClassesTest.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUnsealedUninheritedInternalClassesTest.cs @@ -63,7 +63,7 @@ namespace Test.Rules.Performance { private ITypeRule rule; private AssemblyDefinition assembly; - private Runner runner; + private TestRunner runner; [TestFixtureSetUp] @@ -72,7 +72,7 @@ namespace Test.Rules.Performance { string unit = System.Reflection.Assembly.GetExecutingAssembly ().Location; assembly = AssemblyFactory.GetAssembly (unit); rule = new AvoidUnsealedUninheritedInternalClassesRule (); - runner = new MinimalRunner (); + runner = new TestRunner (rule); } private TypeDefinition GetTest<T> () @@ -91,53 +91,46 @@ namespace Test.Rules.Performance { [Test] public void TestVisable () { - MessageCollection messages = rule.CheckType (GetTest<Visible> (), runner); - Assert.IsNull (messages); + Assert.AreEqual (RuleResult.Success, runner.CheckType (GetTest<Visible> ())); } [Test] public void TestUnsealedInner () { - MessageCollection messages = rule.CheckType (GetTest ("Outer/UnsealedInner"), runner); - Assert.IsNotNull (messages); - Assert.AreEqual (1, messages.Count); + Assert.AreEqual (RuleResult.Failure, runner.CheckType (GetTest ("Outer/UnsealedInner"))); + Assert.AreEqual (1, runner.Defects.Count); } [Test] public void TestSealedInner () { - MessageCollection messages = rule.CheckType (GetTest ("Outer/SealedInner"), runner); - Assert.IsNull (messages); + Assert.AreEqual (RuleResult.Success, runner.CheckType (GetTest ("Outer/SealedInner"))); } [Test] public void TestAbstract () { - MessageCollection messages = rule.CheckType (GetTest<Abstract> (), runner); - Assert.IsNull (messages); + Assert.AreEqual (RuleResult.Success, runner.CheckType (GetTest<Abstract> ())); } [Test] public void TestConcrete () { - MessageCollection messages = rule.CheckType (GetTest<Concrete> (), runner); - Assert.IsNotNull (messages); - Assert.AreEqual (1, messages.Count); + Assert.AreEqual (RuleResult.Failure, runner.CheckType (GetTest<Concrete> ())); + Assert.AreEqual (1, runner.Defects.Count); } [Test] public void TestSealed () { - MessageCollection messages = rule.CheckType (GetTest<Sealed> (), runner); - Assert.IsNull (messages); + Assert.AreEqual (RuleResult.Success, runner.CheckType (GetTest<Sealed> ())); } [Test] public void TestUnsealed () { - MessageCollection messages = rule.CheckType (GetTest<Unsealed> (), runner); - Assert.IsNotNull (messages); - Assert.AreEqual (1, messages.Count); + Assert.AreEqual (RuleResult.Failure, runner.CheckType (GetTest<Unsealed> ())); + Assert.AreEqual (1, runner.Defects.Count); } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUnusedParametersTest.cs b/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUnusedParametersTest.cs index 749a6c07..5abb733d 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUnusedParametersTest.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/Test/AvoidUnusedParametersTest.cs @@ -60,7 +60,7 @@ namespace Test.Rules.Performance { private IMethodRule rule; private AssemblyDefinition assembly; private MethodDefinition method; - private MessageCollection messageCollection; + private TestRunner runner; public void PrintBannerUsingParameter (Version version) { @@ -181,73 +181,65 @@ namespace Test.Rules.Performance { string unit = Assembly.GetExecutingAssembly ().Location; assembly = AssemblyFactory.GetAssembly (unit); rule = new AvoidUnusedParametersRule (); - messageCollection = null; + runner = new TestRunner (rule); } [Test] public void PrintBannerUsingParameterTest () { method = GetMethodForTest ("PrintBannerUsingParameter"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } [Test] public void PrintBannerUsingAssemblyTest () { method = GetMethodForTest ("PrintBannerUsingAssembly"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNotNull (messageCollection); - Assert.AreEqual (1, messageCollection.Count); + Assert.AreEqual (RuleResult.Failure ,runner.CheckMethod (method)); + Assert.AreEqual (1, runner.Defects.Count); } - + [Test] public void PrintBannerWithoutParametersTest () { method = GetMethodForTest ("PrintBannerWithoutParameters"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } [Test] public void MethodWithUnusedParametersTest () { method = GetMethodForTest ("MethodWithUnusedParameters"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNotNull (messageCollection); - Assert.AreEqual (2, messageCollection.Count); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method)); + Assert.AreEqual (2, runner.Defects.Count); } [Test] public void AbstractMethodTest () { method = GetMethodForTestFrom ("Test.Rules.Performance.AbstractClass", "AbstractMethod"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckMethod (method)); } [Test] public void VirtualMethodTest () { method = GetMethodForTestFrom ("Test.Rules.Performance.VirtualClass", "VirtualMethod"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckMethod (method)); } [Test] public void OverrideMethodTest () { method = GetMethodForTestFrom ("Test.Rules.Performance.OverrideClass", "VirtualMethod"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckMethod (method)); } [Test] public void ExternMethodTest () { method = GetMethodForTest ("cos"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckMethod (method)); } [Test] @@ -255,8 +247,7 @@ namespace Test.Rules.Performance { { SimpleCallback callback = new SimpleCallback (SimpleCallbackImpl); method = GetMethodForTest ("SimpleCallbackImpl"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckMethod (method)); } [Test] @@ -275,7 +266,7 @@ namespace Test.Rules.Performance { } } Assert.IsNotNull (method, "method not found!"); - Assert.IsNull (rule.CheckMethod (method, new MinimalRunner ()), "rule result"); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckMethod (method)); } [Test] @@ -283,89 +274,84 @@ namespace Test.Rules.Performance { { SimpleEvent += new SimpleEventHandler (OnSimpleEvent); method = GetMethodForTest ("OnSimpleEvent"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckMethod (method)); } [Test] public void MethodWith5UsedParametersTest () { method = GetMethodForTest ("MethodWith5UsedParameters"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } [Test] public void StaticMethodWithUnusedParametersTest () { method = GetMethodForTest ("StaticMethodWithUnusedParameters"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNotNull (messageCollection); - Assert.AreEqual (2, messageCollection.Count); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method)); + Assert.AreEqual (2, runner.Defects.Count); } [Test] public void StaticMethodWithUsedParametersTest () { method = GetMethodForTest ("StaticMethodWithUsedParameters"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } [Test] public void StaticMethodWith5UsedParametersTest () { method = GetMethodForTest ("StaticMethodWith5UsedParameters"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNull (messageCollection); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } [Test] public void StaticMethodWith5UnusedParametersTest () { method = GetMethodForTest ("StaticMethodWith5UnusedParameters"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNotNull (messageCollection); - Assert.AreEqual (5, messageCollection.Count); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method)); + Assert.AreEqual (5, runner.Defects.Count); } [Test] public void EmptyMethodTest () { method = GetMethodForTest ("EmptyMethod"); - messageCollection = rule.CheckMethod (method, new MinimalRunner ()); - Assert.IsNotNull (messageCollection); - Assert.AreEqual (1, messageCollection.Count); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method)); + Assert.AreEqual (1, runner.Defects.Count); } [Test] public void OperatorsClass () { method = GetMethodForTest ("op_Equality"); - Assert.IsNull (rule.CheckMethod (method, new MinimalRunner ()), "op_Equality"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), "op_Equality"); method = GetMethodForTest ("op_Inequality"); - Assert.IsNull (rule.CheckMethod (method, new MinimalRunner ()), "op_Inequality"); + Assert.AreEqual (RuleResult.Success, rule.CheckMethod (method), "op_Inequality"); } [Test] public void OperatorsStructureOk () { method = GetMethodForTestFrom ("Test.Rules.Performance.AvoidUnusedParametersTest/StructureOk", "op_Equality"); - Assert.IsNull (rule.CheckMethod (method, new MinimalRunner ()), "op_Equality"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), "op_Equality"); method = GetMethodForTestFrom ("Test.Rules.Performance.AvoidUnusedParametersTest/StructureOk", "op_Inequality"); - Assert.IsNull (rule.CheckMethod (method, new MinimalRunner ()), "op_Inequality"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), "op_Inequality"); } [Test] public void OperatorsStructureBad () { method = GetMethodForTestFrom ("Test.Rules.Performance.AvoidUnusedParametersTest/StructureBad", "op_Equality"); - Assert.AreEqual (2, rule.CheckMethod (method, new MinimalRunner ()).Count, "op_Equality"); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method), "op_Equality"); + Assert.AreEqual (2, runner.Defects.Count); method = GetMethodForTestFrom ("Test.Rules.Performance.AvoidUnusedParametersTest/StructureBad", "op_Inequality"); - Assert.AreEqual (2, rule.CheckMethod (method, new MinimalRunner ()).Count, "op_Inequality"); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method), "op_Inequality"); + Assert.AreEqual (2, runner.Defects.Count); } [Test] @@ -375,12 +361,12 @@ namespace Test.Rules.Performance { AssemblyDefinition ad = AssemblyFactory.GetAssembly (cecil); method = GetMethodFromAssembly (ad, "Mono.Cecil.Cil.OpCode", "op_Equality"); - Assert.IsNull (rule.CheckMethod (method, new MinimalRunner ()), "op_Equality"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), "op_Equality"); method = GetMethodFromAssembly (ad, "Mono.Cecil.Cil.OpCode", "op_Inequality"); - Assert.IsNull (rule.CheckMethod (method, new MinimalRunner ()), "op_Inequality"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), "op_Inequality"); } - + private MethodDefinition GetMethodForTest (string methodName) { return GetMethodFromAssembly (assembly, "Test.Rules.Performance.AvoidUnusedParametersTest", methodName); diff --git a/gendarme/rules/Gendarme.Rules.Performance/Test/DontIgnoreMethodResultTest.cs b/gendarme/rules/Gendarme.Rules.Performance/Test/DontIgnoreMethodResultTest.cs index a2ff1c00..de6f5b01 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/Test/DontIgnoreMethodResultTest.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/Test/DontIgnoreMethodResultTest.cs @@ -125,6 +125,7 @@ namespace Test.Rules.Performance { private AssemblyDefinition assembly; private TypeDefinition type; private ModuleDefinition module; + private TestRunner runner; [TestFixtureSetUp] public void FixtureSetUp() @@ -134,6 +135,7 @@ namespace Test.Rules.Performance { module = assembly.MainModule; type = module.Types["Test.Rules.Performance.DontIgnoreMethodResultTest/Item"]; rule = new DontIgnoreMethodResultRule(); + runner = new TestRunner (rule); } MethodDefinition GetTest(string name) @@ -145,60 +147,55 @@ namespace Test.Rules.Performance { return null; } - MessageCollection CheckMethod(MethodDefinition method) - { - return rule.CheckMethod(method, new MinimalRunner()); - } - [Test] public void TestStringMethods() { MethodDefinition method = GetTest("Violations"); - Assert.IsNotNull(CheckMethod(method)); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod(method)); } [Test] public void TestConstructor() { MethodDefinition method = GetTest("CreateItem"); - Assert.IsNotNull(CheckMethod(method)); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod(method)); } [Test] public void TestStringBuilder () { MethodDefinition method = GetTest ("StringBuilderOk"); - Assert.IsNull (CheckMethod (method), "Ok"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), "Ok"); method = GetTest ("StringBuilderBad"); - Assert.IsNotNull (CheckMethod (method), "Bad"); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method), "Bad"); } [Test] public void TestDirectory () { MethodDefinition method = GetTest ("DirectoryOk"); - Assert.IsNull (CheckMethod (method), "Ok"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), "Ok"); method = GetTest ("DirectoryBad"); - Assert.IsNotNull (CheckMethod (method), "Bad"); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method), "Bad"); } [Test] public void TestPermissionSet () { MethodDefinition method = GetTest ("PermissionSetOk"); - Assert.IsNull (CheckMethod (method), "Ok"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), "Ok"); method = GetTest ("PermissionSetBad"); - Assert.IsNotNull (CheckMethod (method), "Bad"); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method), "Bad"); } [Test] public void TestTimer () { MethodDefinition method = GetTest ("TimerOk"); - Assert.IsNull (CheckMethod (method), "Ok"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), "Ok"); } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/Test/EmptyDestructorTest.cs b/gendarme/rules/Gendarme.Rules.Performance/Test/EmptyDestructorTest.cs index 509c2ebd..0aa66c11 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/Test/EmptyDestructorTest.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/Test/EmptyDestructorTest.cs @@ -71,6 +71,7 @@ namespace Test.Rules.Performance { private ITypeRule rule; private AssemblyDefinition assembly; private ModuleDefinition module; + private TestRunner runner; [TestFixtureSetUp] public void FixtureSetUp () @@ -79,6 +80,7 @@ namespace Test.Rules.Performance { assembly = AssemblyFactory.GetAssembly (unit); module = assembly.MainModule; rule = new EmptyDestructorRule (); + runner = new TestRunner (rule); } private TypeDefinition GetTest (string name) @@ -91,21 +93,21 @@ namespace Test.Rules.Performance { public void NoDestructor () { TypeDefinition type = GetTest ("NoDestructorClass"); - Assert.IsNull (rule.CheckType (type, new MinimalRunner ())); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckType (type)); } [Test] public void EmptyDestructor () { TypeDefinition type = GetTest ("EmptyDestructorClass"); - Assert.IsNotNull (rule.CheckType (type, new MinimalRunner ())); + Assert.AreEqual (RuleResult.Failure, runner.CheckType (type)); } [Test] public void Destructor () { TypeDefinition type = GetTest ("DestructorClass"); - Assert.IsNull (rule.CheckType (type, new MinimalRunner ())); + Assert.AreEqual (RuleResult.Success, runner.CheckType (type)); } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/Test/IDisposableWithDestructorWithoutSuppressFinalizeTest.cs b/gendarme/rules/Gendarme.Rules.Performance/Test/IDisposableWithDestructorWithoutSuppressFinalizeTest.cs index 93f187ea..58e9be06 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/Test/IDisposableWithDestructorWithoutSuppressFinalizeTest.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/Test/IDisposableWithDestructorWithoutSuppressFinalizeTest.cs @@ -125,6 +125,7 @@ namespace Test.Rules.Performance { private ITypeRule rule; private AssemblyDefinition assembly; private ModuleDefinition module; + private TestRunner runner; [TestFixtureSetUp] public void FixtureSetUp () @@ -133,6 +134,7 @@ namespace Test.Rules.Performance { assembly = AssemblyFactory.GetAssembly (unit); module = assembly.MainModule; rule = new IDisposableWithDestructorWithoutSuppressFinalizeRule (); + runner = new TestRunner (rule); } private TypeDefinition GetTest (string name) @@ -145,56 +147,56 @@ namespace Test.Rules.Performance { public void NoDestructor () { TypeDefinition type = GetTest ("NoDestructorClass"); - Assert.IsNull (rule.CheckType (type, new MinimalRunner ())); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckType (type)); } [Test] public void Destructor () { TypeDefinition type = GetTest ("DestructorClass"); - Assert.IsNull (rule.CheckType (type, new MinimalRunner ())); + Assert.AreEqual (RuleResult.DoesNotApply, runner.CheckType (type)); } [Test] public void IDisposableNoDestructorWithoutSuppressFinalize () { TypeDefinition type = GetTest ("IDisposableNoDestructorWithoutSuppressFinalizeClass"); - Assert.IsNull (rule.CheckType (type, new MinimalRunner ())); + Assert.AreEqual (RuleResult.Success, runner.CheckType (type)); } [Test] public void IDisposableNoDestructorWithSuppressFinalize () { TypeDefinition type = GetTest ("IDisposableNoDestructorWithSuppressFinalizeClass"); - Assert.IsNull (rule.CheckType (type, new MinimalRunner ())); + Assert.AreEqual (RuleResult.Success, runner.CheckType (type)); } [Test] public void IDisposableDestructorWithoutSuppressFinalize () { TypeDefinition type = GetTest ("IDisposableDestructorWithoutSuppressFinalizeClass"); - Assert.IsNotNull (rule.CheckType (type, new MinimalRunner ())); + Assert.AreEqual (RuleResult.Failure, runner.CheckType (type)); } [Test] public void IDisposableDestructorWithSuppressFinalize () { TypeDefinition type = GetTest ("IDisposableDestructorWithSuppressFinalizeClass"); - Assert.IsNull (rule.CheckType (type, new MinimalRunner ())); + Assert.AreEqual (RuleResult.Success, runner.CheckType (type)); } [Test] public void ExplicitIDisposableDestructorWithoutSuppressFinalize () { TypeDefinition type = GetTest ("ExplicitIDisposableDestructorWithoutSuppressFinalizeClass"); - Assert.IsNotNull (rule.CheckType (type, new MinimalRunner ())); + Assert.AreEqual (RuleResult.Failure, runner.CheckType (type)); } [Test] public void ExplicitIDisposableDestructorWithSuppressFinalize () { TypeDefinition type = GetTest ("ExplicitIDisposableDestructorWithSuppressFinalizeClass"); - Assert.IsNull (rule.CheckType (type, new MinimalRunner ())); + Assert.AreEqual (RuleResult.Success, runner.CheckType (type)); } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/Test/UseIsOperatorTest.cs b/gendarme/rules/Gendarme.Rules.Performance/Test/UseIsOperatorTest.cs index 11737a25..b65801fc 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/Test/UseIsOperatorTest.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/Test/UseIsOperatorTest.cs @@ -102,7 +102,7 @@ namespace Test.Rules.Performance { private IMethodRule rule; private AssemblyDefinition assembly; private TypeDefinition type; - private Runner runner; + private TestRunner runner; [TestFixtureSetUp] public void FixtureSetUp () @@ -111,7 +111,7 @@ namespace Test.Rules.Performance { assembly = AssemblyFactory.GetAssembly (unit); type = assembly.MainModule.Types ["Test.Rules.Performance.UseIsOperatorTest"]; rule = new UseIsOperatorRule (); - runner = new MinimalRunner (); + runner = new TestRunner (rule); } private MethodDefinition GetTest (string name) @@ -128,26 +128,26 @@ namespace Test.Rules.Performance { public void Return () { MethodDefinition method = GetTest ("ReturnEqualityBad"); - Assert.IsNotNull (rule.CheckMethod (method, runner), "ReturnEqualityBad"); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method), "ReturnEqualityBad"); method = GetTest ("ReturnInequalityBad"); - Assert.IsNotNull (rule.CheckMethod (method, runner), "ReturnInequalityBad"); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method), "ReturnInequalityBad"); method = GetTest ("ReturnEqualityOk"); - Assert.IsNull (rule.CheckMethod (method, runner), "ReturnEqualityOk"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), "ReturnEqualityOk"); method = GetTest ("ReturnInequalityOk"); - Assert.IsNull (rule.CheckMethod (method, runner), "ReturnInequalityOk"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), "ReturnInequalityOk"); } [Test] public void Conditions () { MethodDefinition method = GetTest ("ConditionIsOk"); - Assert.IsNull (rule.CheckMethod (method, runner), "ConditionIsOk"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), "ConditionIsOk"); method = GetTest ("ConditionAsOk"); - Assert.IsNull (rule.CheckMethod (method, runner), "ConditionAsOk"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method), "ConditionAsOk"); } [Test] @@ -156,13 +156,13 @@ namespace Test.Rules.Performance { { // missed opportunities are less problematic than false positives ;-) MethodDefinition method = GetTest ("ConditionEqualityBad"); - Assert.IsNotNull (rule.CheckMethod (method, runner), "ConditionEqualityBad"); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method), "ConditionEqualityBad"); method = GetTest ("ConditionInequalityBad"); - Assert.IsNotNull (rule.CheckMethod (method, runner), "ConditionInequalityBad"); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method), "ConditionInequalityBad"); method = GetTest ("ConditionSplitBad"); - Assert.IsNotNull (rule.CheckMethod (method, runner), "ConditionSplitBad"); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method), "ConditionSplitBad"); } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/Test/UseStringEmptyTest.cs b/gendarme/rules/Gendarme.Rules.Performance/Test/UseStringEmptyTest.cs index a53bfd4b..998e951f 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/Test/UseStringEmptyTest.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/Test/UseStringEmptyTest.cs @@ -91,6 +91,7 @@ namespace Test.Rules.Performance { private AssemblyDefinition assembly; private ModuleDefinition module; private TypeDefinition type; + private TestRunner runner; [TestFixtureSetUp] public void FixtureSetUp () @@ -100,6 +101,7 @@ namespace Test.Rules.Performance { module = assembly.MainModule; type = assembly.MainModule.Types["Test.Rules.Performance.UseStringEmptyTest/TestCase"]; rule = new UseStringEmptyRule (); + runner = new TestRunner (rule); } private MethodDefinition GetTest (string name) @@ -119,21 +121,21 @@ namespace Test.Rules.Performance { public void GetConstField () { MethodDefinition method = GetTest ("GetConstField"); - Assert.IsNotNull (rule.CheckMethod (method, new MinimalRunner())); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method)); } [Test] public void Append () { MethodDefinition method = GetTest ("Append"); - Assert.IsNotNull (rule.CheckMethod (method, new MinimalRunner())); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method)); } [Test] public void Enclose () { MethodDefinition method = GetTest ("Enclose"); - Assert.IsNotNull (rule.CheckMethod (method, new MinimalRunner())); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method)); } [Test] @@ -141,7 +143,7 @@ namespace Test.Rules.Performance { { // the "public_field" field is set to "" in the (hidden) ctor MethodDefinition method = GetTest (".ctor"); - Assert.IsNotNull (rule.CheckMethod (method, new MinimalRunner())); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method)); } [Test] @@ -149,35 +151,35 @@ namespace Test.Rules.Performance { { // the "private_static_field" field is set to "" in the (hidden) class ctor MethodDefinition method = GetTest (".cctor"); - Assert.IsNotNull (rule.CheckMethod (method, new MinimalRunner())); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method)); } [Test] public void GetField () { MethodDefinition method = GetTest ("GetField"); - Assert.IsNull (rule.CheckMethod (method, new MinimalRunner())); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } [Test] public void GetStaticField () { MethodDefinition method = GetTest ("GetStaticField"); - Assert.IsNull (rule.CheckMethod (method, new MinimalRunner())); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } [Test] public void Prepend () { MethodDefinition method = GetTest ("Prepend"); - Assert.IsNull (rule.CheckMethod (method, new MinimalRunner())); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } [Test] public void NoHarm () { MethodDefinition method = GetTest ("NoStringWereHarmedInThisTestCase"); - Assert.IsNull (rule.CheckMethod (method, new MinimalRunner())); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/Test/UsingStringLengthInsteadOfCheckingEmptyStringTest.cs b/gendarme/rules/Gendarme.Rules.Performance/Test/UsingStringLengthInsteadOfCheckingEmptyStringTest.cs index 87d348f3..b2710142 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/Test/UsingStringLengthInsteadOfCheckingEmptyStringTest.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/Test/UsingStringLengthInsteadOfCheckingEmptyStringTest.cs @@ -28,6 +28,7 @@ using System; using System.Reflection; using Gendarme.Framework; +using Gendarme.Framework.Rocks; using Gendarme.Rules.Performance; using Mono.Cecil; using NUnit.Framework; @@ -105,8 +106,8 @@ namespace Test.Rules.Performance private IMethodRule methodRule; private AssemblyDefinition assembly; - private TypeDefinition type; - MessageCollection messageCollection; + private MethodDefinition method; + private TestRunner runner; [TestFixtureSetUp] public void FixtureSetUp () @@ -114,76 +115,57 @@ namespace Test.Rules.Performance string unit = Assembly.GetExecutingAssembly ().Location; assembly = AssemblyFactory.GetAssembly (unit); methodRule = new UsingStringLengthInsteadOfCheckingEmptyStringRule(); - messageCollection = null; + runner = new TestRunner (methodRule); } - private TypeDefinition GetTest (string name) + private MethodDefinition GetTest (string type) { - string fullname = "Test.Rules.Performance.UsingStringLengthInsteadOfCheckingEmptyStringTest/" + name; - return assembly.MainModule.Types[fullname]; + string fullname = "Test.Rules.Performance.UsingStringLengthInsteadOfCheckingEmptyStringTest/" + type; + return assembly.MainModule.Types[fullname].GetMethod("Main"); } [Test] public void usingStringEqualsTest () { - type = GetTest ("UsingStringEquals"); - foreach (MethodDefinition method in type.Methods) { - messageCollection = methodRule.CheckMethod (method, new MinimalRunner ()); - } - Assert.IsNotNull (messageCollection); - Assert.AreEqual (1, messageCollection.Count); + method = GetTest ("UsingStringEquals"); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method)); + Assert.AreEqual (1, runner.Defects.Count); } - [Test] public void usingStringLengthTest () { - type = GetTest ("UsingStringLength"); - foreach (MethodDefinition method in type.Methods) { - messageCollection = methodRule.CheckMethod (method, new MinimalRunner ()); - } - Assert.IsNull (messageCollection); + method = GetTest ("UsingStringLength"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } [Test] public void usingEquqlsWithNonStringArgTest () { - type = GetTest ("UsingEquqlsWithNonStringArg"); - foreach (MethodDefinition method in type.Methods) { - messageCollection = methodRule.CheckMethod (method, new MinimalRunner ()); - } - Assert.IsNull (messageCollection); + method = GetTest ("UsingEquqlsWithNonStringArg"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } [Test] public void anotherUseOfEqualsWithEmptyStringTest () { - type = GetTest ("AnotherUseOfEqualsWithEmptyString"); - foreach (MethodDefinition method in type.Methods) { - messageCollection = methodRule.CheckMethod (method, new MinimalRunner ()); - } - Assert.IsNotNull (messageCollection); - Assert.AreEqual (1, messageCollection.Count); + method = GetTest ("AnotherUseOfEqualsWithEmptyString"); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method)); + Assert.AreEqual (1, runner.Defects.Count); } - + [Test] public void oneMoreUseOfEqualsWithEmptyStringTest () { - type = GetTest ("OneMoreUseOfEqualsWithEmptyString"); - foreach (MethodDefinition method in type.Methods) { - messageCollection = methodRule.CheckMethod (method, new MinimalRunner ()); - } - Assert.IsNotNull (messageCollection); - Assert.AreEqual (2, messageCollection.Count); + method = GetTest ("OneMoreUseOfEqualsWithEmptyString"); + Assert.AreEqual (RuleResult.Failure, runner.CheckMethod (method)); + Assert.AreEqual (2, runner.Defects.Count); } [Test] public void usingEqualsWithNonEmptyStringTest () { - type = GetTest ("UsingEqualsWithNonEmptyString"); - foreach (MethodDefinition method in type.Methods) { - messageCollection = methodRule.CheckMethod (method, new MinimalRunner ()); - } - Assert.IsNull (messageCollection); + method = GetTest ("UsingEqualsWithNonEmptyString"); + Assert.AreEqual (RuleResult.Success, runner.CheckMethod (method)); } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/UseIsOperatorRule.cs b/gendarme/rules/Gendarme.Rules.Performance/UseIsOperatorRule.cs index 8573df69..d66cb19f 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/UseIsOperatorRule.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/UseIsOperatorRule.cs @@ -32,16 +32,18 @@ using Gendarme.Framework; namespace Gendarme.Rules.Performance { - public class UseIsOperatorRule : IMethodRule { + [Problem ("The method should use the \"is\" operator and avoid the cast and compare to null.")] + [Solution ("Replace the cast and compare to null with the simpler \"is\" operator.")] + public class UseIsOperatorRule : Rule, IMethodRule { - private const string Text = "Using the 'is' operator would produce better (less) IL and would be easier to understand."; + private const string ErrorText = "Using the 'is' operator would produce better (less) IL and would be easier to understand."; - public MessageCollection CheckMethod (MethodDefinition method, Runner runner) + public RuleResult CheckMethod (MethodDefinition method) { if (!method.HasBody) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; - MessageCollection messages = null; + bool notContainsIsOperator = false; InstructionCollection instructions = method.Body.Instructions; int n = instructions.Count - 2; for (int i = 0; i < n; i++) { @@ -55,14 +57,11 @@ namespace Gendarme.Rules.Performance { if (code2 != Code.Ceq) continue; - if (messages == null) - messages = new MessageCollection (); - Location loc = new Location (method, i); - Message msg = new Message (Text, loc, MessageType.Warning); - messages.Add (msg); + notContainsIsOperator = true; + Runner.Report (method, instructions[i], Severity.High, Confidence.High, ErrorText); } - return messages; + return notContainsIsOperator? RuleResult.Failure : RuleResult.Success; } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/UseStringEmptyRule.cs b/gendarme/rules/Gendarme.Rules.Performance/UseStringEmptyRule.cs index d4b12d3c..ceeadeff 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/UseStringEmptyRule.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/UseStringEmptyRule.cs @@ -35,37 +35,31 @@ using Gendarme.Framework; namespace Gendarme.Rules.Performance { - public class UseStringEmptyRule : IMethodRule { + [Problem ("The method uses literal \"\" instead of String.Empty.")] + [Solution ("Change the empty string for String.Empty.")] + public class UseStringEmptyRule : Rule, IMethodRule { - public MessageCollection CheckMethod (MethodDefinition method, Runner runner) + public RuleResult CheckMethod (MethodDefinition method) { // #1 - rule apply only if the method has a body (e.g. p/invokes, icalls don't) if (!method.HasBody) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; // *** ok, the rule applies! *** - - MessageCollection results = null; - + bool failure = false; // #2 - look for string references foreach (Instruction ins in method.Body.Instructions) { switch (ins.OpCode.OperandType) { case OperandType.InlineString: string s = (ins.Operand as string); if (s.Length == 0) { - Location loc = new Location (method, ins.Offset); - Message msg = new Message ("instance of an empty string has been found.", - loc, MessageType.Warning); - if (results == null) - results = new MessageCollection (msg); - else - results.Add (msg); + Runner.Report (method, ins, Severity.Medium, Confidence.High, "Instance of an empty string has been found"); + failure = true; } break; } } - - return results; + return failure ? RuleResult.Failure : RuleResult.Success; } } } diff --git a/gendarme/rules/Gendarme.Rules.Performance/UsingStringLengthInsteadOfCheckingEmptyStringRule.cs b/gendarme/rules/Gendarme.Rules.Performance/UsingStringLengthInsteadOfCheckingEmptyStringRule.cs index 7f8bfeab..f09d8bbd 100644 --- a/gendarme/rules/Gendarme.Rules.Performance/UsingStringLengthInsteadOfCheckingEmptyStringRule.cs +++ b/gendarme/rules/Gendarme.Rules.Performance/UsingStringLengthInsteadOfCheckingEmptyStringRule.cs @@ -30,35 +30,30 @@ using Mono.Cecil; using Mono.Cecil.Cil; using Gendarme.Framework; -namespace Gendarme.Rules.Performance -{ - public class UsingStringLengthInsteadOfCheckingEmptyStringRule: IMethodRule - { - public MessageCollection CheckMethod (MethodDefinition method, Runner runner) +namespace Gendarme.Rules.Performance { + [Problem ("The method compares the empty string by using Equals (\"\").")] + [Solution ("Use String.Length instead, it's faster compare ints than compare strings.")] + public class UsingStringLengthInsteadOfCheckingEmptyStringRule: Rule, IMethodRule { + public RuleResult CheckMethod (MethodDefinition method) { // rule apply only if the method has a body (e.g. p/invokes, icalls don't) if (!method.HasBody) - return runner.RuleSuccess; - - MessageCollection messageCollection = null; + return RuleResult.DoesNotApply; + bool checksEqualsInsteadOfLength = false; foreach (Instruction instruction in method.Body.Instructions) { if (instruction.Operand != null) { if (instruction.Operand.ToString () == "System.Boolean System.String::Equals(System.String)") { Instruction prevInstr = instruction.Previous; if (prevInstr.OpCode.Name == "ldstr" && prevInstr.Operand.ToString().Length == 0) { - Location location = new Location (method, instruction.Offset); - Message message = new Message ("Method uses .Equals (\"\") method to check for empty string instead of using Length property ", location, MessageType.Error); - if (messageCollection == null) - messageCollection = new MessageCollection (message); - else - messageCollection.Add (message); + Runner.Report (method, instruction, Severity.Medium, Confidence.High, "Method uses Equals method to check for an empty string instead of using Length property"); + checksEqualsInsteadOfLength = true; } } } } - return messageCollection; + return checksEqualsInsteadOfLength ? RuleResult.Failure : RuleResult.Success; } } } |