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

Extract method refactoring should handle tuples in VB #13278

Merged
merged 6 commits into from
Aug 24, 2016
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenTupleTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5146,6 +5146,17 @@ public void CreateTupleTypeSymbol_ElementTypeIsError()
Assert.Equal(SymbolKind.ErrorType, types[1].Kind);
}

[Fact, WorkItem(13277, "https://github.com/dotnet/roslyn/issues/13277")]
public void CreateTupleTypeSymbol_UnderlyingTypeIsError()
{
var comp = CSharpCompilation.Create("test", references: new[] { MscorlibRef });

TypeSymbol intType = comp.GetSpecialType(SpecialType.System_Int32);
var vt2 = comp.CreateErrorTypeSymbol(null, "ValueTuple", 2).Construct(intType, intType);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass intType.ContainingNamespace for container?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is on purpose. I'm trying to do the equivalent of code that crashes the compiler in VB. See the VB test of the same name.


Assert.Throws<ArgumentException>(() => comp.CreateTupleTypeSymbol(vt2, default(ImmutableArray<string>)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor point: Since elementNames argument is optional, consider dropping that parameter so it's clear the ArgumentException is from the vt2 argument.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

}

[Fact]
public void CreateTupleTypeSymbol_BadNames()
{
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/VisualBasic/Portable/Syntax/SyntaxFacts.vb
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
Else
Return parentQualName.Right Is node
End If
Case SyntaxKind.TupleElement
Return DirectCast(parent, TupleElementSyntax).Type Is node
End Select
End If

Expand Down
41 changes: 41 additions & 0 deletions src/Compilers/VisualBasic/Test/Emit/CodeGen/CodeGenTuples.vb
Original file line number Diff line number Diff line change
Expand Up @@ -5896,6 +5896,47 @@ fourth]]>)

End Sub

<Fact(Skip:="https://github.com/dotnet/roslyn/issues/13277")>
<WorkItem(13277, "https://github.com/dotnet/roslyn/issues/13277")>
Public Sub CreateTupleTypeSymbol_UnderlyingTypeIsError()

Dim comp = VisualBasicCompilation.Create("test", references:={MscorlibRef})

Dim intType As TypeSymbol = comp.GetSpecialType(SpecialType.System_Int32)
Dim vt2 = comp.CreateErrorTypeSymbol(Nothing, "ValueTuple", 2).Construct(intType, intType)

Dim tuple = comp.CreateTupleTypeSymbol(vt2, Nothing)
' Crashes in IsTupleCompatible

End Sub

<Fact>
<WorkItem(13042, "https://github.com/dotnet/roslyn/issues/13042")>
Public Sub GetSymbolInfoOnTupleType()
Dim verifier = CompileAndVerify(
<compilation>
<file name="a.vb">
Module C
Function M() As (System.Int32, String)
throw new System.Exception()
End Function
End Module

</file>
</compilation>, additionalRefs:={ValueTupleRef, SystemRuntimeFacadeRef})

Dim comp = verifier.Compilation
Dim tree = comp.SyntaxTrees(0)
Dim model = comp.GetSemanticModel(tree, ignoreAccessibility:=False)
Dim nodes = tree.GetCompilationUnitRoot().DescendantNodes()

Dim type = nodes.OfType(Of QualifiedNameSyntax)().First()
Assert.Equal("System.Int32", type.ToString())
Assert.NotNull(model.GetSymbolInfo(type))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetSymbolInfo() returns a struct. Can this fail?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently not ;-) Removed

Assert.NotNull(model.GetSymbolInfo(type).Symbol)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider checking .ToTestDisplayString() instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure


End Sub

End Class

End Namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,5 +293,41 @@ End Class
</Text>.Value.Replace(vbLf, vbCrLf),
compareTokens:=False)
End Function

<Fact, Trait(Traits.Feature, Traits.Features.CodeActionsExtractMethod), Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Tuples)>
<WorkItem(13042, "https://github.com/dotnet/roslyn/issues/13042")>
Public Async Function TestTuples() As Task

Await TestAsync(
"Class Program
Sub Main(args As String())
[|Dim x = (1, 2)|]
M(x)
End Sub
Private Sub M(x As (Integer, Integer))
End Sub
End Class
Namespace System
Structure ValueTuple(Of T1, T2)
End Structure
End Namespace",
"Class Program
Sub Main(args As String())
Dim x As (Integer, Integer) = {|Rename:NewMethod|}()
M(x)
End Sub
Private Shared Function NewMethod() As (Integer, Integer)
Return (1, 2)
End Function
Private Sub M(x As (Integer, Integer))
End Sub
End Class
Namespace System
Structure ValueTuple(Of T1, T2)
End Structure
End Namespace")

End Function

End Class
End Namespace
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Extensions
Return SyntaxFactory.QualifiedName(SyntaxFactory.IdentifierName("System"), SyntaxFactory.IdentifierName("DateTime"))
End Select

If symbol.IsTupleType Then
Return CreateTupleTypeSyntax(symbol)
End If

If symbol.Name = String.Empty OrElse symbol.IsAnonymousType Then
Return SyntaxFactory.QualifiedName(SyntaxFactory.IdentifierName("System"), SyntaxFactory.IdentifierName("Object"))
End If
Expand All @@ -119,6 +123,17 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Extensions
SyntaxFactory.TypeArgumentList(SyntaxFactory.SeparatedList(symbol.TypeArguments.[Select](Function(t) t.Accept(Me)))))
End Function

Private Function CreateTupleTypeSyntax(symbol As INamedTypeSymbol) As TypeSyntax
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shared?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Dim list = New SeparatedSyntaxList(Of TupleElementSyntax)()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

list is not used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about that. Removed

Dim types = symbol.TupleElementTypes
Dim names = symbol.TupleElementNames
Dim hasNames = Not names.IsDefault

Return SyntaxFactory.TupleType(SyntaxFactory.SeparatedList(
types.Select(Function(t, i) SyntaxFactory.TupleElement(
If(hasNames, SyntaxFactory.IdentifierName(names(i)), Nothing), GenerateTypeSyntax(t)))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor point: Use t.GenerateTypeSyntax() instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

End Function

Public Overrides Function VisitNamedType(symbol As INamedTypeSymbol) As TypeSyntax
Dim typeSyntax = CreateSimpleTypeSyntax(symbol)
If Not (TypeOf typeSyntax Is SimpleNameSyntax) Then
Expand Down