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/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");
 }

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);

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; }
 }

src/Framework/AspNetCoreAnalyzers/test/WebApplicationBuilder/DisallowConfigureHostLoggingTest.cs

+// 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));
+    }
+
+}
+