-
Notifications
You must be signed in to change notification settings - Fork 459
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 5 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,6 @@ | ||
; 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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. | ||
|
||
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 | ||
{ | ||
private 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); | ||
|
||
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.MicrosoftCodeAnalysisModelExtensions, out var modelExtensions) | ||
|| (getDeclaredSymbolMethod = modelExtensions.GetMembers(nameof(ModelExtensions.GetDeclaredSymbol)).FirstOrDefault() as IMethodSymbol) is null | ||
|| !typeProvider.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftCodeAnalysisCSharpSyntaxGlobalStatementSyntax, out var globalStatementSymbol) | ||
|| !typeProvider.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftCodeAnalysisCSharpSyntaxIncompleteMemberSyntax, out var incompleteMemberSymbol) | ||
|| !typeProvider.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftCodeAnalysisCSharpSyntaxBaseFieldDeclarationSyntax, out var baseFieldDeclarationSymbol)) | ||
{ | ||
return; | ||
} | ||
|
||
context.RegisterOperationAction(ctx => AnalyzeInvocation(ctx, getDeclaredSymbolMethod, globalStatementSymbol, incompleteMemberSymbol, baseFieldDeclarationSymbol), OperationKind.Invocation); | ||
}); | ||
} | ||
|
||
private static void AnalyzeInvocation(OperationAnalysisContext context, IMethodSymbol getDeclaredSymbolMethod, INamedTypeSymbol globalStatementSymbol, INamedTypeSymbol incompleteMemberSymbol, INamedTypeSymbol baseFieldDeclarationSymbol) | ||
{ | ||
var invocation = (IInvocationOperation)context.Operation; | ||
if (SymbolEqualityComparer.Default.Equals(invocation.TargetMethod, getDeclaredSymbolMethod)) | ||
{ | ||
var syntaxNodeType = invocation.Arguments[1].Value.WalkDownConversion().Type; | ||
if (syntaxNodeType is not null && syntaxNodeType.GetBaseTypesAndThis().ToSet().Overlaps(new[] { globalStatementSymbol, incompleteMemberSymbol, baseFieldDeclarationSymbol })) | ||
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. This allocates a new array for every invocation of Also note there is already a |
||
{ | ||
var diagnostic = invocation.CreateDiagnostic(DiagnosticDescriptor, syntaxNodeType.Name); | ||
context.ReportDiagnostic(diagnostic); | ||
} | ||
} | ||
} | ||
|
||
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(DiagnosticDescriptor); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -571,4 +571,13 @@ | |
<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>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.
I wonder if we could instead look for all the extension
GetDeclaredSymbol
methods, and use that a list of types that will return positive results? With maybe a special case for the field decl scenario, since that's somewhat misleading?If we don't want to try doing that, I think we should at least include
VariableDeclarationSyntax
, as that is also a common error.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.
Thanks for the suggestion, I have implemented it.