diff --git a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj index 353b9ef022..e3ed1d21d4 100644 --- a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj +++ b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj @@ -426,6 +426,7 @@ + diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExceptionRethrowIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExceptionRethrowIssue.cs new file mode 100644 index 0000000000..c822e003a2 --- /dev/null +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExceptionRethrowIssue.cs @@ -0,0 +1,130 @@ +// +// ExceptionRethrowIssue.cs +// +// Author: +// Simon Lindgren +// +// Copyright (c) 2012 Simon Lindgren +// +// 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.Generic; +using ICSharpCode.NRefactory.TypeSystem; +using ICSharpCode.NRefactory.Semantics; + +namespace ICSharpCode.NRefactory.CSharp.Refactoring +{ + [IssueDescription("A throw statement throws the caught exception by passing it explicitly", + Description = "Finds throws that throws the caught exception and therefore should be empty.", + Category = IssueCategories.CodeQualityIssues, + Severity = Severity.Warning)] + public class ExceptionRethrowIssue : ICodeIssueProvider + { + public IEnumerable GetIssues(BaseRefactoringContext context) + { + return new GatherVisitor(context).GetIssues(); + } + + class GatherVisitor : GatherVisitorBase + { + public GatherVisitor(BaseRefactoringContext context) : base (context) + { + } + + public override void VisitCatchClause(CatchClause catchClause) + { + base.VisitCatchClause(catchClause); + var exceptionResolveResult = ctx.Resolve(catchClause.VariableNameToken) as LocalResolveResult; + if (exceptionResolveResult == null) + return; + + var catchVisitor = new CatchClauseVisitor(ctx, exceptionResolveResult.Variable); + catchClause.Body.AcceptVisitor(catchVisitor); + + foreach (var throwStatement in catchVisitor.OffendingThrows) { + var localThrowStatement = throwStatement; + var title = ctx.TranslateString("The exception is rethrown with explicit usage of the variable"); + var action = new CodeAction(ctx.TranslateString("Change to 'throw;'"), script => { + script.Replace(localThrowStatement, new ThrowStatement()); + }); + AddIssue(localThrowStatement, title, new [] { action }); + } + } + } + + class CatchClauseVisitor : DepthFirstAstVisitor + { + BaseRefactoringContext ctx; + + IVariable parameter; + + bool variableWritten = false; + + public CatchClauseVisitor(BaseRefactoringContext context, IVariable parameter) + { + ctx = context; + this.parameter = parameter; + OffendingThrows = new List(); + } + + public IList OffendingThrows { get; private set; } + + void HandlePotentialWrite (Expression expression) + { + var variableResolveResult = ctx.Resolve(expression) as LocalResolveResult; + if (variableResolveResult == null) + return; + variableWritten |= variableResolveResult.Equals(parameter); + } + + public override void VisitAssignmentExpression(AssignmentExpression assignmentExpression) + { + base.VisitAssignmentExpression(assignmentExpression); + + var variableResolveResult = ctx.Resolve(assignmentExpression.Left) as LocalResolveResult; + if (variableResolveResult == null) + return; + variableWritten |= variableResolveResult.Variable.Equals(parameter); + } + + public override void VisitDirectionExpression(DirectionExpression directionExpression) + { + base.VisitDirectionExpression(directionExpression); + + HandlePotentialWrite(directionExpression); + } + + public override void VisitThrowStatement(ThrowStatement throwStatement) + { + base.VisitThrowStatement(throwStatement); + + if (variableWritten) + return; + + var argumentResolveResult = ctx.Resolve(throwStatement.Expression) as LocalResolveResult; + if (argumentResolveResult == null) + return; + if (parameter.Equals(argumentResolveResult.Variable)) + OffendingThrows.Add(throwStatement); + } + } + + } +} + diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/ExceptionRethrowTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/ExceptionRethrowTests.cs new file mode 100644 index 0000000000..b9b899208f --- /dev/null +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/ExceptionRethrowTests.cs @@ -0,0 +1,88 @@ +// +// ExceptionRethrowIssue.cs +// +// Author: +// Simon Lindgren +// +// Copyright (c) 2012 Simon Lindgren +// +// 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 NUnit.Framework; +using ICSharpCode.NRefactory.CSharp.CodeActions; +using ICSharpCode.NRefactory.CSharp.Refactoring; + +namespace ICSharpCode.NRefactory.CSharp.CodeIssues +{ + public class ExceptionRethrowTests : InspectionActionTestBase + { + [Test] + public void BasicCase () + { + var input = @" +class Foo +{ + void Bar (int i) + { + try {} + catch (Exception e) { + throw e; + } + } +}"; + + TestRefactoringContext context; + var issues = GetIssues (new ExceptionRethrowIssue (), input, out context); + Assert.AreEqual (1, issues.Count); + CheckFix (context, issues, @" +class Foo +{ + void Bar (int i) + { + try {} + catch (Exception e) { + throw; + } + } +}"); + } + + [Test] + public void HandlesSimpleWriteToVariable () + { + var input = @" +class Foo +{ + void Bar (int i) + { + try {} + catch (Exception e) { + e = new Exception(e); + throw e; + } + } +}"; + + TestRefactoringContext context; + var issues = GetIssues (new ExceptionRethrowIssue (), input, out context); + Assert.AreEqual (0, issues.Count); + } + } +} + diff --git a/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj b/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj index 262d3af6ca..9c8f1b4049 100644 --- a/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj +++ b/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj @@ -341,6 +341,7 @@ +