Browse Source

Fixed some convert foreach to for action bugs.

pull/32/merge
Mike Krüger 13 years ago
parent
commit
243b39310e
  1. 45
      ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertForeachToForAction.cs
  2. 31
      ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertForeachToForTests.cs

45
ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/ConvertForeachToForAction.cs

@ -37,30 +37,63 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
[ContextAction("Convert 'foreach' loop to 'for'", Description = "Works on 'foreach' loops that allow direct access to its elements.")] [ContextAction("Convert 'foreach' loop to 'for'", Description = "Works on 'foreach' loops that allow direct access to its elements.")]
public class ConvertForeachToForAction : ICodeActionProvider public class ConvertForeachToForAction : ICodeActionProvider
{ {
static string[] VariableNames = new string[] { "i", "j", "k", "l", "n", "m", "x", "y", "z"};
static string[] CollectionNames = new string[] { "list" };
static string GetName(ICSharpCode.NRefactory.CSharp.Resolver.CSharpResolver state, string[] variableNames)
{
for (int i = 0; i < 1000; i++) {
foreach (var vn in variableNames) {
string id = i > 0 ? vn + i : vn;
var rr = state.LookupSimpleNameOrTypeName(id, new IType[0], NameLookupMode.Expression);
if (rr.IsError)
return id;
}
}
return null;
}
public IEnumerable<CodeAction> GetActions(RefactoringContext context) public IEnumerable<CodeAction> GetActions(RefactoringContext context)
{ {
var foreachStatement = GetForeachStatement(context); var foreachStatement = GetForeachStatement(context);
if (foreachStatement == null) { if (foreachStatement == null) {
yield break; yield break;
} }
var state = context.GetResolverStateBefore (foreachStatement.EmbeddedStatement);
string name = GetName(state, VariableNames);
if (name == null) // very unlikely, but just in case ...
yield break;
yield return new CodeAction(context.TranslateString("Convert 'foreach' loop to 'for'"), script => { yield return new CodeAction(context.TranslateString("Convert 'foreach' loop to 'for'"), script => {
var result = context.Resolve(foreachStatement.InExpression); var result = context.Resolve(foreachStatement.InExpression);
var countProperty = GetCountProperty(result.Type); var countProperty = GetCountProperty(result.Type);
// TODO: use another variable name if 'i' is already in use // TODO: use another variable name if 'i' is already in use
var initializer = new VariableDeclarationStatement(new PrimitiveType("int"), "i", new PrimitiveExpression(0)); var initializer = new VariableDeclarationStatement(new PrimitiveType("int"), name, new PrimitiveExpression(0));
var id1 = new IdentifierExpression("i"); var id1 = new IdentifierExpression(name);
var id2 = id1.Clone(); var id2 = id1.Clone();
var id3 = id1.Clone(); var id3 = id1.Clone();
var inExpression = foreachStatement.InExpression;
Statement declarationStatement = null;
if (inExpression is ObjectCreateExpression || inExpression is ArrayCreateExpression) {
string listName = GetName(state, CollectionNames) ?? "col";
declarationStatement = new VariableDeclarationStatement (
new PrimitiveType ("var"),
listName,
inExpression.Clone ()
);
inExpression = new IdentifierExpression (listName);
}
var variableDeclarationStatement = new VariableDeclarationStatement( var variableDeclarationStatement = new VariableDeclarationStatement(
foreachStatement.VariableType.Clone(), foreachStatement.VariableType.Clone(),
foreachStatement.VariableName, foreachStatement.VariableName,
new IndexerExpression(foreachStatement.InExpression.Clone(), id3) new IndexerExpression(inExpression.Clone(), id3)
); );
var forStatement = new ForStatement() { var forStatement = new ForStatement() {
Initializers = { initializer }, Initializers = { initializer },
Condition = new BinaryOperatorExpression (id1, BinaryOperatorType.LessThan, new MemberReferenceExpression (foreachStatement.InExpression.Clone (), countProperty)), Condition = new BinaryOperatorExpression (id1, BinaryOperatorType.LessThan, new MemberReferenceExpression (inExpression.Clone (), countProperty)),
Iterators = { new ExpressionStatement (new UnaryOperatorExpression (UnaryOperatorType.PostIncrement, id2)) }, Iterators = { new ExpressionStatement (new UnaryOperatorExpression (UnaryOperatorType.PostIncrement, id2)) },
EmbeddedStatement = new BlockStatement { EmbeddedStatement = new BlockStatement {
variableDeclarationStatement variableDeclarationStatement
@ -79,6 +112,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
} else { } else {
forStatement.EmbeddedStatement.AddChild (foreachStatement.EmbeddedStatement.Clone (), BlockStatement.StatementRole); forStatement.EmbeddedStatement.AddChild (foreachStatement.EmbeddedStatement.Clone (), BlockStatement.StatementRole);
} }
if (declarationStatement != null)
script.InsertBefore (foreachStatement, declarationStatement);
script.Replace (foreachStatement, forStatement); script.Replace (foreachStatement, forStatement);
script.Link (initializer.Variables.First ().NameToken, id1, id2, id3); script.Link (initializer.Variables.First ().NameToken, id1, id2, id3);
}); });

31
ICSharpCode.NRefactory.Tests/CSharp/CodeActions/ConvertForeachToForTests.cs

@ -113,5 +113,36 @@ namespace ICSharpCode.NRefactory.CSharp.CodeActions
"}" "}"
); );
} }
/// <summary>
/// Bug 9876 - Convert to for loop created invalid code if iteration variable is called i
/// </summary>
[Test]
public void TestBug9876 ()
{
Test<ConvertForeachToForAction> (@"class TestClass
{
void TestMethod ()
{
$foreach (var i in new[] { 1, 2, 3 }) {
Console.WriteLine (i);
}
}
}", @"class TestClass
{
void TestMethod ()
{
var list = new[] {
1,
2,
3
};
for (int j = 0; j < list.Length; j++) {
var i = list [j];
Console.WriteLine (i);
}
}
}");
}
} }
} }
Loading…
Cancel
Save