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

Add IOperation support for anonymous object creation expressions. #20364

Merged
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Linq;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Semantics
{
Expand Down Expand Up @@ -179,12 +180,16 @@ private IOperation CreateInternal(BoundNode boundNode)
return CreateBoundInterpolationOperation((BoundStringInsert)boundNode);
case BoundKind.LocalFunctionStatement:
return CreateBoundLocalFunctionStatementOperation((BoundLocalFunctionStatement)boundNode);
case BoundKind.AnonymousObjectCreationExpression:
return CreateBoundAnonymousObjectCreationExpressionOperation((BoundAnonymousObjectCreationExpression)boundNode);
case BoundKind.AnonymousPropertyDeclaration:
return CreateBoundAnonymousPropertyDeclarationOperation((BoundAnonymousPropertyDeclaration)boundNode);
case BoundKind.ConstantPattern:
return CreateBoundConstantPatternOperation((BoundConstantPattern)boundNode);
case BoundKind.DeclarationPattern:
return CreateBoundDeclarationPatternOperation((BoundDeclarationPattern)boundNode);
case BoundKind.WildcardPattern:
return null;
Copy link
Member

@jcouv jcouv Jun 29, 2017

Choose a reason for hiding this comment

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

I didn't understand this change. How do you know this is unreachable? #Closed

Copy link
Contributor Author

@mavasani mavasani Jul 3, 2017

Choose a reason for hiding this comment

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

@jcouv - This is addressing feedback from #20276 (comment). We handle this bound node while operation on it's parent here: https://github.com/dotnet/roslyn/pull/20276/files#diff-df80689b64fbe29db28a562eb0dac8caR1154 #Resolved

throw ExceptionUtilities.Unreachable;
case BoundKind.PatternSwitchStatement:
return CreateBoundPatternSwitchStatementOperation((BoundPatternSwitchStatement)boundNode);
case BoundKind.PatternSwitchLabel:
Expand Down Expand Up @@ -352,6 +357,29 @@ private ILiteralExpression CreateBoundLiteralOperation(BoundLiteral boundLiteral
return new LiteralExpression(text, isInvalid, syntax, type, constantValue);
}

private IAnonymousObjectCreationExpression CreateBoundAnonymousObjectCreationExpressionOperation(BoundAnonymousObjectCreationExpression boundAnonymousObjectCreationExpression)
{
Lazy<ImmutableArray<IOperation>> memberInitializers = new Lazy<ImmutableArray<IOperation>>(() => GetAnonymousObjectCreationInitializers(boundAnonymousObjectCreationExpression));
bool isInvalid = boundAnonymousObjectCreationExpression.HasErrors;
SyntaxNode syntax = boundAnonymousObjectCreationExpression.Syntax;
ITypeSymbol type = boundAnonymousObjectCreationExpression.Type;
Optional<object> constantValue = ConvertToOptional(boundAnonymousObjectCreationExpression.ConstantValue);
return new LazyAnonymousObjectCreationExpression(memberInitializers, isInvalid, syntax, type, constantValue);
}

private IPropertyReferenceExpression CreateBoundAnonymousPropertyDeclarationOperation(BoundAnonymousPropertyDeclaration boundAnonymousPropertyDeclaration)
{
PropertySymbol property = boundAnonymousPropertyDeclaration.Property;
Lazy<IOperation> instance = new Lazy<IOperation>(() => null);
Copy link
Member

@jaredpar jaredpar Jun 29, 2017

Choose a reason for hiding this comment

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

Why have a lazy that always returns null? Seems like null coudl just be used directly here. #Resolved

Copy link
Contributor Author

@mavasani mavasani Jul 3, 2017

Choose a reason for hiding this comment

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

I have followed the pattern used in this file. @heejaechang Will having a null child node be handled by the operation factory?

I have added a comment to #20204 to track this feedback for the entire file. #Resolved

ISymbol member = boundAnonymousPropertyDeclaration.Property;
Lazy<ImmutableArray<IArgument>> argumentsInEvaluationOrder = new Lazy<ImmutableArray<IArgument>>(() => ImmutableArray<IArgument>.Empty);
bool isInvalid = boundAnonymousPropertyDeclaration.HasErrors;
SyntaxNode syntax = boundAnonymousPropertyDeclaration.Syntax;
ITypeSymbol type = boundAnonymousPropertyDeclaration.Type;
Optional<object> constantValue = ConvertToOptional(boundAnonymousPropertyDeclaration.ConstantValue);
return new LazyPropertyReferenceExpression(property, instance, member, argumentsInEvaluationOrder, isInvalid, syntax, type, constantValue);
Copy link
Member

Choose a reason for hiding this comment

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

Is this change (adding argumentsInEvaluationOrder) covered by test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently the dumper doesn't print out child operation arrays which are empty, hence we cannot cover it in tests. @jinujoseph is working on updating the operation tree dumper to print a header with 0 element count for this case, at which point the unit tests will cover it.

}

private IObjectCreationExpression CreateBoundObjectCreationExpressionOperation(BoundObjectCreationExpression boundObjectCreationExpression)
{
IMethodSymbol constructor = boundObjectCreationExpression.Constructor;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// 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 System;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Symbols;
Expand Down Expand Up @@ -92,6 +94,33 @@ private ImmutableArray<IArgument> DeriveArguments(
invokedAsExtensionMethod: invokedAsExtensionMethod);
}

private ImmutableArray<IOperation> GetAnonymousObjectCreationInitializers(BoundAnonymousObjectCreationExpression expression)
{
// For error cases, the binder generates only the argument.
Copy link
Contributor Author

@mavasani mavasani Jun 21, 2017

Choose a reason for hiding this comment

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

See #20338 for additional gotchas here. #Resolved

Debug.Assert(expression.Arguments.Length >= expression.Declarations.Length);

var builder = ArrayBuilder<IOperation>.GetInstance(expression.Arguments.Length);
for (int i = 0; i < expression.Arguments.Length; i++)
{
IOperation value = Create(expression.Arguments[i]);
if (i >= expression.Declarations.Length)
{
builder.Add(value);
continue;
}

IOperation target = Create(expression.Declarations[i]);
bool isInvalid = target.IsInvalid || value.IsInvalid;
Copy link
Member

@cston cston Jun 23, 2017

Choose a reason for hiding this comment

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

Is it necessary to consider the entire assignment invalid if either target or value are invalid? Will that mean analyzers will typically skip the valid parts of the assignment? #Pending

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@heejaechang Is actually working on a PR to remove IOperation.IsInvalid flag. It is currently a very fragile heuristic, and we are replacing it with an extension method on IOperation that determines the result by computing if the span has syntax or semantic diagnostics within it.

SyntaxNode syntax = value.Syntax?.Parent ?? expression.Syntax;
ITypeSymbol type = target.Type;
Optional<object> constantValue = value.ConstantValue;
var assignment = new SimpleAssignmentExpression(target, value, isInvalid, syntax, type, constantValue);
builder.Add(assignment);
}

return builder.ToImmutableAndFree();
}

private ImmutableArray<IOperation> GetObjectCreationInitializers(BoundObjectCreationExpression expression)
{
return BoundObjectCreationExpression.GetChildInitializers(expression.InitializerExpressionOpt).SelectAsArray(n => Create(n));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ protected T CompileAndGetModelAndExpression<T>(string program, Func<SemanticMode
var comp = CreateStandardCompilation(program, new[] { LinqAssemblyRef });
var tree = comp.SyntaxTrees[0];
var model = comp.GetSemanticModel(tree);
int start = program.IndexOf(startString, StringComparison.Ordinal) + startString.Length;
int end = program.IndexOf(endString, StringComparison.Ordinal);
int start = program.IndexOf(StartString, StringComparison.Ordinal) + StartString.Length;
int end = program.IndexOf(EndString, StringComparison.Ordinal);
ExpressionSyntax syntaxToBind = null;
foreach (var expr in GetSyntaxNodeList(tree).OfType<ExpressionSyntax>())
{
Expand All @@ -114,8 +114,8 @@ protected T CompileAndGetModelAndStatements<T>(string program, Func<SemanticMode
var comp = CreateStandardCompilation(program, new[] { LinqAssemblyRef });
var tree = comp.SyntaxTrees[0];
var model = comp.GetSemanticModel(tree);
int start = program.IndexOf(startString, StringComparison.Ordinal) + startString.Length;
int end = program.IndexOf(endString, StringComparison.Ordinal);
int start = program.IndexOf(StartString, StringComparison.Ordinal) + StartString.Length;
int end = program.IndexOf(EndString, StringComparison.Ordinal);
StatementSyntax firstStatement = null, lastStatement = null;
foreach (var stmt in GetSyntaxNodeList(tree).OfType<StatementSyntax>())
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,15 @@ public void M(int x, string y)
}
";
string expectedOperationTree = @"
IOperation: (OperationKind.None) (Syntax: 'new { Amoun ... ello"" + y }')
Children(2): IParameterReferenceExpression: x (OperationKind.ParameterReferenceExpression, Type: System.Int32) (Syntax: 'x')
IBinaryOperatorExpression (BinaryOperationKind.StringConcatenate) (OperationKind.BinaryOperatorExpression, Type: System.String) (Syntax: '""Hello"" + y')
Left: ILiteralExpression (OperationKind.LiteralExpression, Type: System.String, Constant: ""Hello"") (Syntax: '""Hello""')
Right: IParameterReferenceExpression: y (OperationKind.ParameterReferenceExpression, Type: System.String) (Syntax: 'y')
IAnonymousObjectCreationExpression (OperationKind.AnonymousObjectCreationExpression, Type: <anonymous type: System.Int32 Amount, System.String Message>) (Syntax: 'new { Amoun ... ello"" + y }')
Initializers(2): ISimpleAssignmentExpression (OperationKind.SimpleAssignmentExpression, Type: System.Int32) (Syntax: 'Amount = x')
Left: IPropertyReferenceExpression: System.Int32 <anonymous type: System.Int32 Amount, System.String Message>.Amount { get; } (Static) (OperationKind.PropertyReferenceExpression, Type: System.Int32) (Syntax: 'Amount')
Right: IParameterReferenceExpression: x (OperationKind.ParameterReferenceExpression, Type: System.Int32) (Syntax: 'x')
ISimpleAssignmentExpression (OperationKind.SimpleAssignmentExpression, Type: System.String) (Syntax: 'Message = ""Hello"" + y')
Left: IPropertyReferenceExpression: System.String <anonymous type: System.Int32 Amount, System.String Message>.Message { get; } (Static) (OperationKind.PropertyReferenceExpression, Type: System.String) (Syntax: 'Message')
Right: IBinaryOperatorExpression (BinaryOperationKind.StringConcatenate) (OperationKind.BinaryOperatorExpression, Type: System.String) (Syntax: '""Hello"" + y')
Left: ILiteralExpression (OperationKind.LiteralExpression, Type: System.String, Constant: ""Hello"") (Syntax: '""Hello""')
Right: IParameterReferenceExpression: y (OperationKind.ParameterReferenceExpression, Type: System.String) (Syntax: 'y')
";
var expectedDiagnostics = DiagnosticDescription.None;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -675,5 +675,54 @@ void M(int? x)

VerifyOperationTreeAndDiagnosticsForTest<CaseSwitchLabelSyntax>(source, expectedOperationTree, expectedDiagnostics);
}

[Fact, WorkItem(19927, "https://github.com/dotnet/roslyn/issues/19927")]
public void TestPatternCaseClause_RedundantPatternDeclarationClauses()
{
string source = @"
using System;
class X
{
void M(object p)
{
/*<bind>*/switch (p)
{
case int x:
break;
case int y:
break;
case X z:
break;
}/*</bind>*/
}
}
";
string expectedOperationTree = @"
ISwitchStatement (3 cases) (OperationKind.SwitchStatement) (Syntax: 'switch (p) ... }')
Switch expression: IParameterReferenceExpression: p (OperationKind.ParameterReferenceExpression, Type: System.Object) (Syntax: 'p')
Sections: ISwitchCase (1 case clauses, 1 statements) (OperationKind.SwitchCase) (Syntax: 'case int x: ... break;')
Clauses: IPatternCaseClause (Label Symbol: case int x:) (CaseKind.Pattern) (OperationKind.PatternCaseClause) (Syntax: 'case int x:')
Pattern: IDeclarationPattern (Declared Symbol: System.Int32 x) (OperationKind.DeclarationPattern) (Syntax: 'int x')
Body: IBranchStatement (BranchKind.Break) (OperationKind.BranchStatement) (Syntax: 'break;')
ISwitchCase (1 case clauses, 1 statements) (OperationKind.SwitchCase) (Syntax: 'case int y: ... break;')
Clauses: IPatternCaseClause (Label Symbol: case int y:) (CaseKind.Pattern) (OperationKind.PatternCaseClause) (Syntax: 'case int y:')
Pattern: IDeclarationPattern (Declared Symbol: System.Int32 y) (OperationKind.DeclarationPattern) (Syntax: 'int y')
Body: IBranchStatement (BranchKind.Break) (OperationKind.BranchStatement) (Syntax: 'break;')
ISwitchCase (1 case clauses, 1 statements) (OperationKind.SwitchCase) (Syntax: 'case X z: ... break;')
Clauses: IPatternCaseClause (Label Symbol: case X z:) (CaseKind.Pattern) (OperationKind.PatternCaseClause) (Syntax: 'case X z:')
Pattern: IDeclarationPattern (Declared Symbol: X z) (OperationKind.DeclarationPattern) (Syntax: 'X z')
Body: IBranchStatement (BranchKind.Break) (OperationKind.BranchStatement) (Syntax: 'break;')
";
var expectedDiagnostics = new DiagnosticDescription[] {
// CS8120: The switch case has already been handled by a previous case.
// case int y:
Diagnostic(ErrorCode.ERR_PatternIsSubsumed, "int y").WithLocation(11, 18),
// CS0162: Unreachable code detected
// break;
Diagnostic(ErrorCode.WRN_UnreachableCode, "break").WithLocation(12, 17)
};

VerifyOperationTreeAndDiagnosticsForTest<SwitchStatementSyntax>(source, expectedOperationTree, expectedDiagnostics);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -265,60 +265,6 @@ public void BindQueryVariable()

#region helpers

protected List<SyntaxNode> GetSyntaxNodeList(SyntaxTree syntaxTree)
{
return GetSyntaxNodeList(syntaxTree.GetCompilationUnitRoot(), null);
}

protected List<SyntaxNode> GetSyntaxNodeList(SyntaxNode node, List<SyntaxNode> synList)
{
if (synList == null)
synList = new List<SyntaxNode>();

synList.Add(node);

foreach (var child in node.ChildNodesAndTokens())
{
if (child.IsNode)
synList = GetSyntaxNodeList(child.AsNode(), synList);
}

return synList;
}

protected SyntaxNode GetSyntaxNodeForBinding(List<SyntaxNode> synList)
{
foreach (var node in synList)
{
string exprFullText = node.ToFullString();
exprFullText = exprFullText.Trim();

if (exprFullText.StartsWith("/*<bind>*/", StringComparison.Ordinal))
{
if (exprFullText.Contains("/*</bind>*/"))
if (exprFullText.EndsWith("/*</bind>*/", StringComparison.Ordinal))
return node;
else
continue;
else
return node;
}

if (exprFullText.EndsWith("/*</bind>*/", StringComparison.Ordinal))
{
if (exprFullText.Contains("/*<bind>*/"))
if (exprFullText.StartsWith("/*<bind>*/", StringComparison.Ordinal))
return node;
else
continue;
else
return node;
}
}

return null;
}

private List<ExpressionSyntax> GetExprSyntaxList(SyntaxTree syntaxTree)
{
return GetExprSyntaxList(syntaxTree.GetCompilationUnitRoot(), null);
Expand Down
Loading