-
Notifications
You must be signed in to change notification settings - Fork 467
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
Support VB for ImmutableObjectMethodAnalyzer #6321
Conversation
src/Microsoft.CodeAnalysis.Analyzers/Core/CodeAnalysisDiagnosticsResources.resx
Show resolved
Hide resolved
5a6207e
to
ad3be7b
Compare
INamedTypeSymbol? type = null; | ||
if (invocation.Instance is not null) | ||
{ | ||
type = invocation.Instance.Type as INamedTypeSymbol; | ||
} | ||
else if (invocation.TargetMethod.IsExtensionMethod && invocation.TargetMethod.Parameters is [{ Type: var parameterType }, ..]) | ||
{ | ||
type = parameterType as INamedTypeSymbol; | ||
} |
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.
@mavasani Does this logic sound correct? I'm surprised there is no property on IInvocationOperation to get this info directly (or at least I couldn't find it)
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.
Can you instead use
roslyn-analyzers/src/Utilities/Compiler/Extensions/IOperationExtensions.cs
Lines 23 to 28 in b7bb138
/// <summary> | |
/// Gets the receiver type for an invocation expression (i.e. type of 'A' in invocation 'A.B()') | |
/// If the invocation actually involves a conversion from A to some other type, say 'C', on which B is invoked, | |
/// then this method returns type A if <paramref name="beforeConversion"/> is true, and C if false. | |
/// </summary> | |
public static INamedTypeSymbol? GetReceiverType(this IInvocationOperation invocation, Compilation compilation, bool beforeConversion, CancellationToken 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.
It looks like GetReceiverType
implementation is incorrect when beforeConversion
is false.
firstArg.Type
is always null. I'll use this extension and fix its implementation.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #6321 +/- ##
==========================================
+ Coverage 96.07% 96.12% +0.04%
==========================================
Files 1367 1361 -6
Lines 315348 316084 +736
Branches 10187 10183 -4
==========================================
+ Hits 302985 303841 +856
+ Misses 9930 9814 -116
+ Partials 2433 2429 -4 |
Write the analyzer using IOperation for easy VB support.
Also fixes #1128
@mavasani I'm not sure whether using IOperation improves performance, doesn't affect it, or make it worse, but if you think this should remain using syntax node actions, let me know and I'll add VB support using syntax node actions.