Skip to content
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.

Sign up for GitHub

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

Refactor internal usage analyzer #30767

Merged
7 commits merged into from
Apr 26, 2023
Merged

Refactor internal usage analyzer #30767

Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ab179a0
Refactor `InternalUsageDiagnosticAnalyzer`
DoctorKrolic Apr 26, 2023
76fdf8b
Add line breaks to match `switch` style below
DoctorKrolic Apr 26, 2023
2124ad7
Use pattern
DoctorKrolic Apr 26, 2023
1cc2c97
Nop, this is still an issue, but other one)
DoctorKrolic Apr 26, 2023
b3007ae
Properly compare symbols
DoctorKrolic Apr 26, 2023
bc8e1ed
Nulls
DoctorKrolic Apr 26, 2023
e88910d
Simplify
DoctorKrolic Apr 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 31 additions & 27 deletions src/EFCore.Analyzers/InternalUsageDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public sealed class InternalUsageDiagnosticAnalyzer : DiagnosticAnalyzer
private static readonly int EFLen = "EntityFrameworkCore".Length;

private static readonly DiagnosticDescriptor Descriptor
// HACK: Work around dotnet/roslyn-analyzers#5828 by not using target-typed new
// HACK: Work around dotnet/roslyn-analyzers#5890 by not using target-typed new
= new DiagnosticDescriptor(
Id,
title: AnalyzerStrings.InternalUsageTitle,
Expand Down Expand Up @@ -55,41 +55,48 @@ public override void Initialize(AnalysisContext context)

private static void AnalyzeNode(OperationAnalysisContext context)
{
switch (context.Operation.Kind)
switch (context.Operation)
{
case OperationKind.FieldReference:
AnalyzeMember(context, ((IFieldReferenceOperation)context.Operation).Field);
case IFieldReferenceOperation fieldReference:
AnalyzeMember(context, fieldReference.Field);
break;
case OperationKind.PropertyReference:
AnalyzeMember(context, ((IPropertyReferenceOperation)context.Operation).Property);

case IPropertyReferenceOperation propertyReference:
AnalyzeMember(context, propertyReference.Property);
break;
case OperationKind.EventReference:
AnalyzeMember(context, ((IEventReferenceOperation)context.Operation).Event);

case IEventReferenceOperation eventReference:
AnalyzeMember(context, eventReference.Event);
break;
case OperationKind.MethodReference:
AnalyzeMember(context, ((IMethodReferenceOperation)context.Operation).Method);

case IMethodReferenceOperation methodReference:
AnalyzeMember(context, methodReference.Method);
break;
case OperationKind.ObjectCreation when ((IObjectCreationOperation)context.Operation).Constructor is { } constructor:

case IObjectCreationOperation { Constructor: { } constructor }:
AnalyzeMember(context, constructor);
break;
case OperationKind.Invocation:
AnalyzeInvocation(context, (IInvocationOperation)context.Operation);

case IInvocationOperation invocation:
AnalyzeInvocation(context, invocation);
break;
case OperationKind.VariableDeclaration:
AnalyzeVariableDeclaration(context, ((IVariableDeclarationOperation)context.Operation));

case IVariableDeclarationOperation variableDeclaration:
AnalyzeVariableDeclaration(context, variableDeclaration);
break;
case OperationKind.TypeOf:
AnalyzeTypeof(context, ((ITypeOfOperation)context.Operation));

case ITypeOfOperation typeOf:
AnalyzeTypeof(context, typeOf);
break;

default:
throw new ArgumentException($"Unexpected {nameof(OperationKind)}: {context.Operation.Kind}");
throw new ArgumentException($"Unexpected operation: {context.Operation.Kind}");
}
}

private static void AnalyzeMember(OperationAnalysisContext context, ISymbol symbol)
{
// ReSharper disable once RedundantCast
if ((object)symbol.ContainingAssembly == context.Compilation.Assembly)
if (symbol.ContainingAssembly.Equals(context.Compilation.Assembly, SymbolEqualityComparer.Default))
{
// Skip all methods inside the same assembly - internal access is fine
return;
Expand All @@ -99,7 +106,7 @@ private static void AnalyzeMember(OperationAnalysisContext context, ISymbol symb

if (HasInternalAttribute(symbol))
{
ReportDiagnostic(context, symbol.Name == ".ctor" ? (object)containingType : $"{containingType}.{symbol.Name}");
ReportDiagnostic(context, symbol.Name == WellKnownMemberNames.InstanceConstructorName ? containingType : $"{containingType}.{symbol.Name}");
return;
}

Expand Down Expand Up @@ -213,8 +220,7 @@ CSharpSyntax.ClassDeclarationSyntax s when s.BaseList?.Types.Count > 0

private static void AnalyzeMethodTypeSymbol(SymbolAnalysisContext context, IMethodSymbol symbol)
{
if (symbol.MethodKind == MethodKind.PropertyGet
|| symbol.MethodKind == MethodKind.PropertySet)
if (symbol.MethodKind is MethodKind.PropertyGet or MethodKind.PropertySet)
{
// Property getters/setters are handled via IPropertySymbol
return;
Expand Down Expand Up @@ -295,13 +301,11 @@ CSharpSyntax.VariableDeclaratorSyntax declarator
};

private static bool IsInternal(SymbolAnalysisContext context, ITypeSymbol symbol)
// ReSharper disable once RedundantCast
=> (object)symbol.ContainingAssembly != context.Compilation.Assembly
=> !symbol.ContainingAssembly.Equals(context.Compilation.Assembly, SymbolEqualityComparer.Default)
&& (IsInInternalNamespace(symbol) || HasInternalAttribute(symbol));

private static bool IsInternal(OperationAnalysisContext context, ITypeSymbol symbol)
// ReSharper disable once RedundantCast
=> (object)symbol.ContainingAssembly != context.Compilation.Assembly
=> !symbol.ContainingAssembly.Equals(context.Compilation.Assembly, SymbolEqualityComparer.Default)
&& (IsInInternalNamespace(symbol) || HasInternalAttribute(symbol));

private static bool HasInternalAttribute(ISymbol symbol)
Expand Down