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.

Sign up for GitHub

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

Add helper methods to Verify operation tree for annonated text within… #17856

Merged
merged 3 commits into from
Mar 15, 2017
Merged

Add helper methods to Verify operation tree for annonated text within… #17856

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9d77278
Add helper methods to Verify operation tree for annonated text within…
mavasani Mar 14, 2017
cb01f0b
Address PR feedback and add comment
mavasani Mar 14, 2017
715e6d7
Don't assert for OperationKind.None in OperationTreeVerifier
mavasani Mar 15, 2017
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
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

namespace Microsoft.CodeAnalysis.CSharp.UnitTests
{
public partial class IOperationTests : CompilingTestBase
public partial class IOperationTests : SemanticModelTestBase
{
[Fact]
[WorkItem(382240, "https://devdiv.visualstudio.com/DevDiv/_workitems?id=382240")]
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_ISymbolInitializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

namespace Microsoft.CodeAnalysis.CSharp.UnitTests
{
public partial class IOperationTests : CompilingTestBase
public partial class IOperationTests : SemanticModelTestBase
{
[Fact, WorkItem(17595, "https://github.com/dotnet/roslyn/issues/17595")]
public void NoInitializers()
Expand Down
39 changes: 37 additions & 2 deletions src/Compilers/Test/Utilities/CSharp/SemanticModelTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
using Microsoft.CodeAnalysis.Text;
using Microsoft.CodeAnalysis.Test.Utilities;
using Xunit;
using Roslyn.Test.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.UnitTests
{
Expand Down Expand Up @@ -220,5 +219,41 @@ protected CompilationUtils.SemanticInfoSummary GetSemanticInfoForTest(string tes
{
return GetSemanticInfoForTest<ExpressionSyntax>(testSrc);
}

protected string GetOperationTreeForTest<TSyntaxNode>(CSharpCompilation compilation)
where TSyntaxNode: SyntaxNode
{
var tree = compilation.SyntaxTrees[0];
var model = compilation.GetSemanticModel(tree);
SyntaxNode syntaxNode = GetSyntaxNodeOfTypeForBinding<TSyntaxNode>(GetSyntaxNodeList(tree));
Copy link
Contributor

Choose a reason for hiding this comment

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

what happen if tree has multiple nodes matching given type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will behave similar to the compiler binding tests for those cases. Seems that this method adds the nodes from bottom up, and picks the first node that matches full text and is of expected type. Also I am not sure if we can have 2 nodes of same kind with same span in tree.

Copy link
Contributor

Choose a reason for hiding this comment

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

but this test doesn't give in span? so it will just pick the first node in the tree?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The unit test generator will add comments denoting the span, like following:

  1. VB:
        Dim x1 = New F()
        Dim x2 = New F() With {.Field = 2}'BIND:"Dim x2 = New F() With {.Field = 2}"
        Dim x3 = New F() With {.Property1 = ""}
        Dim x4 = New F() With {.Property1 = "", .Field = 2}
        Dim x5 = New F() With {.Property2 = New B() With {.Field = True}}
  1. C#
class C
{
    public void M1()
    /*<bind>*/{
        var x1 = new F();
        var x2 = new F() { Field = 2 };
        var x3 = new F() { Property1 = """""""" };
        var x4 = new F() { Property1 = """""""", Field = 2 };
        var x5 = new F() { Property2 = new B { Field = true } };

        var e1 = new F() { Property2 = 1 };
        var e2 = new F() { """""""" };
    }/*</bind>*/
}

bind comments specify the span that the test helper will use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See example unit test: https://github.com/dotnet/roslyn-internal/pull/1692#issuecomment-286570489

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, I see, no span argument is needed since it will get those from source, got it.

if (syntaxNode == null)
{
return null;
}

var operation = model.GetOperationInternal(syntaxNode);
Copy link
Contributor

Choose a reason for hiding this comment

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

can a node have multiple operations?

Copy link
Contributor Author

@mavasani mavasani Mar 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, and we need to test that SemanticModel.GetOperation does give us the expected bound node, lowest or highest. I see that VB and C# seem to have different defaults. C# picks the lowest bound node when there are multiple bound nodes for a syntax (see here) and VB picks the highest (see here). I will open a separate work item to track why this difference.

Copy link
Contributor

Choose a reason for hiding this comment

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

so when testing.. do we want to explicit on IOperation we get from the API? or we use the default behavior?

can user get IOperation off a node? or is it only internal, testing stuff?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a API discussion I believe. SemanticModel.GetOperation doesn't seem to take an options, but we have an internal enum GetOperationOptions that decides what bound node to choose for clashes. Until we expose that publically, unit tests should just use the default that the GetOperation API passes down.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed #17861

Copy link
Contributor

Choose a reason for hiding this comment

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

so for users, they expect 1 node to have 1 IOperation 1:1 matching?

hmm.. but when they examine SyntaxNode of IOperation, they might get multiple IOperation with same node right? how users get to those IOperations? only through walker?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, seems we need to expose the options when there are conflict - needs design discussion. You can probably add comment to #17861

return operation != null ? OperationTreeVerifier.GetOperationTree(operation) : null;
}

protected string GetOperationTreeForTest<TSyntaxNode>(string testSrc, string expectedOperationTree, CSharpParseOptions parseOptions = null)
where TSyntaxNode : SyntaxNode
{
var compilation = CreateCompilationWithMscorlib(testSrc, new[] { SystemCoreRef }, parseOptions: parseOptions);
return GetOperationTreeForTest<TSyntaxNode>(compilation);
}

protected void VerifyOperationTreeForTest<TSyntaxNode>(CSharpCompilation compilation, string expectedOperationTree)
where TSyntaxNode : SyntaxNode
{
var actualOperationTree = GetOperationTreeForTest<TSyntaxNode>(compilation);
OperationTreeVerifier.Verify(expectedOperationTree, actualOperationTree);
}

protected void VerifyOperationTreeForTest<TSyntaxNode>(string testSrc, string expectedOperationTree, CSharpParseOptions parseOptions = null)
where TSyntaxNode : SyntaxNode
{
var actualOperationTree = GetOperationTreeForTest<TSyntaxNode>(testSrc, expectedOperationTree, parseOptions);
OperationTreeVerifier.Verify(expectedOperationTree, actualOperationTree);
}
}
}
29 changes: 29 additions & 0 deletions src/Compilers/Test/Utilities/VisualBasic/SemanticModelTestBase.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
' Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

Imports Microsoft.CodeAnalysis.Test.Utilities
Imports Microsoft.CodeAnalysis.Text
Imports Microsoft.CodeAnalysis.VisualBasic.Symbols
Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
Expand Down Expand Up @@ -151,4 +152,32 @@ Public MustInherit Class SemanticModelTestBase : Inherits BasicTestBase
).Where(Function(s) anyArity OrElse DirectCast(s, Symbol).GetArity() = arity.Value).ToList()
End Function

Friend Function GetOperationTreeForTest(Of TSyntaxNode As SyntaxNode)(compilation As VisualBasicCompilation, fileName As String, Optional which As Integer = 0) As String
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting so compiler test framework between VB and C# is different slightly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it seems there are differences between binding test framework helpers, and I had to retain them so I can re-use it.

Dim node As SyntaxNode = CompilationUtils.FindBindingText(Of TSyntaxNode)(compilation, fileName, which)
If node Is Nothing Then
Return Nothing
End If

Dim tree = (From t In compilation.SyntaxTrees Where t.FilePath = fileName).Single()
Dim semanticModel = compilation.GetSemanticModel(tree)
Dim operation = semanticModel.GetOperationInternal(node)
Return If(operation IsNot Nothing, OperationTreeVerifier.GetOperationTree(operation), Nothing)
End Function

Friend Function GetOperationTreeForTest(Of TSyntaxNode As SyntaxNode)(testSrc As String, Optional parseOptions As VisualBasicParseOptions = Nothing, Optional which As Integer = 0) As String
Dim fileName = "a.vb"
Dim syntaxTree = Parse(testSrc, fileName, parseOptions)
Dim compilation = CreateCompilationWithMscorlib(syntaxTree)
Return GetOperationTreeForTest(Of TSyntaxNode)(compilation, fileName, which)
End Function

Friend Sub VerifyOperationTreeForTest(Of TSyntaxNode As SyntaxNode)(compilation As VisualBasicCompilation, fileName As String, expectedOperationTree As String, Optional which As Integer = 0)
Dim actualOperationTree = GetOperationTreeForTest(Of TSyntaxNode)(compilation, fileName, which)
OperationTreeVerifier.Verify(expectedOperationTree, actualOperationTree)
End Sub

Friend Sub VerifyOperationTreeForTest(Of TSyntaxNode As SyntaxNode)(testSrc As String, expectedOperationTree As String, Optional parseOptions As VisualBasicParseOptions = Nothing, Optional which As Integer = 0)
Dim actualOperationTree = GetOperationTreeForTest(Of TSyntaxNode)(testSrc, parseOptions, which)
OperationTreeVerifier.Verify(expectedOperationTree, actualOperationTree)
End Sub
End Class
2 changes: 1 addition & 1 deletion src/Compilers/VisualBasic/Test/Semantic/IOperation/IOperationTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Imports Roslyn.Test.Utilities
Namespace Microsoft.CodeAnalysis.VisualBasic.UnitTests.Semantics

Partial Public Class IOperationTests
Inherits BasicTestBase
Inherits SemanticModelTestBase

<Fact>
Public Sub InvalidUserDefinedOperators()
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/VisualBasic/Test/Semantic/IOperation/IOperationTests_ISymbolInitializer.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Imports Roslyn.Test.Utilities
Namespace Microsoft.CodeAnalysis.VisualBasic.UnitTests.Semantics

Partial Public Class IOperationTests
Inherits BasicTestBase
Inherits SemanticModelTestBase

<Fact, WorkItem(17595, "https://github.com/dotnet/roslyn/issues/17595")>
Public Sub NoInitializers()
Expand Down
13 changes: 2 additions & 11 deletions src/Test/Utilities/Portable/Compilation/CompilationExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ internal static void VerifyOperationTree(this Compilation compilation, SyntaxNod
var actualTextBuilder = new StringBuilder();
SemanticModel model = compilation.GetSemanticModel(node.SyntaxTree);
AppendOperationTree(model, node, actualTextBuilder);
VerifyOperationTree(expectedOperationTree, actualTextBuilder.ToString());
OperationTreeVerifier.Verify(expectedOperationTree, actualTextBuilder.ToString());
}

internal static void VerifyOperationTree(this Compilation compilation, string expectedOperationTree, bool skipImplicitlyDeclaredSymbols = false)
Expand Down Expand Up @@ -218,7 +218,7 @@ internal static void VerifyOperationTree(this Compilation compilation, string sy
actualTextBuilder.Append(Environment.NewLine);
}

VerifyOperationTree(expectedOperationTree, actualTextBuilder.ToString());
OperationTreeVerifier.Verify(expectedOperationTree, actualTextBuilder.ToString());
}

private static void AppendOperationTree(SemanticModel model, SyntaxNode node, StringBuilder actualTextBuilder, int initialIndent = 0)
Expand All @@ -235,15 +235,6 @@ private static void AppendOperationTree(SemanticModel model, SyntaxNode node, St
}
}

private static void VerifyOperationTree(string expectedOperationTree, string actualOperationTree)
{
char[] newLineChars = Environment.NewLine.ToCharArray();
string actual = actualOperationTree.Trim(newLineChars);
expectedOperationTree = expectedOperationTree.Trim(newLineChars);
expectedOperationTree = Regex.Replace(expectedOperationTree, "([^\r])\n", "$1" + Environment.NewLine);
AssertEx.AreEqual(expectedOperationTree, actual);
}

internal static bool CanHaveExecutableCodeBlock(ISymbol symbol)
{
switch (symbol.Kind)
Expand Down
11 changes: 11 additions & 0 deletions src/Test/Utilities/Portable/Compilation/OperationTreeVerifier.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
using System.Collections.Immutable;
using System.Diagnostics;
using System.Text;
using System.Text.RegularExpressions;
using Microsoft.CodeAnalysis.Semantics;
using Microsoft.CodeAnalysis.Test.Extensions;
using Roslyn.Test.Utilities;
using Xunit;

namespace Microsoft.CodeAnalysis.Test.Utilities
Expand Down Expand Up @@ -42,6 +44,15 @@ public static string GetOperationTree(IOperation operation, int initialIndent =
return walker._builder.ToString();
}

public static void Verify(string expectedOperationTree, string actualOperationTree)
{
char[] newLineChars = Environment.NewLine.ToCharArray();
string actual = actualOperationTree.Trim(newLineChars);
expectedOperationTree = expectedOperationTree.Trim(newLineChars);
expectedOperationTree = Regex.Replace(expectedOperationTree, "([^\r])\n", "$1" + Environment.NewLine);
Copy link
Contributor

Choose a reason for hiding this comment

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

what does it actually trying to do?

Copy link
Contributor Author

@mavasani mavasani Mar 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This is due to the CDATA for VB. It seems to have \n characters instead of new lines, and this causes equality check to fail. This line replaces \n characters, that do not have a \r before it, with \r\n

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm.. but you trimmed new lines right above, regex so, there will be no \r or \n in text anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will trim just \r\n from start and end. We need to retain the newline characters within the text so that assert failure displays the actual and expected IOperation tree in a nicely formatted manner in the test window.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. so escaped char is retained but actually "\r\n" will be removed. okay.

AssertEx.AreEqual(expectedOperationTree, actual);
}

#region Logging helpers

private void LogCommonPropertiesAndNewLine(IOperation operation)
Expand Down
12 changes: 12 additions & 0 deletions src/Test/Utilities/Portable/Extensions/SemanticModelExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.CodeAnalysis.Test.Extensions
{
public static class SemanticModelExtensions
{
public static IOperation GetOperationInternal(this SemanticModel model, SyntaxNode node)
{
return model.GetOperationInternal(node);
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure what this extension method is trying to do.

Copy link
Contributor

Choose a reason for hiding this comment

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

is it calling itself?

Copy link
Member

Choose a reason for hiding this comment

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

I believe this is to bypass the feature flag check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GetOperationInternal is internal method on semantic model that by-passes the IOperation feature flag check. Only core TestUtilties have IVT, so this just exposes it to other test layers, such as ETA.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, got it. some comment would help :) since it looks like it is calling itself ha ha.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sure.

}
}
}
1 change: 1 addition & 0 deletions src/Test/Utilities/Portable/TestUtilities.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@
<Compile Include="Diagnostics\TrackingDiagnosticAnalyzer.cs" />
<Compile Include="ExceptionHelper.cs" />
<Compile Include="CompilerTraitAttribute.cs" />
<Compile Include="Extensions\SemanticModelExtensions.cs" />
<Compile Include="Extensions\SymbolExtensions.cs" />
<Compile Include="FX\ConsoleOutput.cs" />
<Compile Include="FX\CultureHelpers.cs" />
Expand Down