diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantCatchIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantCatchIssue.cs index 9887a4642a..6a25508a5d 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantCatchIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantCatchIssue.cs @@ -25,6 +25,7 @@ // THE SOFTWARE. using System; using System.Collections.Generic; +using ICSharpCode.NRefactory.TypeSystem; namespace ICSharpCode.NRefactory.CSharp.Refactoring { @@ -122,17 +123,35 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring AddIssue(tryCatchStatement.TryBlock.EndLocation, lastCatch.EndLocation, removeTryCatchMessage, fixes); } - bool IsRedundant(CatchClause catchClause) + static bool IsThrowsClause (CatchClause catchClause) { var firstStatement = catchClause.Body.Statements.FirstOrNullObject(); - if (firstStatement.IsNull) { + if (firstStatement.IsNull) return false; - } var throwStatement = firstStatement as ThrowStatement; - if (throwStatement == null) { + if (throwStatement == null) return false; + return true; + } + + bool IsRedundant(CatchClause catchClause) + { + if (!IsThrowsClause (catchClause)) + return false; + + var type = ctx.Resolve (catchClause.Type).Type; + var n = catchClause.NextSibling; + while (n != null) { + var nextClause = n as CatchClause; + if (nextClause != null) { + if (nextClause.Type.IsNull && !IsThrowsClause(nextClause)) + return false; + if (!IsThrowsClause(nextClause) && type.GetDefinition ().IsDerivedFrom (ctx.Resolve (nextClause.Type).Type.GetDefinition ())) + return false; + } + n = n.NextSibling; } - return throwStatement.Expression.IsNull; + return true; } } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantCatchTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantCatchTests.cs index 7a67ccb6d7..afae8f7399 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantCatchTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantCatchTests.cs @@ -218,6 +218,79 @@ class A } }"); } + + /// + /// Bug 12273 - Incorrect redundant catch warning + /// + [Test] + public void TestBug12273() + { + var input = BaseInput + @" + try { + F (); + } catch (ArgumentOutOfRangeException) { + throw; + } catch (Exception e) { + Console.WriteLine (e); + } + } +}"; + TestRefactoringContext context; + var issues = GetIssues(new RedundantCatchIssue(), input, out context); + Assert.AreEqual(0, issues.Count); + + input = BaseInput + @" + try { + F (); + } catch (ArgumentOutOfRangeException) { + throw; + } catch (Exception e) { + throw; + } + } +}"; + issues = GetIssues(new RedundantCatchIssue(), input, out context); + Assert.AreEqual(1, issues.Count); + CheckFix(context, issues, BaseInput + @" + F (); + } +}"); + + } + + /// + /// Bug 12273 - Incorrect redundant catch warning + /// + [Test] + public void TestBug12273Case2() + { + var input = BaseInput + @" + try { + F (); + } catch (ArgumentOutOfRangeException) { + throw; + } catch { + Console.WriteLine (""hello world""); + } + } +}"; + TestRefactoringContext context; + var issues = GetIssues(new RedundantCatchIssue(), input, out context); + Assert.AreEqual(0, issues.Count); + + input = BaseInput + @" + try { + F (); + } catch (ArgumentOutOfRangeException) { + throw; + } catch { + throw; + } + } +}"; + issues = GetIssues(new RedundantCatchIssue(), input, out context); + Assert.AreEqual(1, issues.Count); + } } }