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

Feature/disambiguate using keyword #48898

Merged

Conversation

TheSench
Copy link
Contributor

This PR adds a separate f1_keyword for the using-statement semantics of the using keyword. This allows the F1 help to route to the appropriate page based on the context, rather than having to route to a generic using page. It also updates the logic detecting a using-static to also catch the normal using directive, since it already knows at that point and doesn't need to waste time doing other checks before falling through to the using keyword.

Fixes part of #48392

Related docs PR:
dotnet/docs#21209

Co-authored-by: CyrusNajmabadi <cyrus.najmabadi@gmail.com>
@@ -375,6 +379,12 @@ private static bool TryGetTextForKeyword(SyntaxToken token, ISyntaxFactsService
return true;
}

if (token.IsKind(SyntaxKind.UsingKeyword) && token.Parent is UsingStatementSyntax)
{
text = Keyword("using-statement");
Copy link
Contributor

Choose a reason for hiding this comment

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

The docs page this leads to also covers using declarations (using var x = Foo();) so it would be good to get a test and support for that at the same time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the PR to now include using declarations. I only have a simple test for it right now, and it looks like these are disallowed inside places like the expression of an if-statement. Let me know if I should add additional tests beyond simply testing a local declaration.

@jinujoseph jinujoseph added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Oct 26, 2020
@jinujoseph jinujoseph added this to InQueue in IDE: CommunityPR via automation Oct 26, 2020
@jinujoseph jinujoseph moved this from InQueue to BackToCommunity in IDE: CommunityPR Oct 26, 2020
@jinujoseph jinujoseph moved this from BackToCommunity to BuddyAssigned in IDE: CommunityPR Oct 26, 2020
IDE: CommunityPR automation moved this from BuddyAssigned to LGTM Oct 28, 2020
@davidwengier
Copy link
Contributor

Thanks @TheSench, will wait for the docs PR to be merged, but this looks good to me!

@davidwengier
Copy link
Contributor

Docs PR was merged! Thanks again for the contribution.

@davidwengier davidwengier merged commit 85aa538 into dotnet:master Dec 3, 2020
IDE: CommunityPR automation moved this from LGTM to Completed Dec 3, 2020
@ghost ghost added this to the Next milestone Dec 3, 2020
333fred added a commit to 333fred/roslyn that referenced this pull request Dec 5, 2020
* upstream/master: (143 commits)
  Use Unicode for OutputEncoding (dotnet#49721)
  Fix tests
  Remove unnecessary parens when converting switch statement ot expression.
  Fix flaky test
  Simplify implementation of IsOrInGenericType helper and reduce code duplication. (dotnet#49763)
  Increase CoreCLR dump information
  NRT
  Be resilient to a null-document in the code-folding service.
  Change cache implementation from dictionary to list
  Fix APIs for retrieving solution snapshot from Razor, UT (dotnet#49591)
  Handles optional parameters
  Feature/disambiguate using keyword (dotnet#48898)
  Check all overloads when inferring first argument in an invocation with nothing typed
  Remove optional parameter
  lint
  Clean up option handling
  Remove double blank line
  Add + update tests
  Ensure completion and completion resolve handler always pass the same options
  Change name of telemetry
  ...
@TheSench TheSench deleted the feature/disambiguate-using-keyword branch December 6, 2020 16:37
@dibarbet dibarbet removed this from the Next milestone Dec 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.
Projects
IDE: CommunityPR
  
Completed 2020
Development

Successfully merging this pull request may close these issues.

None yet

6 participants