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

Add codefixer and completion provider to install OpenAPI package from extension methods #55963

Merged
merged 10 commits into from
Jun 14, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public class DiagnosticProject
private static readonly ICompilationAssemblyResolver _assemblyResolver = new AppBaseCompilationAssemblyResolver();
private static readonly Dictionary<Assembly, Solution> _solutionCache = new Dictionary<Assembly, Solution>();

public static Project Create(Assembly testAssembly, string[] sources, Func<Workspace> workspaceFactory = null, Type analyzerReference = null)
public static Project Create(Assembly testAssembly, string[] sources, Func<Workspace> workspaceFactory = null, Type[] analyzerReferences = null)
{
Solution solution;
lock (_solutionCache)
Expand All @@ -50,11 +50,14 @@ public static Project Create(Assembly testAssembly, string[] sources, Func<Works
}
}

if (analyzerReference != null)
if (analyzerReferences != null)
{
solution = solution.AddAnalyzerReference(
projectId,
new AnalyzerFileReference(analyzerReference.Assembly.Location, AssemblyLoader.Instance));
foreach (var analyzerReference in analyzerReferences)
{
solution = solution.AddAnalyzerReference(
projectId,
new AnalyzerFileReference(analyzerReference.Assembly.Location, AssemblyLoader.Instance));
}
}

_solutionCache.Add(testAssembly, solution);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Immutable;
using System.Composition;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.App.Analyzers.Infrastructure;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.ExternalAccess.AspNetCore.AddPackage;

namespace Microsoft.AspNetCore.Analyzers.Dependencies;

/// <summary>
/// This fixer uses Roslyn's AspNetCoreAddPackageCodeAction to support providing a code fix for a missing
/// package based on APIs defined in that package that are called in user code. This fixer is particularly
/// helpful for providing guidance to users on how to add a missing package when they are using an extension
/// method on well-known types like `IServiceCollection` and `IApplicationBuilder`.
/// </summary>
/// <remarks>
/// This class is not sealed to support mocking of the virtual method `TryCreateCodeActionAsync` in unit tests.
/// </remarks>
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AddPackageFixer)), Shared]
public class AddPackageFixer : CodeFixProvider
JamesNK marked this conversation as resolved.
Show resolved Hide resolved
captainsafia marked this conversation as resolved.
Show resolved Hide resolved
{
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
if (root == null)
{
return;
}

var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
if (semanticModel == null)
{
return;
}

var wellKnownTypes = WellKnownTypes.GetOrCreate(semanticModel.Compilation);
var wellKnownExtensionMethodCache = ExtensionMethodsCache.ConstructFromWellKnownTypes(wellKnownTypes);

// Diagnostics are already filtered by FixableDiagnosticIds values.
foreach (var diagnostic in context.Diagnostics)
captainsafia marked this conversation as resolved.
Show resolved Hide resolved
{
var location = diagnostic.Location.SourceSpan;
var node = root.FindNode(location);
if (node == null)
{
return;
}
var methodName = node is IdentifierNameSyntax identifier ? identifier.Identifier.Text : null;
if (methodName == null)
{
return;
}

if (node.Parent is not MemberAccessExpressionSyntax)
{
return;
}

var symbol = semanticModel.GetSymbolInfo(((MemberAccessExpressionSyntax)node.Parent).Expression).Symbol;
var symbolType = symbol switch
{
IMethodSymbol methodSymbol => methodSymbol.ReturnType,
IPropertySymbol propertySymbol => propertySymbol.Type,
ILocalSymbol localSymbol => localSymbol.Type,
_ => null
};

if (symbolType == null)
{
return;
}

var targetThisAndExtensionMethod = new ThisAndExtensionMethod(symbolType, methodName);
if (wellKnownExtensionMethodCache.TryGetValue(targetThisAndExtensionMethod, out var packageSourceAndNamespace))
{
var position = diagnostic.Location.SourceSpan.Start;
var packageInstallData = new AspNetCoreInstallPackageData(
packageSource: null,
packageName: packageSourceAndNamespace.packageName,
packageVersionOpt: null,
packageNamespaceName: packageSourceAndNamespace.namespaceName);
var codeAction = await TryCreateCodeActionAsync(
context.Document,
position,
packageInstallData,
context.CancellationToken);

if (codeAction != null)
{
context.RegisterCodeFix(codeAction, diagnostic);
}
}

}
}

public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;

/// <example>
/// 'IServiceCollection' does not contain a definition for 'AddOpenApi' and no accessible extension method 'AddOpenApi' accepting
/// a first argument of type 'IServiceCollection' could be found (are you missing a using directive or an assembly reference?).
/// </example>
public override ImmutableArray<string> FixableDiagnosticIds { get; } = ["CS1061"];
captainsafia marked this conversation as resolved.
Show resolved Hide resolved

internal virtual async Task<CodeAction?> TryCreateCodeActionAsync(
Document document,
int position,
AspNetCoreInstallPackageData packageInstallData,
CancellationToken cancellationToken)
{
var codeAction = await AspNetCoreAddPackageCodeAction.TryCreateCodeActionAsync(
document,
position,
packageInstallData,
cancellationToken);

return codeAction;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using Microsoft.AspNetCore.Analyzers;
using Microsoft.AspNetCore.App.Analyzers.Infrastructure;

internal static class ExtensionMethodsCache
{
public static Dictionary<ThisAndExtensionMethod, PackageSourceAndNamespace> ConstructFromWellKnownTypes(WellKnownTypes wellKnownTypes)
{
return new()
{
{
new(wellKnownTypes.Get(WellKnownTypeData.WellKnownType.Microsoft_Extensions_DependencyInjection_IServiceCollection), "AddOpenApi"),
new("Microsoft.AspNetCore.OpenApi", "Microsoft.Extensions.DependencyInjection")
},
{
new(wellKnownTypes.Get(WellKnownTypeData.WellKnownType.Microsoft_AspNetCore_Builder_WebApplication), "MapOpenApi"),
new("Microsoft.AspNetCore.OpenApi", "Microsoft.AspNetCore.Builder")
}
};
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

@JamesNK Can I get your review on this given your experience implementing completion providers in route tooling?

The goal is to add completions for extension methods defined in a separate package then provide an installation codefix for them.

Copy link
Member

Choose a reason for hiding this comment

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

It's been a minute since my head was in The Roslyn Zone. I'll do my best.

Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Composition;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.App.Analyzers.Infrastructure;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Completion;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;

namespace Microsoft.AspNetCore.Analyzers.RouteEmbeddedLanguage;

/// <summary>
/// This completion provider expands the completion list of target symbols defined in the
/// ExtensionMethodsCache to include extension methods that can be invoked on the target
/// type that are defined in auxillary packages. This completion provider is designed to be
/// used in conjunction with the `AddPackageFixer` to recommend adding the missing packages
/// extension methods are defined in.
/// </summary>
[ExportCompletionProvider(nameof(ExtensionMethodsCompletionProvider), LanguageNames.CSharp)]
[Shared]
public sealed class ExtensionMethodsCompletionProvider : CompletionProvider
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment summarizing what the provider's deal is.

Does it replace completion for known types or are values from the completion provider added to the auto-complete? Replacing auto-complete for some types (if that's how it works) feels pretty invasive.

Copy link
Member Author

Choose a reason for hiding this comment

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

The completion provider is additive (assuming I understand the intent of the context.AddItem method 😅 ) and is intended to augment the completions list with methods that we think might be helpful. I'll add a comment to this effect.

Copy link
Member

@JamesNK JamesNK Jun 14, 2024

Choose a reason for hiding this comment

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

What happens if the value is already present in auto-complete? For example, someone might already have a reference to the OpenAPI package. Do they see the AddOpenApi option from built-in autocomplete and a item from this provider?

Do items added by this provider look the same or different to other items? Could I see a screenshot?

Copy link
Member

Choose a reason for hiding this comment

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

So excited! 🥳

{
public override async Task ProvideCompletionsAsync(CompletionContext context)
{
if (!context.Document.SupportsSemanticModel)
{
return;
}

var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
if (root == null)
{
return;
}

var span = context.CompletionListSpan;
var token = root.FindToken(span.Start);
if (token.Parent == null)
{
return;
}

var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
if (semanticModel == null)
{
return;
}

var wellKnownTypes = WellKnownTypes.GetOrCreate(semanticModel.Compilation);
var wellKnownExtensionMethodCache = ExtensionMethodsCache.ConstructFromWellKnownTypes(wellKnownTypes);

// We find the nearest member access expression to the adjacent expression to resolve the
// target type of the extension method that the user is invoking. For example, `app.` should
// allow us to resolve to a `WebApplication` instance and `builder.Services.Add` should resolve
// to an `IServiceCollection`.
var nearestMemberAccessExpression = FindNearestMemberAccessExpression(token.Parent);
if (nearestMemberAccessExpression is not null && nearestMemberAccessExpression is MemberAccessExpressionSyntax memberAccess)
{
var symbol = semanticModel.GetSymbolInfo(memberAccess.Expression);
var symbolType = symbol.Symbol switch
{
IMethodSymbol methodSymbol => methodSymbol.ReturnType,
IPropertySymbol propertySymbol => propertySymbol.Type,
ILocalSymbol localSymbol => localSymbol.Type,
_ => null
};

var matchingExtensionMethods = wellKnownExtensionMethodCache.Where(pair => IsMatchingExtensionMethod(pair, symbolType, token));
foreach (var item in matchingExtensionMethods)
{
context.CompletionListSpan = span;
context.AddItem(CompletionItem.Create(
displayText: item.Key.ExtensionMethod,
sortText: item.Key.ExtensionMethod,
filterText: item.Key.ExtensionMethod
));
}
}
}

private static SyntaxNode? FindNearestMemberAccessExpression(SyntaxNode? node)
{
var current = node;
while (current != null)
{
if (current?.IsKind(SyntaxKind.SimpleMemberAccessExpression) ?? false)
{
return current;
}

current = current?.Parent;
}

return null;
}

private static bool IsMatchingExtensionMethod(
KeyValuePair<ThisAndExtensionMethod, PackageSourceAndNamespace> pair,
ISymbol? symbolType,
SyntaxToken token)
{
if (symbolType is null)
{
return false;
}

// If the token that we are parsing is some sort of identifier, this indicates that the user
// has triggered a completion with characters already inserted into the invocation (e.g. `builder.Services.Ad$$).
// In this case, we only want to provide completions that match the characters that have been inserted.
var isIdentifierToken = token.IsKind(SyntaxKind.IdentifierName) || token.IsKind(SyntaxKind.IdentifierToken);
return SymbolEqualityComparer.Default.Equals(pair.Key.ThisType, symbolType) &&
(!isIdentifierToken || pair.Key.ExtensionMethod.Contains(token.ValueText));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.AspNetCore.Analyzers;

internal record struct PackageSourceAndNamespace(string packageName, string namespaceName);
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.CodeAnalysis;

namespace Microsoft.AspNetCore.Analyzers;

internal readonly struct ThisAndExtensionMethod(ITypeSymbol thisType, string extensionMethod)
{
public ITypeSymbol ThisType { get; } = thisType;
public string ExtensionMethod { get; } = extensionMethod;

public override bool Equals(object obj)
{
return obj is ThisAndExtensionMethod other &&
SymbolEqualityComparer.Default.Equals(ThisType, other.ThisType) &&
ExtensionMethod == other.ExtensionMethod;
}

public override int GetHashCode()
{
return HashCode.Combine(ThisType, ExtensionMethod);
captainsafia marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,12 @@
<ProjectReference Include="..\Analyzers\Microsoft.AspNetCore.App.Analyzers.csproj" />
</ItemGroup>

<ItemGroup>
<InternalsVisibleTo Include="Microsoft.AspNetCore.App.Analyzers.Test" />
</ItemGroup>

<ItemGroup>
<Compile Include="$(RepoRoot)\src\Shared\HashCode.cs" />
</ItemGroup>

</Project>
Loading
Loading