Overescape identifiers

CHANGELOG.md

@@ -9,7 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 
 ### VB -> C#
-
+* Remove square brackets from identifiers [#1043](https://github.com/icsharpcode/CodeConverter/issues/1043)
 
 ### C# -> VB
 

CodeConverter/CSharp/CommonConversions.cs

@@ -341,8 +341,11 @@ private static string WithDeclarationName(SyntaxToken id, ISymbol idSymbol, stri
 
  public static SyntaxToken CsEscapedIdentifier(string text)
  {
- text = text.TrimStart('[').TrimEnd(']');
- if (SyntaxFacts.GetKeywordKind(text) != CSSyntaxKind.None) text = "@" + text;
+ if (!CS.SyntaxFacts.IsValidIdentifier(text)) {
+ text = new string(text.Where(CS.SyntaxFacts.IsIdentifierPartCharacter).ToArray());
+ if (!CS.SyntaxFacts.IsIdentifierStartCharacter(text[0])) text = "a" + text;
+ }
+ if (SyntaxFacts.GetKeywordKind(text) != CSSyntaxKind.None || SyntaxFacts.GetContextualKeywordKind(text) != CSSyntaxKind.None) text = "@" + text;
  return SyntaxFactory.Identifier(text);
  }
 

CodeConverter/CSharp/DeclarationNodeVisitor.cs

@@ -95,7 +95,7 @@ public override async Task<CSharpSyntaxNode> VisitCompilationUnit(VBSyntax.Compi
 
  var convertedMembers = string.IsNullOrEmpty(options.RootNamespace)
  ? sourceAndConverted.Select(sd => sd.Converted)
- : PrependRootNamespace(sourceAndConverted, ValidSyntaxFactory.IdentifierName(options.RootNamespace));
+ : PrependRootNamespace(sourceAndConverted, options.RootNamespace);
 
  var usings = await importsClauses
  .SelectAsync(async c => await c.AcceptAsync<UsingDirectiveSyntax>(TriviaConvertingDeclarationVisitor));
@@ -126,13 +126,13 @@ private static bool HasSourceMapAnnotations(UsingDirectiveSyntax c)
 
  private IReadOnlyCollection<MemberDeclarationSyntax> PrependRootNamespace(
  IReadOnlyCollection<(VBSyntax.StatementSyntax VbNode, MemberDeclarationSyntax CsNode)> membersConversions,
- IdentifierNameSyntax rootNamespaceIdentifier)
+ string rootNamespace)
  {
 
  if (_topAncestorNamespace != null) {
- var csMembers = membersConversions.ToLookup(c => ShouldBeNestedInRootNamespace(c.VbNode, rootNamespaceIdentifier.Identifier.Text), c => c.CsNode);
+ var csMembers = membersConversions.ToLookup(c => ShouldBeNestedInRootNamespace(c.VbNode, rootNamespace), c => c.CsNode);
  var nestedMembers = csMembers[true].Select<MemberDeclarationSyntax, SyntaxNode>(x => x);
- var newNamespaceDecl = (MemberDeclarationSyntax) _csSyntaxGenerator.NamespaceDeclaration(rootNamespaceIdentifier.Identifier.Text, nestedMembers);
+ var newNamespaceDecl = (MemberDeclarationSyntax) _csSyntaxGenerator.NamespaceDeclaration(rootNamespace, nestedMembers);
  return csMembers[false].Concat(new[] { newNamespaceDecl }).ToArray();
  }
  return membersConversions.Select(n => n.CsNode).ToArray();

CodeConverter/CSharp/ExpressionNodeVisitor.cs

@@ -726,7 +726,7 @@ await node.Name.AcceptAsync<ExpressionSyntax>(TriviaConvertingExpressionVisitor)
  );
  }
 
- private string GenerateUniqueVariableName(VisualBasicSyntaxNode existingNode, string varNameBase) => NameGenerator.GetUniqueVariableNameInScope(_semanticModel, _generatedNames, existingNode, varNameBase);
+ private string GenerateUniqueVariableName(VisualBasicSyntaxNode existingNode, string varNameBase) => NameGenerator.CS.GetUniqueVariableNameInScope(_semanticModel, _generatedNames, existingNode, varNameBase);
 
  private static ExpressionSyntax DeclareVariableInline(ExpressionSyntax csExpressionSyntax, string temporaryName)
  {

CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs

@@ -962,7 +962,7 @@ public override async Task<SyntaxList<StatementSyntax>> VisitWithBlock(VBSyntax.
 
  private string GetUniqueVariableNameInScope(VBasic.VisualBasicSyntaxNode node, string variableNameBase)
  {
- return NameGenerator.GetUniqueVariableNameInScope(_semanticModel, _generatedNames, node, variableNameBase);
+ return NameGenerator.CS.GetUniqueVariableNameInScope(_semanticModel, _generatedNames, node, variableNameBase);
  }
 
  public override async Task<SyntaxList<StatementSyntax>> VisitTryBlock(VBSyntax.TryBlockSyntax node)

CodeConverter/CSharp/PerScopeState.cs

@@ -100,7 +100,7 @@ public SyntaxList<StatementSyntax> CreateStatements(VBasic.VisualBasicSyntaxNode
  {
  var localFunctions = GetParameterlessFunctions(); 
  var newNames = localFunctions.ToDictionary(f => f.Id, f =>
- NameGenerator.GetUniqueVariableNameInScope(semanticModel, generatedNames, vbNode, f.Prefix)
+ NameGenerator.CS.GetUniqueVariableNameInScope(semanticModel, generatedNames, vbNode, f.Prefix)
  );
  var functions = localFunctions.Select(f => f.AsLocalFunction(newNames[f.Id]));
  statements = ReplaceNames(functions.Concat(statements), newNames);
@@ -121,7 +121,7 @@ public async Task<SyntaxList<StatementSyntax>> CreateLocalsAsync(VBasic.VisualBa
  HoistToParent(variable);
  } else {
  // The variable comes from the VB scope, only check for conflict with other hoisted definitions
- string name = NameGenerator.GenerateUniqueVariableName(generatedNames, variable.OriginalVariableName);
+ string name = NameGenerator.CS.GenerateUniqueVariableName(generatedNames, CommonConversions.CsEscapedIdentifier(variable.OriginalVariableName).Text);
  if (variable.Nested) {
  newNames.Add(variable.Id, name);
  } else if (name != variable.OriginalVariableName) {
@@ -135,7 +135,7 @@ public async Task<SyntaxList<StatementSyntax>> CreateLocalsAsync(VBasic.VisualBa
 
  var additionalDeclarationInfo = GetDeclarations();
  foreach (var additionalLocal in additionalDeclarationInfo) {
- newNames.Add(additionalLocal.Id, NameGenerator.GetUniqueVariableNameInScope(semanticModel, generatedNames, vbNode, additionalLocal.Prefix)); 
+ newNames.Add(additionalLocal.Id, NameGenerator.CS.GetUniqueVariableNameInScope(semanticModel, generatedNames, vbNode, additionalLocal.Prefix)); 
  var decl = CommonConversions.CreateVariableDeclarationAndAssignment(newNames[additionalLocal.Id],
  additionalLocal.Initializer, additionalLocal.Type);
  preDeclarations.Add(CS.SyntaxFactory.LocalDeclarationStatement(decl));
@@ -158,7 +158,7 @@ public async Task<SyntaxList<MemberDeclarationSyntax>> CreateVbStaticFieldsAsync
 
  var fieldInfo = GetFields();
  var newNames = fieldInfo.ToDictionary(f => f.OriginalVariableName, f =>
- NameGenerator.GetUniqueVariableNameInScope(semanticModel, generatedNames, typeNode, f.FieldName)
+ NameGenerator.CS.GetUniqueVariableNameInScope(semanticModel, generatedNames, typeNode, f.FieldName)
  );
  foreach (var field in fieldInfo) {
  var decl = (field.Initializer != null) 

CodeConverter/Common/NameGenerator.cs

@@ -1,40 +1,35 @@
 // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using ICSharpCode.CodeConverter.CSharp;
+
 namespace ICSharpCode.CodeConverter.Common;
 
-internal static class NameGenerator
+internal class NameGenerator
 {
- public static string GenerateUniqueName(string baseName, Func<string, bool> canUse)
- {
- return GenerateUniqueName(baseName, string.Empty, canUse);
- }
+ private readonly Func<string, string> _getValidIdentifier;
 
- private static string GenerateUniqueName(string baseName, string extension, Func<string, bool> canUse)
- {
- if (!string.IsNullOrEmpty(extension) && !extension.StartsWith(".", StringComparison.InvariantCulture)) {
- extension = "." + extension;
- }
+ public static NameGenerator CS { get; } = new NameGenerator(x => CommonConversions.CsEscapedIdentifier(x).Text);
+ public static NameGenerator Generic { get; } = new NameGenerator(x => x);
 
- var name = baseName + extension;
- var index = 1;
+ private NameGenerator(Func<string, string> getValidIdentifier) => _getValidIdentifier = getValidIdentifier;
 
- // Check for collisions
- while (!canUse(name)) {
- name = baseName + index + extension;
- index++;
- }
-
- return name;
- }
+ public string GenerateUniqueName(string baseName, Func<string, bool> canUse) => GenerateUniqueName(baseName, string.Empty, canUse);
 
- public static string GetUniqueVariableNameInScope(SemanticModel semanticModel, HashSet<string> generatedNames, VBasic.VisualBasicSyntaxNode node, string variableNameBase)
+ public string GetUniqueVariableNameInScope(SemanticModel semanticModel, HashSet<string> generatedNames, VBasic.VisualBasicSyntaxNode node, string variableNameBase)
  {
  // Need to check not just the symbols this node has access to, but whether there are any nested blocks which have access to this node and contain a conflicting name
  var scopeStarts = GetScopeStarts(node);
  return GenerateUniqueVariableNameInScope(semanticModel, generatedNames, variableNameBase, scopeStarts);
  }
 
- private static string GenerateUniqueVariableNameInScope(SemanticModel semanticModel, HashSet<string> generatedNames,
+ public string GenerateUniqueVariableName(HashSet<string> generatedNames, string variableNameBase)
+ {
+ string uniqueName = GenerateUniqueName(variableNameBase, string.Empty, n => !generatedNames.Contains(n));
+ generatedNames.Add(uniqueName);
+ return uniqueName;
+ }
+
+ private string GenerateUniqueVariableNameInScope(SemanticModel semanticModel, HashSet<string> generatedNames,
  string variableNameBase, List<int> scopeStarts)
  {
  string uniqueName = GenerateUniqueName(variableNameBase, string.Empty,
@@ -47,16 +42,29 @@ private static string GenerateUniqueVariableNameInScope(SemanticModel semanticMo
  return uniqueName;
  }
 
+ private string GenerateUniqueName(string baseName, string extension, Func<string, bool> canUse)
+ {
+ baseName = _getValidIdentifier(baseName);
+ if (!string.IsNullOrEmpty(extension) && !extension.StartsWith(".", StringComparison.InvariantCulture)) {
+ extension = "." + extension;
+ }
+
+ var name = baseName + extension;
+ var index = 1;
+
+ // Check for collisions
+ while (!canUse(name)) {
+ name = baseName + index + extension;
+ index++;
+ }
+
+ return name;
+ }
+
  private static List<int> GetScopeStarts(VBasic.VisualBasicSyntaxNode node)
  {
  return node.GetAncestorOrThis<VBSyntax.StatementSyntax>().DescendantNodesAndSelf()
  .OfType<VBSyntax.StatementSyntax>().Select(n => n.SpanStart).ToList();
  }
 
- public static string GenerateUniqueVariableName(HashSet<string> generatedNames, string variableNameBase)
- {
- string uniqueName = GenerateUniqueName(variableNameBase, string.Empty, n => !generatedNames.Contains(n));
- generatedNames.Add(uniqueName);
- return uniqueName;
- }
 }

CodeConverter/Common/SymbolRenamer.cs

@@ -5,6 +5,8 @@ namespace ICSharpCode.CodeConverter.Common;
 
 internal static class SymbolRenamer
 {
+ private static readonly NameGenerator NameGenerator = NameGenerator.Generic;
+
  public static IEnumerable<(ISymbol Original, string NewName)> GetSymbolsWithNewNames(
  IEnumerable<ISymbol> toRename, Func<string, bool> canUse, bool canKeepOne)
  {

CodeConverter/VB/NodesVisitor.cs

@@ -475,7 +475,7 @@ private ImplementsClauseSyntax CreateImplementsClauseSyntaxOrNull(ISymbol member
 #pragma warning restore RS1024 // Compare symbols correctly
  string explicitMemberName = UndottedMemberName(memberInfo.Name);
  var hasDuplicateNames = memberNames[explicitMemberName].Count() > 1;
- if (hasDuplicateNames) id = SyntaxFactory.Identifier(NameGenerator.GenerateUniqueName(explicitMemberName, n => !memberNames.Contains(n) && _addedNames.Add(n)));
+ if (hasDuplicateNames) id = SyntaxFactory.Identifier(NameGenerator.Generic.GenerateUniqueName(explicitMemberName, n => !memberNames.Contains(n) && _addedNames.Add(n)));
  } else {
  var containingType = memberInfo.ContainingType;
  var baseClassesAndInterfaces = containingType.GetAllBaseClassesAndInterfaces(true);

Tests/CSharp/ExpressionTests/ExpressionTests.cs

@@ -1029,7 +1029,7 @@ End Sub
 
 public static partial class MyExtensions
 {
- public static void NewColumn(Type type, string strV1 = null, string code = ""code"", int argInt = 1)
+ public static void NewColumn(Type @type, string strV1 = null, string code = ""code"", int argInt = 1)
  {
  }
 
@@ -2524,7 +2524,7 @@ Static i As Integer
  @"
 internal partial class SurroundingClass
 {
- private int _[Step]_i;
+ private int _Step_i;
 
  public void Step()
  {