huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Use operations in our analyzers that need symbols (#17001) 

Fixes: #16922

This change updates our analyzers that need access to the symbols to use
`IOperation` where possible. Using syntax analysis for this kind of
analyzer has worse performance. These analyzers run on generated code,
which can include EF migrations, the design of which amplifies these
effects.

On the path to this, I also added support for a few more cases that
operations make easy. Since we're doing this anyway, I want to have
confidence that we're checking everything (within reason). In some cases
the diagnostic experience changed a bit (including more of the
declaration/code) - I felt like all of these were OK changes. Given the
kinds of error message reported by the analyzers "don't use that type"
it seems like it's still a good enough experience without
micro-optimizing all of the locations.
This commit is contained in:
Ryan Nowak 2019-11-12 17:48:12 -08:00 committed by GitHub
parent fc639d148a
commit a159473c57
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 231 additions and 101 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

191
src/Components/Analyzers/src/InternalUsageAnalyzer.cs
View File

@ -2,10 +2,10 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
namespace Microsoft.Extensions.Internal
{
@ -35,87 +35,148 @@ namespace Microsoft.Extensions.Internal
public void Register(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.RegisterSyntaxNodeAction(AnalyzeNode,
SyntaxKind.SimpleMemberAccessExpression,
SyntaxKind.ObjectCreationExpression,
SyntaxKind.ClassDeclaration,
SyntaxKind.Parameter);
// Analyze usage of our internal types in method bodies.
context.RegisterOperationAction(
AnalyzeOperation,
OperationKind.ObjectCreation,
OperationKind.FieldReference,
OperationKind.MethodReference,
OperationKind.PropertyReference,
OperationKind.EventReference);
// Analyze declarations that use our internal types in API surface.
context.RegisterSymbolAction(
AnalyzeSymbol,
SymbolKind.NamedType,
SymbolKind.Field,
SymbolKind.Method,
SymbolKind.Property,
SymbolKind.Event);
}
private void AnalyzeNode(SyntaxNodeAnalysisContext context)
private void AnalyzeOperation(OperationAnalysisContext context)
{
switch (context.Node)
var symbol = context.Operation switch
{
case MemberAccessExpressionSyntax memberAccessSyntax:
IObjectCreationOperation creation => creation.Constructor,
IInvocationOperation invocation => invocation.TargetMethod,
IFieldReferenceOperation field => field.Member,
IMethodReferenceOperation method => method.Member,
IPropertyReferenceOperation property => property.Member,
IEventReferenceOperation @event => @event.Member,
_ => throw new InvalidOperationException("Unexpected operation kind: " + context.Operation.Kind),
};
VisitOperationSymbol(context, symbol);
}
private void AnalyzeSymbol(SymbolAnalysisContext context)
{
// Note: we don't currently try to detect second-order usage of these types
// like public Task<InternalFoo> GetFooAsync() { }.
//
// This probably accomplishes our goals OK for now, which are focused on use of these
// types in method bodies.
switch (context.Symbol)
{
case INamedTypeSymbol type:
VisitDeclarationSymbol(context, type.BaseType, type);
foreach (var @interface in type.Interfaces)
{
if (context.SemanticModel.GetSymbolInfo(context.Node, context.CancellationToken).Symbol is ISymbol symbol &&
symbol.ContainingAssembly != context.Compilation.Assembly)
{
var containingType = symbol.ContainingType;
VisitDeclarationSymbol(context, @interface, type);
}
break;
if (HasInternalAttribute(symbol))
{
context.ReportDiagnostic(Diagnostic.Create(_descriptor, memberAccessSyntax.Name.GetLocation(), $"{containingType}.{symbol.Name}"));
return;
}
case IFieldSymbol field:
VisitDeclarationSymbol(context, field.Type, field);
break;
if (IsInInternalNamespace(containingType) || HasInternalAttribute(containingType))
{
context.ReportDiagnostic(Diagnostic.Create(_descriptor, memberAccessSyntax.Name.GetLocation(), containingType));
return;
}
}
return;
case IMethodSymbol method:
// Ignore return types on property-getters. Those will be reported through
// the property analysis.
if (method.MethodKind != MethodKind.PropertyGet)
{
VisitDeclarationSymbol(context, method.ReturnType, method);
}
case ObjectCreationExpressionSyntax creationSyntax:
// Ignore parameters on property-setters. Those will be reported through
// the property analysis.
if (method.MethodKind != MethodKind.PropertySet)
{
if (context.SemanticModel.GetSymbolInfo(context.Node, context.CancellationToken).Symbol is ISymbol symbol &&
symbol.ContainingAssembly != context.Compilation.Assembly)
foreach (var parameter in method.Parameters)
{
var containingType = symbol.ContainingType;
if (HasInternalAttribute(symbol))
{
context.ReportDiagnostic(Diagnostic.Create(_descriptor, creationSyntax.GetLocation(), containingType));
return;
}
if (IsInInternalNamespace(containingType) || HasInternalAttribute(containingType))
{
context.ReportDiagnostic(Diagnostic.Create(_descriptor, creationSyntax.Type.GetLocation(), containingType));
return;
}
VisitDeclarationSymbol(context, parameter.Type, method);
}
return;
}
break;
case ClassDeclarationSyntax declarationSyntax:
{
if (context.SemanticModel.GetDeclaredSymbol(declarationSyntax)?.BaseType is ISymbol symbol &&
symbol.ContainingAssembly != context.Compilation.Assembly &&
(IsInInternalNamespace(symbol) || HasInternalAttribute(symbol)) &&
declarationSyntax.BaseList?.Types.Count > 0)
{
context.ReportDiagnostic(Diagnostic.Create(_descriptor, declarationSyntax.BaseList.Types[0].GetLocation(), symbol));
}
case IPropertySymbol property:
VisitDeclarationSymbol(context, property.Type, property);
break;
return;
}
case IEventSymbol @event:
VisitDeclarationSymbol(context, @event.Type, @event);
break;
}
}
case ParameterSyntax parameterSyntax:
{
if (context.SemanticModel.GetDeclaredSymbol(parameterSyntax)?.Type is ISymbol symbol &&
symbol.ContainingAssembly != context.Compilation.Assembly &&
(IsInInternalNamespace(symbol) || HasInternalAttribute(symbol)))
{
// Similar logic here to VisitDeclarationSymbol, keep these in sync.
private void VisitOperationSymbol(OperationAnalysisContext context, ISymbol symbol)
{
if (symbol.ContainingAssembly == context.Compilation.Assembly)
{
// The type is being referenced within the same assembly. This is valid use of an "internal" type
return;
}
context.ReportDiagnostic(Diagnostic.Create(_descriptor, parameterSyntax.GetLocation(), symbol));
}
if (HasInternalAttribute(symbol))
{
context.ReportDiagnostic(Diagnostic.Create(
_descriptor,
context.Operation.Syntax.GetLocation(),
symbol.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat)));
return;
}
return;
}
var containingType = symbol.ContainingType;
if (IsInInternalNamespace(containingType) || HasInternalAttribute(containingType))
{
context.ReportDiagnostic(Diagnostic.Create(
_descriptor,
context.Operation.Syntax.GetLocation(),
containingType.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat)));
return;
}
}
// Similar logic here to VisitOperationSymbol, keep these in sync.
private void VisitDeclarationSymbol(SymbolAnalysisContext context, ISymbol symbol, ISymbol symbolForDiagnostic)
{
if (symbol.ContainingAssembly == context.Compilation.Assembly)
{
// This is part of the compilation, avoid this analyzer when building from source.
return;
}
if (HasInternalAttribute(symbol))
{
context.ReportDiagnostic(Diagnostic.Create(
_descriptor,
symbolForDiagnostic.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax().GetLocation() ?? Location.None,
symbol.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat)));
return;
}
var containingType = symbol as INamedTypeSymbol ?? symbol.ContainingType;
if (IsInInternalNamespace(containingType) || HasInternalAttribute(containingType))
{
context.ReportDiagnostic(Diagnostic.Create(
_descriptor,
symbolForDiagnostic.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax().GetLocation() ?? Location.None,
containingType.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat)));
return;
}
}

42
src/Components/Analyzers/test/ComponentInternalUsageDiagnosticsAnalyzerTest.cs
View File

@ -20,10 +20,10 @@ namespace Microsoft.AspNetCore.Components.Analyzers
private ComponentAnalyzerDiagnosticAnalyzerRunner Runner { get; }
[Fact]
public async Task InternalUsage_FindsUseOfRenderTreeFrameAsParameter()
public async Task InternalUsage_FindsUseOfInternalTypesInDeclarations()
{
// Arrange
var source = Read("UsesRenderTreeFrameAsParameter");
var source = Read("UsesRendererTypesInDeclarations");
// Act
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
@ -34,15 +34,35 @@ namespace Microsoft.AspNetCore.Components.Analyzers
diagnostic =>
{
Assert.Same(DiagnosticDescriptors.DoNotUseRenderTreeTypes, diagnostic.Descriptor);
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MMBaseClass"], diagnostic.Location);
},
diagnostic =>
{
Assert.Same(DiagnosticDescriptors.DoNotUseRenderTreeTypes, diagnostic.Descriptor);
AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MMField"], diagnostic.Location);
},
diagnostic =>
{
Assert.Same(DiagnosticDescriptors.DoNotUseRenderTreeTypes, diagnostic.Descriptor);
AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MMProperty"], diagnostic.Location);
},
diagnostic =>
{
Assert.Same(DiagnosticDescriptors.DoNotUseRenderTreeTypes, diagnostic.Descriptor);
AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MMParameter"], diagnostic.Location);
},
diagnostic =>
{
Assert.Same(DiagnosticDescriptors.DoNotUseRenderTreeTypes, diagnostic.Descriptor);
AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MMReturnType"], diagnostic.Location);
});
}
[Fact]
public async Task InternalUsage_FindsUseOfRenderTreeType()
public async Task InternalUsage_FindsUseOfInternalTypesInMethodBody()
{
// Arrange
var source = Read("UsesRenderTreeFrameTypeAsLocal");
var source = Read("UsersRendererTypesInMethodBody");
// Act
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
@ -53,7 +73,17 @@ namespace Microsoft.AspNetCore.Components.Analyzers
diagnostic =>
{
Assert.Same(DiagnosticDescriptors.DoNotUseRenderTreeTypes, diagnostic.Descriptor);
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MMField"], diagnostic.Location);
},
diagnostic =>
{
Assert.Same(DiagnosticDescriptors.DoNotUseRenderTreeTypes, diagnostic.Descriptor);
AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MMNewObject"], diagnostic.Location);
},
diagnostic =>
{
Assert.Same(DiagnosticDescriptors.DoNotUseRenderTreeTypes, diagnostic.Descriptor);
AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MMProperty"], diagnostic.Location);
});
}
}

7
src/Components/Analyzers/test/TestFiles/ComponentInternalUsageDiagnosticsAnalyzerTest/UsesRenderTreeFrameTypeAsLocal.cs → src/Components/Analyzers/test/TestFiles/ComponentInternalUsageDiagnosticsAnalyzerTest/UsersRendererTypesInMethodBody.cs
View File

@ -3,12 +3,15 @@ using Microsoft.AspNetCore.Components.RenderTree;
namespace Microsoft.AspNetCore.Components.Analyzers.Tests.TestFiles.ComponentInternalUsageDiagnosticsAnalyzerTest
{
class UsesRenderTreeFrameTypeAsLocal
class UsersRendererTypesInMethodBody
{
private void Test()
{
var test = RenderTreeFrameType./*MM*/Attribute;
var test = /*MMField*/RenderTreeFrameType.Attribute;
GC.KeepAlive(test);
var frame = /*MMNewObject*/new RenderTreeFrame();
GC.KeepAlive(/*MMProperty*/frame.Component);
}
}

11
src/Components/Analyzers/test/TestFiles/ComponentInternalUsageDiagnosticsAnalyzerTest/UsesRenderTreeFrameAsParameter.cs
View File

@ -1,11 +0,0 @@
using Microsoft.AspNetCore.Components.RenderTree;
namespace Microsoft.AspNetCore.Components.Analyzers.Tests.TestFiles.ComponentInternalUsageDiagnosticsAnalyzerTest
{
class UsesRenderTreeFrameAsParameter
{
private void Test(/*MM*/RenderTreeFrame frame)
{
}
}
}

26
src/Components/Analyzers/test/TestFiles/ComponentInternalUsageDiagnosticsAnalyzerTest/UsesRendererAsBaseClass.cs Normal file
View File

@ -0,0 +1,26 @@
using System;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Components.RenderTree;
namespace Microsoft.AspNetCore.Components.Analyzers.Tests.TestFiles.ComponentInternalUsageDiagnosticsAnalyzerTest
{
/*MM*/class UsesRendererAsBaseClass : Renderer
{
public UsesRendererAsBaseClass()
: base(null, null)
{
}
public override Dispatcher Dispatcher => throw new NotImplementedException();
protected override void HandleException(Exception exception)
{
throw new NotImplementedException();
}
protected override Task UpdateDisplayAsync(/*M1*/in RenderBatch renderBatch)
{
throw new NotImplementedException();
}
}
}

32
src/Components/Analyzers/test/TestFiles/ComponentInternalUsageDiagnosticsAnalyzerTest/UsesRendererTypesInDeclarations.cs Normal file
View File

@ -0,0 +1,32 @@
using System;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Components.RenderTree;
namespace Microsoft.AspNetCore.Components.Analyzers.Tests.TestFiles.ComponentInternalUsageDiagnosticsAnalyzerTest
{
/*MMBaseClass*/class UsesRendererTypesInDeclarations : Renderer
{
private Renderer /*MMField*/_field = null;
public UsesRendererTypesInDeclarations()
: base(null, null)
{
}
public override Dispatcher Dispatcher => throw new NotImplementedException();
/*MMProperty*/public Renderer Property { get; set; }
protected override void HandleException(Exception exception)
{
throw new NotImplementedException();
}
/*MMParameter*/protected override Task UpdateDisplayAsync(in RenderBatch renderBatch)
{
throw new NotImplementedException();
}
/*MMReturnType*/private Renderer GetRenderer() => _field;
}
}

18
src/Mvc/Mvc.Analyzers/src/AvoidHtmlPartialAnalyzer.cs
View File

@ -6,6 +6,7 @@ using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
namespace Microsoft.AspNetCore.Mvc.Analyzers
{
@ -19,16 +20,9 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers
protected override void InitializeWorker(ViewFeaturesAnalyzerContext analyzerContext)
{
analyzerContext.Context.RegisterSyntaxNodeAction(context =>
analyzerContext.Context.RegisterOperationAction(context =>
{
var invocationExpression = (InvocationExpressionSyntax)context.Node;
var symbol = context.SemanticModel.GetSymbolInfo(invocationExpression, context.CancellationToken).Symbol;
if (symbol == null || symbol.Kind != SymbolKind.Method)
{
return;
}
var method = (IMethodSymbol)symbol;
var method = ((IInvocationOperation)context.Operation).TargetMethod;
if (!analyzerContext.IsHtmlHelperExtensionMethod(method))
{
return;
@ -38,17 +32,17 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers
{
context.ReportDiagnostic(Diagnostic.Create(
SupportedDiagnostic,
invocationExpression.GetLocation(),
context.Operation.Syntax.GetLocation(),
new[] { SymbolNames.PartialMethod }));
}
else if (string.Equals(SymbolNames.RenderPartialMethod, method.Name, StringComparison.Ordinal))
{
context.ReportDiagnostic(Diagnostic.Create(
SupportedDiagnostic,
invocationExpression.GetLocation(),
context.Operation.Syntax.GetLocation(),
new[] { SymbolNames.RenderPartialMethod }));
}
}, SyntaxKind.InvocationExpression);
}, OperationKind.Invocation);
}
}
}

5
src/Mvc/Mvc.Analyzers/src/ViewFeaturesAnalyzerContext.cs
View File

@ -32,11 +32,6 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers
return false;
}
if (method.ReceiverType != HtmlHelperType)
{
return false;
}
if (method.ContainingType != HtmlHelperPartialExtensionsType)
{
return false;