diff options
author | Sebastien Pouliot <sebastien@ximian.com> | 2008-02-15 22:13:38 +0300 |
---|---|---|
committer | Sebastien Pouliot <sebastien@ximian.com> | 2008-02-15 22:13:38 +0300 |
commit | 06048ff0fb3db23763890e456a5780b27fef0610 (patch) | |
tree | 5b54d8a66a5bc87d5dba2c8f53d02e0628368c64 /gendarme/rules | |
parent | 6eb27f47fd57f0a9f937db5a1512dfeceb426935 (diff) | |
parent | d7967e2385b4fb3c8af16029c2bd8896de8371b8 (diff) |
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
Diffstat (limited to 'gendarme/rules')
-rw-r--r-- | gendarme/rules/Gendarme.Rules.Exceptions/ChangeLog | 7 | ||||
-rw-r--r-- | gendarme/rules/Gendarme.Rules.Exceptions/DoNotDestroyStackTraceRule.cs (renamed from gendarme/rules/Gendarme.Rules.Exceptions/DontDestroyStackTrace.cs) | 62 | ||||
-rw-r--r-- | gendarme/rules/Gendarme.Rules.Exceptions/DontSwallowErrorsCatchingNonspecificExceptionsRule.cs | 29 | ||||
-rw-r--r-- | gendarme/rules/Gendarme.Rules.Exceptions/Makefile.am | 4 |
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) |