-
Notifications
You must be signed in to change notification settings - Fork 466
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
Add analyzer for SemanticModel.GetDeclaredSymbol #6779
Changes from 14 commits
3fe1d7c
ae44cc9
20fa602
de68e84
efe5f87
46b50dd
40e64c0
5354e2b
03afce0
3180f1d
f34878e
2c13b86
ea2bf51
5b1a52b
7d39a9b
71f0540
da0b5d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,8 @@ | ||
; Please do not edit this file manually, it should only be updated through code fix application. | ||
|
||
### New Rules | ||
|
||
Rule ID | Category | Severity | Notes | ||
--------|----------|----------|------- | ||
RS1039 | MicrosoftCodeAnalysisCorrectness | Warning | SemanticModelGetDeclaredSymbolAlwaysReturnsNullAnalyzer | ||
RS1040 | MicrosoftCodeAnalysisCorrectness | Warning | CSharpSemanticModelGetDeclaredSymbolAlwaysReturnsNullAnalyzer |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,97 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
using System.Collections.Generic; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
using System.Collections.Immutable; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
using System.Linq; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
using Analyzer.Utilities; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
using Analyzer.Utilities.Extensions; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
using Microsoft.CodeAnalysis.Analyzers; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
using Microsoft.CodeAnalysis.Diagnostics; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
using Microsoft.CodeAnalysis.Operations; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
namespace Microsoft.CodeAnalysis.CSharp.Analyzers.MetaAnalyzers | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
using static CodeAnalysisDiagnosticsResources; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[DiagnosticAnalyzer(LanguageNames.CSharp)] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public sealed class CSharpSemanticModelGetDeclaredSymbolAlwaysReturnsNullAnalyzer : DiagnosticAnalyzer | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
internal static readonly DiagnosticDescriptor DiagnosticDescriptor = new( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
DiagnosticIds.SemanticModelGetDeclaredSymbolAlwaysReturnsNull, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
CreateLocalizableResourceString(nameof(SemanticModelGetDeclaredSymbolAlwaysReturnsNullTitle)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
CreateLocalizableResourceString(nameof(SemanticModelGetDeclaredSymbolAlwaysReturnsNullMessage)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
DiagnosticCategory.MicrosoftCodeAnalysisCorrectness, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
DiagnosticSeverity.Warning, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
isEnabledByDefault: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
description: CreateLocalizableResourceString(nameof(SemanticModelGetDeclaredSymbolAlwaysReturnsNullDescription)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
helpLinkUri: null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
customTags: WellKnownDiagnosticTagsExtensions.Telemetry); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
internal static readonly DiagnosticDescriptor FieldDiagnosticDescriptor = new( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
DiagnosticIds.SemanticModelGetDeclaredSymbolAlwaysReturnsNullForField, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
CreateLocalizableResourceString(nameof(SemanticModelGetDeclaredSymbolAlwaysReturnsNullTitle)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
CreateLocalizableResourceString(nameof(SemanticModelGetDeclaredSymbolAlwaysReturnsNullForFieldMessage)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
DiagnosticCategory.MicrosoftCodeAnalysisCorrectness, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
DiagnosticSeverity.Warning, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
isEnabledByDefault: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
description: CreateLocalizableResourceString(nameof(SemanticModelGetDeclaredSymbolAlwaysReturnsNullForFieldDescription)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
helpLinkUri: null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
customTags: WellKnownDiagnosticTagsExtensions.Telemetry); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public override void Initialize(AnalysisContext context) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
context.EnableConcurrentExecution(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
context.RegisterCompilationStartAction(static context => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var typeProvider = WellKnownTypeProvider.GetOrCreate(context.Compilation); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
IMethodSymbol? getDeclaredSymbolMethod; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (!typeProvider.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftCodeAnalysisCSharpCSharpExtensions, out var csharpExtensions) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|| !typeProvider.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftCodeAnalysisModelExtensions, out var modelExtensions) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|| !typeProvider.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftCodeAnalysisCSharpSyntaxBaseFieldDeclarationSyntax, out var baseFieldDeclaration) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|| !typeProvider.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftCodeAnalysisSyntaxNode, out var syntaxNode) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|| (getDeclaredSymbolMethod = modelExtensions.GetMembers(nameof(ModelExtensions.GetDeclaredSymbol)).FirstOrDefault() as IMethodSymbol) is null) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var allowedTypes = csharpExtensions.GetMembers(nameof(CSharpExtensions.GetDeclaredSymbol)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.OfType<IMethodSymbol>() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.Where(m => m.Parameters.Length >= 2) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.Select(m => m.Parameters[1].Type); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
context.RegisterOperationAction(ctx => AnalyzeInvocation(ctx, getDeclaredSymbolMethod, allowedTypes, baseFieldDeclaration, syntaxNode), OperationKind.Invocation); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private static void AnalyzeInvocation(OperationAnalysisContext context, IMethodSymbol getDeclaredSymbolMethod, IEnumerable<ITypeSymbol> allowedTypes, INamedTypeSymbol baseFieldDeclarationType, INamedTypeSymbol syntaxNodeType) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var invocation = (IInvocationOperation)context.Operation; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (SymbolEqualityComparer.Default.Equals(invocation.TargetMethod, getDeclaredSymbolMethod)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
var syntaxNodeDerivingType = invocation.Arguments[1].Value.WalkDownConversion().Type; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (syntaxNodeDerivingType is null || syntaxNodeDerivingType.Equals(syntaxNodeType)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diagnostic? diagnostic = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (syntaxNodeDerivingType.DerivesFrom(baseFieldDeclarationType)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
diagnostic = invocation.CreateDiagnostic(FieldDiagnosticDescriptor, syntaxNodeDerivingType.Name); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else if (allowedTypes.All(type => !syntaxNodeDerivingType.DerivesFrom(type, baseTypesOnly: true, checkTypeParameterConstraints: false))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
diagnostic = invocation.CreateDiagnostic(DiagnosticDescriptor, syntaxNodeDerivingType.Name); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (diagnostic is not null) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
context.ReportDiagnostic(diagnostic); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(DiagnosticDescriptor, FieldDiagnosticDescriptor); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move this to the top of the file above |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -571,4 +571,19 @@ | |
<data name="DoNotRegisterCompilerTypesWithBadAssemblyReferenceRuleTitle" xml:space="preserve"> | ||
<value>Compiler extensions should be implemented in assemblies with compiler-provided references</value> | ||
</data> | ||
<data name="SemanticModelGetDeclaredSymbolAlwaysReturnsNullDescription" xml:space="preserve"> | ||
<value>Calling 'SemanticModel.GetDeclaredSymbol' with an argument of type 'GlobalStatementSyntax', 'IncompleteMemberSyntax' or a type inheriting from 'BaseFieldDeclarationSyntax' will always return 'null'.</value> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels like fields should get a specific error message that instructs the user to extract the variable declarators. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like this description needs adjustment as fields now have a separate descriptor/description. Additionally, not sure if we should limit the description to just 'GlobalStatementSyntax' and 'IncompleteMemberSyntax' as the code handles all sub-types of |
||
</data> | ||
<data name="SemanticModelGetDeclaredSymbolAlwaysReturnsNullMessage" xml:space="preserve"> | ||
<value>A call to 'SemanticModel.GetDeclaredSymbol({0})' will always return 'null'</value> | ||
</data> | ||
<data name="SemanticModelGetDeclaredSymbolAlwaysReturnsNullTitle" xml:space="preserve"> | ||
<value>This call to 'SemanticModel.GetDeclaredSymbol()' will always return 'null'</value> | ||
</data> | ||
<data name="SemanticModelGetDeclaredSymbolAlwaysReturnsNullForFieldDescription" xml:space="preserve"> | ||
<value>Calling 'SemanticModel.GetDeclaredSymbol' with an argument of type 'FieldDeclarationSyntax' or 'EventFieldDeclarationSyntax' will always return 'null'. Call 'GetDeclaredSymbol' with the variable declarators from the field instead.</value> | ||
</data> | ||
<data name="SemanticModelGetDeclaredSymbolAlwaysReturnsNullForFieldMessage" xml:space="preserve"> | ||
<value>A call to 'SemanticModel.GetDeclaredSymbol({0})' will always return 'null'</value> | ||
333fred marked this conversation as resolved.
Show resolved
Hide resolved
|
||
</data> | ||
</root> |
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.
Where do we ensure that
getDeclaredSymbolMethod
has at least 2 parameters?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.
This would also not work as expected if named arguments are used and arguments are out of order. You probably want to use one of the below helper methods:
roslyn-analyzers/src/Utilities/Compiler/Extensions/IOperationExtensions.cs
Line 850 in b4ed6a3
roslyn-analyzers/src/Utilities/Compiler/Extensions/IOperationExtensions.cs
Line 871 in b4ed6a3