2008-02-15 Sebastien Pouliot <sebastien@ximian.com>

	* DoNotDestroyStackTraceRule.cs: (renamed from DontDestroyStackTrace.cs)
	* DontSwallowErrorsCatchingNonspecificExceptionsRule.cs:
		Updated rules wrt framework changes.
	* Makefile.am: Adjusted for file name changes.


svn path=/trunk/mono-tools/; revision=95791

4 files changed, 64 insertions, 38 deletions

diff --git a/gendarme/rules/Gendarme.Rules.Exceptions/ChangeLog b/gendarme/rules/Gendarme.Rules.Exceptions/ChangeLog
index 6f075133..cebb32d4 100644
--- a/gendarme/rules/Gendarme.Rules.Exceptions/ChangeLog
+++ b/gendarme/rules/Gendarme.Rules.Exceptions/ChangeLog
@@ -1,3 +1,10 @@
+2008-02-15  Sebastien Pouliot  <sebastien@ximian.com> 
+
+	* DoNotDestroyStackTraceRule.cs: (renamed from DontDestroyStackTrace.cs)
+	* DontSwallowErrorsCatchingNonspecificExceptionsRule.cs:
+		Updated rules wrt framework changes.
+	* Makefile.am: Adjusted for file name changes.
+
 2008-01-11  Sebastien Pouliot  <sebastien@ximian.com> 
 
 	* DontDestroyStackTrace.cs: Use new Location ctors. Change some 
diff --git a/gendarme/rules/Gendarme.Rules.Exceptions/DontDestroyStackTrace.cs b/gendarme/rules/Gendarme.Rules.Exceptions/DoNotDestroyStackTraceRule.cs
index ec793aff..d81af634 100644
--- a/gendarme/rules/Gendarme.Rules.Exceptions/DontDestroyStackTrace.cs
+++ b/gendarme/rules/Gendarme.Rules.Exceptions/DoNotDestroyStackTraceRule.cs
@@ -1,27 +1,57 @@
+//
+// DoNotDestroyStackTraceRule
+//
+// 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 in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to
+// permit persons to whom the Software is furnished to do so, subject to
+// the following conditions:
+// 
+// The above copyright notice and this permission notice shall be
+// included in all copies or substantial portions of the Software.
+// 
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+// LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+// WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+//
+
 using System;
-using System.Collections;
+using System.Collections.Generic;
+
 using Mono.Cecil;
 using Mono.Cecil.Cil;
+
 using Gendarme.Framework;
 using Gendarme.Rules.Exceptions.Impl;
 
 namespace Gendarme.Rules.Exceptions {
 
-	public class DontDestroyStackTrace : IMethodRule {
+	[Problem ("A catch block in the method throws back the caught exception which destroys the stack trace.")]
+	[Solution ("If you need to throw the exception caught by the catch block, use 'throw;' instead of 'throw ex;'")]
+	public class DontDestroyStackTrace : Rule, IMethodRule {
 
 		private TypeReference void_reference;
-		private ArrayList warned_offsets_in_method;
+		private List<int> warned_offsets_in_method = new List<int> ();
 
-		public MessageCollection CheckMethod (MethodDefinition method, Runner runner)
+		public RuleResult CheckMethod (MethodDefinition method)
 		{
+			// rule only applies to methods with IL
 			if (!method.HasBody)
-				return runner.RuleSuccess;
+				return RuleResult.DoesNotApply;
 
 			ModuleDefinition module = method.DeclaringType.Module;
 			if (void_reference == null)
 				void_reference = module.Import (typeof (void));
 
-			ArrayList executionPaths = new ArrayList ();
+			List<ExecutionPath> executionPaths = new List<ExecutionPath> ();
 			ExecutionPathFactory epf = new ExecutionPathFactory (method);
 			ISEHGuardedBlock[] guardedBlocks = ExceptionBlockParser.GetExceptionBlocks (method);
 			foreach (ISEHGuardedBlock guardedBlock in guardedBlocks) {
@@ -36,19 +66,16 @@ namespace Gendarme.Rules.Exceptions {
 				}
 			}
 
-			MessageCollection violations = new MessageCollection ();
-			warned_offsets_in_method = new ArrayList ();
+			warned_offsets_in_method.Clear ();
 
 			// Look for paths that 'throw ex;' instead of 'throw'
 			foreach (ExecutionPath catchPath in executionPaths)
-				ProcessCatchPath (catchPath, method, violations);
+				ProcessCatchPath (catchPath, method);
 
-			return (violations.Count == 0) ? null : violations;
+			return Runner.CurrentRuleResult;
 		}
 
-		private void ProcessCatchPath (ExecutionPath catchPath,
-					       MethodDefinition method,
-					       MessageCollection violations)
+		private void ProcessCatchPath (ExecutionPath catchPath, MethodDefinition method)
 		{
 			// Track original exception (top of stack at start) through to the final
 			// return (be it throw, rethrow, leave, or leave.s)
@@ -102,13 +129,7 @@ namespace Gendarme.Rules.Exceptions {
 						// If our original exception is on top of the stack,
 						// we're rethrowing it.This is deemed naughty...
 						if (!warned_offsets_in_method.Contains(cur.Offset)) {
-							Location loc =
-								new Location (method, cur.Offset);
-							Message msg = new Message (
-								"Throwing original exception - destroys stack trace!",
-								loc,
-								MessageType.Error);
-							violations.Add (msg);
+							Runner.Report (method, cur, Severity.Critical, Confidence.High, String.Empty);
 							warned_offsets_in_method.Add (cur.Offset);
 						}
 						return;
@@ -126,7 +147,6 @@ namespace Gendarme.Rules.Exceptions {
 					}
 				}
 			}
-			return;
 		}
 
 		private static int GetNumPops (Instruction instr)
diff --git a/gendarme/rules/Gendarme.Rules.Exceptions/DontSwallowErrorsCatchingNonspecificExceptionsRule.cs b/gendarme/rules/Gendarme.Rules.Exceptions/DontSwallowErrorsCatchingNonspecificExceptionsRule.cs
index 1fcfc015..ef91d4e2 100644
--- a/gendarme/rules/Gendarme.Rules.Exceptions/DontSwallowErrorsCatchingNonspecificExceptionsRule.cs
+++ b/gendarme/rules/Gendarme.Rules.Exceptions/DontSwallowErrorsCatchingNonspecificExceptionsRule.cs
@@ -33,7 +33,10 @@ using Mono.Cecil.Cil;
 using Gendarme.Framework;
 
 namespace Gendarme.Rules.Exceptions {
-	public class DontSwallowErrorsCatchingNonspecificExceptionsRule : IMethodRule {
+
+	[Problem ("The method catch a nonspecific exception.")]
+	[Solution ("You can rethrow the original exception, to avoid destroying the stacktrace, or you can handle more specific exceptions.")]
+	public class DontSwallowErrorsCatchingNonspecificExceptionsRule : Rule, IMethodRule {
 
 		//Added System.Object because is the code behind the following block:
 		//try {
@@ -53,39 +56,35 @@ namespace Gendarme.Rules.Exceptions {
 			return false;
 		}
 
-		private static bool ThrowsGeneralException (ExceptionHandler exceptionHandler)
+		private static Instruction ThrowsGeneralException (ExceptionHandler exceptionHandler)
 		{
 			for (Instruction currentInstruction = exceptionHandler.HandlerStart; currentInstruction != exceptionHandler.HandlerEnd; currentInstruction = currentInstruction.Next) {
 				if (currentInstruction.OpCode.Code == Code.Rethrow)
-					return true;
+					return null;
 			}
-			return false;
+			return exceptionHandler.HandlerStart;
 		}
 
-		public MessageCollection CheckMethod (MethodDefinition methodDefinition, Runner runner)
+		public RuleResult CheckMethod (MethodDefinition methodDefinition)
 		{
+			// rule only applies to methods with IL
 			if (!methodDefinition.HasBody)
-				return runner.RuleSuccess;
+				return RuleResult.DoesNotApply;
 
-			MessageCollection messageCollection = null;
 			ExceptionHandlerCollection exceptionHandlerCollection = methodDefinition.Body.ExceptionHandlers;
 			foreach (ExceptionHandler exceptionHandler in exceptionHandlerCollection) {
 				if (exceptionHandler.Type == ExceptionHandlerType.Catch) {
 					string catchTypeName = exceptionHandler.CatchType.FullName;
 					if (IsForbiddenTypeInCatches (catchTypeName)) {
-						if (!ThrowsGeneralException (exceptionHandler)) {
-							Location location = new Location (methodDefinition, exceptionHandler.HandlerStart.Offset);
-							Message message = new Message ("Do not swallow errors catching nonspecific exceptions.", location, MessageType.Error);
-							if (messageCollection == null)
-								messageCollection = new MessageCollection (message);
-							else
-								messageCollection.Add (message);
+						Instruction throw_instruction = ThrowsGeneralException (exceptionHandler);
+						if (throw_instruction != null) {
+							Runner.Report (methodDefinition, throw_instruction, Severity.Medium, Confidence.High, String.Empty);
 						}
 					}
 				}
 			}
 
-			return messageCollection;
+			return Runner.CurrentRuleResult;
 		}
 	}
 }
diff --git a/gendarme/rules/Gendarme.Rules.Exceptions/Makefile.am b/gendarme/rules/Gendarme.Rules.Exceptions/Makefile.am
index 63dfdb5d..a01a4b3e 100644
--- a/gendarme/rules/Gendarme.Rules.Exceptions/Makefile.am
+++ b/gendarme/rules/Gendarme.Rules.Exceptions/Makefile.am
@@ -10,7 +10,7 @@ DISTCLEANFILES = Makefile.in Gendarme.Rules.Exceptions.xml TestResult.xml
 
 exceptions_rules_sources_in = ../../AssemblyInfo.cs.in
 exceptions_rules_generated_sources = $(exceptions_rules_sources_in:.in=)
-exceptions_rules_sources = DontDestroyStackTrace.cs ISEHCatchBlock.cs \
+exceptions_rules_sources = DoNotDestroyStackTraceRule.cs ISEHCatchBlock.cs \
 	ISEHGuardedBlock.cs ISEHHandlerBlock.cs \
 	SEHHandlerType.cs Impl/ExceptionBlockParser.cs Impl/ExecutionBlock.cs \
 	Impl/ExecutionPath.cs Impl/ExecutionPathFactory.cs Impl/SEHCatchBlock.cs \
@@ -24,7 +24,7 @@ exceptions_rules_build_sources += $(exceptions_rules_generated_sources)
 	$(GMCS) -debug -target:library -r:$(top_builddir)/gendarme/bin/Mono.Cecil.dll -r:../../bin/Gendarme.Framework.dll -out:$@ $(exceptions_rules_build_sources)
 	cp Gendarme.Rules.*.xml ../../bin/
 
-exceptions_test_sources = TestPatterns.cs DontSwallowErrorsCatchingNonspecificExceptionsTest.cs 
+exceptions_test_sources = DontDestroyStackTraceTest.cs DontSwallowErrorsCatchingNonspecificExceptionsTest.cs 
 exceptions_test_build_sources = $(addprefix $(srcdir)/Test/, $(exceptions_test_sources))
 
 Test.Rules.Exceptions.dll: $(exceptions_test_build_sources) $(exceptions_rules_SCRIPTS)