diff options
author | Sebastien Pouliot <sebastien@ximian.com> | 2008-02-15 22:01:34 +0300 |
---|---|---|
committer | Sebastien Pouliot <sebastien@ximian.com> | 2008-02-15 22:01:34 +0300 |
commit | fe5915eea71a19498b2bf26d0adf60a5893095e9 (patch) | |
tree | a0f29992617b547f1304cdab662a464edc6ff47b /gendarme/rules | |
parent | 7cae4892c6a6c1b2d154cd46bb3e46fc89c44339 (diff) |
2008-02-15 Sebastien Pouliot <sebastien@ximian.com>
* AbstractTypesShouldNotHavePublicConstructorsRule.cs
* AttributeArgumentsShouldHaveAccessorsRule.cs
* AvoidEmptyInterfaceRule.cs
* AvoidPropertiesWithoutGetAccessorRule.cs
* AvoidPublicInstanceFieldsRule.cs
* ConsiderConvertingMethodToPropertyRule.cs
* DefineAZeroValueRule.cs
* DisposableFieldsShouldBeDisposedRule.cs
* DisposableTypesShouldHaveFinalizerRule.cs
* DontDeclareProtectedFieldsInSealedClassRule.cs
* EnsureSymmetryForOverloadedOperatorsRule.cs
* EnumsShouldDefineAZeroValueRule.cs
* EnumsShouldUseInt32Rule.cs
* FinalizersShouldBeProtectedRule.cs
* FinalizersShouldCallBaseClassFinalizerRule.cs
* FlagsShouldNotDefineAZeroValueRule.cs
* ImplementEqualsAndGetHashCodeInPairRule.cs
* MainShouldNotBePublicRule.cs
* MissingAttributeUsageOnCustomAttributeRule.cs
* OperatorEqualsShouldBeOverloadedRule.cs
* OverrideEqualsMethodRule.cs
* ProvideAlternativeNamesForOperatorOverloadsRule.cs
* TypesShouldBeInsideNamespacesRule.cs
* TypesWithDisposableFieldsShouldBeDisposableRule.cs
* TypesWithNativeFieldsShouldBeDisposableRule.cs
* UsingCloneWithoutImplementingICloneableRule.cs:
Update rules wrt framework changes.
svn path=/trunk/mono-tools/; revision=95789
Diffstat (limited to 'gendarme/rules')
27 files changed, 388 insertions, 403 deletions
diff --git a/gendarme/rules/Gendarme.Rules.Design/AbstractTypesShouldNotHavePublicConstructorsRule.cs b/gendarme/rules/Gendarme.Rules.Design/AbstractTypesShouldNotHavePublicConstructorsRule.cs index b4adb6fd..226d477d 100644 --- a/gendarme/rules/Gendarme.Rules.Design/AbstractTypesShouldNotHavePublicConstructorsRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/AbstractTypesShouldNotHavePublicConstructorsRule.cs @@ -4,7 +4,7 @@ // Authors: // Sebastien Pouliot <sebastien@ximian.com> // -// Copyright (C) 2007 Novell, Inc (http://www.novell.com) +// Copyright (C) 2007-2008 Novell, Inc (http://www.novell.com) // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal @@ -32,28 +32,25 @@ using Gendarme.Framework; namespace Gendarme.Rules.Design { - public class AbstractTypesShouldNotHavePublicConstructorsRule: ITypeRule { + [Problem ("This abstract type provides public constructor(s).")] + [Solution ("Change constructor visibility to protected.")] + public class AbstractTypesShouldNotHavePublicConstructorsRule : Rule, ITypeRule { - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // rule apply only on abstract types if (!type.IsAbstract) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; // rule applies! - MessageCollection mc = null; foreach (MethodDefinition ctor in type.Constructors) { if (ctor.IsPublic) { - Message msg = new Message ("Public constructor found.", new Location (ctor), MessageType.Warning); - if (mc == null) - mc = new MessageCollection (msg); - else - mc.Add (msg); + Runner.Report (ctor, Severity.Low, Confidence.Total, String.Empty); } } - return mc; + return Runner.CurrentRuleResult; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/AttributeArgumentsShouldHaveAccessorsRule.cs b/gendarme/rules/Gendarme.Rules.Design/AttributeArgumentsShouldHaveAccessorsRule.cs index 6fe35cf7..9c4b4648 100644 --- a/gendarme/rules/Gendarme.Rules.Design/AttributeArgumentsShouldHaveAccessorsRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/AttributeArgumentsShouldHaveAccessorsRule.cs @@ -34,43 +34,38 @@ using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class AttributeArgumentsShouldHaveAccessorsRule : ITypeRule { + [Problem ("All parameter values passed to this type constructors should be visible through read-only properties.")] + [Solution ("Add the missing properties getters to this type.")] + public class AttributeArgumentsShouldHaveAccessorsRule : Rule, ITypeRule { private List<string> allProperties = new List<string> (); - public MessageCollection CheckType (TypeDefinition typeDefinition, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // rule applies only to attributes - if (!typeDefinition.IsAttribute ()) - return runner.RuleSuccess; + if (!type.IsAttribute ()) + return RuleResult.DoesNotApply; // look through getters allProperties.Clear (); - foreach (PropertyDefinition property in typeDefinition.Properties) { + foreach (PropertyDefinition property in type.Properties) { if (property.GetMethod != null) { allProperties.Add (property.Name); } } - Location loc = new Location (typeDefinition); - MessageCollection messages = runner.RuleSuccess; - // look through parameters - foreach (MethodDefinition constructor in typeDefinition.Constructors) { + foreach (MethodDefinition constructor in type.Constructors) { foreach (ParameterDefinition param in constructor.Parameters) { string correspondingPropertyName = char.ToUpper (param.Name [0]) + param.Name.Substring (1); // pascal case it if (!allProperties.Contains (correspondingPropertyName)) { - Message msg = new Message ( - string.Format ("All parameter values passed to an attribute constructor must be visible through read-only properties. Add '{0}' property to the attribute class.", correspondingPropertyName), - loc, MessageType.Error); - if (messages == runner.RuleSuccess) - messages = new MessageCollection (); - messages.Add (msg); + string s = String.Format ("Add '{0}' property to the attribute class.", correspondingPropertyName); + Runner.Report (param, Severity.Medium, Confidence.High, s); allProperties.Add (correspondingPropertyName); // to avoid double catching same property (e.g. from different constructors) } } } - return messages; + return Runner.CurrentRuleResult; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/AvoidEmptyInterfaceRule.cs b/gendarme/rules/Gendarme.Rules.Design/AvoidEmptyInterfaceRule.cs index 8244fd34..9c4b4648 100644 --- a/gendarme/rules/Gendarme.Rules.Design/AvoidEmptyInterfaceRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/AvoidEmptyInterfaceRule.cs @@ -1,10 +1,10 @@ // -// Gendarme.Rules.Design.AvoidEmptyInterfaceRule +// Gendarme.Rules.Design.AttributeArgumentsShouldHaveAccessorsRule // // Authors: -// Sebastien Pouliot <sebastien@ximian.com> +// Daniel Abramov <ex@vingrad.ru> // -// Copyright (C) 2007 Novell, Inc (http://www.novell.com) +// Copyright (C) 2007 Daniel Abramov // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal @@ -25,33 +25,47 @@ // THE SOFTWARE. using System; +using System.Collections.Generic; using Mono.Cecil; using Gendarme.Framework; +using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class AvoidEmptyInterfaceRule : ITypeRule { + [Problem ("All parameter values passed to this type constructors should be visible through read-only properties.")] + [Solution ("Add the missing properties getters to this type.")] + public class AttributeArgumentsShouldHaveAccessorsRule : Rule, ITypeRule { - public MessageCollection CheckType (TypeDefinition type, Runner runner) - { - // rule only applies to interfaces - if (!type.IsInterface) - return runner.RuleSuccess; - - // rule applies! + private List<string> allProperties = new List<string> (); - // first check if the interface defines it's own members - if (type.Methods.Count > 0) - return runner.RuleSuccess; + public RuleResult CheckType (TypeDefinition type) + { + // rule applies only to attributes + if (!type.IsAttribute ()) + return RuleResult.DoesNotApply; - // otherwise it may implement more than one interface itself - if (type.Interfaces.Count > 0) - return runner.RuleSuccess; + // look through getters + allProperties.Clear (); + foreach (PropertyDefinition property in type.Properties) { + if (property.GetMethod != null) { + allProperties.Add (property.Name); + } + } - Message msg = new Message ("Interface is empty.", new Location (type), MessageType.Warning); - return new MessageCollection (msg); + // look through parameters + foreach (MethodDefinition constructor in type.Constructors) { + foreach (ParameterDefinition param in constructor.Parameters) { + string correspondingPropertyName = char.ToUpper (param.Name [0]) + param.Name.Substring (1); // pascal case it + if (!allProperties.Contains (correspondingPropertyName)) { + string s = String.Format ("Add '{0}' property to the attribute class.", correspondingPropertyName); + Runner.Report (param, Severity.Medium, Confidence.High, s); + allProperties.Add (correspondingPropertyName); // to avoid double catching same property (e.g. from different constructors) + } + } + } + return Runner.CurrentRuleResult; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/AvoidPropertiesWithoutGetAccessorRule.cs b/gendarme/rules/Gendarme.Rules.Design/AvoidPropertiesWithoutGetAccessorRule.cs index 4d404add..3dc7d35a 100644 --- a/gendarme/rules/Gendarme.Rules.Design/AvoidPropertiesWithoutGetAccessorRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/AvoidPropertiesWithoutGetAccessorRule.cs @@ -4,7 +4,7 @@ // Authors: // Sebastien Pouliot <sebastien@ximian.com> // -// Copyright (C) 2007 Novell, Inc (http://www.novell.com) +// Copyright (C) 2007-2008 Novell, Inc (http://www.novell.com) // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal @@ -33,31 +33,27 @@ using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class AvoidPropertiesWithoutGetAccessorRule : IMethodRule { + [Problem ("This type contains some properties that have only setters.")] + [Solution ("Add a getter to the property or change the property into a method.")] + public class AvoidPropertiesWithoutGetAccessorRule : Rule, ITypeRule { - public MessageCollection CheckMethod (MethodDefinition method, Runner runner) + public RuleResult CheckType (TypeDefinition type) { - // rule applies to setters - if (!method.IsSetter) - return runner.RuleSuccess; + // rule applies to type with properties + if (type.Properties.Count == 0) + return RuleResult.DoesNotApply; // rule applies - // check if there is a getter for the same property - string name = method.Name.Replace ("set_", "get_"); - TypeDefinition type = method.DeclaringType as TypeDefinition; - // inside the declaring type - foreach (MethodDefinition md in type.Methods) { - // look for the getter name - if (md.Name == name) { - // and confirm it's getter - if (md.IsGetter) - return runner.RuleSuccess; + foreach (PropertyDefinition property in type.Properties) { + MethodDefinition setter = property.SetMethod; + if (setter != null) { + if (property.GetMethod == null) { + Runner.Report (setter, Severity.Medium, Confidence.Total, String.Empty); + } } } - - Message msg = new Message ("The property only provide a set (and no get)", new Location (method), MessageType.Warning); - return new MessageCollection (msg); + return Runner.CurrentRuleResult; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/AvoidPublicInstanceFieldsRule.cs b/gendarme/rules/Gendarme.Rules.Design/AvoidPublicInstanceFieldsRule.cs index a9d4d200..5c388aa7 100644 --- a/gendarme/rules/Gendarme.Rules.Design/AvoidPublicInstanceFieldsRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/AvoidPublicInstanceFieldsRule.cs @@ -33,39 +33,30 @@ using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class AvoidPublicInstanceFieldsRule : ITypeRule { + [Problem ("This type contains public instance fields.")] + [Solution ("If possible change the public fields to properties.")] + public class AvoidPublicInstanceFieldsRule : Rule, ITypeRule { - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // rule doesn't apply on enums if (type.IsEnum) - return runner.RuleSuccess; - - MessageCollection results = null; - Location loc = null; + return RuleResult.DoesNotApply; foreach (FieldDefinition fd in type.Fields) { - if (!fd.IsPublic || fd.IsSpecialName || fd.IsStatic || fd.HasConstant || fd.IsInitOnly) continue; - if (results == null) { - results = new MessageCollection (); - loc = new Location (type); - } - if (fd.FieldType.IsArray ()) { - results.Add (new Message ("Consider converting the public instance field \'" + fd.Name + - "\' into a private field and a \'Set" + Char.ToUpper (fd.Name [0]) + fd.Name.Substring (1) - + "\' method.", loc, MessageType.Warning)); + string s = String.Format ("Consider converting the public instance field '{0}' into a private field and add a 'Set{1}' method.", + fd.Name, Char.ToUpper (fd.Name [0]) + fd.Name.Substring (1)); + Runner.Report (fd, Severity.Medium, Confidence.Total, s); } else { - results.Add (new Message ("Public instance field \'" + fd.Name + "\' should probably be a property.", loc, MessageType.Warning)); + string s = String.Format ("Public instance field '{0}' should probably be a property.", fd.Name); + Runner.Report (fd, Severity.Medium, Confidence.Total, s); } } - - if (results == null) - return runner.RuleSuccess; - return results; + return Runner.CurrentRuleResult; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/ChangeLog b/gendarme/rules/Gendarme.Rules.Design/ChangeLog index cd62ca49..ef9636d6 100644 --- a/gendarme/rules/Gendarme.Rules.Design/ChangeLog +++ b/gendarme/rules/Gendarme.Rules.Design/ChangeLog @@ -1,3 +1,33 @@ +2008-02-15 Sebastien Pouliot <sebastien@ximian.com> + + * AbstractTypesShouldNotHavePublicConstructorsRule.cs + * AttributeArgumentsShouldHaveAccessorsRule.cs + * AvoidEmptyInterfaceRule.cs + * AvoidPropertiesWithoutGetAccessorRule.cs + * AvoidPublicInstanceFieldsRule.cs + * ConsiderConvertingMethodToPropertyRule.cs + * DefineAZeroValueRule.cs + * DisposableFieldsShouldBeDisposedRule.cs + * DisposableTypesShouldHaveFinalizerRule.cs + * DontDeclareProtectedFieldsInSealedClassRule.cs + * EnsureSymmetryForOverloadedOperatorsRule.cs + * EnumsShouldDefineAZeroValueRule.cs + * EnumsShouldUseInt32Rule.cs + * FinalizersShouldBeProtectedRule.cs + * FinalizersShouldCallBaseClassFinalizerRule.cs + * FlagsShouldNotDefineAZeroValueRule.cs + * ImplementEqualsAndGetHashCodeInPairRule.cs + * MainShouldNotBePublicRule.cs + * MissingAttributeUsageOnCustomAttributeRule.cs + * OperatorEqualsShouldBeOverloadedRule.cs + * OverrideEqualsMethodRule.cs + * ProvideAlternativeNamesForOperatorOverloadsRule.cs + * TypesShouldBeInsideNamespacesRule.cs + * TypesWithDisposableFieldsShouldBeDisposableRule.cs + * TypesWithNativeFieldsShouldBeDisposableRule.cs + * UsingCloneWithoutImplementingICloneableRule.cs: + Update rules wrt framework changes. + 2008-02-07 Sebastien Pouliot <sebastien@ximian.com> * EnumsShouldUseInt32Rule.cs: New. Rule to warn about enumations that diff --git a/gendarme/rules/Gendarme.Rules.Design/ConsiderConvertingMethodToPropertyRule.cs b/gendarme/rules/Gendarme.Rules.Design/ConsiderConvertingMethodToPropertyRule.cs index 48f9cd80..a56b3959 100644 --- a/gendarme/rules/Gendarme.Rules.Design/ConsiderConvertingMethodToPropertyRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/ConsiderConvertingMethodToPropertyRule.cs @@ -33,7 +33,9 @@ using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class ConsiderConvertingMethodToPropertyRule : IMethodRule { + [Problem ("This method looks like a candidate to be a property.")] + [Solution ("If possible change this method into a property, otherwise you can ignore the rule.")] + public class ConsiderConvertingMethodToPropertyRule : Rule, IMethodRule { static readonly string [] whitelist = { "GetEnumerator", "GetHashCode", @@ -46,22 +48,22 @@ namespace Gendarme.Rules.Design { return name.StartsWith (start, true, null); } - public MessageCollection CheckMethod (MethodDefinition method, Runner runner) + public RuleResult CheckMethod (MethodDefinition method) { // rules do not apply to constructors, methods returning an array, properties if (method.IsConstructor || method.IsSpecialName || method.ReturnType.ReturnType.IsArray () || method.IsProperty ()) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; // rules do not apply to code generated by the compiler (e.g. anonymous methods) // or generated by a tool (e.g. web services) if (method.IsGeneratedCode ()) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; // ignore the some common Get* method names used in the framework foreach (string s in whitelist) { if (method.Name == s) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; } // rule applies @@ -71,12 +73,12 @@ namespace Gendarme.Rules.Design { // If it starts with "set" and has 1 parameter if ((StartsWith ("get", method.Name) || StartsWith ("is", method.Name)) && method.Parameters.Count == 0 && method.ReturnType.ReturnType.Name != "void" || StartsWith ("set", method.Name) && method.Parameters.Count == 1) { - Location loc = new Location (method); - Message msg = new Message ("This property-like method should probably be a property.", loc, MessageType.Warning); - return new MessageCollection (msg); + + Runner.Report (method, Severity.Low, Confidence.Normal, String.Empty); + return RuleResult.Failure; } - return runner.RuleSuccess; + return RuleResult.Success; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/DefineAZeroValueRule.cs b/gendarme/rules/Gendarme.Rules.Design/DefineAZeroValueRule.cs index cff50e70..2d9d2da1 100644 --- a/gendarme/rules/Gendarme.Rules.Design/DefineAZeroValueRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/DefineAZeroValueRule.cs @@ -4,7 +4,7 @@ // Authors: // Sebastien Pouliot <sebastien@ximian.com> // -// Copyright (C) 2007 Novell, Inc (http://www.novell.com) +// Copyright (C) 2007-2008 Novell, Inc (http://www.novell.com) // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal @@ -32,24 +32,18 @@ using Gendarme.Framework; namespace Gendarme.Rules.Design { - abstract public class DefineAZeroValueRule { + abstract public class DefineAZeroValueRule : Rule { - protected static bool HasZeroValue (TypeDefinition type) + protected static FieldDefinition GetZeroValueField (TypeDefinition type) { foreach (FieldDefinition field in type.Fields) { - switch (field.Name) { - case "value__": - // special case - break; - default: - if (field.Constant is int) { - if (((int) (field.Constant)) == 0) - return true; - } - break; + // the special __value field is not static like the others (user defined) + if (field.IsStatic && (field.Constant is int)) { + if (((int) (field.Constant)) == 0) + return field; } } - return false; + return null; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/DisposableFieldsShouldBeDisposedRule.cs b/gendarme/rules/Gendarme.Rules.Design/DisposableFieldsShouldBeDisposedRule.cs index 041cc27e..4db5a781 100644 --- a/gendarme/rules/Gendarme.Rules.Design/DisposableFieldsShouldBeDisposedRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/DisposableFieldsShouldBeDisposedRule.cs @@ -32,26 +32,25 @@ using System.Collections.Generic; using Mono.Cecil; using Mono.Cecil.Cil; using Gendarme.Framework; +using Gendarme.Framework.Helpers; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class DisposableFieldsShouldBeDisposedRule : ITypeRule { + [Problem ("This type contains disposable field(s) that aren't disposed.")] + [Solution ("Ensure that every disposable field(s) are disposed correctly.")] + public class DisposableFieldsShouldBeDisposedRule : Rule, ITypeRule { - private static void EnsureExists (ref MessageCollection result) - { - if (result == null) - result = new MessageCollection (); - } - - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // rule doesn't apply to generated code (out of developer control) if (type.IsGeneratedCode ()) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; + // note: other rule will complain if there are disposable or native fields + // in a type that doesn't implement IDisposable, so we don't bother here if (!type.Implements ("System.IDisposable")) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; MethodDefinition implicitDisposeMethod = type.GetMethod (MethodSignatures.Dispose); MethodDefinition explicitDisposeMethod = type.GetMethod (MethodSignatures.DisposeExplicit); @@ -61,28 +60,29 @@ namespace Gendarme.Rules.Design { if (explicitDisposeMethod == null || explicitDisposeMethod.IsAbstract) explicitDisposeMethod = null; - if (implicitDisposeMethod == null && explicitDisposeMethod == null) //handled by TypesWithDisposableFieldsShouldBeDisposableRule - return runner.RuleSuccess; - - MessageCollection results = null; + // note: handled by TypesWithDisposableFieldsShouldBeDisposableRule + if (implicitDisposeMethod == null && explicitDisposeMethod == null) + return RuleResult.Success; //Check for baseDispose TypeDefinition baseType = type; while (baseType.BaseType.FullName != "System.Object") { - baseType = baseType.BaseType as TypeDefinition; + baseType = baseType.BaseType.Resolve (); if (baseType == null) - break; //TODO Implements for TypeReference + break; + // also checks parents, so no need to search further if (!baseType.Implements ("System.IDisposable")) - break; //also checks parents, so no need to search further - MethodDefinition baseDisposeMethod = baseType.GetMethod (MethodSignatures.Dispose); //we just check for Dispose() here + break; + //we just check for Dispose() here + MethodDefinition baseDisposeMethod = baseType.GetMethod (MethodSignatures.Dispose); if (baseDisposeMethod == null) continue; //no dispose method (yet) if (baseDisposeMethod.IsAbstract) break; //abstract if (implicitDisposeMethod != null) - CheckIfBaseDisposeIsCalled (implicitDisposeMethod, baseDisposeMethod, ref results); + CheckIfBaseDisposeIsCalled (implicitDisposeMethod, baseDisposeMethod); if (explicitDisposeMethod != null) - CheckIfBaseDisposeIsCalled (explicitDisposeMethod, baseDisposeMethod, ref results); + CheckIfBaseDisposeIsCalled (explicitDisposeMethod, baseDisposeMethod); break; } @@ -94,9 +94,9 @@ namespace Gendarme.Rules.Design { continue; if (field.FieldType.IsArray ()) continue; - TypeDefinition fieldType = field.FieldType as TypeDefinition; + TypeDefinition fieldType = field.FieldType.Resolve (); if (fieldType == null) - continue; //TODO: Implemts for TypeReference + continue; if (fieldType.Implements ("System.IDisposable")) disposeableFields.Add (field); } @@ -112,14 +112,14 @@ namespace Gendarme.Rules.Design { } if (implicitDisposeMethod != null) - CheckIfAllFieldsAreDisposed (implicitDisposeMethod, iList, ref results); + CheckIfAllFieldsAreDisposed (implicitDisposeMethod, iList); if (explicitDisposeMethod != null) - CheckIfAllFieldsAreDisposed (explicitDisposeMethod, eList, ref results); + CheckIfAllFieldsAreDisposed (explicitDisposeMethod, eList); - return results; + return Runner.CurrentRuleResult; } - private static void CheckIfBaseDisposeIsCalled (MethodDefinition method, MethodDefinition baseMethod, ref MessageCollection results) + private void CheckIfBaseDisposeIsCalled (MethodDefinition method, MethodDefinition baseMethod) { bool found = false; //Check for a call to base.Dispose(); @@ -139,14 +139,12 @@ namespace Gendarme.Rules.Design { } if (!found) { - EnsureExists (ref results); - Location loc = new Location (method); - Message msg = new Message (string.Format ("{0}::{1}() should call base.Dispose().", method.DeclaringType.FullName, method.Name), loc, MessageType.Warning); - results.Add (msg); + string s = String.Format ("{0} should call base.Dispose().", method.ToString ()); + Runner.Report (method, Severity.Medium, Confidence.High, s); } } - private static void CheckIfAllFieldsAreDisposed (MethodDefinition method, List<FieldDefinition> fields, ref MessageCollection results) + private void CheckIfAllFieldsAreDisposed (MethodDefinition method, List<FieldDefinition> fields) { //Check if Dispose(bool) is called and if all fields are disposed foreach (Instruction ins in method.Body.Instructions) { @@ -169,12 +167,9 @@ namespace Gendarme.Rules.Design { if (fields.Count == 0) return; - EnsureExists (ref results); - foreach (FieldDefinition field in fields) { - Location loc = new Location (field); - Message msg = new Message (string.Format ("{0} is Disposable. {1}() should call {0}.Dispose()", field.Name, method.Name), loc, MessageType.Error); - results.Add (msg); + string s = string.Format ("{0} is Disposable. {1}() should call {0}.Dispose()", field.Name, method.Name); + Runner.Report (field, Severity.High, Confidence.High, s); } } diff --git a/gendarme/rules/Gendarme.Rules.Design/DisposableTypesShouldHaveFinalizerRule.cs b/gendarme/rules/Gendarme.Rules.Design/DisposableTypesShouldHaveFinalizerRule.cs index 1177c809..f03c62e6 100644 --- a/gendarme/rules/Gendarme.Rules.Design/DisposableTypesShouldHaveFinalizerRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/DisposableTypesShouldHaveFinalizerRule.cs @@ -29,31 +29,36 @@ using System; using Mono.Cecil; using Gendarme.Framework; +using Gendarme.Framework.Helpers; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class DisposableTypesShouldHaveFinalizerRule : ITypeRule { + [Problem ("This type contains native fields but does not have a finalizer.")] + [Solution ("Add a finalizer, calling Dispose(true), to release unmanaged resources.")] + public class DisposableTypesShouldHaveFinalizerRule : Rule, ITypeRule { - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { + // rule onyly applies to type that implements IDisposable if (!type.Implements ("System.IDisposable")) - return runner.RuleSuccess; - if (type.GetMethod (MethodSignatures.Finalize) != null) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; + // no problem is a finalizer is found + if (type.HasMethod (MethodSignatures.Finalize)) + return RuleResult.Success; + + // otherwise check for native types foreach (FieldDefinition field in type.Fields) { // we can't dispose static fields in IDisposable if (field.IsStatic) continue; - if (field.FieldType.GetOriginalType ().IsNative ()) { - Location loc = new Location (type); - Message msg = new Message (string.Format ("{0} implements IDisposable and has native fields. It should implement a finalizer to release unmanaged resources.", type.FullName), loc, MessageType.Error); - return new MessageCollection (msg); - } + if (!field.FieldType.GetOriginalType ().IsNative ()) + continue; + Runner.Report (field, Severity.High, Confidence.High, String.Empty); } - return runner.RuleSuccess; + return Runner.CurrentRuleResult; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/DontDeclareProtectedFieldsInSealedClassRule.cs b/gendarme/rules/Gendarme.Rules.Design/DontDeclareProtectedFieldsInSealedClassRule.cs index db914964..d7a56428 100644 --- a/gendarme/rules/Gendarme.Rules.Design/DontDeclareProtectedFieldsInSealedClassRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/DontDeclareProtectedFieldsInSealedClassRule.cs @@ -1,10 +1,12 @@ // -// Gendarme.Rules.Dodgy.DontDeclareProtectedFieldsInSealedClassRule +// Gendarme.Rules.Dodgy.DoNotDeclareProtectedFieldsInSealedTypeRule // // Authors: // Nidhi Rawal <sonu2404@gmail.com> +// Sebastien Pouliot <sebastien@ximian.com> // // Copyright (c) <2007> Nidhi Rawal +// Copyright (C) 2008 Novell, Inc (http://www.novell.com) // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal @@ -32,27 +34,23 @@ using Gendarme.Framework; namespace Gendarme.Rules.Design { - public class DontDeclareProtectedFieldsInSealedClassRule: ITypeRule { + [Problem ("This sealed type contains protected (family) field(s).")] + [Solution ("Change the field visibility to public or private to represent the true intended use of the field.")] + public class DoNotDeclareProtectedFieldsInSealedTypeRule: Rule, ITypeRule { - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // rule applies only to sealed types if (!type.IsSealed) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; - MessageCollection mc = null; foreach (FieldDefinition field in type.Fields) { if (field.IsFamily) { - Location location = new Location (type); - Message message = new Message ("This sealed class contains protected field(s)", location, MessageType.Error); - if (mc == null) - mc = new MessageCollection (message); - else - mc.Add (message); + Runner.Report (field, Severity.Low, Confidence.Total, String.Empty); } } - return mc; + return Runner.CurrentRuleResult; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/EnsureSymmetryForOverloadedOperatorsRule.cs b/gendarme/rules/Gendarme.Rules.Design/EnsureSymmetryForOverloadedOperatorsRule.cs index 3fa893bd..78f5c00b 100644 --- a/gendarme/rules/Gendarme.Rules.Design/EnsureSymmetryForOverloadedOperatorsRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/EnsureSymmetryForOverloadedOperatorsRule.cs @@ -29,12 +29,18 @@ using System; using System.Collections.Generic; using Mono.Cecil; + using Gendarme.Framework; +using Gendarme.Framework.Helpers; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class EnsureSymmetryForOverloadedOperatorsRule : ITypeRule { + [Problem ("This type should overload operators in symmetry (e.g. == and !=, + and -).")] + [Solution ("Add the missing operator and keep the type operators symmetrical.")] + public class EnsureSymmetryForOverloadedOperatorsRule : Rule, ITypeRule { + + private const string Message = "The '{0}' operator is present, for symmetry, the '{1}' operator should be added."; static KeyValuePair<MethodSignature, MethodSignature> [] SymmetricOperators_Warning = new KeyValuePair<MethodSignature, MethodSignature> [] { new KeyValuePair<MethodSignature,MethodSignature> (MethodSignatures.op_Addition, MethodSignatures.op_Subtraction), @@ -48,23 +54,21 @@ namespace Gendarme.Rules.Design { new KeyValuePair<MethodSignature, MethodSignature> (MethodSignatures.op_True, MethodSignatures.op_False), }; - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { if (type.IsInterface || type.IsEnum) - return runner.RuleSuccess; - - MessageCollection results = null; + return RuleResult.DoesNotApply; foreach (var kv in SymmetricOperators_Warning) - CheckOperatorPair (kv, type, MessageType.Warning, ref results); + CheckOperatorPair (kv, type, Severity.Medium); foreach (var kv in SymmetricOperators_Error) - CheckOperatorPair (kv, type, MessageType.Error, ref results); + CheckOperatorPair (kv, type, Severity.High); - return results; + return Runner.CurrentRuleResult; } - private static void CheckOperatorPair (KeyValuePair<MethodSignature, MethodSignature> pair, TypeDefinition type, - MessageType msgType, ref MessageCollection results) + private void CheckOperatorPair (KeyValuePair<MethodSignature, MethodSignature> pair, + TypeDefinition type, Severity severity) { MethodDefinition op = type.GetMethod (pair.Key); if (op == null) { //first one not defined @@ -76,13 +80,9 @@ namespace Gendarme.Rules.Design { if (type.HasMethod (pair.Value)) return; //both are defined } - if (results == null) - results = new MessageCollection (); - Location loc = new Location (op); - string s = string.Format ("This type implements the '{0}' operator so, for symmetry, it should also implement the '{1}' operator.", - pair.Key.Name, pair.Value.Name); - Message msg = new Message (s, loc, msgType); - results.Add (msg); + + string s = string.Format (Message, pair.Key.Name, pair.Value.Name); + Runner.Report (op, severity, Confidence.Total, s); } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/EnumsShouldDefineAZeroValueRule.cs b/gendarme/rules/Gendarme.Rules.Design/EnumsShouldDefineAZeroValueRule.cs index 49905392..18e4cecb 100644 --- a/gendarme/rules/Gendarme.Rules.Design/EnumsShouldDefineAZeroValueRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/EnumsShouldDefineAZeroValueRule.cs @@ -33,25 +33,28 @@ using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { + [Problem ("This enumeration does not provide a member with a value of 0.")] + [Solution ("Add a new member in the enum with a value of 0.")] public class EnumsShouldDefineAZeroValueRule : DefineAZeroValueRule, ITypeRule { - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // rule apply only on enums if (!type.IsEnum) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; // rule doesn't apply on [Flags] if (type.IsFlags ()) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; // rule applies! - if (HasZeroValue (type)) - return runner.RuleSuccess; + FieldDefinition field = GetZeroValueField (type); + if (field != null) + return RuleResult.Success; - Message msg = new Message ("Enum doesn't define a 0 value.", new Location (type), MessageType.Error); - return new MessageCollection (msg); + Runner.Report (type, Severity.Low, Confidence.Total, String.Empty); + return RuleResult.Failure; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/EnumsShouldUseInt32Rule.cs b/gendarme/rules/Gendarme.Rules.Design/EnumsShouldUseInt32Rule.cs index f5a2f5c0..3f98ea95 100644 --- a/gendarme/rules/Gendarme.Rules.Design/EnumsShouldUseInt32Rule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/EnumsShouldUseInt32Rule.cs @@ -33,13 +33,15 @@ using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class EnumsShouldUseInt32Rule : ITypeRule { + [Problem ("Unless required for interoperability this enumeration should use Int32 as its underling storage type.")] + [Solution ("Remove the extra type from the enumeration declaration (Int32 will be used as default).")] + public class EnumsShouldUseInt32Rule : Rule, ITypeRule { - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // rule apply only on enums if (!type.IsEnum) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; // rule applies! @@ -54,30 +56,30 @@ namespace Gendarme.Rules.Design { break; } - MessageType criticality = MessageType.Error; + Severity severity = Severity.Critical; switch (value_type) { case "System.Int32": - return runner.RuleSuccess; + return RuleResult.Success; // some are bad choice (when possible) but usable by all CLS compliant languages case "System.Byte": case "System.Int16": case "System.Int64": - criticality = MessageType.Warning; + severity = Severity.High; break; // while others are not usable in non-CLS compliant languages case "System.SByte": case "System.UInt16": case "System.UInt32": case "System.UInt64": - criticality = MessageType.Error; + severity = Severity.Critical; break; default: throw new NotSupportedException (value_type + " unexpected as a Enum value type"); } string text = String.Format ("Enums should use System.Int32 instead of '{0}'.", value_type); - Message msg = new Message (text, new Location (type), criticality); - return new MessageCollection (msg); + Runner.Report (type, severity, Confidence.Total, text); + return RuleResult.Failure; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/FinalizersShouldBeProtectedRule.cs b/gendarme/rules/Gendarme.Rules.Design/FinalizersShouldBeProtectedRule.cs index 56b27653..b246c6a3 100644 --- a/gendarme/rules/Gendarme.Rules.Design/FinalizersShouldBeProtectedRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/FinalizersShouldBeProtectedRule.cs @@ -31,26 +31,27 @@ using Mono.Cecil; using Mono.Cecil.Cil; using Gendarme.Framework; +using Gendarme.Framework.Helpers; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class FinalizersShouldBeProtectedRule : ITypeRule { + [Problem ("The finalizer for this type isn't protected (family) and is not callable only from the runtime.")] + [Solution ("Change finalizer visibility to protected (family).")] + public class FinalizersShouldBeProtectedRule : Rule, ITypeRule { - public MessageCollection CheckType (TypeDefinition typeDefinition, Runner runner) + public RuleResult CheckType (TypeDefinition type) { - MethodDefinition finalizer = typeDefinition.GetMethod (MethodSignatures.Finalize); - + MethodDefinition finalizer = type.GetMethod (MethodSignatures.Finalize); if (finalizer == null) // no finalizer found - return runner.RuleSuccess; + return RuleResult.DoesNotApply; // good finalizer: if (finalizer.IsFamily && !finalizer.IsFamilyAndAssembly && !finalizer.IsFamilyOrAssembly) - return runner.RuleSuccess; + return RuleResult.Success; - Location loc = new Location (finalizer); - Message msg = new Message ("Finalizer must be protected in order not to be called from user code.", loc, MessageType.Error); - return new MessageCollection (msg); + Runner.Report (finalizer, Severity.High, Confidence.Total, String.Empty); + return RuleResult.Failure; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/FinalizersShouldCallBaseClassFinalizerRule.cs b/gendarme/rules/Gendarme.Rules.Design/FinalizersShouldCallBaseClassFinalizerRule.cs index 7ea83363..d68c3672 100644 --- a/gendarme/rules/Gendarme.Rules.Design/FinalizersShouldCallBaseClassFinalizerRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/FinalizersShouldCallBaseClassFinalizerRule.cs @@ -31,29 +31,30 @@ using Mono.Cecil; using Mono.Cecil.Cil; using Gendarme.Framework; +using Gendarme.Framework.Helpers; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class FinalizersShouldCallBaseClassFinalizerRule : ITypeRule { + [Problem ("The finalizer for this type does not call its base class finalizer.")] + [Solution ("Since your language does not do this automatically, like C#, add a call to the base type finalizer just before the finalizer exits.")] + public class FinalizersShouldCallBaseClassFinalizerRule : Rule, ITypeRule { - public MessageCollection CheckType (TypeDefinition typeDefinition, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // handle System.Object (which can't call base class) - if (typeDefinition.BaseType == null) - return runner.RuleSuccess; - - MethodDefinition finalizer = typeDefinition.GetMethod (MethodSignatures.Finalize); + if (type.BaseType == null) + return RuleResult.DoesNotApply; + MethodDefinition finalizer = type.GetMethod (MethodSignatures.Finalize); if (finalizer == null) // no finalizer found - return runner.RuleSuccess; + return RuleResult.DoesNotApply; if (IsBaseFinalizeCalled (finalizer)) - return runner.RuleSuccess; + return RuleResult.Success; - Location loc = new Location (finalizer); - Message msg = new Message ("Base class finalizer must be called just before the method exits.", loc, MessageType.Error); - return new MessageCollection (msg); + Runner.Report (finalizer, Severity.Critical, Confidence.Total, String.Empty); + return RuleResult.Failure; } private static bool IsBaseFinalizeCalled (MethodDefinition finalizer) diff --git a/gendarme/rules/Gendarme.Rules.Design/FlagsShouldNotDefineAZeroValueRule.cs b/gendarme/rules/Gendarme.Rules.Design/FlagsShouldNotDefineAZeroValueRule.cs index 12e5439c..7f2c506f 100644 --- a/gendarme/rules/Gendarme.Rules.Design/FlagsShouldNotDefineAZeroValueRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/FlagsShouldNotDefineAZeroValueRule.cs @@ -33,21 +33,24 @@ using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { + [Problem ("This enumeration flag defines a value of 0, which cannot be used as a real value.")] + [Solution ("Remove the 0 value from the flag.")] public class FlagsShouldNotDefineAZeroValueRule : DefineAZeroValueRule, ITypeRule { - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // rule apply only on [Flags] (this takes care of checking for enums) if (!type.IsFlags ()) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; // rule applies! - if (!HasZeroValue (type)) - return runner.RuleSuccess; + FieldDefinition field = GetZeroValueField (type); + if (field == null) + return RuleResult.Success; - Message msg = new Message ("Flags define a 0 value.", new Location (type), MessageType.Error); - return new MessageCollection (msg); + Runner.Report (field, Severity.High, Confidence.Total, String.Empty); + return RuleResult.Failure; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/ImplementEqualsAndGetHashCodeInPairRule.cs b/gendarme/rules/Gendarme.Rules.Design/ImplementEqualsAndGetHashCodeInPairRule.cs index 3bc98535..9aec96f5 100644 --- a/gendarme/rules/Gendarme.Rules.Design/ImplementEqualsAndGetHashCodeInPairRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/ImplementEqualsAndGetHashCodeInPairRule.cs @@ -31,41 +31,40 @@ using System; using Mono.Cecil; using Gendarme.Framework; +using Gendarme.Framework.Helpers; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class ImplementEqualsAndGetHashCodeInPairRule : ITypeRule { + [Problem ("This type only implements one of the Equals(Object) or GetHashCode() methods.")] + [Solution ("Implement the missing method.")] + public class ImplementEqualsAndGetHashCodeInPairRule : Rule, ITypeRule { private const string Message = "Type implements '{0}' but does not implement '{1}'"; - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // rule doesn't apply to interfaces and enums if (type.IsInterface || type.IsEnum) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; bool equals = type.HasMethod (MethodSignatures.Equals); bool getHashCode = type.HasMethod (MethodSignatures.GetHashCode); // if we have Equals but no GetHashCode method if (equals && !getHashCode) { - Location location = new Location (type); string text = String.Format (Message, MethodSignatures.Equals, MethodSignatures.GetHashCode); - Message message = new Message (text, location, MessageType.Error); - return new MessageCollection (message); + Runner.Report (type, Severity.Critical, Confidence.High, text); } // if we have GetHashCode but no Equals method if (!equals && getHashCode) { - Location location = new Location (type); - string text = String.Format (Message, MethodSignatures.GetHashCode, MethodSignatures.Equals); - Message message = new Message (text, location, MessageType.Warning); - return new MessageCollection (message); + string text = String.Format (Message, MethodSignatures.Equals, MethodSignatures.GetHashCode); + Runner.Report (type, Severity.Medium, Confidence.High, text); } // we either have both Equals and GetHashCode or none (both fine) - return runner.RuleSuccess; + return Runner.CurrentRuleResult; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/MainShouldNotBePublicRule.cs b/gendarme/rules/Gendarme.Rules.Design/MainShouldNotBePublicRule.cs index 77d33dcf..9a11494e 100644 --- a/gendarme/rules/Gendarme.Rules.Design/MainShouldNotBePublicRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/MainShouldNotBePublicRule.cs @@ -29,29 +29,22 @@ using System; using Mono.Cecil; using Gendarme.Framework; +using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class MainShouldNotBePublicRule : IAssemblyRule { - - // might be moved to Rocks as well - private static bool IsVBAssembly (AssemblyDefinition assemblyDefinition) - { - // as of now, we check for Microsoft.VisualBasic.dll reference - foreach (AssemblyNameReference r in assemblyDefinition.MainModule.AssemblyReferences) { - if (r.Name == "Microsoft.VisualBasic") - return true; - } - return false; - } + [Problem ("The entry point (Main) of this assembly is visible to the outside world (ref: C# Programming Guide).")] + [Solution ("Reduce entry point visibility if your language allows it. It may not be possible in some language, like VB.NET).")] + public class MainShouldNotBePublicRule : Rule, IAssemblyRule { + private const string VisualBasic = "Microsoft.VisualBasic"; - public MessageCollection CheckAssembly (AssemblyDefinition assemblyDefinition, Runner runner) + public RuleResult CheckAssembly (AssemblyDefinition assembly) { // assembly must have an entry point to be examined - MethodDefinition entryPoint = assemblyDefinition.EntryPoint; - if (entryPoint == null) - return runner.RuleSuccess; + MethodDefinition entry_point = assembly.EntryPoint; + if (entry_point == null) + return RuleResult.DoesNotApply; // RULE APPLIES @@ -59,25 +52,20 @@ namespace Gendarme.Rules.Design { // if we can't get access to it (is this possible?) we abandon // also, if it is not public, we don't have to continue our work // - we can't reach Main () anyways - TypeDefinition declaringType = entryPoint.DeclaringType as TypeDefinition; - if (declaringType == null || !declaringType.IsPublic) - return runner.RuleSuccess; + TypeDefinition type = entry_point.DeclaringType.Resolve (); + if (type == null || !type.IsPublic) + return RuleResult.Success; // at last, if Main () is not public, then it's okay - if (!entryPoint.IsPublic) - return runner.RuleSuccess; + if (!entry_point.IsPublic) + return RuleResult.Success; - Location loc; - string message; - if (!IsVBAssembly (assemblyDefinition)) { - loc = new Location (entryPoint); - message = "Main () method should not be visible outside the assembly. Make it private or internal."; + if (assembly.References (VisualBasic)) { + Runner.Report (type, Severity.Medium, Confidence.High, "Reduce class or module visibility (from public)."); } else { - loc = new Location (declaringType); // point at Module - message = "Main () method should not be visible outside the assembly. Do not make the module or class containing it public."; + Runner.Report (entry_point, Severity.Medium, Confidence.Total, "Change method visibility to private or internal."); } - Message msg = new Message (message, loc, MessageType.Error); - return new MessageCollection (msg); + return RuleResult.Failure; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/MissingAttributeUsageOnCustomAttributeRule.cs b/gendarme/rules/Gendarme.Rules.Design/MissingAttributeUsageOnCustomAttributeRule.cs index 8d26b6b0..ecf9c745 100644 --- a/gendarme/rules/Gendarme.Rules.Design/MissingAttributeUsageOnCustomAttributeRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/MissingAttributeUsageOnCustomAttributeRule.cs @@ -33,22 +33,23 @@ using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class MissingAttributeUsageOnCustomAttributeRule : ITypeRule { + [Problem ("This attribute does not specify on which items it can be used.")] + [Solution ("Specify [AttributeUsage] on this attribute type.")] + public class MissingAttributeUsageOnCustomAttributeRule : Rule, ITypeRule { private const string AttributeUsageAttribute = "System.AttributeUsageAttribute"; - 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.HasAttribute (AttributeUsageAttribute)) // it's ok - return runner.RuleSuccess; + return RuleResult.Success; - Location loc = new Location (type); - Message msg = new Message ("When declaring an attribute, you should specify its usage targets using 'AttributeUsage' attribute to avoid one's wrong use.", loc, MessageType.Error); - return new MessageCollection (msg); + Runner.Report (type, Severity.High, Confidence.Total, String.Empty); + return RuleResult.Failure; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/OperatorEqualsShouldBeOverloadedRule.cs b/gendarme/rules/Gendarme.Rules.Design/OperatorEqualsShouldBeOverloadedRule.cs index 939dcb37..8e758652 100644 --- a/gendarme/rules/Gendarme.Rules.Design/OperatorEqualsShouldBeOverloadedRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/OperatorEqualsShouldBeOverloadedRule.cs @@ -29,39 +29,33 @@ using System; using Mono.Cecil; using Gendarme.Framework; +using Gendarme.Framework.Helpers; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class OperatorEqualsShouldBeOverloadedRule : ITypeRule { + [Problem ("This type overloads the == operator but doesn't override the Equals method.")] + [Solution ("Override the Equals method to match the results of the == operator.")] + public class OperatorEqualsShouldBeOverloadedRule : Rule, ITypeRule { - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { if (type.IsEnum || type.IsInterface) - return runner.RuleSuccess; - - MessageCollection results = null; + return RuleResult.DoesNotApply; if (type.HasMethod (MethodSignatures.op_Addition) && type.HasMethod (MethodSignatures.op_Subtraction)) { if (!type.HasMethod (MethodSignatures.op_Equality)) { - results = new MessageCollection (); - Location loc = new Location (type); - Message msg = new Message ("This type implements the addition (+) and subtraction (-) operators. It should also implement the equality (==) operator.", loc, MessageType.Warning); - results.Add (msg); + Runner.Report (type, Severity.Low, Confidence.High, "This type implements the addition (+) and subtraction (-) operators. It should also implement the equality (==) operator."); } } if (type.IsValueType) { if (type.HasMethod (MethodSignatures.Equals) && !type.HasMethod (MethodSignatures.op_Equality)) { - if (results == null) - results = new MessageCollection (); - Location loc = new Location (type); - Message msg = new Message ("This type overrides Object.Equals. It should also implement the equality (==) operator.", loc, MessageType.Warning); - results.Add (msg); + Runner.Report (type, Severity.Medium, Confidence.High, "This type overrides Object.Equals. It should also implement the equality (==) operator."); } } - return results; + return Runner.CurrentRuleResult; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/OverrideEqualsMethodRule.cs b/gendarme/rules/Gendarme.Rules.Design/OverrideEqualsMethodRule.cs index 7d72f6a4..d5541ab4 100644 --- a/gendarme/rules/Gendarme.Rules.Design/OverrideEqualsMethodRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/OverrideEqualsMethodRule.cs @@ -29,25 +29,26 @@ using System; using Mono.Cecil; using Gendarme.Framework; +using Gendarme.Framework.Helpers; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class OverrideEqualsMethodRule : ITypeRule { + [Problem ("This type overloads the == operator but doesn't override the Equals method.")] + [Solution ("Override the Equals method to match the results of the == operator.")] + public class OverrideEqualsMethodRule : Rule, ITypeRule { - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { if (type.IsEnum || type.IsInterface) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; - if (type.HasMethod (MethodSignatures.op_Equality)) { - if (!type.HasMethod (MethodSignatures.Equals)) { - Location loc = new Location (type); - Message msg = new Message ("This type implements the equality (==) operator. It should also override the Object.Equals method.", loc, MessageType.Warning); - return new MessageCollection (msg); - } - } - return runner.RuleSuccess; + MethodDefinition equality = type.GetMethod (MethodSignatures.op_Equality); + if ((equality == null) || type.HasMethod (MethodSignatures.Equals)) + return RuleResult.Success; + + Runner.Report (equality, Severity.High, Confidence.High, String.Empty); + return RuleResult.Failure; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/ProvideAlternativeNamesForOperatorOverloadsRule.cs b/gendarme/rules/Gendarme.Rules.Design/ProvideAlternativeNamesForOperatorOverloadsRule.cs index a76e6180..6f60a97f 100644 --- a/gendarme/rules/Gendarme.Rules.Design/ProvideAlternativeNamesForOperatorOverloadsRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/ProvideAlternativeNamesForOperatorOverloadsRule.cs @@ -29,12 +29,16 @@ using System; using System.Collections.Generic; using Mono.Cecil; + using Gendarme.Framework; +using Gendarme.Framework.Helpers; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class ProvideAlternativeNamesForOperatorOverloadsRule : ITypeRule { + [Problem ("This type contains overloads for some operators but doesn't provide named alternatives.")] + [Solution ("Add named methods equivalent to the operators for language that do not support them (e.g. VB.NET).")] + public class ProvideAlternativeNamesForOperatorOverloadsRule : Rule, ITypeRule { static string [] NoParameter = new string [] { }; static string [] OneParameter = new string [] { null }; //new string [1] = one parameter of any type @@ -76,12 +80,10 @@ namespace Gendarme.Rules.Design { new KeyValuePair<MethodSignature,MethodSignature> (MethodSignatures.op_LessThanOrEqual, Compare), }; - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { if (type.IsEnum || type.IsInterface) - return runner.RuleSuccess; - - MessageCollection results = null; + return RuleResult.DoesNotApply; foreach (var kv in AlternativeMethodNames) { MethodDefinition op = type.GetMethod (kv.Key); @@ -96,17 +98,13 @@ namespace Gendarme.Rules.Design { } if (!alternativeDefined) { - if (results == null) - results = new MessageCollection (); - Location loc = new Location (op); string s = String.Format ("This type implements the '{0}' operator. Some languages do not support overloaded operators so an alternative '{1}' method should be provided.", kv.Key.Name, kv.Value.Name); - Message msg = new Message (s, loc, MessageType.Warning); - results.Add (msg); + Runner.Report (op, Severity.Medium, Confidence.High, s); } } - return results; + return Runner.CurrentRuleResult; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/TypesShouldBeInsideNamespacesRule.cs b/gendarme/rules/Gendarme.Rules.Design/TypesShouldBeInsideNamespacesRule.cs index cceffe6c..76b15acc 100644 --- a/gendarme/rules/Gendarme.Rules.Design/TypesShouldBeInsideNamespacesRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/TypesShouldBeInsideNamespacesRule.cs @@ -33,26 +33,28 @@ using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class TypesShouldBeInsideNamespacesRule: ITypeRule { + [Problem ("This type is visible outside the assembly so it should be defined inside a namespace to avoid conflicts.")] + [Solution ("Move this type inside a namespace or reduce it's visibility (e.g. internal or private).")] + public class TypesShouldBeInsideNamespacesRule: Rule, ITypeRule { - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // rule doesn't apply to nested types, since the declaring type will already be reported if (type.IsNested) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; // rule applies to only to types visible outside the assembly if (!type.IsVisible ()) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; // rule applies! // check if the type resides inside a namespace if (!String.IsNullOrEmpty (type.Namespace)) - return runner.RuleSuccess; + return RuleResult.Success; - Message msg = new Message ("Type is not declared inside a namespace.", new Location (type), MessageType.Warning); - return new MessageCollection (msg); + Runner.Report (type, Severity.Low, Confidence.Total, String.Empty); + return RuleResult.Failure; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/TypesWithDisposableFieldsShouldBeDisposableRule.cs b/gendarme/rules/Gendarme.Rules.Design/TypesWithDisposableFieldsShouldBeDisposableRule.cs index 34487909..ea9984b0 100644 --- a/gendarme/rules/Gendarme.Rules.Design/TypesWithDisposableFieldsShouldBeDisposableRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/TypesWithDisposableFieldsShouldBeDisposableRule.cs @@ -35,17 +35,24 @@ using System.IO; using Mono.Cecil; using Mono.Cecil.Cil; using Gendarme.Framework; +using Gendarme.Framework.Helpers; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class TypesWithDisposableFieldsShouldBeDisposableRule : ITypeRule { + [Problem ("This type contains disposable field(s) but doesn't implement IDisposable.")] + [Solution ("Implement IDisposable and free the disposable field(s) in it's Dispose method.")] + public class TypesWithDisposableFieldsShouldBeDisposableRule : Rule, ITypeRule { - public MessageCollection CheckType (TypeDefinition type, Runner runner) + private const string AbstractTypeMessage = "Field implement IDisposable. Type should implement a non-abstract Dispose() method"; + private const string TypeMessage = "Field implement IDisposable. Type should implement a Dispose() method"; + private const string AbstractDisposeMessage = "Some field(s) implement IDisposable. Making this method abstract shifts the reponsability of disposing those fields to the inheritors of this class."; + + public RuleResult CheckType (TypeDefinition type) { // rule doesn't apply to enums, interfaces or structs if (type.IsEnum || type.IsInterface || type.IsValueType) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; MethodDefinition explicitDisposeMethod = null; MethodDefinition implicitDisposeMethod = null; @@ -56,56 +63,37 @@ namespace Gendarme.Rules.Design { implicitDisposeMethod = type.GetMethod (MethodSignatures.Dispose); explicitDisposeMethod = type.GetMethod (MethodSignatures.DisposeExplicit); - if (IsAbstractMethod (implicitDisposeMethod)) - abstractWarning = true; - if (IsAbstractMethod (explicitDisposeMethod)) + if (((implicitDisposeMethod != null) && implicitDisposeMethod.IsAbstract) || + ((explicitDisposeMethod != null) && explicitDisposeMethod.IsAbstract)) { abstractWarning = true; - - if (abstractWarning == false) - return runner.RuleSuccess; + } else { + return RuleResult.Success; + } } - MessageCollection results = null; - foreach (FieldDefinition field in type.Fields) { // we can't dispose static fields in IDisposable if (field.IsStatic) continue; - TypeDefinition fieldType = field.FieldType.GetOriginalType () as TypeDefinition; + TypeDefinition fieldType = field.FieldType.GetOriginalType ().Resolve (); if (fieldType == null) - continue; //TODO: Implemts for TypeReference + continue; if (fieldType.Implements ("System.IDisposable")) { - if (results == null) - results = new MessageCollection (); - Location loc = new Location (field); if (abstractWarning) - results.Add (new Message (string.Format ("{1} is Disposeable. {0} shoud implement a non abstract Dispose() method.", type.FullName, field.Name), loc, MessageType.Warning)); + Runner.Report (field, Severity.High, Confidence.High, AbstractTypeMessage); else - results.Add (new Message (string.Format ("{1} is Disposeable. {0} should implement System.IDisposable to release unmanaged resources.", type.FullName, field.Name), loc, MessageType.Error)); - + Runner.Report (field, Severity.High, Confidence.High, TypeMessage); } } - if (results == null) - return runner.RuleSuccess; + // Warn about possible confusion if the Dispose methods are abstract + if (implicitDisposeMethod != null && implicitDisposeMethod.IsAbstract) + Runner.Report (implicitDisposeMethod, Severity.Medium, Confidence.High, AbstractDisposeMessage); - if (IsAbstractMethod (implicitDisposeMethod)) - results.Add (GenerateAbstractWarning (implicitDisposeMethod)); - if (IsAbstractMethod (explicitDisposeMethod)) - results.Add (GenerateAbstractWarning (explicitDisposeMethod)); + if (explicitDisposeMethod != null && explicitDisposeMethod.IsAbstract) + Runner.Report (explicitDisposeMethod, Severity.Medium, Confidence.High, AbstractDisposeMessage); - return results; - } - - private static bool IsAbstractMethod (MethodDefinition method) - { - return method != null && method.IsAbstract; - } - - private static Message GenerateAbstractWarning (MethodDefinition method) - { - Location loc = new Location (method); - return new Message (string.Format ("{0} has at least one Disposable field. Marking {1}() as abstract shifts the job of disposing those fields to the inheritors of this class.", method.DeclaringType.FullName, method.Name), loc, MessageType.Warning); + return Runner.CurrentRuleResult; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/TypesWithNativeFieldsShouldBeDisposableRule.cs b/gendarme/rules/Gendarme.Rules.Design/TypesWithNativeFieldsShouldBeDisposableRule.cs index 7ecf910e..c830167f 100644 --- a/gendarme/rules/Gendarme.Rules.Design/TypesWithNativeFieldsShouldBeDisposableRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/TypesWithNativeFieldsShouldBeDisposableRule.cs @@ -29,17 +29,24 @@ using System; using Mono.Cecil; using Gendarme.Framework; +using Gendarme.Framework.Helpers; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class TypesWithNativeFieldsShouldBeDisposableRule : ITypeRule { + [Problem ("This type contains native field(s) but doesn't implement IDisposable.")] + [Solution ("Implement IDisposable and free the native field(s) in it's Dispose method.")] + public class TypesWithNativeFieldsShouldBeDisposableRule : Rule, ITypeRule { - public MessageCollection CheckType (TypeDefinition type, Runner runner) + private const string AbstractTypeMessage = "Field is native. Type should implement a non-abstract Dispose() method"; + private const string TypeMessage = "Field is native. Type should implement a Dispose() method"; + private const string AbstractDisposeMessage = "Some fields are native pointers. Making this method abstract shifts the reponsability of disposing those fields to the inheritors of this class."; + + public RuleResult CheckType (TypeDefinition type) { // rule doesn't apply to enums, interfaces or structs if (type.IsEnum || type.IsInterface || type.IsValueType) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; MethodDefinition explicitDisposeMethod = null; MethodDefinition implicitDisposeMethod = null; @@ -50,52 +57,34 @@ namespace Gendarme.Rules.Design { implicitDisposeMethod = type.GetMethod (MethodSignatures.Dispose); explicitDisposeMethod = type.GetMethod (MethodSignatures.DisposeExplicit); - if (IsAbstractMethod (implicitDisposeMethod)) - abstractWarning = true; - if (IsAbstractMethod (explicitDisposeMethod)) + if (((implicitDisposeMethod != null) && implicitDisposeMethod.IsAbstract) || + ((explicitDisposeMethod != null) && explicitDisposeMethod.IsAbstract)) { abstractWarning = true; - - if (abstractWarning == false) - return runner.RuleSuccess; + } else { + return RuleResult.Success; + } } - MessageCollection results = null; - foreach (FieldDefinition field in type.Fields) { // we can't dispose static fields in IDisposable if (field.IsStatic) continue; if (field.FieldType.GetOriginalType ().IsNative ()) { - if (results == null) - results = new MessageCollection (); - Location loc = new Location (field); if (abstractWarning) - results.Add (new Message (string.Format ("{1} is a native field. {0} shoud implement a non abstract Dispose() method.", type.FullName, field.Name), loc, MessageType.Warning)); + Runner.Report (field, Severity.High, Confidence.High, AbstractTypeMessage); else - results.Add (new Message (string.Format ("{1} is a native field. {0} should implement System.IDisposable to release unmanaged resources.", type.FullName, field.Name), loc, MessageType.Error)); + Runner.Report (field, Severity.High, Confidence.High, TypeMessage); } } - if (results == null) - return runner.RuleSuccess; //no native fields + // Warn about possible confusion if the Dispose methods are abstract + if (implicitDisposeMethod != null && implicitDisposeMethod.IsAbstract) + Runner.Report (implicitDisposeMethod, Severity.Medium, Confidence.High, AbstractDisposeMessage); - if (IsAbstractMethod (implicitDisposeMethod)) - results.Add (GenerateAbstractWarning (implicitDisposeMethod)); - if (IsAbstractMethod (explicitDisposeMethod)) - results.Add (GenerateAbstractWarning (explicitDisposeMethod)); + if (explicitDisposeMethod != null && explicitDisposeMethod.IsAbstract) + Runner.Report (explicitDisposeMethod, Severity.Medium, Confidence.High, AbstractDisposeMessage); - return results; - } - - private static bool IsAbstractMethod (MethodDefinition method) - { - return method != null && method.IsAbstract; - } - - private static Message GenerateAbstractWarning (MethodDefinition method) - { - Location loc = new Location (method); - return new Message (string.Format ("{0} has at least one native field. Marking {1}() as abstract shifts the job of releasing those fields to the inheritors of this class.", method.DeclaringType.FullName, method.Name), loc, MessageType.Warning); + return Runner.CurrentRuleResult; } } } diff --git a/gendarme/rules/Gendarme.Rules.Design/UsingCloneWithoutImplementingICloneableRule.cs b/gendarme/rules/Gendarme.Rules.Design/UsingCloneWithoutImplementingICloneableRule.cs index bcf1c399..03b32cac 100644 --- a/gendarme/rules/Gendarme.Rules.Design/UsingCloneWithoutImplementingICloneableRule.cs +++ b/gendarme/rules/Gendarme.Rules.Design/UsingCloneWithoutImplementingICloneableRule.cs @@ -6,7 +6,7 @@ // Sebastien Pouliot <sebastien@ximian.com> // // Copyright (c) <2007> Nidhi Rawal -// Copyright (C) 2007 Novell, Inc (http://www.novell.com) +// Copyright (C) 2007-2008 Novell, Inc (http://www.novell.com) // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal @@ -31,43 +31,41 @@ using System; using Mono.Cecil; using Gendarme.Framework; +using Gendarme.Framework.Helpers; using Gendarme.Framework.Rocks; namespace Gendarme.Rules.Design { - public class UsingCloneWithoutImplementingICloneableRule: ITypeRule { + [Problem ("This type provides a Clone() method but does not implement the ICloneable interface.")] + [Solution ("Implement the ICloneable interface.")] + public class UsingCloneWithoutImplementingICloneableRule: Rule, ITypeRule { - // copy-pasted from gendarme\rules\Gendarme.Rules.BadPractice\CloneMethodShouldNotReturnNullRule.cs private static bool IsCloneMethod (MethodDefinition method) { return (method.Name == "Clone" && (method.Parameters.Count == 0)); } - public MessageCollection CheckType (TypeDefinition type, Runner runner) + public RuleResult CheckType (TypeDefinition type) { // rule applies to type that doesn't implement System.IClonable if (type.Implements ("System.ICloneable")) - return runner.RuleSuccess; + return RuleResult.DoesNotApply; - MessageCollection mc = null; foreach (MethodDefinition method in type.Methods) { - // we check for methods name Clone + // note: we don't use MethodSignatures.Clone because we want to + // (a) check the return value ourselves + // (b) deal with possibly multiple Clone methods if (!IsCloneMethod (method)) continue; // that return System.Object, e.g. public object Clone() // or the current type, e.g. public <type> Clone() if ((method.ReturnType.ReturnType.FullName == "System.Object") || (method.ReturnType.ReturnType == type)) { - Location location = new Location (type); - Message message = new Message ("A Clone() method is provided but System.ICloneable is not implemented", location, MessageType.Error); - if (mc == null) - mc = new MessageCollection (message); - else - mc.Add (message); + Runner.Report (method, Severity.Low, Confidence.High, String.Empty); } } - return mc; + return Runner.CurrentRuleResult; } } } |