From f43d5df0e22c3477ccf32585aa19af731efc73a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Fri, 24 Aug 2012 06:55:30 +0200 Subject: [PATCH 01/29] [CodeIssues] New issue to convert .Where(p).Any() to .Any(p). (from shani) --- .../ICSharpCode.NRefactory.CSharp.csproj | 1 + .../RedundantWhereWithPredicateIssue.cs | 64 +++++++++++++++++ .../RedundantWhereWithPredicateIssueTests.cs | 70 +++++++++++++++++++ .../ICSharpCode.NRefactory.Tests.csproj | 1 + 4 files changed, 136 insertions(+) create mode 100644 ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantWhereWithPredicateIssue.cs create mode 100644 ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantWhereWithPredicateIssueTests.cs diff --git a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj index a62692406f..65cbb19826 100644 --- a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj +++ b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj @@ -498,6 +498,7 @@ + diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantWhereWithPredicateIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantWhereWithPredicateIssue.cs new file mode 100644 index 0000000000..45c930748f --- /dev/null +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantWhereWithPredicateIssue.cs @@ -0,0 +1,64 @@ +using System.Collections.Generic; +using System.Linq; +using ICSharpCode.NRefactory.Semantics; +using ICSharpCode.NRefactory.TypeSystem; +using ICSharpCode.NRefactory.PatternMatching; + +namespace ICSharpCode.NRefactory.CSharp.Refactoring +{ + [IssueDescription("Any() should be used with predicate and Where() removed", + Description= "Detects redundant Where() with predicate calls followed by Any().", + Category = IssueCategories.CodeQualityIssues, + Severity = Severity.Hint)] + public class RedundantWhereWithPredicateIssue : ICodeIssueProvider + { + static readonly AstNode pattern = + new InvocationExpression ( + new MemberReferenceExpression ( + new NamedNode ("whereInvoke", + new InvocationExpression ( + new MemberReferenceExpression (new AnyNode ("target"), "Where"), + new AnyNode ())), + "Any")); + + public IEnumerable GetIssues(BaseRefactoringContext context) + { + return new GatherVisitor(context).GetIssues(); + } + + class GatherVisitor : GatherVisitorBase + { + public GatherVisitor (BaseRefactoringContext ctx) : base (ctx) + { + } + + public override void VisitInvocationExpression (InvocationExpression anyInvoke) + { + base.VisitInvocationExpression (anyInvoke); + + var match = pattern.Match (anyInvoke); + if (!match.Success) + return; + + var anyResolve = ctx.Resolve (anyInvoke) as InvocationResolveResult; + if (anyResolve == null || anyResolve.Member.FullName != "System.Linq.Enumerable.Any") + return; + var whereInvoke = match.Get ("whereInvoke").Single (); + var whereResolve = ctx.Resolve (whereInvoke) as InvocationResolveResult; + if (whereResolve == null || whereResolve.Member.FullName != "System.Linq.Enumerable.Where") + return; + if (whereResolve.Member.Parameters.Count != 2) + return; + var predResolve = whereResolve.Member.Parameters [1]; + if (predResolve.Type.TypeParameterCount != 2) + return; + + AddIssue (anyInvoke, "Redundant Where() call with predicate followed by Any()", script => { + var arg = whereInvoke.Arguments.Single ().Clone (); + var target = match.Get ("target").Single ().Clone (); + script.Replace (anyInvoke, new InvocationExpression (new MemberReferenceExpression (target, "Any"), arg)); + }); + } + } + } +} diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantWhereWithPredicateIssueTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantWhereWithPredicateIssueTests.cs new file mode 100644 index 0000000000..9c712e6726 --- /dev/null +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantWhereWithPredicateIssueTests.cs @@ -0,0 +1,70 @@ +using System; +using NUnit.Framework; +using ICSharpCode.NRefactory.CSharp.Refactoring; +using ICSharpCode.NRefactory.CSharp.CodeActions; + +namespace ICSharpCode.NRefactory.CSharp.CodeIssues +{ + [TestFixture] + public class RedundantWhereWithPredicateIssueTests : InspectionActionTestBase + { + [Test] + public void TestWhereAnyCase1 () + { + var input = @"using System.Linq; +public class CSharpDemo { + public void Bla () { + int[] arr; + var bla = arr.Where (x => x < 4).Any (); + } +}"; + + TestRefactoringContext context; + var issues = GetIssues (new RedundantWhereWithPredicateIssue (), input, out context); + Assert.AreEqual (1, issues.Count); + CheckFix (context, issues, @"using System.Linq; +public class CSharpDemo { + public void Bla () { + int[] arr; + var bla = arr.Any (x => x < 4); + } +}"); + } + + [Test] + public void TestWhereAnyWrongWhere1 () + { + var input = @"using System.Linq; +public class CSharpDemo { + public void Bla () { + int[] arr; + var bla = arr.Where ((x, i) => x + i < 4).Any (); + } +}"; + + TestRefactoringContext context; + var issues = GetIssues (new RedundantWhereWithPredicateIssue (), input, out context); + Assert.AreEqual (0, issues.Count); + } + + [Test] + public void TestWhereAnyWrongWhere2 () + { + var input = @"using System; +using System.Linq; +public class X +{ + X Where (Func f) { return null; } + bool Any () { return false; } + public void Bla () { + X ex = null; + var bla = ex.Where (x => x + 1).Any (); + } +}"; + + TestRefactoringContext context; + var issues = GetIssues (new RedundantWhereWithPredicateIssue (), 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 37f23a1085..f738c7aa97 100644 --- a/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj +++ b/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj @@ -359,6 +359,7 @@ + From e639b4bcbbef6ebbc639ecae7348bd607ea31cf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Tue, 28 Aug 2012 06:57:38 +0200 Subject: [PATCH 02/29] [Analysis] Handled null conditions in do while & while statements. --- ICSharpCode.NRefactory.CSharp/Analysis/ControlFlow.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Analysis/ControlFlow.cs b/ICSharpCode.NRefactory.CSharp/Analysis/ControlFlow.cs index 21b4526963..b74c2fe1df 100644 --- a/ICSharpCode.NRefactory.CSharp/Analysis/ControlFlow.cs +++ b/ICSharpCode.NRefactory.CSharp/Analysis/ControlFlow.cs @@ -535,7 +535,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis Connect(data, conditionNode); - bool? cond = builder.EvaluateCondition(whileStatement.Condition); + bool? cond = whileStatement.Condition.IsNull ? true : builder.EvaluateCondition(whileStatement.Condition); ControlFlowNode bodyStart = builder.CreateStartNode(whileStatement.EmbeddedStatement); if (cond != false) Connect(conditionNode, bodyStart, ControlFlowEdgeType.ConditionTrue); @@ -563,7 +563,7 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis ControlFlowNode bodyEnd = doWhileStatement.EmbeddedStatement.AcceptVisitor(this, bodyStart); Connect(bodyEnd, conditionNode); - bool? cond = builder.EvaluateCondition(doWhileStatement.Condition); + bool? cond = doWhileStatement.Condition.IsNull ? true : builder.EvaluateCondition(doWhileStatement.Condition); if (cond != false) Connect(conditionNode, bodyStart, ControlFlowEdgeType.ConditionTrue); if (cond != true) From 6c62b5b2c39a0cb65ce756430ada91038cfb1b08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Tue, 28 Aug 2012 08:19:18 +0200 Subject: [PATCH 03/29] [Ast] Fixed end location of multi line strings. --- .../Ast/Expressions/PrimitiveExpression.cs | 35 ++++++++++++++++++- .../ICSharpCode.NRefactory.CSharp.csproj | 4 +++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/ICSharpCode.NRefactory.CSharp/Ast/Expressions/PrimitiveExpression.cs b/ICSharpCode.NRefactory.CSharp/Ast/Expressions/PrimitiveExpression.cs index 9f867fe1eb..514bd9b027 100644 --- a/ICSharpCode.NRefactory.CSharp/Ast/Expressions/PrimitiveExpression.cs +++ b/ICSharpCode.NRefactory.CSharp/Ast/Expressions/PrimitiveExpression.cs @@ -43,9 +43,14 @@ namespace ICSharpCode.NRefactory.CSharp } string literalValue; + TextLocation? endLocation; public override TextLocation EndLocation { get { - return new TextLocation (StartLocation.Line, StartLocation.Column + literalValue.Length); + if (!endLocation.HasValue) { + endLocation = value is string ? AdvanceLocation (StartLocation, literalValue) : + new TextLocation (StartLocation.Line, StartLocation.Column + literalValue.Length); + } + return endLocation.Value; } } @@ -102,6 +107,34 @@ namespace ICSharpCode.NRefactory.CSharp { return visitor.VisitPrimitiveExpression (this, data); } + + unsafe static TextLocation AdvanceLocation(TextLocation startLocation, string str) + { + int line = startLocation.Line; + int col = startLocation.Column; + fixed (char* start = str) { + char* p = start; + char* endPtr = start + str.Length; + while (p < endPtr) { + switch (*p) { + case '\r': + char* nextp = p + 1; + if (nextp < endPtr && *nextp == '\n') + p++; + goto case '\n'; + case '\n': + line++; + col = 1; + break; + default: + col++; + break; + } + p++; + } + } + return new TextLocation (line, col); + } protected internal override bool DoMatch(AstNode other, PatternMatching.Match match) { diff --git a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj index 65cbb19826..10a85c68b2 100644 --- a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj +++ b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj @@ -42,10 +42,12 @@ PdbOnly false + True Full true + True False @@ -56,6 +58,7 @@ Full true + True True @@ -66,6 +69,7 @@ PdbOnly false + True From 3451f5cb38ec750b81d4786b01674729840d8e7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Thu, 30 Aug 2012 07:41:54 +0200 Subject: [PATCH 04/29] [Analysis] Handle null reference in control flow graph builder. btw. assumes that if true statement is null the if is incomplete. --- ICSharpCode.NRefactory.CSharp/Analysis/ControlFlow.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ICSharpCode.NRefactory.CSharp/Analysis/ControlFlow.cs b/ICSharpCode.NRefactory.CSharp/Analysis/ControlFlow.cs index b74c2fe1df..b8eb4bf212 100644 --- a/ICSharpCode.NRefactory.CSharp/Analysis/ControlFlow.cs +++ b/ICSharpCode.NRefactory.CSharp/Analysis/ControlFlow.cs @@ -405,7 +405,9 @@ namespace ICSharpCode.NRefactory.CSharp.Analysis public override ControlFlowNode VisitIfElseStatement(IfElseStatement ifElseStatement, ControlFlowNode data) { - bool? cond = builder.EvaluateCondition(ifElseStatement.Condition); + bool? cond = ifElseStatement.Condition.IsNull ? true : builder.EvaluateCondition(ifElseStatement.Condition); + if (ifElseStatement.TrueStatement.IsNull) + return data; ControlFlowNode trueBegin = builder.CreateStartNode(ifElseStatement.TrueStatement); if (cond != false) Connect(data, trueBegin, ControlFlowEdgeType.ConditionTrue); From d69edd367078492b1c84569c905c6b699c955bea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Thu, 30 Aug 2012 08:07:20 +0200 Subject: [PATCH 05/29] [CodeIssues] ')edundant field initializer' issue no longer greys out the field name. --- .../Refactoring/CodeIssues/RedundantFieldInitializerIssue.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantFieldInitializerIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantFieldInitializerIssue.cs index fa952c614b..42db6f44d8 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantFieldInitializerIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantFieldInitializerIssue.cs @@ -62,7 +62,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (!defaultValueExpr.Match (variable.Initializer).Success) continue; - AddIssue (variable, ctx.TranslateString ("Remove redundant field initializer"), + AddIssue (variable.Initializer, ctx.TranslateString ("Remove redundant field initializer"), script => script.Replace (variable, new VariableInitializer (variable.Name))); } } From 1a80006cacb793dfbbd5a55015f8ef2f405f276d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Thu, 30 Aug 2012 10:06:22 +0200 Subject: [PATCH 06/29] [CodeIssues] Fixed an issue with try/catch statement in redundant assignment issue. --- .../Refactoring/VariableReferenceGraph.cs | 37 ++++++++++++++-- .../RedundantAssignmentIssueTests.cs | 44 +++++++++++++++++++ 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/VariableReferenceGraph.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/VariableReferenceGraph.cs index 0a687f6260..56f99e3a17 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/VariableReferenceGraph.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/VariableReferenceGraph.cs @@ -28,6 +28,7 @@ using System.Collections.Generic; using System.Linq; using ICSharpCode.NRefactory.CSharp.Analysis; using ICSharpCode.NRefactory.CSharp.Resolver; +using System.Threading; namespace ICSharpCode.NRefactory.CSharp.Refactoring { @@ -80,6 +81,13 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring return cfgVrNodeBuilder.Build (cfg [0], references, refStatements, context.Resolver); } + public static VariableReferenceNode Build (Statement statement, ISet references, + ISet refStatements, CSharpAstResolver resolver, CancellationToken cancellationToken = default(CancellationToken)) + { + var cfg = cfgBuilder.BuildControlFlowGraph (statement, resolver, cancellationToken); + return cfgVrNodeBuilder.Build (cfg [0], references, refStatements, resolver); + } + class GetExpressionsVisitor : DepthFirstAstVisitor> { @@ -153,6 +161,10 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring yield return yieldReturnStatement.Expression; } + public override IEnumerable VisitBlockStatement(BlockStatement blockStatement) + { + yield break; + } } class CfgVariableReferenceNodeBuilder @@ -216,16 +228,36 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring } nodeDict [cfNode] = node; - if (IsValidControlFlowNode (cfNode) && refStatements.Contains (cfNode.NextStatement)) + if (IsValidControlFlowNode (cfNode) && refStatements.Contains (cfNode.NextStatement)) { node = GetStatementEndNode (node, cfNode.NextStatement); + } if (cfNode.Outgoing.Count == 1) { cfNode = cfNode.Outgoing [0].To; } else { - foreach (var e in cfNode.Outgoing) + foreach (var e in cfNode.Outgoing) { node.AddNextNode (AddNode (e.To)); + } break; } + + // Hack for handling try ... catch ... finally. + var tryc = cfNode.NextStatement as TryCatchStatement; + if (tryc != null) { + VariableReferenceNode outNode = null; + foreach (var n in tryc.CatchClauses) { + var catchNode = VariableReferenceGraphBuilder.Build(n.Body, references, refStatements, this.resolver); + (outNode ?? node).AddNextNode (catchNode); + outNode = catchNode; + } + if (!tryc.FinallyBlock.IsNull) { + var finallyNode = VariableReferenceGraphBuilder.Build(tryc.FinallyBlock, references, refStatements, this.resolver); + (outNode ?? node).AddNextNode (finallyNode); + outNode = finallyNode; + } + if (outNode != null) + return outNode; + } } return nodeDict [startNode]; } @@ -282,7 +314,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring } #region Skipped Expressions - public override void VisitAnonymousMethodExpression (AnonymousMethodExpression anonymousMethodExpression) { } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantAssignmentIssueTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantAssignmentIssueTests.cs index b111864055..934d1435f8 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantAssignmentIssueTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantAssignmentIssueTests.cs @@ -290,5 +290,49 @@ class TestClass }"; Test (input, 0); } + + + + [Test] + public void TestAssignmentInTryCatch () + { + var input = @"using System; +class TestClass +{ + void TestMethod () + { + var a = new TestClass (); + try { + a = null; + } catch (Exception) { + if (a != null) { + a.TestMethod (); + } + } + } +}"; + Test (input, 0); + } + + [Test] + public void TestAssignmentInTryCatchFinally () + { + var input = @" +class TestClass +{ + void TestMethod () + { + var a = new TestClass (); + try { + a = null; + } finally { + if (a != null) { + a.TestMethod (); + } + } + } +}"; + Test (input, 0); + } } } From 24c0222e20a2a8fe9865e90776578f397c7e8b3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Thu, 30 Aug 2012 10:36:37 +0200 Subject: [PATCH 07/29] [CodeAction] Fixed bug in splet declaration and assignment action. --- .../SplitDeclarationAndAssignmentAction.cs | 11 +++++++---- .../CodeActions/SplitDeclarationAndAssignmentTests.cs | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/SplitDeclarationAndAssignmentAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/SplitDeclarationAndAssignmentAction.cs index 5ee9cab932..da4056b867 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/SplitDeclarationAndAssignmentAction.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/SplitDeclarationAndAssignmentAction.cs @@ -28,6 +28,7 @@ using System.Linq; using ICSharpCode.NRefactory.PatternMatching; using System.Threading; using System.Collections.Generic; +using ICSharpCode.NRefactory.TypeSystem; namespace ICSharpCode.NRefactory.CSharp.Refactoring { @@ -65,10 +66,12 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { var result = context.GetNode (); if (result != null && result.Variables.Count == 1 && !result.Variables.First ().Initializer.IsNull && result.Variables.First ().NameToken.Contains (context.Location.Line, context.Location.Column)) { - resolvedType = result.Type.Clone (); - // resolvedType = context.Resolve (result.Variables.First ().Initializer).Type.ConvertToAstType (); - // if (resolvedType == null) - // return null; + var type = context.Resolve(result.Variables.First ().Initializer).Type; + if (type.Equals(SpecialType.NullType) || type.Equals(SpecialType.UnknownType)) { + resolvedType = new PrimitiveType ("object"); + } else { + resolvedType = context.CreateShortType (type); + } return result; } resolvedType = null; diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/SplitDeclarationAndAssignmentTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/SplitDeclarationAndAssignmentTests.cs index 8dc1dd9a1f..cc2a6a488a 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/SplitDeclarationAndAssignmentTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/SplitDeclarationAndAssignmentTests.cs @@ -75,7 +75,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions "{" + Environment.NewLine + " void Test ()" + Environment.NewLine + " {" + Environment.NewLine + - " var aVar;" + Environment.NewLine + + " TestClass aVar;" + Environment.NewLine + " aVar = this;" + Environment.NewLine + " }" + Environment.NewLine + "}", result); From 1b41ee02ca843826fbfcf597332590f5a59a75cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Thu, 30 Aug 2012 10:50:20 +0200 Subject: [PATCH 08/29] [CodeAction] Convert lambda to delegate action no longer puts a redundant () in the anonymous delegate. --- .../Expressions/AnonymousMethodExpression.cs | 1 + .../ConvertLambdaToAnonymousDelegateAction.cs | 7 ++++-- .../ConvertLamdaToAnonymousDelegateTests.cs | 22 +++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Ast/Expressions/AnonymousMethodExpression.cs b/ICSharpCode.NRefactory.CSharp/Ast/Expressions/AnonymousMethodExpression.cs index 07de16197a..c1f1acd606 100644 --- a/ICSharpCode.NRefactory.CSharp/Ast/Expressions/AnonymousMethodExpression.cs +++ b/ICSharpCode.NRefactory.CSharp/Ast/Expressions/AnonymousMethodExpression.cs @@ -80,6 +80,7 @@ namespace ICSharpCode.NRefactory.CSharp public AnonymousMethodExpression (BlockStatement body, IEnumerable parameters = null) { if (parameters != null) { + hasParameterList = true; foreach (var parameter in parameters) { AddChild (parameter, Roles.Parameter); } diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertLambdaToAnonymousDelegateAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertLambdaToAnonymousDelegateAction.cs index cb6a555f1e..08103bf591 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertLambdaToAnonymousDelegateAction.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertLambdaToAnonymousDelegateAction.cs @@ -56,7 +56,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring }; } var method = new AnonymousMethodExpression (newBody, GetParameters(lambdaResolveResult.Parameters, context)); - method.HasParameterList = true; script.Replace(node, method); }); } @@ -64,6 +63,9 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring IEnumerable GetParameters(IList parameters, RefactoringContext context) { + if (parameters == null || parameters.Count == 0) + return null; + var result = new List (); foreach (var parameter in parameters) { var type = context.CreateShortType(parameter.Type); var name = parameter.Name; @@ -72,8 +74,9 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring modifier |= ParameterModifier.Ref; if (parameter.IsOut) modifier |= ParameterModifier.Out; - yield return new ParameterDeclaration(type, name, modifier); + result.Add (new ParameterDeclaration(type, name, modifier)); } + return result; } } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertLamdaToAnonymousDelegateTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertLamdaToAnonymousDelegateTests.cs index b1fd140cdd..b55d90dedc 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertLamdaToAnonymousDelegateTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertLamdaToAnonymousDelegateTests.cs @@ -72,6 +72,28 @@ class A System.Console.WriteLine (i1); }; } +}"); + } + + [Test] + public void ParameterLessLambdaTest () + { + Test(@" +class A +{ + void F () + { + System.Action = ()$ => { System.Console.WriteLine (); }; + } +}", @" +class A +{ + void F () + { + System.Action = delegate { + System.Console.WriteLine (); +}; + } }"); } } From aad32a0e66fe7ef6fdbb273ee6637a3d97c85647 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Thu, 30 Aug 2012 11:33:59 +0200 Subject: [PATCH 09/29] [CodeAction] Fixed some 'cast expression of incompatible type' issue bugs. --- .../CastExpressionOfIncompatibleTypeIssue.cs | 9 +++++++-- ...CastExpressionOfIncompatibleTypeIssueTests.cs | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/CastExpressionOfIncompatibleTypeIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/CastExpressionOfIncompatibleTypeIssue.cs index ff2ad06ef5..0cd039c107 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/CastExpressionOfIncompatibleTypeIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/CastExpressionOfIncompatibleTypeIssue.cs @@ -26,6 +26,8 @@ using System.Collections.Generic; using ICSharpCode.NRefactory.TypeSystem; +using ICSharpCode.NRefactory.CSharp.Resolver; +using ICSharpCode.NRefactory.Semantics; namespace ICSharpCode.NRefactory.CSharp.Refactoring { @@ -43,9 +45,12 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring class GatherVisitor : GatherVisitorBase { + readonly CSharpConversions conversion; + public GatherVisitor (BaseRefactoringContext ctx) : base (ctx) { + conversion = new CSharpConversions(ctx.Compilation); } public override void VisitCastExpression (CastExpression castExpression) @@ -66,8 +71,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring void VisitTypeCastExpression (Expression expression, IType exprType, IType castToType) { - if (TypeCompatibilityHelper.CheckTypeCompatibility (exprType, castToType) == - TypeCompatibilityHelper.TypeCompatiblity.NeverOfProvidedType) + var foundConversion = conversion.ExplicitConversion(exprType, castToType); + if (foundConversion == Conversion.None) AddIssue (expression, ctx.TranslateString ("Type cast expression of incompatible type")); } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CastExpressionOfIncompatibleTypeIssueTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CastExpressionOfIncompatibleTypeIssueTests.cs index cdc813315d..be33b5ff15 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CastExpressionOfIncompatibleTypeIssueTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CastExpressionOfIncompatibleTypeIssueTests.cs @@ -46,5 +46,21 @@ class TestClass }"; Test (input, 2); } + + [Test] + public void TestCompatibleTypes () + { + var input = @" +class TestClass +{ + void TestMethod () + { + var x1 = (int)123; + var x2 = (int)System.ConsoleKey.A; + } +}"; + + Test (input, 0); + } } } From d9f68d3e67985c358a26e65b0addaf2eea17255d Mon Sep 17 00:00:00 2001 From: Mansheng Yang Date: Thu, 30 Aug 2012 23:27:00 +0800 Subject: [PATCH 10/29] [CodeIssues] ExpressionIsAlwaysOfProvidedTypeIssue: use Conversions instead of TypeCompatibilityHelper --- .../CodeIssues/ExpressionIsAlwaysOfProvidedTypeIssue.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExpressionIsAlwaysOfProvidedTypeIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExpressionIsAlwaysOfProvidedTypeIssue.cs index 80b005473c..e5eb242e76 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExpressionIsAlwaysOfProvidedTypeIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExpressionIsAlwaysOfProvidedTypeIssue.cs @@ -25,6 +25,8 @@ // THE SOFTWARE. using System.Collections.Generic; +using ICSharpCode.NRefactory.CSharp.Resolver; +using ICSharpCode.NRefactory.Semantics; namespace ICSharpCode.NRefactory.CSharp.Refactoring { @@ -42,9 +44,11 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring class GatherVisitor : GatherVisitorBase { + static CSharpConversions conversion; public GatherVisitor (BaseRefactoringContext ctx) : base (ctx) { + conversion = new CSharpConversions(ctx.Compilation); } public override void VisitIsExpression (IsExpression isExpression) @@ -54,8 +58,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring var type = ctx.Resolve (isExpression.Expression).Type; var providedType = ctx.ResolveType (isExpression.Type); - if (TypeCompatibilityHelper.CheckTypeCompatibility (type, providedType) != - TypeCompatibilityHelper.TypeCompatiblity.AlwaysOfProvidedType) + var foundConversion = conversion.ImplicitConversion(type, providedType); + if (foundConversion == Conversion.None) return; var action = new CodeAction (ctx.TranslateString ("Compare with 'null'"), From e23568fc5132f35642c2c76841f4eaf46bd8af92 Mon Sep 17 00:00:00 2001 From: Mansheng Yang Date: Fri, 31 Aug 2012 08:20:16 +0800 Subject: [PATCH 11/29] [CodeActions] ConvertCastToAsAction: insert parentheses when necessary --- .../CodeActions/ConvertCastToAsAction.cs | 23 ++++++++++---- .../CodeActions/ConvertCastToAsTests.cs | 30 +++++++++++++++---- 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertCastToAsAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertCastToAsAction.cs index 85c694bd71..5aecc69d82 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertCastToAsAction.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertCastToAsAction.cs @@ -34,7 +34,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring [ContextAction ("Convert cast to 'as'.", Description = "Convert cast to 'as'.")] public class ConvertCastToAsAction : SpecializedCodeAction { - + static InsertParenthesesVisitor insertParentheses = new InsertParenthesesVisitor (); protected override CodeAction GetAction (RefactoringContext context, CastExpression node) { if (node.Expression.Contains (context.Location)) @@ -42,10 +42,23 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring // only works on reference and nullable types var type = context.ResolveType (node.Type); var typeDef = type.GetDefinition (); - if (type.IsReferenceType == true || (typeDef != null && typeDef.KnownTypeCode == KnownTypeCode.NullableOfT)) - return new CodeAction (context.TranslateString ("Convert cast to 'as'"), script => - script.Replace (node, new AsExpression (node.Expression.Clone (), node.Type.Clone ()))); - + var isNullable = typeDef != null && typeDef.KnownTypeCode == KnownTypeCode.NullableOfT; + if (type.IsReferenceType == true || isNullable) { + return new CodeAction (context.TranslateString ("Convert cast to 'as'"), script => { + var asExpr = new AsExpression (node.Expression.Clone (), node.Type.Clone ()); + // if parent is an expression, clone parent and replace the case expression with asExpr, + // so that we can inset parentheses + var parentExpr = node.Parent.Clone () as Expression; + if (parentExpr != null) { + var castExpr = parentExpr.GetNodeContaining (node.StartLocation, node.EndLocation); + castExpr.ReplaceWith (asExpr); + parentExpr.AcceptVisitor (insertParentheses); + script.Replace (node.Parent, parentExpr); + } else { + script.Replace (node, asExpr); + } + }); + } return null; } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertCastToAsTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertCastToAsTests.cs index f25a0c9653..3ab5a801f4 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertCastToAsTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertCastToAsTests.cs @@ -32,7 +32,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions [TestFixture] public class ConvertCastToAsTests : ContextActionTestBase { - void TestType(string type) + void TestType (string type) { string input = @" using System; @@ -56,19 +56,19 @@ class TestClass } [Test] - public void Test() + public void Test () { TestType ("Exception"); } [Test] - public void TestNullable() + public void TestNullable () { TestType ("int?"); } [Test] - public void TestNonReferenceType() + public void TestNonReferenceType () { TestWrongContext (@" using System; @@ -76,9 +76,29 @@ class TestClass { void Test (object a) { - var b = (int)$a; + var b = ($int)a; } }"); } + + [Test] + public void TestInsertParentheses () + { + string input = @" +class TestClass { + void TestMethod (object o) + { + var b = 1 + ($TestClass)o; + } +}"; + string output = @" +class TestClass { + void TestMethod (object o) + { + var b = 1 + (o as TestClass); + } +}"; + Test (input, output); + } } } From 84126bbddcad952aeb35fb06d29c74b0eb2c7a3e Mon Sep 17 00:00:00 2001 From: Mansheng Yang Date: Fri, 31 Aug 2012 08:35:13 +0800 Subject: [PATCH 12/29] [CodeActions] ConvertAsToCastAction: fixed some parentheses issues --- .../CodeActions/ConvertAsToCastAction.cs | 20 +++++++-- .../CodeActions/ConvertAsToCastTests.cs | 42 ++++++++++++++++++- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertAsToCastAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertAsToCastAction.cs index 3d9e262962..8a120134e5 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertAsToCastAction.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertAsToCastAction.cs @@ -32,13 +32,27 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring [ContextAction("Convert 'as' to cast.", Description = "Convert 'as' to cast.")] public class ConvertAsToCastAction : SpecializedCodeAction { - + static InsertParenthesesVisitor insertParentheses = new InsertParenthesesVisitor (); protected override CodeAction GetAction (RefactoringContext context, AsExpression node) { if (!node.AsToken.Contains (context.Location)) return null; - return new CodeAction (context.TranslateString ("Convert 'as' to cast"), - script => script.Replace (node, new CastExpression (node.Type.Clone (), node.Expression.Clone ()))); + return new CodeAction (context.TranslateString ("Convert 'as' to cast"), script => { + var castExpr = new CastExpression (node.Type.Clone (), node.Expression.Clone ()); + var parenthesizedExpr = node.Parent as ParenthesizedExpression; + if (parenthesizedExpr != null && parenthesizedExpr.Parent is Expression) { + // clone parent expression and replace the ParenthesizedExpression with castExpr to remove + // parentheses, then insert parentheses if necessary + var parentExpr = (Expression)parenthesizedExpr.Parent.Clone (); + parentExpr.GetNodeContaining (parenthesizedExpr.StartLocation, parenthesizedExpr.EndLocation) + .ReplaceWith (castExpr); + parentExpr.AcceptVisitor (insertParentheses); + script.Replace (parenthesizedExpr.Parent, parentExpr); + } else { + castExpr.AcceptVisitor (insertParentheses); + script.Replace (node, castExpr); + } + }); } } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertAsToCastTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertAsToCastTests.cs index 721bf18e98..90c3468fe2 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertAsToCastTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertAsToCastTests.cs @@ -34,7 +34,7 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions public class ConvertAsToCastTests : ContextActionTestBase { [Test] - public void Test() + public void Test () { Test (@" using System; @@ -54,5 +54,45 @@ class TestClass } }"); } + + [Test] + public void TestRemoveParentheses () + { + string input = @" +class TestClass { + void TestMethod (object o) + { + var b = 1 + (o $as TestClass); + } +}"; + string output = @" +class TestClass { + void TestMethod (object o) + { + var b = 1 + (TestClass)o; + } +}"; + Test (input, output); + } + + [Test] + public void TestInsertParentheses () + { + string input = @" +class TestClass { + void TestMethod (object o) + { + var b = 1 + o $as TestClass; + } +}"; + string output = @" +class TestClass { + void TestMethod (object o) + { + var b = (TestClass)(1 + o); + } +}"; + Test (input, output); + } } } From 0270ce48b01ba42e0a15b20f2db7a7750b023efb Mon Sep 17 00:00:00 2001 From: Mansheng Yang Date: Fri, 31 Aug 2012 08:53:45 +0800 Subject: [PATCH 13/29] [CodeActions] CreateOverloadWithoutParameterAction: fixed missing ref/out in generated call --- .../CreateOverloadWithoutParameterAction.cs | 18 ++++++++++++--- .../CreateOverloadWithoutParameterTests.cs | 23 +++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/CreateOverloadWithoutParameterAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/CreateOverloadWithoutParameterAction.cs index 9dbd124cab..6ca79fca39 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/CreateOverloadWithoutParameterAction.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/CreateOverloadWithoutParameterAction.cs @@ -66,15 +66,15 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring Expression argExpr; if (node.ParameterModifier == ParameterModifier.Ref) { body.Add (new VariableDeclarationStatement (node.Type.Clone (), node.Name, defaultExpr)); - argExpr = new DirectionExpression (FieldDirection.Ref, new IdentifierExpression (node.Name)); + argExpr = GetArgumentExpression (node); } else if (node.ParameterModifier == ParameterModifier.Out) { body.Add (new VariableDeclarationStatement (node.Type.Clone (), node.Name)); - argExpr = new DirectionExpression (FieldDirection.Out, new IdentifierExpression (node.Name)); + argExpr = GetArgumentExpression (node); } else { argExpr = defaultExpr; } body.Add (new InvocationExpression (new IdentifierExpression (methodDecl.Name), - methodDecl.Parameters.Select (param => param == node ? argExpr : new IdentifierExpression (param.Name)))); + methodDecl.Parameters.Select (param => param == node ? argExpr : GetArgumentExpression(param)))); var decl = (MethodDeclaration)methodDecl.Clone (); decl.Parameters.Remove (decl.Parameters.First (param => param.Name == node.Name)); @@ -87,6 +87,18 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring }); } + static Expression GetArgumentExpression(ParameterDeclaration parameter) + { + var identifierExpr = new IdentifierExpression(parameter.Name); + switch (parameter.ParameterModifier) { + case ParameterModifier.Out: + return new DirectionExpression (FieldDirection.Out, identifierExpr); + case ParameterModifier.Ref: + return new DirectionExpression (FieldDirection.Ref, identifierExpr); + } + return identifierExpr; + } + static Expression GetDefaultValueExpression (RefactoringContext context, AstType astType) { var type = context.ResolveType (astType); diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/CreateOverloadWithoutParameterTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/CreateOverloadWithoutParameterTests.cs index e1286790e2..ff23a3435b 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/CreateOverloadWithoutParameterTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/CreateOverloadWithoutParameterTests.cs @@ -203,6 +203,29 @@ class Test : ITest void ITest.Test (int a, int $b) { } +}"); + } + + [Test] + public void TestGenereatedCall () + { + Test ( + @"class Test +{ + void TestMethod (ref int $i, ref int j, out int k) + { + } +}", + @"class Test +{ + void TestMethod (ref int j, out int k) + { + int i = 0; + TestMethod (ref i, ref j, out k); + } + void TestMethod (ref int i, ref int j, out int k) + { + } }"); } } From bf33c03cc2efa1262cfa93d676424e32cac8651c Mon Sep 17 00:00:00 2001 From: Mansheng Yang Date: Fri, 31 Aug 2012 09:24:04 +0800 Subject: [PATCH 14/29] [CodeIssues] RedundantArrayInitializerCommaIssue: show appropriate description for different types of initializers --- .../RedundantArrayInitializerCommaIssue.cs | 13 ++++- ...edundantArrayInitializerCommaIssueTests.cs | 53 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantArrayInitializerCommaIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantArrayInitializerCommaIssue.cs index 5a748db76a..aed48ef960 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantArrayInitializerCommaIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/RedundantArrayInitializerCommaIssue.cs @@ -57,7 +57,18 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring var commaToken = arrayInitializerExpression.RBraceToken.PrevSibling as CSharpTokenNode; if (commaToken == null || commaToken.GetText () != ",") return; - AddIssue (commaToken, ctx.TranslateString ("Remove redundant comma in array initializer"), + string initializerType; + if (arrayInitializerExpression.Parent is ObjectCreateExpression) { + if (arrayInitializerExpression.Elements.FirstOrNullObject () is NamedExpression) { + initializerType = "object"; + } else { + initializerType = "collection"; + } + } else { + initializerType = "array"; + } + AddIssue (commaToken, + ctx.TranslateString (string.Format("Remove redundant comma in {0} initializer", initializerType)), script => script.Remove (commaToken)); } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantArrayInitializerCommaIssueTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantArrayInitializerCommaIssueTests.cs index cd8d561ed7..9609e03b7e 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantArrayInitializerCommaIssueTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/RedundantArrayInitializerCommaIssueTests.cs @@ -24,6 +24,7 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. +using ICSharpCode.NRefactory.CSharp.CodeActions; using ICSharpCode.NRefactory.CSharp.Refactoring; using NUnit.Framework; @@ -53,5 +54,57 @@ class TestClass }"; Test (input, 1, output); } + + [Test] + public void TestArrayInitializerDescription () + { + var input = @" +class TestClass +{ + void TestMethod () + { + var a = new int[] { 1, 2, }; + } +}"; + TestRefactoringContext ctx; + var issues = GetIssues (new RedundantArrayInitializerCommaIssue (), input, out ctx); + Assert.AreEqual (issues.Count, 1); + Assert.AreEqual (issues [0].Description, "Remove redundant comma in array initializer"); + } + + [Test] + public void TestObjectInitializerDescription () + { + var input = @" +class TestClass +{ + int Prop { get; set; } + void TestMethod () + { + var a = new TestClass { Prop = 1, }; + } +}"; + TestRefactoringContext ctx; + var issues = GetIssues (new RedundantArrayInitializerCommaIssue (), input, out ctx); + Assert.AreEqual (issues.Count, 1); + Assert.AreEqual (issues [0].Description, "Remove redundant comma in object initializer"); + } + + [Test] + public void TestCollectionInitializerDescrition () + { + var input = @" +class TestClass +{ + void TestMethod () + { + var a = new TestClass { 1, }; + } +}"; + TestRefactoringContext ctx; + var issues = GetIssues (new RedundantArrayInitializerCommaIssue (), input, out ctx); + Assert.AreEqual (issues.Count, 1); + Assert.AreEqual (issues [0].Description, "Remove redundant comma in collection initializer"); + } } } From 862e3d3156569530a4b8c4567e30c877eeec7a8b Mon Sep 17 00:00:00 2001 From: Mansheng Yang Date: Fri, 31 Aug 2012 09:43:27 +0800 Subject: [PATCH 15/29] [CodeActions] PutInsideUsingAction: remove trailing Dispose() invocation --- .../CodeActions/PutInsideUsingAction.cs | 13 ++++++++++ .../CSharp/CodeActions/PutInsideUsingTests.cs | 24 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/PutInsideUsingAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/PutInsideUsingAction.cs index 88dd4bdcbe..0978f12e23 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/PutInsideUsingAction.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/PutInsideUsingAction.cs @@ -29,6 +29,7 @@ using System.Linq; using ICSharpCode.NRefactory.CSharp.Resolver; using ICSharpCode.NRefactory.Semantics; using ICSharpCode.NRefactory.TypeSystem; +using ICSharpCode.NRefactory.PatternMatching; namespace ICSharpCode.NRefactory.CSharp.Refactoring { @@ -99,6 +100,11 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring foreach (var decl in variableToMoveOutside) script.InsertBefore (variableDecl, decl); + if (body.Statements.Count > 0) { + var lastStatement = body.Statements.Last (); + if (IsDisposeInvocation (resolveResult.Variable.Name, lastStatement)) + lastStatement.Remove (); + } var usingStatement = new UsingStatement { ResourceAcquisition = new VariableDeclarationStatement (variableDecl.Type.Clone (), node.Name, @@ -143,5 +149,12 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring }, context.CancellationToken); return lastReference; } + + static bool IsDisposeInvocation (string variableName, Statement statement) + { + var memberReferenceExpr = new MemberReferenceExpression (new IdentifierExpression (variableName), "Dispose"); + var pattern = new ExpressionStatement (new InvocationExpression (memberReferenceExpr)); + return pattern.Match (statement).Success; + } } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/PutInsideUsingTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/PutInsideUsingTests.cs index b681e110b3..f9c746d203 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/PutInsideUsingTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/PutInsideUsingTests.cs @@ -185,6 +185,30 @@ class TestClass } a++; } +}"); + } + + [Test] + public void TestRemoveDisposeInvocation () + { + Test (@" +class TestClass +{ + void TestMethod () + { + System.IDisposable obj $= null; + obj.Method (); + obj.Dispose(); + } +}", @" +class TestClass +{ + void TestMethod () + { + using (System.IDisposable obj = null) { + obj.Method (); + } + } }"); } } From 3001fae71f127171e5f78874cec58604bc6ce951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Fri, 31 Aug 2012 10:12:25 +0200 Subject: [PATCH 16/29] [CodeAction] Fixed bug in create class declaration action. --- .../CreateClassDeclarationAction.cs | 2 +- .../CreateClassDeclarationTests.cs | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/CreateClassDeclarationAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/CreateClassDeclarationAction.cs index 81295afc03..6265cc70be 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/CreateClassDeclarationAction.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/CreateClassDeclarationAction.cs @@ -93,7 +93,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring result = new TypeDeclaration() { Name = className }; var entity = simpleType.GetParent(); if (entity != null) - result.Modifiers |= entity.Modifiers & ~Modifiers.Internal; + result.Modifiers |= entity.Modifiers & Modifiers.Public; return result; } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/CreateClassDeclarationTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/CreateClassDeclarationTests.cs index 4158e3266f..c70de1d472 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/CreateClassDeclarationTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/CreateClassDeclarationTests.cs @@ -301,5 +301,26 @@ class TestClass "); } + [Test()] + public void TestModifierBug () + { + Test ( + @" +class TestClass +{ + private readonly $Foo _foo; +} +", @" +class Foo +{ +} +class TestClass +{ + private readonly Foo _foo; +} +"); + } + + } } From 2733a83a799b46ce7224a36b144d15867a5682ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Fri, 31 Aug 2012 13:35:12 +0200 Subject: [PATCH 17/29] [Semantics] ConversionResolveResult: For nullable conversions return the constant value of the input resolve result. IMHO ConstantValue == null is an error for example for int? i = 1; case. The constant value should be '1' there. --- .../Semantics/ConversionResolveResult.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ICSharpCode.NRefactory/Semantics/ConversionResolveResult.cs b/ICSharpCode.NRefactory/Semantics/ConversionResolveResult.cs index bb97964150..8ac4f67751 100644 --- a/ICSharpCode.NRefactory/Semantics/ConversionResolveResult.cs +++ b/ICSharpCode.NRefactory/Semantics/ConversionResolveResult.cs @@ -63,5 +63,13 @@ namespace ICSharpCode.NRefactory.Semantics { return new [] { Input }; } + + public override object ConstantValue { + get { + if (Conversion.IsNullableConversion) + return Input.ConstantValue; + return null; + } + } } } From 3acaf5eca2f14d97fb049d3388b655c00ccf53fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Fri, 31 Aug 2012 13:44:15 +0200 Subject: [PATCH 18/29] [Semantic] Handled default parameter value conversion in default unresolved parameter. --- .../Semantics/ConversionResolveResult.cs | 7 ------- .../Implementation/DefaultUnresolvedParameter.cs | 5 +++++ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/ICSharpCode.NRefactory/Semantics/ConversionResolveResult.cs b/ICSharpCode.NRefactory/Semantics/ConversionResolveResult.cs index 8ac4f67751..27ecd115f0 100644 --- a/ICSharpCode.NRefactory/Semantics/ConversionResolveResult.cs +++ b/ICSharpCode.NRefactory/Semantics/ConversionResolveResult.cs @@ -64,12 +64,5 @@ namespace ICSharpCode.NRefactory.Semantics return new [] { Input }; } - public override object ConstantValue { - get { - if (Conversion.IsNullableConversion) - return Input.ConstantValue; - return null; - } - } } } diff --git a/ICSharpCode.NRefactory/TypeSystem/Implementation/DefaultUnresolvedParameter.cs b/ICSharpCode.NRefactory/TypeSystem/Implementation/DefaultUnresolvedParameter.cs index c78a92d9e3..4576b373fa 100644 --- a/ICSharpCode.NRefactory/TypeSystem/Implementation/DefaultUnresolvedParameter.cs +++ b/ICSharpCode.NRefactory/TypeSystem/Implementation/DefaultUnresolvedParameter.cs @@ -235,6 +235,11 @@ namespace ICSharpCode.NRefactory.TypeSystem.Implementation public object ConstantValue { get { ResolveResult rr = LazyInit.VolatileRead(ref this.resolvedDefaultValue); + if (rr is ConversionResolveResult) { + var crr = (ConversionResolveResult)rr; + if (crr.Conversion.IsNullableConversion) + return crr.Input.ConstantValue; + } if (rr != null) { return rr.ConstantValue; } else { From a1bcb547474cdac7d988494adf39fa2f0b2fd42c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Fri, 31 Aug 2012 13:59:22 +0200 Subject: [PATCH 19/29] [Semantics] Added test case for nullable constant value / fixed last fix. --- .../TypeSystem/TypeSystemTests.TestCase.cs | 5 +++++ .../TypeSystem/TypeSystemTests.cs | 8 ++++++++ .../Implementation/DefaultUnresolvedParameter.cs | 12 ++++++------ 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/ICSharpCode.NRefactory.Tests/TypeSystem/TypeSystemTests.TestCase.cs b/ICSharpCode.NRefactory.Tests/TypeSystem/TypeSystemTests.TestCase.cs index 6849a73c43..34cdac5d68 100644 --- a/ICSharpCode.NRefactory.Tests/TypeSystem/TypeSystemTests.TestCase.cs +++ b/ICSharpCode.NRefactory.Tests/TypeSystem/TypeSystemTests.TestCase.cs @@ -327,4 +327,9 @@ namespace ICSharpCode.NRefactory.TypeSystem.TestCase get { return 0; } } } + + public class ClassWithMethodThatHasNullableDefaultParameter { + public void Foo (int? bar = 42) { } + } + } diff --git a/ICSharpCode.NRefactory.Tests/TypeSystem/TypeSystemTests.cs b/ICSharpCode.NRefactory.Tests/TypeSystem/TypeSystemTests.cs index 90e4ce1cce..1a3538f9a9 100644 --- a/ICSharpCode.NRefactory.Tests/TypeSystem/TypeSystemTests.cs +++ b/ICSharpCode.NRefactory.Tests/TypeSystem/TypeSystemTests.cs @@ -1200,5 +1200,13 @@ namespace ICSharpCode.NRefactory.TypeSystem var indexer = type.GetProperties(p => p.IsIndexer).Single(); Assert.AreEqual("Foo", indexer.Name); } + + [Test] + public void TestNullableDefaultParameter() + { + ITypeDefinition type = GetTypeDefinition(typeof(ClassWithMethodThatHasNullableDefaultParameter)); + var method = type.GetMethods ().Single (m => m.Name == "Foo"); + Assert.AreEqual(42, method.Parameters.Single ().ConstantValue); + } } } diff --git a/ICSharpCode.NRefactory/TypeSystem/Implementation/DefaultUnresolvedParameter.cs b/ICSharpCode.NRefactory/TypeSystem/Implementation/DefaultUnresolvedParameter.cs index 4576b373fa..3a0a197de9 100644 --- a/ICSharpCode.NRefactory/TypeSystem/Implementation/DefaultUnresolvedParameter.cs +++ b/ICSharpCode.NRefactory/TypeSystem/Implementation/DefaultUnresolvedParameter.cs @@ -235,17 +235,17 @@ namespace ICSharpCode.NRefactory.TypeSystem.Implementation public object ConstantValue { get { ResolveResult rr = LazyInit.VolatileRead(ref this.resolvedDefaultValue); + if (rr == null) { + rr = defaultValue.Resolve(context); + LazyInit.GetOrSet(ref this.resolvedDefaultValue, rr); + } if (rr is ConversionResolveResult) { var crr = (ConversionResolveResult)rr; if (crr.Conversion.IsNullableConversion) return crr.Input.ConstantValue; } - if (rr != null) { - return rr.ConstantValue; - } else { - rr = defaultValue.Resolve(context); - return LazyInit.GetOrSet(ref this.resolvedDefaultValue, rr).ConstantValue; - } + return rr.ConstantValue; + } } From 3dbba420ebdd63a9013e1ce40d6afaa1f5e87c16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Mon, 3 Sep 2012 09:33:47 +0200 Subject: [PATCH 20/29] [Resolver] Fixed infinite loop issue when a type inherits from type parameter. --- .../CSharp/Resolver/ResolveAtLocationTests.cs | 23 +++++++++++++++++++ .../Implementation/BaseTypeCollector.cs | 2 ++ 2 files changed, 25 insertions(+) diff --git a/ICSharpCode.NRefactory.Tests/CSharp/Resolver/ResolveAtLocationTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/Resolver/ResolveAtLocationTests.cs index c5f19646fb..a996956058 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/Resolver/ResolveAtLocationTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/Resolver/ResolveAtLocationTests.cs @@ -179,5 +179,28 @@ class Foo { } }")); } + + [Test] + public void TestBug6758() + { + var rr = ResolveAtLocation( + @"using System; + +namespace TestCrash +{ + class A + { + + } + + class B : T where T: $A + { + + } +} +"); + Assert.AreEqual("A", rr.Type.Name); + } + } } diff --git a/ICSharpCode.NRefactory/TypeSystem/Implementation/BaseTypeCollector.cs b/ICSharpCode.NRefactory/TypeSystem/Implementation/BaseTypeCollector.cs index c0d7d8c83a..d527e55f48 100644 --- a/ICSharpCode.NRefactory/TypeSystem/Implementation/BaseTypeCollector.cs +++ b/ICSharpCode.NRefactory/TypeSystem/Implementation/BaseTypeCollector.cs @@ -55,6 +55,8 @@ namespace ICSharpCode.NRefactory.TypeSystem.Implementation // (e.g. C implements I1 and I2, and both interfaces derive from Object) if (!this.Contains(type)) { foreach (IType baseType in type.DirectBaseTypes) { + if (type.Kind != TypeKind.TypeParameter && baseType.Kind == TypeKind.TypeParameter) + continue; if (SkipImplementedInterfaces && def != null && def.Kind != TypeKind.Interface && def.Kind != TypeKind.TypeParameter) { if (baseType.Kind == TypeKind.Interface) { // skip the interface From 7cab07f13ec9b00adf1f917170bce1cccb8e7aaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Mon, 3 Sep 2012 10:15:40 +0200 Subject: [PATCH 21/29] [Completion] Check for extension method accessibility. --- .../Completion/CSharpCompletionEngine.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngine.cs b/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngine.cs index 704218dd3e..85e0b79919 100644 --- a/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngine.cs +++ b/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngine.cs @@ -2461,6 +2461,8 @@ namespace ICSharpCode.NRefactory.CSharp.Completion } else { foreach (var meths in state.GetExtensionMethods (type)) { foreach (var m in meths) { + if (!lookup.IsAccessible(m, isProtectedAllowed)) + continue; result.AddMember(m); } } From d4f373d995066e2e72816939cc571f7b9856a08d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Mon, 3 Sep 2012 11:37:55 +0200 Subject: [PATCH 22/29] [Refactoring] Fixed potential issue in variable refrence graph builder. --- .../Refactoring/VariableReferenceGraph.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/VariableReferenceGraph.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/VariableReferenceGraph.cs index 56f99e3a17..74a4c08523 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/VariableReferenceGraph.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/VariableReferenceGraph.cs @@ -255,8 +255,10 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring (outNode ?? node).AddNextNode (finallyNode); outNode = finallyNode; } - if (outNode != null) + if (outNode != null) { + nodeDict [cfNode] = outNode; return outNode; + } } } return nodeDict [startNode]; From 465bcdfb61923b12505daa4bc638358e19ddfc1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Tue, 4 Sep 2012 08:01:17 +0200 Subject: [PATCH 23/29] [Completion] Check if types are accessible in the namespace resolve result case. --- .../Completion/CSharpCompletionEngine.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngine.cs b/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngine.cs index 85e0b79919..bec200f310 100644 --- a/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngine.cs +++ b/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngine.cs @@ -2295,11 +2295,18 @@ namespace ICSharpCode.NRefactory.CSharp.Completion return null; } + var lookup = new MemberLookup( + ctx.CurrentTypeDefinition, + Compilation.MainAssembly + ); + if (resolveResult is NamespaceResolveResult) { var nr = (NamespaceResolveResult)resolveResult; var namespaceContents = new CompletionDataWrapper(this); foreach (var cl in nr.Namespace.Types) { + if (!lookup.IsAccessible(cl, false)) + continue; IType addType = typePred != null ? typePred(cl) : cl; if (addType != null) namespaceContents.AddType(addType, addType.Name); @@ -2319,13 +2326,7 @@ namespace ICSharpCode.NRefactory.CSharp.Completion //var typeDef = resolveResult.Type.GetDefinition(); var result = new CompletionDataWrapper(this); bool includeStaticMembers = false; - - var lookup = new MemberLookup( - ctx.CurrentTypeDefinition, - Compilation.MainAssembly - ); - - + if (resolveResult is LocalResolveResult) { if (resolvedNode is IdentifierExpression) { var mrr = (LocalResolveResult)resolveResult; From a2e370fe2d21b92398cc3d90f204e7de9f814273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Kr=C3=BCger?= Date: Tue, 4 Sep 2012 08:18:38 +0200 Subject: [PATCH 24/29] [CodeActions] Fixed implement abstract members bug. --- .../CodeActions/ImplementInterfaceAction.cs | 6 ++++- .../ImplementAbstractMembersTest.cs | 27 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ImplementInterfaceAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ImplementInterfaceAction.cs index 915715cb52..d1a4abcd0e 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ImplementInterfaceAction.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ImplementInterfaceAction.cs @@ -101,7 +101,11 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring decl.Modifiers = Modifiers.None; decl.AddChild(builder.ConvertType(member.DeclaringType), EntityDeclaration.PrivateImplementationTypeRole); } else { - decl.Modifiers = Modifiers.Public; + if (member.IsProtected) { + decl.Modifiers = Modifiers.Protected; + } else { + decl.Modifiers = Modifiers.Public; + } } return decl; } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ImplementAbstractMembersTest.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ImplementAbstractMembersTest.cs index 9bf6d38732..f8e411b778 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ImplementAbstractMembersTest.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ImplementAbstractMembersTest.cs @@ -55,6 +55,33 @@ class Foo : Simple } #endregion } +"); + } + + [Test()] + public void TestProtectedMembers() + { + Test(@"abstract class Simple { + protected abstract string ServiceName { get; } +} + +class Foo : $Simple +{ +} +", @"abstract class Simple { + protected abstract string ServiceName { get; } +} + +class Foo : Simple +{ + #region implemented abstract members of Simple + protected override string ServiceName { + get { + throw new System.NotImplementedException (); + } + } + #endregion +} "); } From 403486104c7fc9001de7de9fc24031470edcb24b Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Thu, 23 Aug 2012 14:26:16 +0200 Subject: [PATCH 25/29] C# -> VB converter: Add support for 'await' operator --- .../OutputVisitor/OutputVisitor.cs | 1 + .../Visitors/CSharpToVBConverterVisitor.cs | 14 +++++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/ICSharpCode.NRefactory.VB/OutputVisitor/OutputVisitor.cs b/ICSharpCode.NRefactory.VB/OutputVisitor/OutputVisitor.cs index 40b17bcf1f..7edd3a525f 100644 --- a/ICSharpCode.NRefactory.VB/OutputVisitor/OutputVisitor.cs +++ b/ICSharpCode.NRefactory.VB/OutputVisitor/OutputVisitor.cs @@ -1963,6 +1963,7 @@ namespace ICSharpCode.NRefactory.VB WriteKeyword("For"); WriteKeyword("Each"); forEachStatement.Variable.AcceptVisitor(this, data); + Space(); WriteKeyword("In"); forEachStatement.InExpression.AcceptVisitor(this, data); NewLine(); diff --git a/ICSharpCode.NRefactory.VB/Visitors/CSharpToVBConverterVisitor.cs b/ICSharpCode.NRefactory.VB/Visitors/CSharpToVBConverterVisitor.cs index e6d4d0f47a..48632e0fd1 100644 --- a/ICSharpCode.NRefactory.VB/Visitors/CSharpToVBConverterVisitor.cs +++ b/ICSharpCode.NRefactory.VB/Visitors/CSharpToVBConverterVisitor.cs @@ -671,6 +671,12 @@ namespace ICSharpCode.NRefactory.VB.Visitors ((InvocationExpression)expr).Target = new IdentifierExpression() { Identifier = "__Dereference" }; ((InvocationExpression)expr).Arguments.Add((Expression)unaryOperatorExpression.Expression.AcceptVisitor(this, data)); break; + case ICSharpCode.NRefactory.CSharp.UnaryOperatorType.Await: + expr = new UnaryOperatorExpression() { + Expression = (Expression)unaryOperatorExpression.Expression.AcceptVisitor(this, data), + Operator = UnaryOperatorType.Await + }; + break; default: throw new Exception("Invalid value for UnaryOperatorType"); } @@ -1912,14 +1918,14 @@ namespace ICSharpCode.NRefactory.VB.Visitors throw new NotImplementedException(); } - public AstNode VisitCompilationUnit(CSharp.CompilationUnit compilationUnit, object data) + public AstNode VisitSyntaxTree(CSharp.SyntaxTree syntaxTree, object data) { var unit = new CompilationUnit(); - foreach (var node in compilationUnit.Children) + foreach (var node in syntaxTree.Children) unit.AddChild(node.AcceptVisitor(this, null), CompilationUnit.MemberRole); - return EndNode(compilationUnit, unit); + return EndNode(syntaxTree, unit); } public AstNode VisitSimpleType(CSharp.SimpleType simpleType, object data) @@ -2134,6 +2140,8 @@ namespace ICSharpCode.NRefactory.VB.Visitors mod |= Modifiers.Override; if ((modifier & CSharp.Modifiers.Virtual) == CSharp.Modifiers.Virtual) mod |= Modifiers.Overridable; + if ((modifier & CSharp.Modifiers.Async) == CSharp.Modifiers.Async) + mod |= Modifiers.Async; return mod; } From 69c1e6e6deeb5b239ec63e3de4a845f37b6aa483 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Tue, 4 Sep 2012 14:40:01 +0200 Subject: [PATCH 26/29] Use custom GetHashCode() implementation in XmlDocumentationProvider as the hash codes may get serialized. The normal .NET string.GetHashCode() isn't guaranteed to be stable across multiple runs of the program (e.g. with .NET 4.5 hash randomization). --- .../Documentation/XmlDocumentationProvider.cs | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/ICSharpCode.NRefactory/Documentation/XmlDocumentationProvider.cs b/ICSharpCode.NRefactory/Documentation/XmlDocumentationProvider.cs index e16b6c0f03..dd3d325f13 100644 --- a/ICSharpCode.NRefactory/Documentation/XmlDocumentationProvider.cs +++ b/ICSharpCode.NRefactory/Documentation/XmlDocumentationProvider.cs @@ -259,13 +259,30 @@ namespace ICSharpCode.NRefactory.Documentation int pos = linePosMapper.GetPositionForLine(reader.LineNumber) + Math.Max(reader.LinePosition - 2, 0); string memberAttr = reader.GetAttribute("name"); if (memberAttr != null) - indexList.Add(new IndexEntry(memberAttr.GetHashCode(), pos)); + indexList.Add(new IndexEntry(GetHashCode(memberAttr), pos)); reader.Skip(); } break; } } } + + /// + /// Hash algorithm used for the index. + /// This is a custom implementation so that old index files work correctly + /// even when the .NET string.GetHashCode implementation changes + /// (e.g. due to .NET 4.5 hash randomization) + /// + static int GetHashCode(string key) + { + unchecked { + int h = 0; + foreach (char c in key) { + h = (h << 5) - h + c; + } + return h; + } + } #endregion #region GetDocumentation @@ -277,7 +294,7 @@ namespace ICSharpCode.NRefactory.Documentation if (key == null) throw new ArgumentNullException("key"); - int hashcode = key.GetHashCode(); + int hashcode = GetHashCode(key); // index is sorted, so we can use binary search int m = Array.BinarySearch(index, new IndexEntry(hashcode, 0)); if (m < 0) From 62204182e7e4552ba9c228bd755b703e290cd388 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Tue, 4 Sep 2012 14:40:18 +0200 Subject: [PATCH 27/29] Demo: show parser errors in message box --- ICSharpCode.NRefactory.Demo/CSDemo.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ICSharpCode.NRefactory.Demo/CSDemo.cs b/ICSharpCode.NRefactory.Demo/CSDemo.cs index 9569f3f1d2..3ad101a701 100644 --- a/ICSharpCode.NRefactory.Demo/CSDemo.cs +++ b/ICSharpCode.NRefactory.Demo/CSDemo.cs @@ -65,7 +65,11 @@ namespace ICSharpCode.NRefactory.Demo void CSharpParseButtonClick(object sender, EventArgs e) { - syntaxTree = new CSharpParser().Parse(csharpCodeTextBox.Text, "demo.cs"); + var parser = new CSharpParser(); + syntaxTree = parser.Parse(csharpCodeTextBox.Text, "demo.cs"); + if (parser.HasErrors) { + MessageBox.Show(string.Join(Environment.NewLine, parser.Errors.Select(err => err.Message))); + } csharpTreeView.Nodes.Clear(); foreach (var element in syntaxTree.Children) { csharpTreeView.Nodes.Add(MakeTreeNode(element)); From 4a337b8ed18943dc4857a2f3013d5b045e7599ed Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Tue, 4 Sep 2012 14:52:00 +0200 Subject: [PATCH 28/29] [CodeActions] fixed bug implemented 'protected internal abstract' members. The 'internal' modifier is necessary in overrides if they are inside the same assembly as the abstract member. --- .../CodeActions/ImplementInterfaceAction.cs | 11 ++++--- .../Refactoring/TypeSystemAstBuilder.cs | 16 ++++++---- .../ImplementAbstractMembersTest.cs | 30 +++++++++++++++++-- 3 files changed, 45 insertions(+), 12 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ImplementInterfaceAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ImplementInterfaceAction.cs index d1a4abcd0e..06079b22d5 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ImplementInterfaceAction.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ImplementInterfaceAction.cs @@ -93,6 +93,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { var builder = context.CreateTypeSytemAstBuilder(); builder.GenerateBody = true; + builder.ShowModifiers = false; + builder.ShowAccessibility = true; builder.ShowConstantValues = !explicitImplementation; builder.ShowTypeParameterConstraints = !explicitImplementation; builder.UseCustomEvents = explicitImplementation; @@ -100,11 +102,12 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (explicitImplementation) { decl.Modifiers = Modifiers.None; decl.AddChild(builder.ConvertType(member.DeclaringType), EntityDeclaration.PrivateImplementationTypeRole); + } else if (member.DeclaringType.Kind == TypeKind.Interface) { + decl.Modifiers |= Modifiers.Public; } else { - if (member.IsProtected) { - decl.Modifiers = Modifiers.Protected; - } else { - decl.Modifiers = Modifiers.Public; + // Remove 'internal' modifier from 'protected internal' members if the override is in a different assembly than the member + if (!member.ParentAssembly.InternalsVisibleTo(context.Compilation.MainAssembly)) { + decl.Modifiers &= ~Modifiers.Internal; } } return decl; diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/TypeSystemAstBuilder.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/TypeSystemAstBuilder.cs index 3f1ea52811..28e68ce546 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/TypeSystemAstBuilder.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/TypeSystemAstBuilder.cs @@ -461,7 +461,10 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring EntityDeclaration ConvertTypeDefinition(ITypeDefinition typeDefinition) { - Modifiers modifiers = ModifierFromAccessibility(typeDefinition.Accessibility); + Modifiers modifiers = Modifiers.None; + if (this.ShowAccessibility) { + modifiers |= ModifierFromAccessibility(typeDefinition.Accessibility); + } if (this.ShowModifiers) { if (typeDefinition.IsStatic) { modifiers |= Modifiers.Static; @@ -598,7 +601,7 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring if (accessor == null) return Accessor.Null; Accessor decl = new Accessor(); - if (accessor.Accessibility != ownerAccessibility) + if (this.ShowAccessibility && accessor.Accessibility != ownerAccessibility) decl.Modifiers = ModifierFromAccessibility(accessor.Accessibility); decl.Body = GenerateBodyBlock(); return decl; @@ -716,10 +719,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring #endregion #region Convert Modifiers - Modifiers ModifierFromAccessibility(Accessibility accessibility) + static Modifiers ModifierFromAccessibility(Accessibility accessibility) { - if (!this.ShowAccessibility) - return Modifiers.None; switch (accessibility) { case Accessibility.Private: return Modifiers.Private; @@ -740,7 +741,10 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring Modifiers GetMemberModifiers(IMember member) { bool isInterfaceMember = member.DeclaringType.Kind == TypeKind.Interface; - Modifiers m = isInterfaceMember ? Modifiers.None : ModifierFromAccessibility(member.Accessibility); + Modifiers m = Modifiers.None; + if (this.ShowAccessibility && !isInterfaceMember) { + m |= ModifierFromAccessibility(member.Accessibility); + } if (this.ShowModifiers) { if (member.IsStatic) { m |= Modifiers.Static; diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ImplementAbstractMembersTest.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ImplementAbstractMembersTest.cs index f8e411b778..1d03a054f2 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ImplementAbstractMembersTest.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ImplementAbstractMembersTest.cs @@ -1,6 +1,6 @@ -// +// // ImplementAbstractMembersTest.cs -// +// // Author: // Mike Krüger // @@ -85,6 +85,32 @@ class Foo : Simple "); } + [Test()] + public void TestProtectedInternalMembers() + { + Test(@"abstract class Simple { + protected internal abstract string ServiceName { get; } +} + +class Foo : $Simple +{ +} +", @"abstract class Simple { + protected internal abstract string ServiceName { get; } +} + +class Foo : Simple +{ + #region implemented abstract members of Simple + protected internal override string ServiceName { + get { + throw new System.NotImplementedException (); + } + } + #endregion +} +"); + } } } From 31474555b2cb2fe64874d363103f519bf1fafabb Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Tue, 4 Sep 2012 15:21:12 +0200 Subject: [PATCH 29/29] Avoid looking for inner classes when resolving a class constraint. --- .../Resolver/ResolveVisitor.cs | 15 +++--- .../TypeSystem/TypeSystemConvertVisitor.cs | 3 +- .../CSharp/Resolver/NameLookupTests.cs | 50 +++++++++++++++++++ .../Implementation/BaseTypeCollector.cs | 2 - 4 files changed, 61 insertions(+), 9 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Resolver/ResolveVisitor.cs b/ICSharpCode.NRefactory.CSharp/Resolver/ResolveVisitor.cs index c7f82cc8ef..d88109fa21 100644 --- a/ICSharpCode.NRefactory.CSharp/Resolver/ResolveVisitor.cs +++ b/ICSharpCode.NRefactory.CSharp/Resolver/ResolveVisitor.cs @@ -3257,13 +3257,16 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver AstType outermostType = type; while (outermostType.Parent is AstType) outermostType = (AstType)outermostType.Parent; - NameLookupMode lookupMode = NameLookupMode.Type; + if (outermostType.Parent is UsingDeclaration || outermostType.Parent is UsingAliasDeclaration) { - lookupMode = NameLookupMode.TypeInUsingDeclaration; - } else if (outermostType.Parent is TypeDeclaration && outermostType.Role == Roles.BaseType) { - lookupMode = NameLookupMode.BaseTypeReference; - } - return lookupMode; + return NameLookupMode.TypeInUsingDeclaration; + } else if (outermostType.Role == Roles.BaseType) { + // Use BaseTypeReference for a type's base type, and for a constraint on a type. + // Do not use it for a constraint on a method. + if (outermostType.Parent is TypeDeclaration || (outermostType.Parent is Constraint && outermostType.Parent.Parent is TypeDeclaration)) + return NameLookupMode.BaseTypeReference; + } + return NameLookupMode.Type; } ResolveResult IAstVisitor.VisitMemberType(MemberType memberType) diff --git a/ICSharpCode.NRefactory.CSharp/TypeSystem/TypeSystemConvertVisitor.cs b/ICSharpCode.NRefactory.CSharp/TypeSystem/TypeSystemConvertVisitor.cs index f9ca28e7e0..65574e3c59 100644 --- a/ICSharpCode.NRefactory.CSharp/TypeSystem/TypeSystemConvertVisitor.cs +++ b/ICSharpCode.NRefactory.CSharp/TypeSystem/TypeSystemConvertVisitor.cs @@ -515,7 +515,8 @@ namespace ICSharpCode.NRefactory.CSharp.TypeSystem continue; } } - tp.Constraints.Add(type.ToTypeReference()); + var lookupMode = (ownerType == EntityType.TypeDefinition) ? NameLookupMode.BaseTypeReference : NameLookupMode.Type; + tp.Constraints.Add(type.ToTypeReference(lookupMode)); } break; } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/Resolver/NameLookupTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/Resolver/NameLookupTests.cs index cdcb819232..4669356f58 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/Resolver/NameLookupTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/Resolver/NameLookupTests.cs @@ -1015,5 +1015,55 @@ namespace foo { Assert.AreEqual("foo.Foo", result.Type.FullName); } + + [Test] + public void BaseTypeReference_refers_to_outer_type() + { + string program = @"class B {} + class A : $B$ { + class B {} + }"; + var result = Resolve(program); + Assert.IsFalse(result.IsError); + Assert.AreEqual("B", result.Type.FullName); + + // also check if the reference in the type system is correct + var a = ((ITypeDefinition)result.Type).Compilation.RootNamespace.GetTypeDefinition("A", 0); + Assert.AreEqual("B", a.DirectBaseTypes.Single().FullName); + } + + [Test] + public void Class_constraint_refers_to_outer_type() + { + string program = @"class B {} + class A where T : $B$ { + class B {} + }"; + var result = Resolve(program); + Assert.IsFalse(result.IsError); + Assert.AreEqual("B", result.Type.FullName); + + // also check if the reference in the type system is correct + var a = ((ITypeDefinition)result.Type).Compilation.RootNamespace.GetTypeDefinition("A", 1); + Assert.AreEqual("B", a.TypeParameters.Single().DirectBaseTypes.Single().FullName); + } + + [Test] + public void Method_constraint_refers_to_inner_type() + { + string program = @"class B {} + class A { + void M() where T : $B$ {} + class B {} + }"; + var result = Resolve(program); + Assert.IsFalse(result.IsError); + Assert.AreEqual("A.B", result.Type.FullName); + + // also check if the reference in the type system is correct + var a = ((ITypeDefinition)result.Type).Compilation.RootNamespace.GetTypeDefinition("A", 0); + var method = a.Methods.Single(m => m.Name == "M"); + Assert.AreEqual("A.B", method.TypeParameters.Single().DirectBaseTypes.Single().FullName); + } } } diff --git a/ICSharpCode.NRefactory/TypeSystem/Implementation/BaseTypeCollector.cs b/ICSharpCode.NRefactory/TypeSystem/Implementation/BaseTypeCollector.cs index d527e55f48..c0d7d8c83a 100644 --- a/ICSharpCode.NRefactory/TypeSystem/Implementation/BaseTypeCollector.cs +++ b/ICSharpCode.NRefactory/TypeSystem/Implementation/BaseTypeCollector.cs @@ -55,8 +55,6 @@ namespace ICSharpCode.NRefactory.TypeSystem.Implementation // (e.g. C implements I1 and I2, and both interfaces derive from Object) if (!this.Contains(type)) { foreach (IType baseType in type.DirectBaseTypes) { - if (type.Kind != TypeKind.TypeParameter && baseType.Kind == TypeKind.TypeParameter) - continue; if (SkipImplementedInterfaces && def != null && def.Kind != TypeKind.Interface && def.Kind != TypeKind.TypeParameter) { if (baseType.Kind == TypeKind.Interface) { // skip the interface