Skip to content

Simplify portions of symbol completion#51811

Merged
9 commits merged intodotnet:mainfrom
CyrusNajmabadi:simplifyCompletion
Mar 12, 2021
Merged

Simplify portions of symbol completion#51811
9 commits merged intodotnet:mainfrom
CyrusNajmabadi:simplifyCompletion

Conversation

@CyrusNajmabadi
Copy link
Contributor

Working on this as part of "intellisense for operators/conversions". These methods often confuse the heck out of me. So i'm just working on making the data flow and checks clearer and simpler.

@CyrusNajmabadi CyrusNajmabadi requested review from Cosifne and genlu March 11, 2021 21:54
@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner March 11, 2021 21:54
@ghost ghost added the Area-IDE label Mar 11, 2021

// should not crash
await VerifyItemExistsAsync(markup, "ToString");
await VerifyNoItemsExistAsync(markup);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this test was bogus. we shoudl not have offered completion here. teh cleanup simplified code and ended up fixing this.

Copy link
Member

@Cosifne Cosifne left a comment

Choose a reason for hiding this comment

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

:shipit:

filterOutOfScopeLocals = false;
}
}
filterOutOfScopeLocals = !_context.LeftToken.GetRequiredParent().IsFoundUnder<LocalDeclarationStatementSyntax>(d => d.Declaration.Type);
Copy link
Member

Choose a reason for hiding this comment

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

The original code is easier to understand for me. Can we just merge the if statements instead?

if (symbol.Kind is SymbolKind.NamedType or SymbolKind.Namespace)
{
return ImmutableArray<ISymbol>.Empty;
// For named typed and namespaces, we flip things around. We only want statics and not instance members.
Copy link
Member

Choose a reason for hiding this comment

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

typo: types

Copy link
Member

@genlu genlu left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Auto-approval

@CyrusNajmabadi
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@ghost ghost merged commit 8da536d into dotnet:main Mar 12, 2021
@ghost ghost added this to the Next milestone Mar 12, 2021
@CyrusNajmabadi CyrusNajmabadi deleted the simplifyCompletion branch March 12, 2021 20:44
@allisonchou allisonchou modified the milestones: Next, 16.10.P2 Mar 29, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants