From f4f4835635a812cfe24b724b8a1c4aab6cee8705 Mon Sep 17 00:00:00 2001
From: pedro-camargo-MSFT
<106186580+pedro-camargo-MSFT@users.noreply.github.com>
Date: Tue, 28 Jun 2022 16:23:16 -0700
Subject: [PATCH] Implement analyzer to favor using builder.Logging over
ConfigureLogging (#42354)
* Add analyzer issue #35816
* Deleting uneccessary using statement and comment
* Make DiagnosticDescriptor title and description consistent with other DiagnosticDescriptors
* Update src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs
Co-authored-by: Safia Abdalla <safia@safia.rocks>
Co-authored-by: Safia Abdalla <safia@safia.rocks>
---
.../src/Analyzers/DiagnosticDescriptors.cs | 9 +
.../WebApplicationBuilderAnalyzer.cs | 40 ++-
.../WebApplicationBuilder/WellKnownTypes.cs | 8 +
.../DisallowConfigureHostLoggingTest.cs | 278 ++++++++++++++++++
4 files changed, 334 insertions(+), 1 deletion(-)
create mode 100644 src/Framework/AspNetCoreAnalyzers/test/WebApplicationBuilder/DisallowConfigureHostLoggingTest.cs
diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs
index c01db137dea..a2d5903de4e 100644
--- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs
+++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs
@@ -79,4 +79,13 @@ internal static class DiagnosticDescriptors
DiagnosticSeverity.Error,
isEnabledByDefault: true,
helpLinkUri: "https://aka.ms/aspnet/analyzers");
+
+ internal static readonly DiagnosticDescriptor DoNotUseHostConfigureLogging = new(
+ "ASP0011",
+ "Suggest using builder.Logging over Host.ConfigureLogging or WebHost.ConfigureLogging",
+ "Suggest using builder.Logging instead of {0}",
+ "Usage",
+ DiagnosticSeverity.Warning,
+ isEnabledByDefault: true,
+ helpLinkUri: "https://aka.ms/aspnet/analyzers");
}
diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/WebApplicationBuilder/WebApplicationBuilderAnalyzer.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/WebApplicationBuilder/WebApplicationBuilderAnalyzer.cs
index 01cfdc829a7..0e1b4f220e4 100644
--- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/WebApplicationBuilder/WebApplicationBuilderAnalyzer.cs
+++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/WebApplicationBuilder/WebApplicationBuilderAnalyzer.cs
@@ -21,6 +21,7 @@ public class WebApplicationBuilderAnalyzer : DiagnosticAnalyzer
DiagnosticDescriptors.DoNotUseConfigureWebHostWithConfigureHostBuilder,
DiagnosticDescriptors.DoNotUseConfigureWithConfigureWebHostBuilder,
DiagnosticDescriptors.DoNotUseUseStartupWithConfigureWebHostBuilder,
+ DiagnosticDescriptors.DoNotUseHostConfigureLogging
});
public override void Initialize(AnalysisContext context)
@@ -44,6 +45,11 @@ public class WebApplicationBuilderAnalyzer : DiagnosticAnalyzer
wellKnownTypes.HostingAbstractionsWebHostBuilderExtensions,
wellKnownTypes.WebHostBuilderExtensions,
};
+ INamedTypeSymbol[] configureLoggingTypes =
+ {
+ wellKnownTypes.HostingHostBuilderExtensions,
+ wellKnownTypes.WebHostBuilderExtensions
+ };
compilationStartAnalysisContext.RegisterOperationAction(operationAnalysisContext =>
{
@@ -98,6 +104,38 @@ public class WebApplicationBuilderAnalyzer : DiagnosticAnalyzer
invocation));
}
+ //var builder = WebApplication.CreateBuilder(args);
+ //builder.Host.ConfigureLogging(x => {})
+ if (IsDisallowedMethod(
+ operationAnalysisContext,
+ invocation,
+ targetMethod,
+ wellKnownTypes.ConfigureHostBuilder,
+ "ConfigureLogging",
+ configureLoggingTypes))
+ {
+ operationAnalysisContext.ReportDiagnostic(
+ CreateDiagnostic(
+ DiagnosticDescriptors.DoNotUseHostConfigureLogging,
+ invocation));
+ }
+
+ //var builder = WebApplication.CreateBuilder(args);
+ //builder.WebHost.ConfigureLogging(x => {})
+ if (IsDisallowedMethod(
+ operationAnalysisContext,
+ invocation,
+ targetMethod,
+ wellKnownTypes.ConfigureWebHostBuilder,
+ "ConfigureLogging",
+ configureLoggingTypes))
+ {
+ operationAnalysisContext.ReportDiagnostic(
+ CreateDiagnostic(
+ DiagnosticDescriptors.DoNotUseHostConfigureLogging,
+ invocation));
+ }
+
static Diagnostic CreateDiagnostic(DiagnosticDescriptor descriptor, IInvocationOperation operation)
{
// Take the location for the whole invocation operation as a starting point.
@@ -147,7 +185,7 @@ public class WebApplicationBuilderAnalyzer : DiagnosticAnalyzer
location = Location.Create(operation.Syntax.SyntaxTree, targetSpan);
}
- return Diagnostic.Create(descriptor, location);
+ return Diagnostic.Create(descriptor, location, methodName);
}
}, OperationKind.Invocation);
diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/WebApplicationBuilder/WellKnownTypes.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/WebApplicationBuilder/WellKnownTypes.cs
index d4c43aa2a84..0e2f1102d4d 100644
--- a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/WebApplicationBuilder/WellKnownTypes.cs
+++ b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/WebApplicationBuilder/WellKnownTypes.cs
@@ -42,6 +42,12 @@ internal sealed class WellKnownTypes
return false;
}
+ const string HostingHostBuilderExtensions = "Microsoft.Extensions.Hosting.HostingHostBuilderExtensions";
+ if (compilation.GetTypeByMetadataName(HostingHostBuilderExtensions) is not { } hostingHostBuilderExtensions)
+ {
+ return false;
+ }
+
wellKnownTypes = new WellKnownTypes
{
ConfigureHostBuilder = configureHostBuilder,
@@ -49,6 +55,7 @@ internal sealed class WellKnownTypes
GenericHostWebHostBuilderExtensions = genericHostWebHostBuilderExtensions,
HostingAbstractionsWebHostBuilderExtensions = hostingAbstractionsWebHostBuilderExtensions,
WebHostBuilderExtensions = webHostBuilderExtensions,
+ HostingHostBuilderExtensions = hostingHostBuilderExtensions
};
return true;
@@ -59,4 +66,5 @@ internal sealed class WellKnownTypes
public INamedTypeSymbol GenericHostWebHostBuilderExtensions { get; private init; }
public INamedTypeSymbol HostingAbstractionsWebHostBuilderExtensions { get; private init; }
public INamedTypeSymbol WebHostBuilderExtensions { get; private init; }
+ public INamedTypeSymbol HostingHostBuilderExtensions { get; private init; }
}
diff --git a/src/Framework/AspNetCoreAnalyzers/test/WebApplicationBuilder/DisallowConfigureHostLoggingTest.cs b/src/Framework/AspNetCoreAnalyzers/test/WebApplicationBuilder/DisallowConfigureHostLoggingTest.cs
new file mode 100644
index 00000000000..38934ee8d16
--- /dev/null
+++ b/src/Framework/AspNetCoreAnalyzers/test/WebApplicationBuilder/DisallowConfigureHostLoggingTest.cs
@@ -0,0 +1,278 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+using System.Globalization;
+using Microsoft.AspNetCore.Analyzer.Testing;
+using Microsoft.CodeAnalysis;
+
+namespace Microsoft.AspNetCore.Analyzers.WebApplicationBuilder;
+public partial class DisallowConfigureHostLoggingTest
+{
+ private TestDiagnosticAnalyzerRunner Runner { get; } = new(new WebApplicationBuilderAnalyzer());
+
+ [Fact]
+ public async Task DoesNotWarnWhenBuilderLoggingIsUsed()
+ {
+ //arrange
+ var source = @"
+using Microsoft.AspNetCore.Builder;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+var builder = WebApplication.CreateBuilder(args);
+builder.Logging.AddJsonConsole();
+";
+ //act
+ var diagnostics = await Runner.GetDiagnosticsAsync(source);
+ //assert
+ Assert.Empty(diagnostics);
+ }
+
+ [Fact]
+ public async Task DoesNotWarnWhenBuilderLoggingIsUsed_InMain()
+ {
+ //arrange
+ var source = @"
+using Microsoft.AspNetCore.Builder;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+public static class Program
+{
+ public static void Main (string[] args)
+ {
+ var builder = WebApplication.CreateBuilder(args);
+ builder.Logging.AddJsonConsole();
+ }
+}
+public class Startup { }
+";
+ //act
+ var diagnostics = await Runner.GetDiagnosticsAsync(source);
+ //assert
+ Assert.Empty(diagnostics);
+ }
+
+ [Fact]
+ public async Task WarnsWhenBuilderLoggingIsNotUsed_Host()
+ {
+ //arrange
+ var source = TestSource.Read(@"
+using Microsoft.AspNetCore.Builder;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+var builder = WebApplication.CreateBuilder(args);
+builder.Host./*MM*/ConfigureLogging(logging =>
+{
+logging.AddJsonConsole();
+});
+");
+ //act
+ var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
+ //assert
+ var diagnostic = Assert.Single(diagnostics);
+ Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic.Descriptor);
+ AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
+ Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic.GetMessage(CultureInfo.InvariantCulture));
+ }
+
+ [Fact]
+ public async Task WarnsWhenBuilderLoggingIsNotUsed_WebHost()
+ {
+ //arrange
+ var source = TestSource.Read(@"
+using Microsoft.AspNetCore.Builder;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Microsoft.AspNetCore.Hosting;
+var builder = WebApplication.CreateBuilder(args);
+builder.WebHost./*MM*/ConfigureLogging(logging =>
+{
+logging.AddJsonConsole();
+});
+");
+ //act
+ var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
+ //assert
+ var diagnostic = Assert.Single(diagnostics);
+ Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic.Descriptor);
+ AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
+ Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic.GetMessage(CultureInfo.InvariantCulture));
+ }
+
+ [Fact]
+ public async Task WarnsWhenBuilderLoggingIsNotUsed_OnDifferentLine_Host()
+ {
+ //arrange
+ var source = TestSource.Read(@"
+using Microsoft.AspNetCore.Builder;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+var builder = WebApplication.CreateBuilder(args);
+builder.Host.
+ /*MM*/ConfigureLogging(logging =>
+{
+ logging.AddJsonConsole();
+});
+");
+ //act
+ var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
+ //assert
+ var diagnostic = Assert.Single(diagnostics);
+ Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic.Descriptor);
+ AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
+ Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic.GetMessage(CultureInfo.InvariantCulture));
+ }
+
+ [Fact]
+ public async Task WarnsWhenBuilderLoggingIsNotUsed_OnDifferentLine_WebHost()
+ {
+ //arrange
+ var source = TestSource.Read(@"
+using Microsoft.AspNetCore.Builder;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Microsoft.AspNetCore.Hosting;
+var builder = WebApplication.CreateBuilder(args);
+builder.WebHost.
+ /*MM*/ConfigureLogging(logging =>
+{
+logging.AddJsonConsole();
+});
+");
+ //act
+ var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
+ //assert
+ var diagnostic = Assert.Single(diagnostics);
+ Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic.Descriptor);
+ AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
+ Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic.GetMessage(CultureInfo.InvariantCulture));
+ }
+
+ [Fact]
+ public async Task WarnsWhenBuilderLoggingIsNotUsed_InMain_Host()
+ {
+ //arrange
+ var source = TestSource.Read(@"
+using Microsoft.AspNetCore.Builder;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+public static class Program
+{
+ public static void Main (string[] args)
+ {
+ var builder = WebApplication.CreateBuilder(args);
+ builder.Host./*MM*/ConfigureLogging(logging =>
+ {
+ logging.AddJsonConsole();
+ });
+ }
+}
+public class Startup { }
+");
+ //act
+ var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
+ //assert
+ var diagnostic = Assert.Single(diagnostics);
+ Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic.Descriptor);
+ AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
+ Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic.GetMessage(CultureInfo.InvariantCulture));
+ }
+
+ [Fact]
+ public async Task WarnsWhenBuilderLoggingIsNotUsed_InMain_WebHost()
+ {
+ //arrange
+ var source = TestSource.Read(@"
+using Microsoft.AspNetCore.Builder;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Microsoft.AspNetCore.Hosting;
+public static class Program
+{
+ public static void Main (string[] args)
+ {
+ var builder = WebApplication.CreateBuilder(args);
+ builder.WebHost./*MM*/ConfigureLogging(logging =>
+ {
+ logging.AddJsonConsole();
+ });
+ }
+}
+public class Startup { }
+");
+ //act
+ var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
+ //assert
+ var diagnostic = Assert.Single(diagnostics);
+ Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic.Descriptor);
+ AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
+ Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic.GetMessage(CultureInfo.InvariantCulture));
+ }
+
+ [Fact]
+ public async Task WarnsWhenBuilderLoggingIsNotUsed_WhenChained_WebHost()
+ {
+ //arrange
+ var source = TestSource.Read(@"
+using Microsoft.AspNetCore.Builder;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Microsoft.AspNetCore.Hosting;
+var builder = WebApplication.CreateBuilder(args);
+builder.WebHost.
+ /*MM*/ConfigureLogging(logging => { })
+ .ConfigureServices(services => { });
+");
+ //act
+ var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
+ //assert
+ var diagnostic = Assert.Single(diagnostics);
+ Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic.Descriptor);
+ AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
+ Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic.GetMessage(CultureInfo.InvariantCulture));
+ }
+
+ [Fact]
+ public async Task WarnsTwiceWhenBuilderLoggingIsNotUsed_Host()
+ {
+ //arrange
+ var source = TestSource.Read(@"
+using Microsoft.AspNetCore.Builder;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+var builder = WebApplication.CreateBuilder(args);
+builder.Host./*MM1*/ConfigureLogging(logging =>
+{
+logging.AddJsonConsole();
+});
+builder.Host./*MM2*/ConfigureLogging(logging =>
+{
+logging.AddJsonConsole();
+});
+");
+ //act
+ var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
+ //assert
+ Assert.Equal(2, diagnostics.Length);
+ var diagnostic1 = diagnostics[0];
+ var diagnostic2 = diagnostics[1];
+
+ Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic1.Descriptor);
+ AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MM1"], diagnostic1.Location);
+ Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic1.GetMessage(CultureInfo.InvariantCulture));
+
+ Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic2.Descriptor);
+ AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MM2"], diagnostic2.Location);
+ Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic2.GetMessage(CultureInfo.InvariantCulture));
+ }
+
+}
+
--
GitLab