Merge pull request #105 from fightangel/fixes

some fixes for code actions and code issues

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 <AsExpression>
 	{
-
+		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);
+				}
+			});
 		}
 	}
 }

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<CastExpression>
 	{
-
+		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;
 		}
 	}

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);

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;
+		}
 	}
 }

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'"), 

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));
 			}
 		}

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<ConvertAsToCastAction> (@"
 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<ConvertAsToCastAction> (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<ConvertAsToCastAction> (input, output);
+		}
 	}
 }

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<ConvertCastToAsAction> (@"
 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<ConvertCastToAsAction> (input, output);
+		}
 	}
 }

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<CreateOverloadWithoutParameterAction> (
+				@"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)
+	{
+	}
 }");
 		}
 	}

ICSharpCode.NRefactory.Tests/CSharp/CodeActions/PutInsideUsingTests.cs

@@ -185,6 +185,30 @@ class TestClass
 		}
 		a++;
 	}
+}");
+		}
+
+		[Test]
+		public void TestRemoveDisposeInvocation ()
+		{
+			Test<PutInsideUsingAction> (@"
+class TestClass
+{
+	void TestMethod ()
+	{
+		System.IDisposable obj $= null;
+		obj.Method ();
+		obj.Dispose();
+	}
+}", @"
+class TestClass
+{
+	void TestMethod ()
+	{
+		using (System.IDisposable obj = null) {
+			obj.Method ();
+		}
+	}
 }");
 		}
 	}

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<RedundantArrayInitializerCommaIssue> (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");
+		}
 	}
 }