-
Notifications
You must be signed in to change notification settings - Fork 4.2k
IntelliSense not shown for overloaded methods, one with enums #36293
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
Conversation
|
Will provide a fix for VB soon #Resolved |
Verified that no fix is required for VB. Everything works fine there. |
| } | ||
|
|
||
| if (type.TypeKind != TypeKind.Enum) | ||
| foreach (var typeIterator in types) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: foreach (var type in types) is clearer since you're not reusing the iterator variable at another point. #ByDesign
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only reason for this is there are assignments to the type variables in the body of the loop:
var type = typeIterator;
type = type.GetTypeArguments().FirstOrDefault();
In reply to: 292626186 [](ancestors = 292626186)
| } | ||
| if (type.TypeKind != TypeKind.Enum) | ||
| { | ||
| type = TryGetEnumTypeInEnumInitializer(semanticModel, token, type, cancellationToken) ?? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little confused by this logic. If the type is not an enum, we try to get an enum? #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There can be multiple types. Some of them can be enums and some are not. The scenarios we're fixing is an overload with enum in one case and not enum in another.
We should bail out the non-enum case but provide results for the enum one. It should not depend which one was taken with FirstOrDefault.
In reply to: 292626711 [](ancestors = 292626711)
| rules: s_rules.WithMatchPriority(MatchPriority.Preselect), | ||
| contextPosition: position); | ||
| var displayService = document.GetLanguageService<ISymbolDisplayService>(); | ||
| var displayText = alias != null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: alias?.Name ?? displayService.ToMinimalDisplayString(semanticModel, position, type) #WontFix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ryzngard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the most part looks good to me. I'm not too familiar with this area, but the change seems straight forward with the exception of commented question.
Fixes #36187