From 3e15d4e9f2e0f2359c38189fbc56cc4fcfab5260 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Wed, 31 May 2017 11:43:28 -0700 Subject: [PATCH 1/2] Ensure that IOperations nodes within a C# local function are part of the operation tree Analysis of any code within a local function with IOperation analyzers will produce false or missing diagnostics, as the operation tree doesn't contain any operation nodes within the BoundLocalFunctionStatement. This can be a dogfood blocker for IOperation analyzers on real world code - we had few false reports due this on the Roslyn repo as we use this new language feature, and corresponding rules had to be disabled in the repo. --- .../CSharp/Portable/BoundTree/Statement.cs | 8 ++-- ...tionTests_IParameterReferenceExpression.cs | 43 +++++++++++++++++++ .../Portable/Operations/OperationVisitor.cs | 10 ----- .../Core/Portable/PublicAPI.Unshipped.txt | 2 - 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/src/Compilers/CSharp/Portable/BoundTree/Statement.cs b/src/Compilers/CSharp/Portable/BoundTree/Statement.cs index c8f3c3f1f7401..92c11dac56cb9 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/Statement.cs +++ b/src/Compilers/CSharp/Portable/BoundTree/Statement.cs @@ -854,16 +854,18 @@ public override TResult Accept(OperationVisitor OperationKind.LocalFunctionStatement; + protected override OperationKind StatementKind => OperationKind.None; + + protected override ImmutableArray Children => ImmutableArray.Create(this.Body); public override void Accept(OperationVisitor visitor) { - visitor.VisitLocalFunctionStatement(this); + visitor.VisitNoneOperation(this); } public override TResult Accept(OperationVisitor visitor, TArgument argument) { - return visitor.VisitLocalFunctionStatement(this, argument); + return visitor.VisitNoneOperation(this, argument); } } diff --git a/src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_IParameterReferenceExpression.cs b/src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_IParameterReferenceExpression.cs index 3299b453ba482..5cda2364c9a05 100644 --- a/src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_IParameterReferenceExpression.cs +++ b/src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_IParameterReferenceExpression.cs @@ -850,5 +850,48 @@ public void Method1(object x) VerifyOperationTreeAndDiagnosticsForTest(source, expectedOperationTree, expectedDiagnostics); } + + [Fact, WorkItem(19902, "https://github.com/dotnet/roslyn/issues/19902")] + public void ParameterReference_LocalFunctionStatement() + { + string source = @" +using System; +using System.Collections.Generic; + +class Class +{ + static IEnumerable MyIterator(IEnumerable source, Func predicate) + { + /**/IEnumerable Iterator() + { + foreach (var element in source) + if (predicate(element)) + yield return element; + }/**/ + + return Iterator(); + } +} + +"; + string expectedOperationTree = @" +IOperation: (OperationKind.None) (Syntax: 'IEnumerable ... }') + Children(1): IBlockStatement (2 statements) (OperationKind.BlockStatement) (Syntax: '{ ... }') + IForEachLoopStatement (Iteration variable: T element) (LoopKind.ForEach) (OperationKind.LoopStatement) (Syntax: 'foreach (va ... rn element;') + Collection: IConversionExpression (ConversionKind.Cast, Implicit) (OperationKind.ConversionExpression, Type: System.Collections.Generic.IEnumerable) (Syntax: 'source') + IParameterReferenceExpression: source (OperationKind.ParameterReferenceExpression, Type: System.Collections.Generic.IEnumerable) (Syntax: 'source') + Body: IIfStatement (OperationKind.IfStatement) (Syntax: 'if (predica ... rn element;') + Condition: IInvocationExpression (virtual System.Boolean System.Func.Invoke(T arg)) (OperationKind.InvocationExpression, Type: System.Boolean) (Syntax: 'predicate(element)') + Instance Receiver: IParameterReferenceExpression: predicate (OperationKind.ParameterReferenceExpression, Type: System.Func) (Syntax: 'predicate') + Arguments(1): IArgument (ArgumentKind.Explicit, Matching Parameter: arg) (OperationKind.Argument) (Syntax: 'element') + ILocalReferenceExpression: element (OperationKind.LocalReferenceExpression, Type: T) (Syntax: 'element') + IfTrue: IReturnStatement (OperationKind.YieldReturnStatement) (Syntax: 'yield return element;') + ILocalReferenceExpression: element (OperationKind.LocalReferenceExpression, Type: T) (Syntax: 'element') + YieldBreakStatement (OperationKind.YieldBreakStatement) (Syntax: '{ ... }') +"; + var expectedDiagnostics = DiagnosticDescription.None; + + VerifyOperationTreeAndDiagnosticsForTest(source, expectedOperationTree, expectedDiagnostics); + } } } \ No newline at end of file diff --git a/src/Compilers/Core/Portable/Operations/OperationVisitor.cs b/src/Compilers/Core/Portable/Operations/OperationVisitor.cs index 317a2efc745ba..c3ab306c0cb7c 100644 --- a/src/Compilers/Core/Portable/Operations/OperationVisitor.cs +++ b/src/Compilers/Core/Portable/Operations/OperationVisitor.cs @@ -389,11 +389,6 @@ public virtual void VisitInvalidExpression(IInvalidExpression operation) { DefaultVisit(operation); } - - public virtual void VisitLocalFunctionStatement(IOperation operation) - { - DefaultVisit(operation); - } } /// @@ -789,10 +784,5 @@ public virtual TResult VisitInvalidExpression(IInvalidExpression operation, TArg { return DefaultVisit(operation, argument); } - - public virtual TResult VisitLocalFunctionStatement(IOperation operation, TArgument argument) - { - return DefaultVisit(operation, argument); - } } } diff --git a/src/Compilers/Core/Portable/PublicAPI.Unshipped.txt b/src/Compilers/Core/Portable/PublicAPI.Unshipped.txt index c20e3b48e9e81..22ed4041a45e9 100644 --- a/src/Compilers/Core/Portable/PublicAPI.Unshipped.txt +++ b/src/Compilers/Core/Portable/PublicAPI.Unshipped.txt @@ -804,7 +804,6 @@ virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLabelStatement(Mi virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLambdaExpression(Microsoft.CodeAnalysis.Semantics.ILambdaExpression operation) -> void virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLateBoundMemberReferenceExpression(Microsoft.CodeAnalysis.Semantics.ILateBoundMemberReferenceExpression operation) -> void virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLiteralExpression(Microsoft.CodeAnalysis.Semantics.ILiteralExpression operation) -> void -virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLocalFunctionStatement(Microsoft.CodeAnalysis.IOperation operation) -> void virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLocalReferenceExpression(Microsoft.CodeAnalysis.Semantics.ILocalReferenceExpression operation) -> void virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLockStatement(Microsoft.CodeAnalysis.Semantics.ILockStatement operation) -> void virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitMethodBindingExpression(Microsoft.CodeAnalysis.Semantics.IMethodBindingExpression operation) -> void @@ -880,7 +879,6 @@ virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.Vi virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLambdaExpression(Microsoft.CodeAnalysis.Semantics.ILambdaExpression operation, TArgument argument) -> TResult virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLateBoundMemberReferenceExpression(Microsoft.CodeAnalysis.Semantics.ILateBoundMemberReferenceExpression operation, TArgument argument) -> TResult virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLiteralExpression(Microsoft.CodeAnalysis.Semantics.ILiteralExpression operation, TArgument argument) -> TResult -virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLocalFunctionStatement(Microsoft.CodeAnalysis.IOperation operation, TArgument argument) -> TResult virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLocalReferenceExpression(Microsoft.CodeAnalysis.Semantics.ILocalReferenceExpression operation, TArgument argument) -> TResult virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLockStatement(Microsoft.CodeAnalysis.Semantics.ILockStatement operation, TArgument argument) -> TResult virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitMethodBindingExpression(Microsoft.CodeAnalysis.Semantics.IMethodBindingExpression operation, TArgument argument) -> TResult From 1288d3f1e430e92c3456f2d344aeecd8a0f1a026 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Wed, 31 May 2017 13:07:14 -0700 Subject: [PATCH 2/2] Fix build break --- .../Utilities/Portable/Compilation/OperationTreeVerifier.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/Test/Utilities/Portable/Compilation/OperationTreeVerifier.cs b/src/Test/Utilities/Portable/Compilation/OperationTreeVerifier.cs index 4a0e6df980f3c..45fd96a5b3dc1 100644 --- a/src/Test/Utilities/Portable/Compilation/OperationTreeVerifier.cs +++ b/src/Test/Utilities/Portable/Compilation/OperationTreeVerifier.cs @@ -1058,12 +1058,6 @@ public override void VisitIfStatement(IIfStatement operation) Visit(operation.IfFalseStatement, "IfFalse"); } - public override void VisitLocalFunctionStatement(IOperation operation) - { - LogString(nameof(VisitLocalFunctionStatement)); - LogCommonPropertiesAndNewLine(operation); - } - private void LogCaseClauseCommon(ICaseClause operation) { var kindStr = $"{nameof(CaseKind)}.{operation.CaseKind}";