From 411d738c541156e27bd15cf933b1a4556a0175e9 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Wed, 29 May 2024 12:31:04 -0700 Subject: [PATCH 01/10] Add analyzer to install OpenAPI package from extension methods --- .../CodeFixes/Dependencies/AddPackageFixer.cs | 101 ++++++++++++++++++ .../Dependencies/PackageSourceAndNamespace.cs | 6 ++ .../Dependencies/ThisAndExtensionMethod.cs | 25 +++++ .../Microsoft.AspNetCore.App.CodeFixes.csproj | 4 + .../test/Dependencies/AddPackageTests.cs | 65 +++++++++++ src/Shared/RoslynUtils/WellKnownTypeData.cs | 6 +- 6 files changed, 205 insertions(+), 2 deletions(-) create mode 100644 src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/PackageSourceAndNamespace.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs new file mode 100644 index 000000000000..c6bdfc1c2997 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs @@ -0,0 +1,101 @@ +// 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.Collections.Immutable; +using System.Composition; +using System.Threading.Tasks; +using Microsoft.AspNetCore.App.Analyzers.Infrastructure; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.ExternalAccess.AspNetCore.AddPackage; + +namespace Microsoft.AspNetCore.Analyzers.Dependencies; + +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AddPackageFixer)), Shared] +public sealed class AddPackageFixer : CodeFixProvider +{ + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); + if (root == null) + { + return; + } + + if (semanticModel == null) + { + return; + } + + var wellKnownTypes = WellKnownTypes.GetOrCreate(semanticModel.Compilation); + + Dictionary _wellKnownExtensionMethodCache = 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") + } + }; + + foreach (var diagnostic in context.Diagnostics) + { + 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; + } + 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 AspNetCoreAddPackageCodeAction.TryCreateCodeActionAsync( + context.Document, + position, + packageInstallData, + context.CancellationToken); + + if (codeAction != null) + { + context.RegisterCodeFix(codeAction, diagnostic); + } + } + + } + } + + public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public override ImmutableArray FixableDiagnosticIds { get; } = ["CS1061"]; +} diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/PackageSourceAndNamespace.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/PackageSourceAndNamespace.cs new file mode 100644 index 000000000000..032abaf96ab5 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/PackageSourceAndNamespace.cs @@ -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); diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs new file mode 100644 index 000000000000..9604088ea3a7 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +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) + { + if (obj is ThisAndExtensionMethod other) + { + return SymbolEqualityComparer.Default.Equals(ThisType, other.ThisType) && + ExtensionMethod.Equals(other.ExtensionMethod, StringComparison.OrdinalIgnoreCase); + } + return false; + } + + public override int GetHashCode() => HashCode.Combine(ThisType, ExtensionMethod); +} diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj index 0275894e5719..3c05565850b7 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj @@ -22,4 +22,8 @@ + + + + diff --git a/src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs new file mode 100644 index 000000000000..c11d2701b78d --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs @@ -0,0 +1,65 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using VerifyCS = Microsoft.AspNetCore.Analyzers.Verifiers.CSharpCodeFixVerifier< + Microsoft.AspNetCore.Analyzers.WebApplicationBuilder.WebApplicationBuilderAnalyzer, + Microsoft.AspNetCore.Analyzers.Dependencies.AddPackageFixer>; + +namespace Microsoft.AspNetCore.Analyzers.Dependencies; +public class AddPackagesTest +{ + [Fact] + public async Task CanFixMissingExtensionMethodForDI() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; + +var builder = WebApplication.CreateBuilder(args); + +builder.Services.{|CS1061:AddOpenApi|}(); +"; + var fixedSource = @" +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +var builder = WebApplication.CreateBuilder(args); + +builder.Services.AddOpenApi(); +"; + + // Assert + await VerifyCS.VerifyCodeFixAsync(source, fixedSource); + } + + [Fact] + public async Task CanFixMissingExtensionMethodForBuilder() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; + +var app = WebApplication.Create(); + +app.{|CS1061:MapOpenApi|}(); +"; + var fixedSource = @" +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; + +var app = WebApplication.Create(args); + +app.MapOpenApi(); +"; + + // Assert + await VerifyCS.VerifyCodeFixAsync(source, fixedSource); + } +} diff --git a/src/Shared/RoslynUtils/WellKnownTypeData.cs b/src/Shared/RoslynUtils/WellKnownTypeData.cs index 6cd79a6c388c..10e62baef94e 100644 --- a/src/Shared/RoslynUtils/WellKnownTypeData.cs +++ b/src/Shared/RoslynUtils/WellKnownTypeData.cs @@ -112,7 +112,8 @@ public enum WellKnownType Microsoft_AspNetCore_Authorization_AuthorizeAttribute, Microsoft_Extensions_DependencyInjection_PolicyServiceCollectionExtensions, Microsoft_Extensions_DependencyInjection_FromKeyedServicesAttribute, - Microsoft_AspNetCore_Authorization_AuthorizationOptions + Microsoft_AspNetCore_Authorization_AuthorizationOptions, + Microsoft_Extensions_DependencyInjection_IServiceCollection } public static string[] WellKnownTypeNames = new[] @@ -222,6 +223,7 @@ public enum WellKnownType "Microsoft.AspNetCore.Authorization.AuthorizeAttribute", "Microsoft.Extensions.DependencyInjection.PolicyServiceCollectionExtensions", "Microsoft.Extensions.DependencyInjection.FromKeyedServicesAttribute", - "Microsoft.AspNetCore.Authorization.AuthorizationOptions" + "Microsoft.AspNetCore.Authorization.AuthorizationOptions", + "Microsoft.Extensions.DependencyInjection.IServiceCollection", }; } From ab4bbe6cd70da09101252e8842b648db5ed26976 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Wed, 29 May 2024 16:45:45 -0700 Subject: [PATCH 02/10] Use Ordinal comparison for extension method names --- .../CodeFixes/Dependencies/AddPackageFixer.cs | 8 ++++- .../Dependencies/ThisAndExtensionMethod.cs | 2 +- .../test/Dependencies/AddPackageTests.cs | 29 +++++-------------- 3 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs index c6bdfc1c2997..908511cc88eb 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs @@ -57,7 +57,13 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) { return; } - var symbol = semanticModel.GetSymbolInfo(((MemberAccessExpressionSyntax)node.Parent!).Expression).Symbol; + + if (node.Parent is not MemberAccessExpressionSyntax) + { + return; + } + + var symbol = semanticModel.GetSymbolInfo(((MemberAccessExpressionSyntax)node.Parent).Expression).Symbol; var symbolType = symbol switch { IMethodSymbol methodSymbol => methodSymbol.ReturnType, diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs index 9604088ea3a7..977539d55b07 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs @@ -16,7 +16,7 @@ public override bool Equals(object? obj) if (obj is ThisAndExtensionMethod other) { return SymbolEqualityComparer.Default.Equals(ThisType, other.ThisType) && - ExtensionMethod.Equals(other.ExtensionMethod, StringComparison.OrdinalIgnoreCase); + ExtensionMethod.Equals(other.ExtensionMethod, StringComparison.Ordinal); } return false; } diff --git a/src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs index c11d2701b78d..fea2d8b03627 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs @@ -6,6 +6,12 @@ Microsoft.AspNetCore.Analyzers.Dependencies.AddPackageFixer>; namespace Microsoft.AspNetCore.Analyzers.Dependencies; + +/// +/// These tests don't assert the fix is applied, since it takes a dependency on the internal +/// VS-specific `PackageInstallerService`. However, the fixer is invoked in these codepaths +/// so we can validate that the symbol resolution and checks function correctly. +/// public class AddPackagesTest { [Fact] @@ -20,20 +26,10 @@ public async Task CanFixMissingExtensionMethodForDI() var builder = WebApplication.CreateBuilder(args); builder.Services.{|CS1061:AddOpenApi|}(); -"; - var fixedSource = @" -using Microsoft.AspNetCore.Builder; -using Microsoft.Extensions.Hosting; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; - -var builder = WebApplication.CreateBuilder(args); - -builder.Services.AddOpenApi(); "; // Assert - await VerifyCS.VerifyCodeFixAsync(source, fixedSource); + await VerifyCS.VerifyCodeFixAsync(source, source); } [Fact] @@ -48,18 +44,9 @@ public async Task CanFixMissingExtensionMethodForBuilder() var app = WebApplication.Create(); app.{|CS1061:MapOpenApi|}(); -"; - var fixedSource = @" -using Microsoft.AspNetCore.Builder; -using Microsoft.Extensions.Hosting; -using Microsoft.Extensions.Logging; - -var app = WebApplication.Create(args); - -app.MapOpenApi(); "; // Assert - await VerifyCS.VerifyCodeFixAsync(source, fixedSource); + await VerifyCS.VerifyCodeFixAsync(source, source); } } From a3531f0d72310eba762d893bb3d69437a555c92b Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Mon, 3 Jun 2024 16:41:25 -0700 Subject: [PATCH 03/10] Use records struct for comparison --- .../Dependencies/ThisAndExtensionMethod.cs | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs index 977539d55b07..a583aa8dd9c3 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs @@ -6,20 +6,4 @@ 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) - { - if (obj is ThisAndExtensionMethod other) - { - return SymbolEqualityComparer.Default.Equals(ThisType, other.ThisType) && - ExtensionMethod.Equals(other.ExtensionMethod, StringComparison.Ordinal); - } - return false; - } - - public override int GetHashCode() => HashCode.Combine(ThisType, ExtensionMethod); -} +internal record struct ThisAndExtensionMethod(ITypeSymbol thisType, string extensionMethod); From 1a8c2866d3cd1794cc8316ff0979eb5135e13e5a Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Mon, 3 Jun 2024 17:27:14 -0700 Subject: [PATCH 04/10] Remove unneeded source include --- .../src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj index 3c05565850b7..0275894e5719 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj @@ -22,8 +22,4 @@ - - - - From f92547b655af00a24b780e6cb53bc2b70e6a42e5 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Mon, 3 Jun 2024 17:34:41 -0700 Subject: [PATCH 05/10] Remove unrequired using --- .../src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs index a583aa8dd9c3..869537197010 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using Microsoft.CodeAnalysis; namespace Microsoft.AspNetCore.Analyzers; From 63e2ba090a60a1f4edabd471321e99695438b243 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Wed, 5 Jun 2024 16:56:44 -0700 Subject: [PATCH 06/10] Check that code action was invoked in tests --- .../CodeFixes/Dependencies/AddPackageFixer.cs | 21 ++++++++++++++-- .../Microsoft.AspNetCore.App.CodeFixes.csproj | 4 +++ .../test/Dependencies/AddPackageTests.cs | 25 ++++++++++++++++++- ...osoft.AspNetCore.App.Analyzers.Test.csproj | 1 + 4 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs index 908511cc88eb..eba982a14f16 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs @@ -4,9 +4,11 @@ using System.Collections.Generic; 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; @@ -14,7 +16,7 @@ namespace Microsoft.AspNetCore.Analyzers.Dependencies; [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AddPackageFixer)), Shared] -public sealed class AddPackageFixer : CodeFixProvider +public class AddPackageFixer : CodeFixProvider { public override async Task RegisterCodeFixesAsync(CodeFixContext context) { @@ -86,7 +88,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) packageName: packageSourceAndNamespace.packageName, packageVersionOpt: null, packageNamespaceName: packageSourceAndNamespace.namespaceName); - var codeAction = await AspNetCoreAddPackageCodeAction.TryCreateCodeActionAsync( + var codeAction = await TryCreateCodeActionAsync( context.Document, position, packageInstallData, @@ -104,4 +106,19 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; public override ImmutableArray FixableDiagnosticIds { get; } = ["CS1061"]; + + internal virtual async Task TryCreateCodeActionAsync( + Document document, + int position, + AspNetCoreInstallPackageData packageInstallData, + CancellationToken cancellationToken) + { + var codeAction = await AspNetCoreAddPackageCodeAction.TryCreateCodeActionAsync( + document, + position, + packageInstallData, + cancellationToken); + + return codeAction; + } } diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj index 0275894e5719..9cb16fc86bd7 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj @@ -22,4 +22,8 @@ + + + + diff --git a/src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs index fea2d8b03627..59ebcd3d2cc2 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs @@ -1,9 +1,13 @@ // 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; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.ExternalAccess.AspNetCore.AddPackage; +using Moq; using VerifyCS = Microsoft.AspNetCore.Analyzers.Verifiers.CSharpCodeFixVerifier< Microsoft.AspNetCore.Analyzers.WebApplicationBuilder.WebApplicationBuilderAnalyzer, - Microsoft.AspNetCore.Analyzers.Dependencies.AddPackageFixer>; + Microsoft.AspNetCore.Analyzers.Dependencies.AddPackagesTest.MockAddPackageFixer>; namespace Microsoft.AspNetCore.Analyzers.Dependencies; @@ -29,7 +33,9 @@ public async Task CanFixMissingExtensionMethodForDI() "; // Assert + MockAddPackageFixer.Invoked = false; await VerifyCS.VerifyCodeFixAsync(source, source); + Assert.True(MockAddPackageFixer.Invoked); } [Fact] @@ -47,6 +53,23 @@ public async Task CanFixMissingExtensionMethodForBuilder() "; // Assert + MockAddPackageFixer.Invoked = false; await VerifyCS.VerifyCodeFixAsync(source, source); + Assert.True(MockAddPackageFixer.Invoked); + } + + public class MockAddPackageFixer : AddPackageFixer + { + internal static bool Invoked { get; set; } + internal override Task TryCreateCodeActionAsync( + Document document, + int position, + AspNetCoreInstallPackageData packageInstallData, + CancellationToken cancellationToken) + { + Invoked = true; + Assert.Equal("Microsoft.AspNetCore.OpenApi", packageInstallData.PackageName); + return Task.FromResult(null); + } } } diff --git a/src/Framework/AspNetCoreAnalyzers/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj b/src/Framework/AspNetCoreAnalyzers/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj index 8070e80c0b51..43e22ae5afec 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj +++ b/src/Framework/AspNetCoreAnalyzers/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj @@ -22,6 +22,7 @@ + From 87be40461b294070686e28b7fb188dd5fc2c4fd7 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Thu, 6 Jun 2024 22:32:21 -0700 Subject: [PATCH 07/10] More feedback --- .../CodeFixes/Dependencies/AddPackageFixer.cs | 2 +- .../Dependencies/ThisAndExtensionMethod.cs | 18 +++++++++++++++++- .../Microsoft.AspNetCore.App.CodeFixes.csproj | 4 ++++ .../test/Dependencies/AddPackageTests.cs | 8 ++++++++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs index eba982a14f16..3c713a8cf471 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs @@ -21,12 +21,12 @@ public class AddPackageFixer : CodeFixProvider public override async Task RegisterCodeFixesAsync(CodeFixContext context) { var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); - var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); if (root == null) { return; } + var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); if (semanticModel == null) { return; diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs index 869537197010..a537e3f4ec61 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs @@ -5,4 +5,20 @@ namespace Microsoft.AspNetCore.Analyzers; -internal record struct ThisAndExtensionMethod(ITypeSymbol thisType, string extensionMethod); +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); + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj index 9cb16fc86bd7..dcf278570a22 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Microsoft.AspNetCore.App.CodeFixes.csproj @@ -26,4 +26,8 @@ + + + + diff --git a/src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs index 59ebcd3d2cc2..81dc77c7b8dc 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/Dependencies/AddPackageTests.cs @@ -60,7 +60,15 @@ public async Task CanFixMissingExtensionMethodForBuilder() public class MockAddPackageFixer : AddPackageFixer { + /// + /// This static property allows us to verify that the fixer was + /// able to successfully resolve the symbol and call into the + /// package install APIs. This is a workaround for the fact that + /// the package install APIs are not readily mockable. Note: this + /// is not intended for use across test classes. + /// internal static bool Invoked { get; set; } + internal override Task TryCreateCodeActionAsync( Document document, int position, From a4797f87a1b86e31f6d6e39fe05a9f0b238158d1 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Mon, 10 Jun 2024 17:04:16 -0700 Subject: [PATCH 08/10] Add completion provider for extension methods in OpenAPI packages --- .../src/DiagnosticProject.cs | 13 +- .../CodeFixes/Dependencies/AddPackageFixer.cs | 15 +- .../Dependencies/ExtensionMethodsCache.cs | 24 +++ .../ExtensionMethodsCompletionProvider.cs | 104 +++++++++++ ...ExtensionMethodsCompletionProviderTests.cs | 162 ++++++++++++++++++ ...osoft.AspNetCore.App.Analyzers.Test.csproj | 1 - .../test/TestDiagnosticAnalyzer.cs | 2 +- 7 files changed, 301 insertions(+), 20 deletions(-) create mode 100644 src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ExtensionMethodsCache.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ExtensionMethodsCompletionProvider.cs create mode 100644 src/Framework/AspNetCoreAnalyzers/test/Dependencies/ExtensionMethodsCompletionProviderTests.cs diff --git a/src/Analyzers/Microsoft.AspNetCore.Analyzer.Testing/src/DiagnosticProject.cs b/src/Analyzers/Microsoft.AspNetCore.Analyzer.Testing/src/DiagnosticProject.cs index 381e38557ccc..ebafe1002dba 100644 --- a/src/Analyzers/Microsoft.AspNetCore.Analyzer.Testing/src/DiagnosticProject.cs +++ b/src/Analyzers/Microsoft.AspNetCore.Analyzer.Testing/src/DiagnosticProject.cs @@ -28,7 +28,7 @@ public class DiagnosticProject private static readonly ICompilationAssemblyResolver _assemblyResolver = new AppBaseCompilationAssemblyResolver(); private static readonly Dictionary _solutionCache = new Dictionary(); - public static Project Create(Assembly testAssembly, string[] sources, Func workspaceFactory = null, Type analyzerReference = null) + public static Project Create(Assembly testAssembly, string[] sources, Func workspaceFactory = null, Type[] analyzerReferences = null) { Solution solution; lock (_solutionCache) @@ -50,11 +50,14 @@ public static Project Create(Assembly testAssembly, string[] sources, Func _wellKnownExtensionMethodCache = 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") - } - }; + var wellKnownExtensionMethodCache = ExtensionMethodsClass.ConstructFromWellKnownTypes(wellKnownTypes); foreach (var diagnostic in context.Diagnostics) { @@ -80,7 +69,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) } var targetThisAndExtensionMethod = new ThisAndExtensionMethod(symbolType, methodName); - if (_wellKnownExtensionMethodCache.TryGetValue(targetThisAndExtensionMethod, out var packageSourceAndNamespace)) + if (wellKnownExtensionMethodCache.TryGetValue(targetThisAndExtensionMethod, out var packageSourceAndNamespace)) { var position = diagnostic.Location.SourceSpan.Start; var packageInstallData = new AspNetCoreInstallPackageData( diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ExtensionMethodsCache.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ExtensionMethodsCache.cs new file mode 100644 index 000000000000..e50fc22c119b --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ExtensionMethodsCache.cs @@ -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 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") + } + }; + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ExtensionMethodsCompletionProvider.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ExtensionMethodsCompletionProvider.cs new file mode 100644 index 000000000000..6d6bed786fb3 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ExtensionMethodsCompletionProvider.cs @@ -0,0 +1,104 @@ +// 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; + +[ExportCompletionProvider(nameof(ExtensionMethodsCompletionProvider), LanguageNames.CSharp)] +[Shared] +public sealed class ExtensionMethodsCompletionProvider : CompletionProvider +{ + 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 = ExtensionMethodsClass.ConstructFromWellKnownTypes(wellKnownTypes); + 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 pair, + ISymbol? symbolType, + SyntaxToken token) + { + if (symbolType is null) + { + return false; + } + + 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)); + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/test/Dependencies/ExtensionMethodsCompletionProviderTests.cs b/src/Framework/AspNetCoreAnalyzers/test/Dependencies/ExtensionMethodsCompletionProviderTests.cs new file mode 100644 index 000000000000..9ea06af21652 --- /dev/null +++ b/src/Framework/AspNetCoreAnalyzers/test/Dependencies/ExtensionMethodsCompletionProviderTests.cs @@ -0,0 +1,162 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Analyzers.RouteEmbeddedLanguage.Infrastructure; +using Microsoft.AspNetCore.Analyzers.WebApplicationBuilder; +using Microsoft.CodeAnalysis.Completion; + +namespace Microsoft.AspNetCore.Analyzers.Dependencies; + +public partial class ExtensionMethodsCompletionProviderTests +{ + private TestDiagnosticAnalyzerRunner Runner { get; } = new(new WebApplicationBuilderAnalyzer()); + + public static object[][] CompletionTriggers => + [ + [CompletionTrigger.Invoke], + [null] + ]; + + [Theory] + [MemberData(nameof(CompletionTriggers))] + public async Task ProvidesAddOpenApiCompletion(CompletionTrigger trigger) + { + // Arrange & Act + var result = await GetCompletionsAndServiceAsync(@" +using Microsoft.AspNetCore.Builder; + +class Program +{ + static void Main() + { + var builder = WebApplication.CreateBuilder(); + builder.Services.$$ + } +} +", trigger); + + // Assert + Assert.True(result.ShouldTriggerCompletion); + Assert.Contains(result.Completions.ItemsList, item => item.DisplayText == "AddOpenApi"); + } + + [Theory] + [MemberData(nameof(CompletionTriggers))] + public async Task ProvidesAddOpenApiCompletionWithPartialToken(CompletionTrigger trigger) + { + // Arrange & Act + var result = await GetCompletionsAndServiceAsync(@" +using Microsoft.AspNetCore.Builder; + +class Program +{ + static void Main() + { + var builder = WebApplication.CreateBuilder(); + builder.Services.[|Ad$$|] + } +} +", trigger); + + // Assert + Assert.True(result.ShouldTriggerCompletion); + Assert.Contains(result.Completions.ItemsList, item => item.DisplayText == "AddOpenApi"); + } + + [Theory] + [MemberData(nameof(CompletionTriggers))] + public async Task DoesNotProvideCompletionIfNoStringMatchForServices(CompletionTrigger trigger) + { + // Arrange & Act + var result = await GetCompletionsAndServiceAsync(@" +using Microsoft.AspNetCore.Builder; + +class Program +{ + static void Main() + { + var builder = WebApplication.CreateBuilder(); + builder.Services.[|Confi$$|] + } +} +", trigger); + + // Assert + Assert.True(result.ShouldTriggerCompletion); + Assert.DoesNotContain(result.Completions.ItemsList, item => item.DisplayText == "AddOpenApi"); + } + + [Theory] + [MemberData(nameof(CompletionTriggers))] + public async Task ProvidesMapOpenApiCompletion(CompletionTrigger trigger) + { + // Arrange & Act + var result = await GetCompletionsAndServiceAsync(@" +using Microsoft.AspNetCore.Builder; + +class Program +{ + static void Main() + { + var app = WebApplication.Create(); + app.$$ + } +} +", trigger); + + // Assert + Assert.True(result.ShouldTriggerCompletion); + Assert.Contains(result.Completions.ItemsList, item => item.DisplayText == "MapOpenApi"); + } + + [Theory] + [MemberData(nameof(CompletionTriggers))] + public async Task ProvidesMapOpenApiCompletionWithPartialToken(CompletionTrigger trigger) + { + // Arrange & Act + var result = await GetCompletionsAndServiceAsync(@" +using Microsoft.AspNetCore.Builder; + +class Program +{ + static void Main() + { + var app = WebApplication.Create(); + app.[|Ma$$|] + } +} +", trigger); + + // Assert + Assert.True(result.ShouldTriggerCompletion); + Assert.Contains(result.Completions.ItemsList, item => item.DisplayText == "MapOpenApi"); + } + + [Theory] + [MemberData(nameof(CompletionTriggers))] + public async Task DoesNotProvideCompletionIfNoStringMatchForWebApplication(CompletionTrigger trigger) + { + // Arrange & Act + var result = await GetCompletionsAndServiceAsync(@" +using Microsoft.AspNetCore.Builder; + +class Program +{ + static void Main() + { + var app = WebApplication.Create(); + app.[|Use$$|] + } +} +", trigger); + + // Assert + Assert.True(result.ShouldTriggerCompletion); + Assert.DoesNotContain(result.Completions.ItemsList, item => item.DisplayText == "MapOpenApi"); + } + + private Task GetCompletionsAndServiceAsync(string source, CompletionTrigger? completionTrigger = null) + { + return CompletionTestHelpers.GetCompletionsAndServiceAsync(Runner, source, completionTrigger); + } +} diff --git a/src/Framework/AspNetCoreAnalyzers/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj b/src/Framework/AspNetCoreAnalyzers/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj index 43e22ae5afec..8070e80c0b51 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj +++ b/src/Framework/AspNetCoreAnalyzers/test/Microsoft.AspNetCore.App.Analyzers.Test.csproj @@ -22,7 +22,6 @@ - diff --git a/src/Framework/AspNetCoreAnalyzers/test/TestDiagnosticAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/test/TestDiagnosticAnalyzer.cs index 44a15625ef9d..8300b2232690 100644 --- a/src/Framework/AspNetCoreAnalyzers/test/TestDiagnosticAnalyzer.cs +++ b/src/Framework/AspNetCoreAnalyzers/test/TestDiagnosticAnalyzer.cs @@ -151,7 +151,7 @@ public static Project CreateProjectWithReferencesInBinDir(Assembly testAssembly, Func createWorkspace = CreateWorkspace; - var project = DiagnosticProject.Create(testAssembly, source, createWorkspace, typeof(RoutePatternClassifier)); + var project = DiagnosticProject.Create(testAssembly, source, createWorkspace, [typeof(RoutePatternClassifier), typeof(ExtensionMethodsCompletionProvider)]); foreach (var assembly in Directory.EnumerateFiles(AppContext.BaseDirectory, "*.dll")) { if (!project.MetadataReferences.Any(c => string.Equals(Path.GetFileNameWithoutExtension(c.Display), Path.GetFileNameWithoutExtension(assembly), StringComparison.OrdinalIgnoreCase))) From b1a4eccff3841245accca802bcd3567c1119f273 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Tue, 11 Jun 2024 13:00:23 -0700 Subject: [PATCH 09/10] Fix up reference and add comments --- .../CodeFixes/Dependencies/AddPackageFixer.cs | 17 +++++++++++++++-- .../ExtensionMethodsCompletionProvider.cs | 18 ++++++++++++++++-- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs index d8a02ad5bc9f..29e4d5e374f0 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/AddPackageFixer.cs @@ -1,7 +1,6 @@ // 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.Collections.Immutable; using System.Composition; using System.Threading; @@ -15,6 +14,15 @@ namespace Microsoft.AspNetCore.Analyzers.Dependencies; +/// +/// 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`. +/// +/// +/// This class is not sealed to support mocking of the virtual method `TryCreateCodeActionAsync` in unit tests. +/// [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AddPackageFixer)), Shared] public class AddPackageFixer : CodeFixProvider { @@ -33,8 +41,9 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) } var wellKnownTypes = WellKnownTypes.GetOrCreate(semanticModel.Compilation); - var wellKnownExtensionMethodCache = ExtensionMethodsClass.ConstructFromWellKnownTypes(wellKnownTypes); + var wellKnownExtensionMethodCache = ExtensionMethodsCache.ConstructFromWellKnownTypes(wellKnownTypes); + // Diagnostics are already filtered by FixableDiagnosticIds values. foreach (var diagnostic in context.Diagnostics) { var location = diagnostic.Location.SourceSpan; @@ -94,6 +103,10 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + /// + /// '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?). + /// public override ImmutableArray FixableDiagnosticIds { get; } = ["CS1061"]; internal virtual async Task TryCreateCodeActionAsync( diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ExtensionMethodsCompletionProvider.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ExtensionMethodsCompletionProvider.cs index 6d6bed786fb3..d5ec910472a1 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ExtensionMethodsCompletionProvider.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ExtensionMethodsCompletionProvider.cs @@ -13,6 +13,13 @@ namespace Microsoft.AspNetCore.Analyzers.RouteEmbeddedLanguage; +/// +/// 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. +/// [ExportCompletionProvider(nameof(ExtensionMethodsCompletionProvider), LanguageNames.CSharp)] [Shared] public sealed class ExtensionMethodsCompletionProvider : CompletionProvider @@ -44,9 +51,13 @@ public override async Task ProvideCompletionsAsync(CompletionContext context) } var wellKnownTypes = WellKnownTypes.GetOrCreate(semanticModel.Compilation); - var wellKnownExtensionMethodCache = ExtensionMethodsClass.ConstructFromWellKnownTypes(wellKnownTypes); - var nearestMemberAccessExpression = FindNearestMemberAccessExpression(token.Parent); + 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); @@ -97,6 +108,9 @@ private static bool IsMatchingExtensionMethod( 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)); From e9f6fba560cee7852858d0eeeeec6efc71eb6204 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Fri, 14 Jun 2024 11:19:35 -0700 Subject: [PATCH 10/10] Fix up ThisAndExtensionMethod.GetHashCode --- .../src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs index a537e3f4ec61..4e87db00617f 100644 --- a/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs +++ b/src/Framework/AspNetCoreAnalyzers/src/CodeFixes/Dependencies/ThisAndExtensionMethod.cs @@ -19,6 +19,6 @@ public override bool Equals(object obj) public override int GetHashCode() { - return HashCode.Combine(ThisType, ExtensionMethod); + return HashCode.Combine(SymbolEqualityComparer.Default.GetHashCode(ThisType), ExtensionMethod); } }