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.

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

D2L0102: Attemped to locate an unlocatable type #920

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
#nullable enable

using System.Collections.Immutable;
using D2L.CodeStyle.Analyzers.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
using static D2L.CodeStyle.Analyzers.ApiUsage.ServiceLocator.SingletonLocatorAnalyzer;

namespace D2L.CodeStyle.Analyzers.ApiUsage.ServiceLocator {

[DiagnosticAnalyzer( LanguageNames.CSharp )]
public sealed class ServiceLocationAnalyzer : DiagnosticAnalyzer {

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(
Diagnostics.LocatedUnlocatable
);

public override void Initialize( AnalysisContext context ) {
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis( GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics );
context.RegisterCompilationStartAction( RegisterSingletonLocatorAnalyzer );
}

public void RegisterSingletonLocatorAnalyzer( CompilationStartAnalysisContext context ) {
LocatedTypeResolver? locatedTypeResolver = GetLocatedTypeResolver( context.Compilation );
if( locatedTypeResolver is null ) {
return;
}

context.RegisterOperationAction(
ctx => EnforceLocatorRules(
ctx,
( (IInvocationOperation)ctx.Operation ).TargetMethod,
locatedTypeResolver
),
OperationKind.Invocation
);

context.RegisterOperationAction(
ctx => EnforceLocatorRules(
ctx,
( (IMethodReferenceOperation)ctx.Operation ).Method,
locatedTypeResolver
),
OperationKind.MethodReference
);
}

private static void EnforceLocatorRules(
OperationAnalysisContext context,
IMethodSymbol methodSymbol,
LocatedTypeResolver locatedTypeResolver
) {
ITypeSymbol? locatedType = locatedTypeResolver( methodSymbol );
if( locatedType is null ) {
return;
}

EnforceUnlocatable( context, locatedType );
}

// Enforce that the locator isn't used to load [Unlocatable] or [Unlocatable.Candidate]
private static void EnforceUnlocatable(
OperationAnalysisContext context,
ITypeSymbol locatedType
) {
if( !Attributes.Unlocatable.IsDefined( locatedType )
&& !Attributes.UnlocatableCandidate.IsDefined( locatedType )
) {
return;
}

context.ReportDiagnostic(
descriptor: Diagnostics.LocatedUnlocatable,
location: context.Operation.Syntax.GetLocation(),
messageArgs: new[] { locatedType.GetFullTypeName() }
);
}

private delegate ITypeSymbol? LocatedTypeResolver( IMethodSymbol method );

private static LocatedTypeResolver? GetLocatedTypeResolver( Compilation compilation ) {
ImmutableDictionary<IMethodSymbol, Func<IMethodSymbol, ITypeSymbol>> locatorMethods = GetLocatorMethods( compilation );

if( locatorMethods.IsEmpty ) {
return null;
}

ContainedTypeResolver containedTypeResolver = GetContainedTypeResolver( compilation );

return method => {
if( !locatorMethods.TryGetValue( method.OriginalDefinition, out var resolver ) ) {
return null;
}

ITypeSymbol type = resolver( method );
type = containedTypeResolver( type );
return type;
};
}

private static ImmutableDictionary<IMethodSymbol, Func<IMethodSymbol, ITypeSymbol>> GetLocatorMethods( Compilation compilation ) {
var locatorMethodMappings = new (string typeName, string methodName, Func<IMethodSymbol, bool> methodPicker, Func<IMethodSymbol, ITypeSymbol> typeResolver)[] {
new( "D2L.LP.Extensibility.Activation.Domain.SingletonLocator", "Get", _ => true, m => m.TypeArguments.Single() ),
new( "D2L.LP.Extensibility.Activation.Domain.IServiceLocator", "Get", m => m.TypeParameters.Length == 1, m => m.TypeArguments.Single() ),
new( "D2L.LP.Extensibility.Activation.Domain.IServiceLocator", "TryGet", m => m.TypeParameters.Length == 1, m => m.TypeArguments.Single() ),
Comment on lines +105 to +107
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should perhaps add IObjectActivator as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering why this is in D2L.CodeStyle vs the spot for analyzers in the LMS repo?
I don't really care, just want to understand where/if we draw the line.

Copy link
Contributor Author

@omsmith omsmith Sep 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was simply putting it beside the other locator analyzers. Would be happy if they all moved. Would need to rewrite / port spectests in that case though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spec tests in the LMS repo are probably a good idea.
Anyway no worries, just wondering.

};

var locatorMethodsBuilder = ImmutableDictionary.CreateBuilder<IMethodSymbol, Func<IMethodSymbol, ITypeSymbol>>( SymbolEqualityComparer.Default );

foreach( (string typeName, string methodName, var methodPicker, var typeResolver ) in locatorMethodMappings ) {

INamedTypeSymbol? type = compilation.GetTypeByMetadataName( typeName );
if( type.IsNullOrErrorType() ) {
continue;
}

IMethodSymbol? method = type
.GetMembers( methodName )
.OfType<IMethodSymbol>()
.Single( methodPicker );

locatorMethodsBuilder.Add( method, typeResolver );
}

return locatorMethodsBuilder.ToImmutable();
}

}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using D2L.CodeStyle.Analyzers.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
Expand All @@ -25,14 +24,14 @@ public void RegisterSingletonLocatorAnalyzer( CompilationStartAnalysisContext co
return;
}

ImmutableDictionary<INamedTypeSymbol, int> containerTypes = GetContainerTypes( context.Compilation );
ContainedTypeResolver containedTypeResolver = GetContainedTypeResolver( context.Compilation );

context.RegisterOperationAction(
ctx => EnforceSingletonsOnly(
ctx,
( (IInvocationOperation)ctx.Operation ).TargetMethod,
IsSingletonLocatorGet,
IsContainerType
containedTypeResolver
),
OperationKind.Invocation
);
Expand All @@ -42,7 +41,7 @@ public void RegisterSingletonLocatorAnalyzer( CompilationStartAnalysisContext co
ctx,
( (IMethodReferenceOperation)ctx.Operation ).Method,
IsSingletonLocatorGet,
IsContainerType
containedTypeResolver
),
OperationKind.MethodReference
);
Expand All @@ -51,51 +50,26 @@ bool IsSingletonLocatorGet( IMethodSymbol? methodSymbol ) => SymbolEqualityCompa
getMethodSymbol,
methodSymbol?.OriginalDefinition
);

bool IsContainerType(
ITypeSymbol type,
[NotNullWhen( true )] out ITypeSymbol? containedType
) {

if( type is not INamedTypeSymbol namedType ) {
containedType = null;
return false;
}

if( containerTypes.TryGetValue( namedType.OriginalDefinition, out int typeArgumentIndex ) ) {
containedType = namedType.TypeArguments[ typeArgumentIndex ];
return true;
}

containedType = null;
return false;
}
}

private delegate bool IsSingletonLocatorGet(
IMethodSymbol? methodSymbol
);

private delegate bool IsContainerType(
ITypeSymbol type,
[NotNullWhen( true )] out ITypeSymbol? containedType
);

// Enforce that SingletonLocator can only load actual [Singleton]s
private static void EnforceSingletonsOnly(
OperationAnalysisContext context,
IMethodSymbol methodSymbol,
IsSingletonLocatorGet isSingletonLocatorGet,
IsContainerType isContainerType
ContainedTypeResolver containedTypeResolver
) {
if( !isSingletonLocatorGet( methodSymbol ) ) {
return;
}

ITypeSymbol typeArg = methodSymbol.TypeArguments.Single();
if( isContainerType( typeArg, out ITypeSymbol? containedType ) ) {
typeArg = containedType;
}
ITypeSymbol typeArg = containedTypeResolver(
methodSymbol.TypeArguments.Single()
);

if( Attributes.Singleton.IsDefined( typeArg ) ) {
return;
Expand Down Expand Up @@ -125,6 +99,23 @@ Compilation compilation
return (IMethodSymbol)getMembers[ 0 ];
}

internal delegate ITypeSymbol ContainedTypeResolver( ITypeSymbol type );
internal static ContainedTypeResolver GetContainedTypeResolver( Compilation compilation ) {
ImmutableDictionary<INamedTypeSymbol, int> containerTypes = GetContainerTypes( compilation );

return type => {
if( type is not INamedTypeSymbol namedType ) {
return type;
}

if( containerTypes.TryGetValue( namedType.OriginalDefinition, out int typeArgumentIndex ) ) {
return namedType.TypeArguments[ typeArgumentIndex ];
}

return type;
};
}

private static ImmutableDictionary<INamedTypeSymbol, int> GetContainerTypes( Compilation compilation ) {
var containerTypeMappings = new (string typeName, int containedTypeIdx)[] {
new ( "D2L.LP.Extensibility.Activation.Domain.IPlugins`1", 0 ),
Expand Down
2 changes: 2 additions & 0 deletions src/D2L.CodeStyle.Analyzers/Attributes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ internal static class Attributes {
internal static readonly RoslynAttribute Singleton = new RoslynAttribute( "D2L.LP.Extensibility.Activation.Domain.SingletonAttribute" );
internal static readonly RoslynAttribute DIFramework = new RoslynAttribute( "D2L.LP.Extensibility.Activation.Domain.DIFrameworkAttribute" );
internal static readonly RoslynAttribute Dependency = new RoslynAttribute( "D2L.LP.Extensibility.Activation.Domain.DependencyAttribute" );
internal static readonly RoslynAttribute Unlocatable = new RoslynAttribute( "D2L.LP.Extensibility.Activation.Domain.UnlocatableAttribute" );
internal static readonly RoslynAttribute UnlocatableCandidate = new RoslynAttribute( "D2L.LP.Extensibility.Activation.Domain.Unlocatable.CandidateAttribute" );

internal sealed class RoslynAttribute {

Expand Down
9 changes: 9 additions & 0 deletions src/D2L.CodeStyle.Analyzers/Diagnostics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -757,5 +757,14 @@ public static class Diagnostics {
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true
);

public static readonly DiagnosticDescriptor LocatedUnlocatable = new DiagnosticDescriptor(
id: "D2L0102",
title: "Attemped to locate an unlocatable type",
messageFormat: "Cannot locate {0} because it is marked [Unlocatable] or [Unlocatable.Candidate]",
category: "Correctness",
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true
);
}
}
Loading
Loading