Skip to content

SignatureHelp: fix display or ref-returning indexer#24359

Merged
jcouv merged 1 commit intodotnet:dev15.6.xfrom
jcouv:ref-quickinfo
Jan 22, 2018
Merged

SignatureHelp: fix display or ref-returning indexer#24359
jcouv merged 1 commit intodotnet:dev15.6.xfrom
jcouv:ref-quickinfo

Conversation

@jcouv
Copy link
Copy Markdown
Member

@jcouv jcouv commented Jan 20, 2018

Customer scenario

SignatureHelp on a ref-returning or ref-readonly-returning indexer should display the "ref" or "ref readonly" modifiers, as the screenshot shows (post fix).

image

Bugs this fixes

Fixes #24311

Risk

Performance impact

Low. Just adding a check for ref-returning symbols in the SignatureHelp and adding a couple of parts to display when appropriate.

Is this a regression from a previous update?

No

Root cause analysis

The SignatureHelper only relies on the SymbolDisplayVisitor to display parts of an indexer's signature. The SymbolDisplayVisitor would display ref int C.this[...], but the SignatureHelper wants to display ref int C[...]. This non-standard behavior is the reason why the fix in SymbolDisplayVisitor did not automatically handle this scenario.

How was the bug found?

Reported by customer.

@dotnet/roslyn-ide for review. Thanks
Tagging @VSadov @OmarTawfik as FYI

@jcouv jcouv added the Area-IDE label Jan 20, 2018
@jcouv jcouv added this to the 15.6 milestone Jan 20, 2018
@jcouv jcouv self-assigned this Jan 20, 2018
@jcouv jcouv requested a review from a team as a code owner January 20, 2018 23:42
@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented Jan 22, 2018

@dotnet/roslyn-ide for review for 15.6. Thanks

@jcouv
Copy link
Copy Markdown
Member Author

jcouv commented Jan 22, 2018

Thanks!
@Pilchie for ask-mode approval for 15.6.

@Pilchie
Copy link
Copy Markdown
Member

Pilchie commented Jan 22, 2018

Approved.

@jcouv jcouv merged commit 991ae7e into dotnet:dev15.6.x Jan 22, 2018
@jcouv jcouv deleted the ref-quickinfo branch January 22, 2018 20:11
jmarolf pushed a commit to jmarolf/roslyn that referenced this pull request Jan 30, 2018
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.

3 participants