Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed #325: #326

Merged
merged 21 commits into from Jul 21, 2019
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -10,6 +10,7 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.FindSymbols;
using Microsoft.CodeAnalysis.Operations;
using Microsoft.VisualBasic.CompilerServices;
using SyntaxFactory = Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
using SyntaxNodeExtensions = ICSharpCode.CodeConverter.Util.SyntaxNodeExtensions;
@@ -1302,23 +1303,24 @@ public override CSharpSyntaxNode VisitMemberAccessExpression(VBSyntax.MemberAcce
{
var simpleNameSyntax = (SimpleNameSyntax)node.Name.Accept(TriviaConvertingVisitor);

var symbolInfo = _semanticModel.GetSymbolInfo(node.Name);
var nodeSymbol = _semanticModel.GetSymbolInfo(node.Name).Symbol;
var isDefaultProperty = nodeSymbol is IPropertySymbol p && VBasic.VisualBasicExtensions.IsDefault(p);
ExpressionSyntax left = null;
if (node.Expression is VBSyntax.MyClassExpressionSyntax) {
if (symbolInfo.Symbol.IsStatic) {
if (nodeSymbol.IsStatic) {
var typeInfo = _semanticModel.GetTypeInfo(node.Expression);
left = _semanticModel.GetCsTypeSyntax(typeInfo.Type, node);
} else {
left = SyntaxFactory.ThisExpression();
if (symbolInfo.Symbol.IsVirtual && !symbolInfo.Symbol.IsAbstract) {
if (nodeSymbol.IsVirtual && !nodeSymbol.IsAbstract) {
simpleNameSyntax = SyntaxFactory.IdentifierName($"MyClass{ConvertIdentifier(node.Name.Identifier).ValueText}");
}
}
}
if (left == null && symbolInfo.Symbol?.IsStatic == true) {
if (left == null && nodeSymbol?.IsStatic == true) {
var typeInfo = _semanticModel.GetTypeInfo(node.Expression);
var symbol = _semanticModel.GetSymbolInfo(node.Expression);
if (typeInfo.Type != null && !symbol.Symbol.IsType()) {
var expressionSymbolInfo = _semanticModel.GetSymbolInfo(node.Expression);
if (typeInfo.Type != null && !expressionSymbolInfo.Symbol.IsType()) {
left = _semanticModel.GetCsTypeSyntax(typeInfo.Type, node);
}
}
@@ -1327,25 +1329,25 @@ public override CSharpSyntaxNode VisitMemberAccessExpression(VBSyntax.MemberAcce
}
if (left == null) {
if (IsSubPartOfConditionalAccess(node)) {
return SyntaxFactory.MemberBindingExpression(simpleNameSyntax);
return isDefaultProperty ? SyntaxFactory.ElementBindingExpression()
: (ExpressionSyntax) SyntaxFactory.MemberBindingExpression(simpleNameSyntax);
}
left = SyntaxFactory.IdentifierName(_withBlockTempVariableNames.Peek());
} else if (TryGetTypePromotedModuleSymbol(node, out var promotedModuleSymbol)) {
left = SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, left,
SyntaxFactory.IdentifierName(promotedModuleSymbol.Name));
}

if (node.Expression.IsKind(VBasic.SyntaxKind.GlobalName)) {
return SyntaxFactory.AliasQualifiedName((IdentifierNameSyntax)left, simpleNameSyntax);
}

var memberAccessExpressionSyntax = SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, left, simpleNameSyntax);
if (_semanticModel.GetSymbolInfo(node).Symbol.IsKind(SymbolKind.Method) && node.Parent?.IsKind(VBasic.SyntaxKind.InvocationExpression) != true) {
var visitMemberAccessExpression = SyntaxFactory.InvocationExpression(memberAccessExpressionSyntax, SyntaxFactory.ArgumentList());
return visitMemberAccessExpression;

if (isDefaultProperty && left != null) {
return left;
}

return memberAccessExpressionSyntax;
var memberAccessExpressionSyntax = SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, left, simpleNameSyntax);
return AddEmptyArgumentListIfImplicit(node, memberAccessExpressionSyntax);
}

/// <remarks>https://docs.microsoft.com/en-us/dotnet/visual-basic/programming-guide/language-features/declared-elements/type-promotion</remarks>
@@ -1738,25 +1740,25 @@ public override CSharpSyntaxNode VisitBinaryExpression(VBSyntax.BinaryExpression
public override CSharpSyntaxNode VisitInvocationExpression(VBSyntax.InvocationExpressionSyntax node)
{
var invocationSymbol = _semanticModel.GetSymbolInfo(node).ExtractBestMatch();
var symbol = _semanticModel.GetSymbolInfo(node.Expression).ExtractBestMatch();
var symbolReturnType = symbol?.GetReturnType();

if (symbol?.ContainingNamespace.MetadataName == "VisualBasic" && TrySubstituteVisualBasicMethod(node, out var csEquivalent)) {
var expressionSymbol = _semanticModel.GetSymbolInfo(node.Expression).ExtractBestMatch();
var expressionReturnType = expressionSymbol?.GetReturnType() ?? _semanticModel.GetTypeInfo(node.Expression).Type;
var operation = _semanticModel.GetOperation(node);
if (expressionSymbol?.ContainingNamespace.MetadataName == "VisualBasic" && TrySubstituteVisualBasicMethod(node, out var csEquivalent)) {
return csEquivalent;
}

if (TryGetElementAccessExpressionToConvert(out var convertedExpression)) {
return SyntaxFactory.ElementAccessExpression(
convertedExpression,
SyntaxFactory.BracketedArgumentList(SyntaxFactory.SeparatedList(node.ArgumentList.Arguments.Select(a => (ArgumentSyntax)a.Accept(TriviaConvertingVisitor)))));
// VB doesn't have a specialized node for element access because the syntax is ambiguous. Instead, it just uses an invocation expression or dictionary access expression, then figures out using the semantic model which one is most likely intended.
// https://github.com/dotnet/roslyn/blob/master/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb#L768
var convertedExpression = ConvertInvocationSubExpression(out var shouldBeElementAccess);
if (shouldBeElementAccess) {
return CreateElementAccess();
}
convertedExpression = (ExpressionSyntax)node.Expression.Accept(TriviaConvertingVisitor);

if (symbol != null && symbol.IsKind(SymbolKind.Property)) {
if (expressionSymbol != null && expressionSymbol.IsKind(SymbolKind.Property)) {
return convertedExpression;
}

if (invocationSymbol?.Name == nameof(Enumerable.ElementAtOrDefault) && !symbol.Equals(invocationSymbol)) {
if (invocationSymbol?.Name == nameof(Enumerable.ElementAtOrDefault) && !expressionSymbol.Equals(invocationSymbol)) {
_extraUsingDirectives.Add(nameof(System) + "." + nameof(System.Linq));
convertedExpression = SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, convertedExpression,
SyntaxFactory.IdentifierName(nameof(Enumerable.ElementAtOrDefault)));
@@ -1765,36 +1767,49 @@ public override CSharpSyntaxNode VisitInvocationExpression(VBSyntax.InvocationEx

return SyntaxFactory.InvocationExpression(convertedExpression, ConvertArgumentListOrEmpty(node.ArgumentList));

bool TryGetElementAccessExpressionToConvert(out ExpressionSyntax converted)
ExpressionSyntax ConvertInvocationSubExpression(out bool isElementAccess)
{
VBSyntax.ExpressionSyntax toConvert = null;
converted = null;
isElementAccess = IsPropertyElementAccess(operation) ||
IsArrayElementAccess(operation) ||
ProbablyNotAMethodCall(node, expressionSymbol, expressionReturnType);

if (invocationSymbol?.IsIndexer() == true || ProbablyNotAMethodCall(node, symbol, symbolReturnType)) {
toConvert = node.Expression;
}
return (ExpressionSyntax)node.Expression.Accept(TriviaConvertingVisitor);
}

// VB can use an imaginary member "Item" when an object has an indexer
if ((toConvert != null || invocationSymbol?.IsErrorType() == true) && node.Expression is VBSyntax.MemberAccessExpressionSyntax memberAccessExpression && memberAccessExpression.Name.Identifier.Text == "Item") {
toConvert = memberAccessExpression.Expression;
CSharpSyntaxNode CreateElementAccess()
{
var bracketedArgumentListSyntax = SyntaxFactory.BracketedArgumentList(SyntaxFactory.SeparatedList(
node.ArgumentList.Arguments.Select(a => (ArgumentSyntax)a.Accept(TriviaConvertingVisitor))
));
if (convertedExpression is ElementBindingExpressionSyntax binding && !binding.ArgumentList.Arguments.Any())
{
// Special case where structure changes due to conditional access (See VisitMemberAccessExpression)
return binding.WithArgumentList(bracketedArgumentListSyntax);
}

if (toConvert != null) {
converted = (ExpressionSyntax)toConvert.Accept(TriviaConvertingVisitor);
else
{
return SyntaxFactory.ElementAccessExpression(convertedExpression,bracketedArgumentListSyntax);
}

return converted != null;
}
}

private static bool IsPropertyElementAccess(IOperation operation)
{
return operation is IPropertyReferenceOperation pro && pro.Arguments.Any();
}

private static bool IsArrayElementAccess(IOperation operation)
{
return operation != null && operation.Kind == OperationKind.ArrayElementReference;
}

/// <summary>
/// Chances of having an unknown delegate stored as a field/local seem lower than having an unknown non-delegate type with an indexer stored.
/// So for a standalone identifier err on the side of assuming it's an indexer.
/// </summary>
private static bool ProbablyNotAMethodCall(VBSyntax.InvocationExpressionSyntax node, ISymbol symbol, ITypeSymbol symbolReturnType)
{
return !(symbol is IMethodSymbol) && (symbolReturnType.IsErrorType() && node.Expression is VBSyntax.IdentifierNameSyntax
|| symbolReturnType.IsArrayType());
return !(symbol is IMethodSymbol) && symbolReturnType.IsErrorType() && node.Expression is VBSyntax.IdentifierNameSyntax && node.ArgumentList.Arguments.Any();
}

private ArgumentListSyntax ConvertArgumentListOrEmpty(VBSyntax.ArgumentListSyntax argumentListSyntax)
@@ -1994,16 +2009,11 @@ private static SyntaxToken GetMethodBlockBaseIdentifierForImplicitReturn(VBSynta
}
}

private CSharpSyntaxNode AddEmptyArgumentListIfImplicit(VBSyntax.IdentifierNameSyntax node, ExpressionSyntax id)
private CSharpSyntaxNode AddEmptyArgumentListIfImplicit(SyntaxNode node, ExpressionSyntax id)
{
var symbol = GetSymbolInfoInDocument(node);
if (symbol != null &&
(symbol.IsOrdinaryMethod() || symbol.IsExtensionMethod()) &&
!node.Parent.IsKind(VBasic.SyntaxKind.InvocationExpression, VBasic.SyntaxKind.SimpleMemberAccessExpression, VBasic.SyntaxKind.AddressOfExpression)) {
return SyntaxFactory.InvocationExpression(id);
}

return id;
return _semanticModel.GetOperation(node)?.Kind == OperationKind.Invocation
? SyntaxFactory.InvocationExpression(id, SyntaxFactory.ArgumentList())
: id;
}

private ExpressionSyntax QualifyNode(SyntaxNode node, SimpleNameSyntax left)
@@ -37,10 +37,12 @@
<EmbeddedResource Include="Shared\DefaultReferences.cs" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="2.3.2" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="2.3.2" />
<PackageReference Include="Microsoft.CodeAnalysis.VisualBasic" Version="2.3.2" />
<PackageReference Include="Microsoft.CodeAnalysis.VisualBasic.Workspaces" Version="2.3.2" />
<PackageReference Include="Microsoft.CodeAnalysis.Common" Version="2.6.1" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="2.6.1" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="2.6.1" />
<PackageReference Include="Microsoft.CodeAnalysis.VisualBasic" Version="2.6.1" />
<PackageReference Include="Microsoft.CodeAnalysis.VisualBasic.Workspaces" Version="2.6.1" />
<PackageReference Include="Microsoft.CodeAnalysis.Workspaces.Common" Version="2.6.1" />
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0-beta-63127-02">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
@@ -1,4 +1,5 @@
using System;
using System.Data;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using CodeConverter.Tests.TestRunners;
@@ -604,6 +605,129 @@ public void Test()
}");
}

[Fact]
public void MethodCallDictionaryAccessConditional()
{
TestConversionVisualBasicToCSharp(@"Public Class A
Public Sub Test()
Dim dict = New Dictionary(Of String, String) From {{""a"", ""AAA""}, {""b"", ""bbb""}}
Dim v = dict?.Item(""a"")

This comment has been minimized.

Copy link
@mrmonday

mrmonday Jul 21, 2019

Contributor

I don't see a test case for plain ?("a"), without the Item anywhere?

Shouldn't make a difference, but better to explicitly test it?

End Sub
End Class", @"using System.Collections.Generic;
public class A
{
public void Test()
{
var dict = new Dictionary<string, string>() { { ""a"", ""AAA"" }, { ""b"", ""bbb"" } };
var v = dict?[""a""];
}
}");
}

[Fact]
public void IndexerWithParameter()
{
//BUG Semicolon ends up on newline after comment instead of before it
TestConversionVisualBasicToCSharpWithoutComments(@"Imports System.Data
Public Class A
Public Function ReadDataSet(myData As DataSet) As String
With myData.Tables(0).Rows(0)
Return .Item(""MY_COLUMN_NAME"").ToString()
End With
End Function
End Class", @"using System.Data;
public class A
{
public string ReadDataSet(DataSet myData)
{
{
var withBlock = myData.Tables[0].Rows[0];
return withBlock[""MY_COLUMN_NAME""].ToString();
}
}
}");
}

[Fact]
public void MethodCallArrayIndexerBrackets()
{
TestConversionVisualBasicToCSharp(@"Public Class A
Public Sub Test()
Dim str1 = Me.GetStringFromNone(0)
str1 = GetStringFromNone(0)
Dim str2 = GetStringFromNone()(1)
Dim str3 = Me.GetStringsFromString(""abc"")
str3 = GetStringsFromString(""abc"")
Dim str4 = GetStringsFromString(""abc"")(1)
Dim fromStr3 = GetMoreStringsFromString(""bc"")(1)(0)
Dim explicitNoParameter = GetStringsFromAmbiguous()(0)(1)
Dim usesParameter1 = GetStringsFromAmbiguous(0)(1)(2)
End Sub
Function GetStringFromNone() As String()
Return New String() { ""A"", ""B"", ""C""}
End Function
Function GetStringsFromString(parm As String) As String()
Return New String() { ""1"", ""2"", ""3""}
End Function
Function GetMoreStringsFromString(parm As String) As String()()
Return New String()() { New String() { ""1"" }}
End Function
Function GetStringsFromAmbiguous() As String()()
Return New String()() { New String() { ""1"" }}
End Function
Function GetStringsFromAmbiguous(amb As Integer) As String()()
Return New String()() { New String() { ""1"" }}
End Function
End Class", @"public class A
{
public void Test()
{
var str1 = this.GetStringFromNone()[0];
str1 = GetStringFromNone()[0];
var str2 = GetStringFromNone()[1];
var str3 = this.GetStringsFromString(""abc"");
str3 = GetStringsFromString(""abc"");
var str4 = GetStringsFromString(""abc"")[1];
var fromStr3 = GetMoreStringsFromString(""bc"")[1][0];
var explicitNoParameter = GetStringsFromAmbiguous()[0][1];
var usesParameter1 = GetStringsFromAmbiguous(0)[1][2];
}
public string[] GetStringFromNone()
{
return new string[] { ""A"", ""B"", ""C"" };
}
public string[] GetStringsFromString(string parm)
{
return new string[] { ""1"", ""2"", ""3"" };
}
public string[][] GetMoreStringsFromString(string parm)
{
return new string[][] { new string[] { ""1"" } };
}
public string[][] GetStringsFromAmbiguous()
{
return new string[][] { new string[] { ""1"" } };
}
public string[][] GetStringsFromAmbiguous(int amb)
{
return new string[][] { new string[] { ""1"" } };
}
}");
}

[Fact]
public void UnknownTypeInvocation()
{
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.