Completion missing members of lambda parameter in fault tolerance case #34312

ivanbasov · 2019-03-21T00:04:54Z

src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationService.cs

CyrusNajmabadi · 2019-03-21T00:11:43Z

src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationService.cs

+            IParameterSymbol parameter,
+            int position,
+            Func<TArgumentListSyntax, SeparatedSyntaxList<TArgumentSyntax>> getArguments,
+            CancellationToken cancellationToken)


whooboy. this is complex.

Consider doc'ing this.

should we pull these type parameters up to the class? Does either language ever call this same method with a different instantiation? #Resolved

CyrusNajmabadi · 2019-03-21T00:14:04Z

src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationService.cs

+            where TLambdaExpressionSyntax : SyntaxNode
+
+        {
+            if (!(parameter.ContainingSymbol is IMethodSymbol containingMethod &&


this whole method needs comments helping explain what is going on. i.e.: // this code is to help give intellisense in the following case: .... And then explain what we're looking for and what this is working around. #Resolved

i'm going to hold off until then, aas that will really help me understand what's going on here and why it's appropriate. Ping me whne you think it's good to look again! #Resolved

CyrusNajmabadi · 2019-03-21T00:14:44Z

src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationService.cs

+                return default;
+            }
+
+            var lambdaSyntax = containingMethod.DeclaringSyntaxReferences.Single().GetSyntax(cancellationToken) as TLambdaExpressionSyntax;


Single is very dangerous. We should verify we only have one ref before doing this. you never know if a future compiler change might break this. #Resolved

ivanbasov · 2019-03-26T01:41:57Z

@CyrusNajmabadi , @dpoeschl , could you please review? #Resolved

CyrusNajmabadi · 2019-03-26T02:12:17Z

src/VisualStudio/Xaml/Impl/Microsoft.VisualStudio.LanguageServices.Xaml.csproj

@@ -32,6 +32,7 @@
    <PackageReference Include="Microsoft.VisualStudio.Shell.15.0" Version="$(MicrosoftVisualStudioShell150Version)" />
    <PackageReference Include="Microsoft.VisualStudio.ComponentModelHost" Version="$(MicrosoftVisualStudioComponentModelHostVersion)" />
    <PackageReference Include="Newtonsoft.Json" Version="$(NewtonsoftJsonVersion)" />
+    <PackageReference Include="StreamJsonRpc" Version="$(StreamJsonRpcVersion)" />


why do we need this? #Resolved

This is from another PR. Thank you!

In reply to: 268920484 [](ancestors = 268920484)

CyrusNajmabadi · 2019-03-26T02:13:16Z

src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationServiceRunner.cs

+        protected abstract bool TryGetOrdinalInArgumentList(SyntaxNode argumentOpt, out int ordinalInInvocation);
+
+        // This code is to help give intellisense in the following case: 
+        // query.Include(a => a.SomeProerty).ThenInclude(a => a.


Proerty #Resolved

CyrusNajmabadi · 2019-03-26T02:13:56Z

src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationServiceRunner.cs

+            // Cannot proceed without DeclaringSyntaxReferences.
+            // We expect that there is a single DeclaringSyntaxReferences in the scenario.
+            // If anything changes on the compiler side, the approach should be revised.
+            if (containingMethod.DeclaringSyntaxReferences.IsDefaultOrEmpty || containingMethod.DeclaringSyntaxReferences.Length > 1)


DeclaringSyntaxReferences is never Default. So you could just simple this to .DeclaringSyntaxReferences.Length != 1 #Resolved

CyrusNajmabadi · 2019-03-26T02:17:23Z

src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationServiceRunner.cs

+                                continue;
+                            }
+
+                            type = type.GetAllTypeArguments()[ordinalInLambda];


allTypeArguments[ordinalInLambda] #Resolved

CyrusNajmabadi · 2019-03-26T02:17:39Z

src/Workspaces/VisualBasic/Portable/Recommendations/VisualBasicRecommendationService.vb

            Return New VisualBasicRecommendationServiceRunner(context, filterOutOfScopeLocals, cancellationToken)
        End Function
+


remove. #Resolved

CyrusNajmabadi · 2019-03-26T02:17:45Z

src/Workspaces/VisualBasic/Portable/Recommendations/VisualBasicRecommendationServiceRunner.vb

@@ -17,6 +17,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Recommendations
        End Sub

        Public Overrides Function GetSymbols() As ImmutableArray(Of ISymbol)
+


remove... #Resolved

CyrusNajmabadi · 2019-03-26T02:17:50Z

src/Workspaces/VisualBasic/Portable/Recommendations/VisualBasicRecommendationServiceRunner.vb

@@ -53,6 +54,29 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Recommendations
            Return ImmutableArray(Of ISymbol).Empty
        End Function

+        Protected Overrides Function IsInvocationExpression(syntaxNode As SyntaxNode) As Boolean
+


remove #Resolved

CyrusNajmabadi · 2019-03-26T02:17:54Z

src/Workspaces/VisualBasic/Portable/Recommendations/VisualBasicRecommendationServiceRunner.vb

+        End Function
+
+        Protected Overrides Function IsLambdaExpression(syntaxNode As SyntaxNode) As Boolean
+


remove #Resolved

I'm not a fan of those empty lines. I just found them in the original code. Was it an obsolete code style? #Resolved

yeah... that's super bizarre... i'm not sure how that happened. def can remove :) (and can get rid of all my comments). #Resolved

OK. Thanks! Will remove all the empty lines #Resolved

CyrusNajmabadi · 2019-03-26T02:17:59Z

src/Workspaces/VisualBasic/Portable/Recommendations/VisualBasicRecommendationServiceRunner.vb

+        End Function
+
+        Protected Overrides Function TryGetOrdinalInArgumentList(argumentOpt As SyntaxNode, ByRef ordinalInInvocation As Integer) As Boolean
+


remove #Resolved

CyrusNajmabadi · 2019-03-26T02:18:28Z

src/Workspaces/VisualBasic/Portable/Recommendations/VisualBasicRecommendationServiceRunner.vb

+
+            ordinalInInvocation = -1
+            Return False
+        End Function


so, fwiw, i think you could use ISyntaxFacts and not need the top two methods. You could also move this method into the base type. #Resolved

CyrusNajmabadi · 2019-03-26T02:18:39Z

src/Workspaces/VisualBasic/Portable/Recommendations/VisualBasicRecommendationServiceRunner.vb

@@ -85,6 +109,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Recommendations
        End Function

        Private Function GetUnqualifiedSymbolsForLabelContext() As ImmutableArray(Of ISymbol)
+


remove. this style is not used at all in the IDE. #Resolved

CyrusNajmabadi · 2019-03-26T02:19:05Z

src/Workspaces/VisualBasic/Portable/Recommendations/VisualBasicRecommendationServiceRunner.vb

@@ -491,6 +523,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Recommendations
        End Function

        Private Function IsOrContainsValidAccessibleClass(namespaceOrTypeSymbol As INamespaceOrTypeSymbol, within As ISymbol) As Boolean
+


please undo all of this :) it's just not the IDE style (i also don't understand why we'd want it). #Resolved

CyrusNajmabadi · 2019-03-26T20:16:18Z

src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb

+            return default(IIncludableQueryable<TEntity, TProperty>);
+        }
+    }
+}]]>


could you extract this into a constant and just include that in all these tests. it really bloats to have to have it in each test. #Resolved

it also makes it harder to tell how each test differs. #Resolved

CyrusNajmabadi · 2019-03-26T20:16:53Z

src/EditorFeatures/Test2/IntelliSense/VisualBasicCompletionCommandHandlerTests.vb

+
+    Friend Class Registration
+        Public Property Activities As ICollection(Of Activity)
+    End Class


same point here. #Resolved

CyrusNajmabadi · 2019-03-26T20:18:10Z

src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationServiceRunner.cs

+        {
+            var builder = ArrayBuilder<ITypeSymbol>.GetInstance();
+            var expressionSymbol = _context.SemanticModel.Compilation.GetTypeByMetadataName(typeof(Expression<>).FullName);
+            var funcSymbol = _context.SemanticModel.Compilation.GetTypeByMetadataName(typeof(Func<>).FullName);


we need to be resilient to these not being around.

it's rare, but it can definitely happen during solution load (when we don't have all references properly attached). Either we can bail here if they're null, or we can check them in the loop accordingly. #Resolved

CyrusNajmabadi · 2019-03-26T20:18:54Z

src/Workspaces/VisualBasic/Portable/Recommendations/VisualBasicRecommendationService.vb

-                                                 cancellationToken As CancellationToken) As AbstractRecommendationServiceRunner(Of VisualBasicSyntaxContext)
+            context As VisualBasicSyntaxContext,
+            filterOutOfScopeLocals As Boolean,
+            cancellationToken As CancellationToken) As AbstractRecommendationServiceRunner(Of VisualBasicSyntaxContext)
            Return New VisualBasicRecommendationServiceRunner(context, filterOutOfScopeLocals, cancellationToken)
        End Function
    End Class


can just revert this file. #Resolved

CyrusNajmabadi

LGTM. would like test cleanup, and we need some null checks. I don't need to review again. Tnx!

ivanbasov · 2019-03-27T15:32:07Z

@dpoeschl please review

ivanbasov · 2019-03-27T21:17:04Z

@genlu could you please review?

src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationServiceRunner.cs

genlu

gafter · 2019-03-29T21:38:45Z

Nice work @ivanbasov !!

ivanbasov added the IDE-IntelliSense Completion, Signature Help, Quick Info label Mar 21, 2019

ivanbasov added this to the 16.1.P1 milestone Mar 21, 2019

ivanbasov requested review from CyrusNajmabadi and a team March 21, 2019 00:04

ivanbasov changed the title ~~Theninclude~~ Completion missing members of lambda parameter in fault tolerance case Mar 21, 2019

CyrusNajmabadi reviewed Mar 21, 2019

View reviewed changes

src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationService.cs Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Mar 21, 2019

View reviewed changes

src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationService.cs Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Mar 21, 2019

View reviewed changes

ivanbasov force-pushed the theninclude branch from a1c40df to 3efb28e Compare March 21, 2019 20:44

ivanbasov mentioned this pull request Mar 22, 2019

Refactoring of AbstractRecommendationService. Required for ThenInclude #34346

Merged

ivanbasov force-pushed the theninclude branch from 3efb28e to 718fe4e Compare March 25, 2019 22:27

ivanbasov added 11 commits March 25, 2019 15:29

Completion missing members of lambda parameter in fault tolerance case

90f092c

original unit test

d514f68

draft VB implementation

ccb8062

more tests (failed yet)

4fd3413

C# tests passed

c199f52

no expression

b4d1c0c

unit tersts for VB. moved some code to abstract class

ef3fba2

all tests passed

bc0f051

code cleanup

5a2117f

streamjsonrpc at LanguageServices.Xaml

8a4b616

merge

2892af9

ivanbasov force-pushed the theninclude branch from 718fe4e to 2892af9 Compare March 25, 2019 23:25

code review feedback

4ed9eeb

CyrusNajmabadi reviewed Mar 26, 2019

View reviewed changes

code review feedback

76b1d50

CyrusNajmabadi reviewed Mar 26, 2019

View reviewed changes

CyrusNajmabadi approved these changes Mar 26, 2019

View reviewed changes

ivanbasov added 2 commits March 26, 2019 16:21

refactoring tests

e9afaab

code review feedback

9ded6e3

null checks

a36fdc5

ivanbasov modified the milestones: 16.1.P1, 16.1.P2 Mar 27, 2019

dpoeschl approved these changes Mar 28, 2019

View reviewed changes

genlu reviewed Mar 28, 2019

View reviewed changes

src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationServiceRunner.cs Outdated Show resolved Hide resolved

genlu approved these changes Mar 28, 2019

View reviewed changes

code review feedback

d497f9b

ivanbasov merged commit f38ba3a into dotnet:master Mar 29, 2019

ivanbasov deleted the theninclude branch March 29, 2019 19:02

ivanbasov mentioned this pull request May 29, 2019

supporting types from params in intellisense #36040

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Completion missing members of lambda parameter in fault tolerance case #34312

Completion missing members of lambda parameter in fault tolerance case #34312

ivanbasov commented Mar 21, 2019

CyrusNajmabadi Mar 21, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 21, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 21, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 21, 2019 •

edited by ivanbasov

Loading

ivanbasov commented Mar 26, 2019 •

edited

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

ivanbasov Mar 26, 2019

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

ivanbasov Mar 26, 2019 •

edited

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

ivanbasov Mar 26, 2019 •

edited

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi left a comment

ivanbasov commented Mar 27, 2019

ivanbasov commented Mar 27, 2019

genlu left a comment

gafter commented Mar 29, 2019

		Return New VisualBasicRecommendationServiceRunner(context, filterOutOfScopeLocals, cancellationToken)
		End Function

		@@ -17,6 +17,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Recommendations
		End Sub

		Public Overrides Function GetSymbols() As ImmutableArray(Of ISymbol)

		End Function

		Protected Overrides Function IsLambdaExpression(syntaxNode As SyntaxNode) As Boolean

		End Function

		Protected Overrides Function TryGetOrdinalInArgumentList(argumentOpt As SyntaxNode, ByRef ordinalInInvocation As Integer) As Boolean

		@@ -85,6 +109,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Recommendations
		End Function

		Private Function GetUnqualifiedSymbolsForLabelContext() As ImmutableArray(Of ISymbol)

		@@ -491,6 +523,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Recommendations
		End Function

		Private Function IsOrContainsValidAccessibleClass(namespaceOrTypeSymbol As INamespaceOrTypeSymbol, within As ISymbol) As Boolean

Completion missing members of lambda parameter in fault tolerance case #34312

Completion missing members of lambda parameter in fault tolerance case #34312

Conversation

ivanbasov commented Mar 21, 2019

CyrusNajmabadi Mar 21, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 21, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 21, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 21, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov commented Mar 26, 2019 • edited Loading

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 26, 2019

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 26, 2019 • edited Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

ivanbasov Mar 26, 2019 • edited Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi Mar 26, 2019 • edited by ivanbasov Loading

Choose a reason for hiding this comment

CyrusNajmabadi left a comment

Choose a reason for hiding this comment

ivanbasov commented Mar 27, 2019

ivanbasov commented Mar 27, 2019

genlu left a comment

Choose a reason for hiding this comment

gafter commented Mar 29, 2019

CyrusNajmabadi Mar 21, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 21, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 21, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 21, 2019 •

edited by ivanbasov

Loading

ivanbasov commented Mar 26, 2019 •

edited

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

ivanbasov Mar 26, 2019 •

edited

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

ivanbasov Mar 26, 2019 •

edited

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading

CyrusNajmabadi Mar 26, 2019 •

edited by ivanbasov

Loading