roslyn
roslyn copied to clipboard
dotnet
Filter only valid symbols for inheritance completions
I was a bit surprised, but there already was symbol filtering, but only for VB. I utilized this behaviour and made it shared between C# and VB. Also transfered some technics to other special case symbol filtering, like getting an underlying symbol form an alias or checking a namespace for valid members. Currently this PR implements:
- Shared
SyntaxContextflags, that allow to utilize inheritance completion behaviour between both languages - Correct handling for C# inheritance of classes (provides both classes and interfaces)
- Correct handling for VB
Inherits/Implements - Correct handling of interface inheritance in both languages
- Correct handling of structs when implementing interfaces
- Correct handling of self-reference both for classes and interfaces (https://github.com/dotnet/roslyn/issues/60935)
- Correct handling of C# keywords
After review:
- Added handling of records
- Do not show classes/records when there is already one item in the base list
As a side result: Fixes: https://github.com/dotnet/roslyn/issues/60935
Example of how it works for basic C# case:
Before:
After:
One thing that seemed odd to me is that I didn't see any distinction on c# between being on the first base type of a class/record or the second onwards. The latter must be an interface.
Are there vb tests?
As I said, VB was there before. So did tests for VB
Are there tests for aliases?
Currently only in VB. Will implement for C#
One thing that seemed odd to me is that I didn't see any distinction on c# between being on the first base type of a class/record or the second onwards. The latter must be an interface.
Yes, I forgot about records and didn't even thought about disabling classes for C# when there is one. Will introduce later as well
@DoctorKrolic The summary of this PR is long, but it doesn't appear to describe the actual change. I'm not sure what I'm supposed to be reviewing, or how it's supposed to be better than what's already there.
@DoctorKrolic can you revise the title of htis PR to reflect what change is happening?
@CyrusNajmabadi It seems that I adressed all your feedback, so I would like a review pass again!
Note: enum base completions where already supported in VB, and it was not some explicit check for this case, but because of VB's symbol recommender structure: it checks for all valid cases and else returns empty list. So I thought, that there was no point in all scaffolding required if I want it to be in s shared layer and added just a simple chick in C#-specific code.
@DoctorKrolic I've got a lot of critical things i'm looking at right now. This is on my list, but i likely can't get to it for a bit. Thank you for your patience.