IntelliSense not shown for overloaded methods, one with enums#36293
IntelliSense not shown for overloaded methods, one with enums#36293ivanbasov merged 1 commit intodotnet:masterfrom
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.
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.
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.
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.
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.
nit: alias?.Name ?? displayService.ToMinimalDisplayString(semanticModel, position, type) #WontFix
There was a problem hiding this comment.
ryzngard
left a comment
There was a problem hiding this comment.
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