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

Fix find references for indexer properties #1399

Merged
merged 3 commits into from
Mar 19, 2019

Conversation

SirIntruder
Copy link
Contributor

I've noticed find references/code lens were broken for indexer properties, in a similar fashion to #1371. Yet this couldn't be fixed by pushing "CanBeReferencedByName" mess any more, as both get/set method and indexer property can't be referenced by name. So the PR would make sure:

  1. Locations of usages returned by Roslyn are used as-is

  2. When request wants locations of definitions to be added to the response, get/set methods of properties are filtered out using this. Works for both normal and indexed properties and add/remove of events . This perserves current behaviour, but removes all of the unintented side-effects.

  3. There is a new test for indexer properties.

Copy link
Member

@bjorkstromm bjorkstromm left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@david-driscoll david-driscoll left a comment

Choose a reason for hiding this comment

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

@SirIntruder thanks for your contribution! Sorry it took us so long to merge.

@filipw filipw merged commit 0fcefe1 into OmniSharp:master Mar 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants