Add BuildServiceProvider analyzer rule

doc/AG0041.md

@@ -0,0 +1,68 @@
+# AG0041: Use message templates in logging
+
+Structured logging should be employed instead of string interpolation or concatenation.
+
+## Why is this an issue?
+
+Logging is a critical aspect of any application, providing insights into its behavior and facilitating debugging.
+However, not all logging approaches are created equal. Using string interpolation or concatenation for log messages is
+a common pitfall that can lead to performance issues and hinder log analysis.
+
+Consider the following problematic code:
+
+```csharp
+logger.Information($"User {userId} logged in at {DateTime.Now}");  // Noncompliant
+logger.Information("Error occurred: " + errorMessage);  // Noncompliant
+```
+
+Instead, you should use message templates:
+
+```csharp
+logger.Information("User {UserId} logged in at {LoginTime}", userId, DateTime.Now);  // Compliant
+logger.Information("Error occurred: {ErrorMessage}", errorMessage);  // Compliant
+```
+
+## Benefits of using message templates
+
+The use of message templates in logging offers several significant advantages over traditional string interpolation or 
+concatenation methods. Firstly, message templates can provide a notable performance boost, particularly in scenarios where 
+log messages aren't actually written due to log level settings. This efficiency is crucial in high-volume logging environments 
+where every millisecond counts.
+
+Structured logging, enabled by message templates, is another key benefit. It allows variables to be captured as separate entities 
+rather than being merged into a single string. This separation enables powerful log analysis capabilities, making it easier to search, 
+filter, and aggregate log data based on specific parameters. The ability to maintain consistent log formats across an application 
+is also greatly enhanced with message templates, leading to more uniform and easily interpretable logs.
+
+Message templates also facilitate semantic logging, allowing developers to attach additional metadata to log events. This extra 
+context can be invaluable when debugging complex issues or analyzing application behavior. Furthermore, modern logging frameworks such 
+as Serilog and Microsoft.Extensions.Logging are optimized for template-style logging. By using message templates, developers can 
+take full advantage of these frameworks' features and performance optimizations, ensuring their logging strategy is future-proof 
+and aligned with industry best practices.
+
+## More Info
+
+This analyzer checks for the use of string interpolation (`$"..."`) or string concatenation in logging method calls. It specifically targets methods of `ILogger` from both `Microsoft.Extensions.Logging` and `Serilog`.
+
+### Noncompliant Code Examples
+
+```csharp
+_logger.LogInformation($"Processing order {orderId}");
+_logger.LogError("Error: " + exception.Message);
+```
+
+### Compliant Solution
+
+```csharp
+_logger.LogInformation("Processing order {OrderId}", orderId);
+_logger.LogError("Error: {ErrorMessage}", exception.Message);
+```
+
+### Exceptions
+
+This rule doesn't apply to non-logging methods or to logging methods that explicitly expect formatted strings.
+
+## Resources
+
+* [Structured logging with ILogger in .NET](https://docs.microsoft.com/en-us/aspnet/core/fundamentals/logging/?view=aspnetcore-5.0#log-message-template)
+* [Serilog message templates](https://github.com/serilog/serilog/wiki/Writing-Log-Events#message-templates)

doc/AG0043.md

@@ -0,0 +1,121 @@
+# AG0043: BuildServiceProvider should not be used in production code
+
+## Problem Description
+Using `BuildServiceProvider()` in production code can lead to memory leaks and other issues because it creates a new container. This is especially problematic when called repeatedly, such as in request processing scenarios.
+
+## Rule Details
+This rule raises an issue when `BuildServiceProvider()` is called on `IServiceCollection` instances.
+
+### Noncompliant Code Examples
+
+#### Traditional ASP.NET Core
+```csharp
+public void ConfigureServices(IServiceCollection services)
+{
+    services.AddTransient<IMyService, MyService>();
+    var serviceProvider = services.BuildServiceProvider(); // Noncompliant
+    var myService = serviceProvider.GetService<IMyService>();
+}
+```
+
+#### Minimal API
+```csharp
+var builder = WebApplication.CreateBuilder(args);
+
+builder.Services.AddTransient<IMyService, MyService>();
+
+var app = builder.Build();
+
+app.MapGet("/", () => 
+{
+    var serviceProvider = app.Services.BuildServiceProvider(); // Noncompliant
+    var myService = serviceProvider.GetService<IMyService>();
+    return myService.GetMessage();
+});
+
+app.Run();
+```
+
+### Compliant Solutions
+
+#### Traditional ASP.NET Core
+```csharp
+public class Startup
+{
+    public void ConfigureServices(IServiceCollection services)
+    {
+        services.AddTransient<IMyService, MyService>();
+        // Let ASP.NET Core build the service provider
+    }
+    public void Configure(IApplicationBuilder app, IMyService myService)
+    {
+        // Services are injected by the framework
+    }
+}
+```
+
+#### Minimal API
+```csharp
+var builder = WebApplication.CreateBuilder(args);
+
+builder.Services.AddTransient<IMyService, MyService>();
+
+var app = builder.Build();
+
+app.MapGet("/", (IMyService myService) => myService.GetMessage());
+
+app.Run();
+
+// Service interfaces and implementations
+public interface IMyService
+{
+    string GetMessage();
+}
+
+public class MyService : IMyService
+{
+    public string GetMessage() => "Hello from MyService!";
+}
+```
+
+## Why is this an Issue?
+1. **Memory Leaks**: Each call to `BuildServiceProvider()` creates a new dependency injection container, which holds references to all registered services. If called repeatedly (e.g., during request processing), this leads to memory leaks.
+2. **Performance Impact**: Creating new service providers is computationally expensive and can impact application performance.
+3. **Singleton Duplication**: Multiple service providers can result in multiple instances of services that should be singletons.
+
+## Exceptions
+`BuildServiceProvider()` may be acceptable in the following scenarios:
+- Unit tests where a full DI container is needed
+- Development-time configuration validation
+- Tools and utilities that run outside the normal application lifecycle
+
+## How to Fix It
+1. In ASP.NET Core applications:
+    - Let the framework handle service provider creation
+    - Use constructor injection to obtain dependencies
+    - Use `IServiceScope` for creating scoped service providers when needed
+    - `HttpContext` and other services have methods like `RequestServices.GetService<T>()` to get scoped services
+2. For configuration validation:
+   ```csharp
+   public void ConfigureServices(IServiceCollection services)
+   {
+       services.AddTransient<IMyService, MyService>();
+       // Only in development
+       if (Environment.IsDevelopment())
+       {
+           // Validate service registration
+           var serviceProvider = services.BuildServiceProvider(validateScopes: true);
+           serviceProvider.Dispose();
+       }
+   }
+   ```
+
+## Benefits
+- Prevents memory leaks
+- Improves application performance
+- Ensures proper singleton behavior
+- Maintains proper service scoping
+
+## References
+- [ASP.NET Core Dependency Injection Best Practices](https://docs.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection#service-lifetimes)
+- [Dependency injection in ASP.NET Core](https://docs.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection)

src/Agoda.Analyzers.Test/AgodaCustom/AG0043UnitTests.cs

@@ -0,0 +1,96 @@
+using System.Threading.Tasks;
+using Agoda.Analyzers.AgodaCustom;
+using Agoda.Analyzers.Test.Helpers;
+using Microsoft.CodeAnalysis.Diagnostics;
+using NUnit.Framework;
+using Microsoft.Extensions.DependencyInjection;
+
+namespace Agoda.Analyzers.Test.AgodaCustom;
+
+[TestFixture]
+class AG0043UnitTests : DiagnosticVerifier
+{
+    protected override DiagnosticAnalyzer DiagnosticAnalyzer => new AG0043NoBuildServiceProvider();
+
+    protected override string DiagnosticId => AG0043NoBuildServiceProvider.DIAGNOSTIC_ID;
+
+    [Test]
+    public async Task BuildServiceProvider_Used_RaisesDiagnostic()
+    {
+        var services = new ServiceCollection();
+        services.BuildServiceProvider();
+
+        var code = new CodeDescriptor
+        {
+            References = new[]
+            {
+                typeof(ServiceCollection).Assembly, 
+                typeof(ServiceProvider).Assembly, 
+                typeof(IServiceCollection).Assembly
+            },
+            Code = @"
+                    using Microsoft.Extensions.DependencyInjection;
+
+                    public class Program
+                    {
+                        public static void Main()
+                        {
+                            var services = new ServiceCollection();
+                            var provider = services.BuildServiceProvider();
+                        }
+                    }"
+        };
+
+        await VerifyDiagnosticsAsync(code, new DiagnosticLocation(9, 53));
+    }
+
+    [Test]
+    public async Task OtherMethod_NoDiagnostic()
+    {
+        var code = new CodeDescriptor
+        {
+            References = new[] {typeof(ServiceCollection).Assembly},
+            Code = @"
+                    using Microsoft.Extensions.DependencyInjection;
+
+                    class TestClass
+                    {
+                        public void ConfigureServices()
+                        {
+                            var services = new ServiceCollection();
+                            services.AddSingleton<object>();
+                        }
+                    }"
+        };
+
+        await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults);
+    }
+
+    [Test]
+    public async Task BuildServiceProvider_WhenChaining_RaiseDiagnostic()
+    {
+        var code = new CodeDescriptor
+        {
+            References = new[]
+            {
+                typeof(ServiceCollection).Assembly, 
+                typeof(ServiceProvider).Assembly, 
+                typeof(IServiceCollection).Assembly
+            },
+            Code = @"
+                    using Microsoft.Extensions.DependencyInjection;
+
+                    public class Program
+                    {
+                        public static void Main()
+                        {
+                            var provider = new ServiceCollection()
+                                .AddSingleton<object>()
+                                .BuildServiceProvider();
+                        }
+                    }"
+        };
+
+        await VerifyDiagnosticsAsync(code, new DiagnosticLocation(10, 34));
+    }
+}

src/Agoda.Analyzers/AgodaCustom/AG0041LogTemplateAnalyzer.cs

@@ -16,13 +16,13 @@
         private static readonly LocalizableString MessageFormat = new LocalizableResourceString(nameof(CustomRulesResources.AG0041Title), CustomRulesResources.ResourceManager, typeof(CustomRulesResources));
         private const string Category = "Best Practices";
 
         internal static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(DiagnosticId, 
             Title, 
             MessageFormat,
             Category, 
             DiagnosticSeverity.Warning, 
             isEnabledByDefault: true, 
-            helpLinkUri: "https://github.com/agoda-com/AgodaAnalyzers/issues/183");
+            helpLinkUri: "https://github.com/agoda-com/AgodaAnalyzers/blob/master/doc/AG0041.md");
 
         public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
 
@@ -87,4 +87,4 @@
                              bes.Right.IsKind(SyntaxKind.StringLiteralExpression)));
         }
     }
-}
+}