Skip to content

Commit

Permalink
Omit the "new" keyword when not required - fixes #504 - fixes #463
Browse files Browse the repository at this point in the history
  • Loading branch information
GrahamTheCoder committed Jan 17, 2020
1 parent 186cd76 commit 7c43378
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 11 deletions.
14 changes: 12 additions & 2 deletions ICSharpCode.CodeConverter/CSharp/CommonConversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -310,12 +310,13 @@ public static SyntaxToken CsEscapedIdentifier(string text)
return SyntaxFactory.Identifier(text);
}

public SyntaxTokenList ConvertModifiers(SyntaxNode node, IEnumerable<SyntaxToken> modifiers,
public SyntaxTokenList ConvertModifiers(SyntaxNode node, IReadOnlyCollection<SyntaxToken> modifiers,
TokenContext context = TokenContext.Global, bool isVariableOrConst = false, params CSSyntaxKind[] extraCsModifierKinds)
{
ISymbol declaredSymbol = _semanticModel.GetDeclaredSymbol(node);
var declaredAccessibility = declaredSymbol?.DeclaredAccessibility ?? Accessibility.NotApplicable;

modifiers = modifiers.Where(m =>
!m.IsKind(SyntaxKind.OverloadsKeyword) || RequiresNewKeyword(declaredSymbol) != false).ToList();
var contextsWithIdenticalDefaults = new[] { TokenContext.Global, TokenContext.Local, TokenContext.InterfaceOrModule, TokenContext.MemberInInterface };
bool isPartial = declaredSymbol.IsPartialClassDefinition() || declaredSymbol.IsPartialMethodDefinition() || declaredSymbol.IsPartialMethodImplementation();
bool implicitVisibility = ContextHasIdenticalDefaults(context, contextsWithIdenticalDefaults, declaredSymbol)
Expand All @@ -328,6 +329,15 @@ public SyntaxTokenList ConvertModifiers(SyntaxNode node, IEnumerable<SyntaxToken
return SyntaxFactory.TokenList(modifierSyntaxs);
}

private static bool? RequiresNewKeyword(ISymbol declaredSymbol)
{
if (!(declaredSymbol is IMethodSymbol methodSymbol)) return null;
if (declaredSymbol.IsOverride ) return false;
var parameterSignature = methodSymbol.GetParameterSignature();
return declaredSymbol.ContainingType.FollowProperty(s => s.BaseType).Skip(1).Any(t => t.GetMembers()
.Any(s => s.Name == declaredSymbol.Name && s is IMethodSymbol m && m.GetParameterSignature() == parameterSignature));
}

private static bool ContextHasIdenticalDefaults(TokenContext context, TokenContext[] contextsWithIdenticalDefaults, ISymbol declaredSymbol)
{
if (!contextsWithIdenticalDefaults.Contains(context)) {
Expand Down
4 changes: 2 additions & 2 deletions ICSharpCode.CodeConverter/CSharp/DeclarationNodeVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ private async Task<List<MemberDeclarationSyntax>> GetMemberDeclarations(VBSyntax
node.Modifiers.Where(m => !SyntaxTokenExtensions.IsKind(m, VBasic.SyntaxKind.WithEventsKeyword));
var isWithEvents = node.Modifiers.Any(m => SyntaxTokenExtensions.IsKind(m, VBasic.SyntaxKind.WithEventsKeyword));
var convertedModifiers =
CommonConversions.ConvertModifiers(node.Declarators[0].Names[0], convertableModifiers, GetMemberContext(node));
CommonConversions.ConvertModifiers(node.Declarators[0].Names[0], convertableModifiers.ToList(), GetMemberContext(node));
var declarations = new List<MemberDeclarationSyntax>(node.Declarators.Count);

foreach (var declarator in node.Declarators)
Expand Down Expand Up @@ -571,7 +571,7 @@ public override async Task<CSharpSyntaxNode> VisitPropertyStatement(VBSyntax.Pro
var isReadonly = node.Modifiers.Any(m => SyntaxTokenExtensions.IsKind(m, VBasic.SyntaxKind.ReadOnlyKeyword));
var isWriteOnly = node.Modifiers.Any(m => SyntaxTokenExtensions.IsKind(m, VBasic.SyntaxKind.WriteOnlyKeyword));
var convertibleModifiers = node.Modifiers.Where(m => !m.IsKind(VBasic.SyntaxKind.ReadOnlyKeyword, VBasic.SyntaxKind.WriteOnlyKeyword, VBasic.SyntaxKind.DefaultKeyword));
var modifiers = CommonConversions.ConvertModifiers(node, convertibleModifiers, GetMemberContext(node));
var modifiers = CommonConversions.ConvertModifiers(node, convertibleModifiers.ToList(), GetMemberContext(node));
var isIndexer = CommonConversions.IsDefaultIndexer(node);
var accessedThroughMyClass = IsAccessedThroughMyClass(node, node.Identifier, _semanticModel.GetDeclaredSymbol(node));
bool isInInterface = node.Ancestors().OfType<VBSyntax.InterfaceBlockSyntax>().FirstOrDefault() != null;
Expand Down
13 changes: 13 additions & 0 deletions ICSharpCode.CodeConverter/Util/IMethodSymbolExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
using System.Linq;
using Microsoft.CodeAnalysis;

namespace ICSharpCode.CodeConverter.Util
{
internal static class IMethodSymbolExtensions
{
public static string GetParameterSignature(this IMethodSymbol methodSymbol)
{
return string.Join(" ", methodSymbol.Parameters.Select(p => p.Type));
}
}
}
2 changes: 1 addition & 1 deletion ICSharpCode.CodeConverter/VB/CaseConflictResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ private static (ISymbol Original, string NewName)[] GetMethodSymbolsWithNewNames
bool specialSymbolUsingName)
{
var methodsByCaseInsensitiveSignature = methodSymbols
.ToLookup(m => (m.Name.ToLowerInvariant(), string.Join(" ", m.Parameters.Select(p => p.Type))))
.ToLookup(m => (m.Name.ToLowerInvariant(), m.GetParameterSignature()))
.Where(g => g.Count() > 1)
.SelectMany(clashingMethodGroup =>
{
Expand Down
15 changes: 11 additions & 4 deletions Tests/CSharp/MemberTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1349,7 +1349,10 @@ await TestConversionVisualBasicToCSharp(
Public Shared Sub CreateStatic()
End Sub
Public Sub CreateInstance()
Public Overloads Sub CreateInstance()
End Sub
Public Overloads Sub CreateInstance(o As Object)
End Sub
Public MustOverride Sub CreateAbstractInstance()
Expand All @@ -1368,8 +1371,8 @@ End Sub
Public Overrides Sub CreateAbstractInstance()
End Sub
Public Overrides Sub CreateVirtualInstance()
Public Overloads Sub CreateVirtualInstance(o As Object)
End Sub
End Class",
@"internal abstract partial class TestClass1
Expand All @@ -1382,6 +1385,10 @@ public void CreateInstance()
{
}
public void CreateInstance(object o)
{
}
public abstract void CreateAbstractInstance();
public virtual void CreateVirtualInstance()
Expand All @@ -1403,7 +1410,7 @@ public override void CreateAbstractInstance()
{
}
public override void CreateVirtualInstance()
public void CreateVirtualInstance(object o)
{
}
}");
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 7c43378

Please # to comment.