From 35475a24144e7f4f9eff01294fde63241a45fa69 Mon Sep 17 00:00:00 2001 From: Simon Lindgren Date: Mon, 30 Jul 2012 11:14:07 +0200 Subject: [PATCH] [Refactoring] Make NamingHelper less horrible and remove the helper from RefactoringContext. --- .../Completion/CSharpCompletionEngine.cs | 63 +++++++++++++++++-- .../CodeActions/IterateViaForeachAction.cs | 3 +- .../Refactoring/NamingHelper.cs | 36 +++++------ .../Refactoring/RefactoringContext.cs | 15 ----- .../CSharp/Refactoring/NamingHelperTests.cs | 53 ++++++++++------ 5 files changed, 110 insertions(+), 60 deletions(-) diff --git a/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngine.cs b/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngine.cs index 9c7e5c5b1a..71fdde7687 100644 --- a/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngine.cs +++ b/ICSharpCode.NRefactory.CSharp/Completion/CSharpCompletionEngine.cs @@ -129,7 +129,62 @@ namespace ICSharpCode.NRefactory.CSharp.Completion } return Enumerable.Empty(); } - + + IEnumerable GenerateNameProposals(AstType type) + { + if (type is PrimitiveType) { + var pt = (PrimitiveType)type; + switch (pt.Keyword) { + case "object": + yield return "o"; + yield return "obj"; + break; + case "bool": + yield return "b"; + yield return "pred"; + break; + case "double": + case "float": + case "decimal": + yield return "d"; + yield return "f"; + yield return "m"; + break; + default: + yield return "i"; + yield return "j"; + yield return "k"; + break; + } + yield break; + } + string name; + if (type is SimpleType) { + name = ((SimpleType)type).Identifier; + } else if (type is MemberType) { + name = ((MemberType)type).MemberName; + } else { + yield break; + } + + var names = WordParser.BreakWords(name); + + var possibleName = new StringBuilder(); + for (int i = 0; i < names.Count; i++) { + possibleName.Length = 0; + for (int j = i; j < names.Count; j++) { + if (string.IsNullOrEmpty(names [j])) { + continue; + } + if (j == i) { + names [j] = Char.ToLower(names [j] [0]) + names [j].Substring(1); + } + possibleName.Append(names [j]); + } + yield return possibleName.ToString(); + } + } + IEnumerable HandleMemberReferenceCompletion(ExpressionResult expr) { if (expr == null) @@ -354,7 +409,7 @@ namespace ICSharpCode.NRefactory.CSharp.Completion if (parent.Variables.Count != 1) return DefaultControlSpaceItems(isAsExpression, controlSpace); - foreach (var possibleName in NamingHelper.GenerateNameProposals (parent.Type)) { + foreach (var possibleName in GenerateNameProposals (parent.Type)) { if (possibleName.Length > 0) { proposeNameList.Result.Add(factory.CreateLiteralCompletionData(possibleName.ToString())); } @@ -1064,7 +1119,7 @@ namespace ICSharpCode.NRefactory.CSharp.Completion } if (node is Identifier && node.Parent is ForeachStatement) { var foreachStmt = (ForeachStatement)node.Parent; - foreach (var possibleName in NamingHelper.GenerateNameProposals (foreachStmt.VariableType)) { + foreach (var possibleName in GenerateNameProposals (foreachStmt.VariableType)) { if (possibleName.Length > 0) { wrapper.Result.Add(factory.CreateLiteralCompletionData(possibleName.ToString())); } @@ -1082,7 +1137,7 @@ namespace ICSharpCode.NRefactory.CSharp.Completion // Try Parameter name case var param = node.Parent as ParameterDeclaration; if (param != null) { - foreach (var possibleName in NamingHelper.GenerateNameProposals (param.Type)) { + foreach (var possibleName in GenerateNameProposals (param.Type)) { if (possibleName.Length > 0) { wrapper.Result.Add(factory.CreateLiteralCompletionData(possibleName.ToString())); } diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/IterateViaForeachAction.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/IterateViaForeachAction.cs index 660ea7dadd..9446fd088c 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/IterateViaForeachAction.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeActions/IterateViaForeachAction.cs @@ -116,9 +116,10 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring static ForeachStatement MakeForeach(Expression node, IType type, RefactoringContext context) { + var namingHelper = new NamingHelper(context); return new ForeachStatement() { VariableType = new SimpleType("var"), - VariableName = context.GenerateVariableName(type), + VariableName = namingHelper.GenerateVariableName(type), InExpression = node.Clone(), EmbeddedStatement = new BlockStatement() }; diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/NamingHelper.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/NamingHelper.cs index fc687f3e66..b444afcc8c 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/NamingHelper.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/NamingHelper.cs @@ -33,17 +33,17 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { public class NamingHelper { - ISet UsedVariableNames; + ISet usedVariableNames; RefactoringContext context; public NamingHelper(RefactoringContext context) { this.context = context; - var astNode = context.GetNode(); - if (UsedVariableNames == null) { + if (usedVariableNames == null) { var visitor = new VariableFinderVisitor(); + var astNode = context.GetNode(); astNode.AcceptVisitor(visitor); - UsedVariableNames = visitor.VariableNames; + usedVariableNames = visitor.VariableNames; } } @@ -88,21 +88,10 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring } var names = WordParser.BreakWords(name); - - var possibleName = new StringBuilder(); - for (int i = 0; i < names.Count; i++) { - possibleName.Length = 0; - for (int j = i; j < names.Count; j++) { - if (string.IsNullOrEmpty(names [j])) { - continue; - } - if (j == i) { - names [j] = Char.ToLower(names [j] [0]) + names [j].Substring(1); - } - possibleName.Append(names [j]); - } - yield return possibleName.ToString(); + if (names.Count > 0) { + names [0] = Char.ToLower(names [0] [0]) + names [0].Substring(1); } + yield return string.Join("", names); } /// @@ -119,8 +108,10 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring string firstSuggestion = null; foreach (var name in NamingHelper.GenerateNameProposals(type)) { firstSuggestion = firstSuggestion ?? name; - if (NameIsUnused(name)) + if (NameIsUnused(name)) { + usedVariableNames.Add(name); return name; + } } // If we get here, all of the standard suggestions are already used. // This will at least be the second variable named based on firstSuggestion, so start at 2 @@ -129,12 +120,13 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring do { proposedName = firstSuggestion + counter++; } while (!NameIsUnused(proposedName)); + usedVariableNames.Add(proposedName); return proposedName; } bool NameIsUnused(string name) { - return !UsedVariableNames.Contains(name) && LookupVariable(name) == null; + return !usedVariableNames.Contains(name) && LookupVariable(name) == null; } /// @@ -192,8 +184,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring IVariable LookupVariable(string name) { - var astNode = context.GetNode(); - var resolverState = context.Resolver.GetResolverStateAfter(astNode.LastChild.PrevSibling); + var blockStatement = context.GetNode(); + var resolverState = context.GetResolverStateAfter(blockStatement.RBraceToken.PrevSibling); var simpleNameRR = resolverState.ResolveSimpleName(name, new List()) as LocalResolveResult; if (simpleNameRR == null) return null; diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/RefactoringContext.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/RefactoringContext.cs index 61c0a44ffc..bd30fe9e31 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/RefactoringContext.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/RefactoringContext.cs @@ -154,21 +154,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { return baseName + (number > 0 ? (number + 1).ToString () : ""); } - - NamingHelper namingHelper = null; - public string GenerateVariableName(AstType type) - { - if (namingHelper == null) - namingHelper = new NamingHelper(this); - return namingHelper.GenerateVariableName(type); - } - - public string GenerateVariableName(IType type) - { - if (namingHelper == null) - namingHelper = new NamingHelper(this); - return namingHelper.GenerateVariableName(type); - } #endregion } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/Refactoring/NamingHelperTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/Refactoring/NamingHelperTests.cs index 0f042f3593..401d93af0a 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/Refactoring/NamingHelperTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/Refactoring/NamingHelperTests.cs @@ -27,11 +27,9 @@ using NUnit.Framework; using ICSharpCode.NRefactory.CSharp.Refactoring; using ICSharpCode.NRefactory.CSharp.CodeActions; using ICSharpCode.NRefactory.CSharp; -using System.Linq; namespace ICSharpCode.NRefactory.CSharp.Refactoring { - [TestFixture] public class NamingHelperTests { @@ -51,7 +49,7 @@ class A { $ } }" ); - var name = context.GenerateVariableName(new PrimitiveType("int")); + var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int")); Assert.NotNull(name); Assert.AreEqual("i", name); } @@ -69,7 +67,7 @@ class A } }" ); - var name = context.GenerateVariableName(new PrimitiveType("int")); + var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int")); Assert.NotNull(name); Assert.IsFalse(name == "i", "i was already used and should not be proposed."); } @@ -87,7 +85,7 @@ class A } }" ); - var name = context.GenerateVariableName(new PrimitiveType("int")); + var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int")); Assert.NotNull(name); Assert.IsFalse(name == "i", "i was already used and should not be proposed."); } @@ -105,7 +103,7 @@ class A } }" ); - var name = context.GenerateVariableName(new PrimitiveType("int")); + var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int")); Assert.NotNull(name); Assert.IsFalse(name == "i", "i was already used and should not be proposed."); } @@ -122,7 +120,7 @@ class A $ } }", true); - var name = context.GenerateVariableName(new PrimitiveType("int")); + var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int")); Assert.NotNull(name); Assert.IsFalse(name == "i", "i was already used and should not be proposed."); } @@ -139,7 +137,7 @@ class A } }" ); - var name = context.GenerateVariableName(new PrimitiveType("int")); + var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int")); Assert.NotNull(name); Assert.IsFalse(name == "i", "i was already used and should not be proposed."); } @@ -156,7 +154,7 @@ class A } }" ); - var name = context.GenerateVariableName(new PrimitiveType("int")); + var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int")); Assert.NotNull(name); Assert.IsFalse(name == "i", "i was already used and should not be proposed."); } @@ -177,7 +175,7 @@ class A } }" ); - var name = context.GenerateVariableName(new PrimitiveType("int")); + var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int")); Assert.NotNull(name); Assert.AreEqual("i3", name); } @@ -202,7 +200,7 @@ class A } }" ); - var name = context.GenerateVariableName(new PrimitiveType("int")); + var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int")); Assert.NotNull(name); Assert.IsTrue(name == "i"); } @@ -221,7 +219,7 @@ class A } }" ); - var name = context.GenerateVariableName(new PrimitiveType("int")); + var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int")); Assert.NotNull(name); Assert.AreEqual("i2", name); } @@ -229,7 +227,6 @@ class A [Test] public void GenerateVariableNameIgnoresFixedVariables() { - // Snippet tests that identifiers from in, into and let clauses are found var context = MakeContext(@" class A { @@ -240,7 +237,7 @@ class A } }" ); - var name = context.GenerateVariableName(new PrimitiveType("int")); + var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int")); Assert.NotNull(name); Assert.AreEqual("j", name); } @@ -258,13 +255,13 @@ class A } }" ); - var name = context.GenerateVariableName(new PrimitiveType("int")); + var name = new NamingHelper(context).GenerateVariableName(new PrimitiveType("int")); Assert.NotNull(name); Assert.AreEqual("i2", name); } - + [Test] - public void GenerateVariableNameForComposedType() + public void GenerateVariableNameForCustomType() { var context = MakeContext(@" class A @@ -275,9 +272,29 @@ class A } }" ); - var name = context.GenerateVariableName(new SimpleType() { Identifier = "VariableNameGenerationTester" }); + var name = new NamingHelper(context).GenerateVariableName(new SimpleType() { Identifier = "VariableNameGenerationTester" }); Assert.NotNull(name); Assert.AreEqual("variableNameGenerationTester", name); } + + [Test] + public void GenerateVariableNameDoesNotRepeatNames() + { + var context = MakeContext(@" +class A +{ + void F() + { + $ + } +}" + ); + var namingHelper = new NamingHelper(context); + var name1 = namingHelper.GenerateVariableName(new PrimitiveType("int")); + var name2 = namingHelper.GenerateVariableName(new PrimitiveType("int")); + Assert.NotNull(name1); + Assert.NotNull(name2); + Assert.AreNotEqual(name1, name2, "The generated names should not repeat."); + } } }