Skip to content

Commit f4f4835

Browse files
pedro-camargo11captainsafia
pedro-camargo11
and
captainsafia
authored
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 <[email protected]> Co-authored-by: Safia Abdalla <[email protected]>
1 parent 4adb425 commit f4f4835

File tree

4 files changed

+334
-1
lines changed

4 files changed

+334
-1
lines changed

‎src/Framework/AspNetCoreAnalyzers/src/Analyzers/DiagnosticDescriptors.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,4 +79,13 @@ internal static class DiagnosticDescriptors
7979
DiagnosticSeverity.Error,
8080
isEnabledByDefault: true,
8181
helpLinkUri: "https://aka.ms/aspnet/analyzers");
82+
83+
internal static readonly DiagnosticDescriptor DoNotUseHostConfigureLogging = new(
84+
"ASP0011",
85+
"Suggest using builder.Logging over Host.ConfigureLogging or WebHost.ConfigureLogging",
86+
"Suggest using builder.Logging instead of {0}",
87+
"Usage",
88+
DiagnosticSeverity.Warning,
89+
isEnabledByDefault: true,
90+
helpLinkUri: "https://aka.ms/aspnet/analyzers");
8291
}

‎src/Framework/AspNetCoreAnalyzers/src/Analyzers/WebApplicationBuilder/WebApplicationBuilderAnalyzer.cs

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public class WebApplicationBuilderAnalyzer : DiagnosticAnalyzer
2121
DiagnosticDescriptors.DoNotUseConfigureWebHostWithConfigureHostBuilder,
2222
DiagnosticDescriptors.DoNotUseConfigureWithConfigureWebHostBuilder,
2323
DiagnosticDescriptors.DoNotUseUseStartupWithConfigureWebHostBuilder,
24+
DiagnosticDescriptors.DoNotUseHostConfigureLogging
2425
});
2526

2627
public override void Initialize(AnalysisContext context)
@@ -44,6 +45,11 @@ public override void Initialize(AnalysisContext context)
4445
wellKnownTypes.HostingAbstractionsWebHostBuilderExtensions,
4546
wellKnownTypes.WebHostBuilderExtensions,
4647
};
48+
INamedTypeSymbol[] configureLoggingTypes =
49+
{
50+
wellKnownTypes.HostingHostBuilderExtensions,
51+
wellKnownTypes.WebHostBuilderExtensions
52+
};
4753

4854
compilationStartAnalysisContext.RegisterOperationAction(operationAnalysisContext =>
4955
{
@@ -98,6 +104,38 @@ public override void Initialize(AnalysisContext context)
98104
invocation));
99105
}
100106

107+
//var builder = WebApplication.CreateBuilder(args);
108+
//builder.Host.ConfigureLogging(x => {})
109+
if (IsDisallowedMethod(
110+
operationAnalysisContext,
111+
invocation,
112+
targetMethod,
113+
wellKnownTypes.ConfigureHostBuilder,
114+
"ConfigureLogging",
115+
configureLoggingTypes))
116+
{
117+
operationAnalysisContext.ReportDiagnostic(
118+
CreateDiagnostic(
119+
DiagnosticDescriptors.DoNotUseHostConfigureLogging,
120+
invocation));
121+
}
122+
123+
//var builder = WebApplication.CreateBuilder(args);
124+
//builder.WebHost.ConfigureLogging(x => {})
125+
if (IsDisallowedMethod(
126+
operationAnalysisContext,
127+
invocation,
128+
targetMethod,
129+
wellKnownTypes.ConfigureWebHostBuilder,
130+
"ConfigureLogging",
131+
configureLoggingTypes))
132+
{
133+
operationAnalysisContext.ReportDiagnostic(
134+
CreateDiagnostic(
135+
DiagnosticDescriptors.DoNotUseHostConfigureLogging,
136+
invocation));
137+
}
138+
101139
static Diagnostic CreateDiagnostic(DiagnosticDescriptor descriptor, IInvocationOperation operation)
102140
{
103141
// Take the location for the whole invocation operation as a starting point.
@@ -147,7 +185,7 @@ static Diagnostic CreateDiagnostic(DiagnosticDescriptor descriptor, IInvocationO
147185
location = Location.Create(operation.Syntax.SyntaxTree, targetSpan);
148186
}
149187

150-
return Diagnostic.Create(descriptor, location);
188+
return Diagnostic.Create(descriptor, location, methodName);
151189
}
152190

153191
}, OperationKind.Invocation);

‎src/Framework/AspNetCoreAnalyzers/src/Analyzers/WebApplicationBuilder/WellKnownTypes.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,20 @@ public static bool TryCreate(Compilation compilation, [NotNullWhen(true)] out We
4242
return false;
4343
}
4444

45+
const string HostingHostBuilderExtensions = "Microsoft.Extensions.Hosting.HostingHostBuilderExtensions";
46+
if (compilation.GetTypeByMetadataName(HostingHostBuilderExtensions) is not { } hostingHostBuilderExtensions)
47+
{
48+
return false;
49+
}
50+
4551
wellKnownTypes = new WellKnownTypes
4652
{
4753
ConfigureHostBuilder = configureHostBuilder,
4854
ConfigureWebHostBuilder = configureWebHostBuilder,
4955
GenericHostWebHostBuilderExtensions = genericHostWebHostBuilderExtensions,
5056
HostingAbstractionsWebHostBuilderExtensions = hostingAbstractionsWebHostBuilderExtensions,
5157
WebHostBuilderExtensions = webHostBuilderExtensions,
58+
HostingHostBuilderExtensions = hostingHostBuilderExtensions
5259
};
5360

5461
return true;
@@ -59,4 +66,5 @@ public static bool TryCreate(Compilation compilation, [NotNullWhen(true)] out We
5966
public INamedTypeSymbol GenericHostWebHostBuilderExtensions { get; private init; }
6067
public INamedTypeSymbol HostingAbstractionsWebHostBuilderExtensions { get; private init; }
6168
public INamedTypeSymbol WebHostBuilderExtensions { get; private init; }
69+
public INamedTypeSymbol HostingHostBuilderExtensions { get; private init; }
6270
}

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

Lines changed: 278 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,278 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
using System.Globalization;
4+
using Microsoft.AspNetCore.Analyzer.Testing;
5+
using Microsoft.CodeAnalysis;
6+
7+
namespace Microsoft.AspNetCore.Analyzers.WebApplicationBuilder;
8+
public partial class DisallowConfigureHostLoggingTest
9+
{
10+
private TestDiagnosticAnalyzerRunner Runner { get; } = new(new WebApplicationBuilderAnalyzer());
11+
12+
[Fact]
13+
public async Task DoesNotWarnWhenBuilderLoggingIsUsed()
14+
{
15+
//arrange
16+
var source = @"
17+
using Microsoft.AspNetCore.Builder;
18+
using Microsoft.Extensions.Hosting;
19+
using Microsoft.Extensions.DependencyInjection;
20+
using Microsoft.Extensions.Logging;
21+
var builder = WebApplication.CreateBuilder(args);
22+
builder.Logging.AddJsonConsole();
23+
";
24+
//act
25+
var diagnostics = await Runner.GetDiagnosticsAsync(source);
26+
//assert
27+
Assert.Empty(diagnostics);
28+
}
29+
30+
[Fact]
31+
public async Task DoesNotWarnWhenBuilderLoggingIsUsed_InMain()
32+
{
33+
//arrange
34+
var source = @"
35+
using Microsoft.AspNetCore.Builder;
36+
using Microsoft.Extensions.Hosting;
37+
using Microsoft.Extensions.DependencyInjection;
38+
using Microsoft.Extensions.Logging;
39+
public static class Program
40+
{
41+
public static void Main (string[] args)
42+
{
43+
var builder = WebApplication.CreateBuilder(args);
44+
builder.Logging.AddJsonConsole();
45+
}
46+
}
47+
public class Startup { }
48+
";
49+
//act
50+
var diagnostics = await Runner.GetDiagnosticsAsync(source);
51+
//assert
52+
Assert.Empty(diagnostics);
53+
}
54+
55+
[Fact]
56+
public async Task WarnsWhenBuilderLoggingIsNotUsed_Host()
57+
{
58+
//arrange
59+
var source = TestSource.Read(@"
60+
using Microsoft.AspNetCore.Builder;
61+
using Microsoft.Extensions.Hosting;
62+
using Microsoft.Extensions.DependencyInjection;
63+
using Microsoft.Extensions.Logging;
64+
var builder = WebApplication.CreateBuilder(args);
65+
builder.Host./*MM*/ConfigureLogging(logging =>
66+
{
67+
logging.AddJsonConsole();
68+
});
69+
");
70+
//act
71+
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
72+
//assert
73+
var diagnostic = Assert.Single(diagnostics);
74+
Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic.Descriptor);
75+
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
76+
Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic.GetMessage(CultureInfo.InvariantCulture));
77+
}
78+
79+
[Fact]
80+
public async Task WarnsWhenBuilderLoggingIsNotUsed_WebHost()
81+
{
82+
//arrange
83+
var source = TestSource.Read(@"
84+
using Microsoft.AspNetCore.Builder;
85+
using Microsoft.Extensions.Hosting;
86+
using Microsoft.Extensions.DependencyInjection;
87+
using Microsoft.Extensions.Logging;
88+
using Microsoft.AspNetCore.Hosting;
89+
var builder = WebApplication.CreateBuilder(args);
90+
builder.WebHost./*MM*/ConfigureLogging(logging =>
91+
{
92+
logging.AddJsonConsole();
93+
});
94+
");
95+
//act
96+
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
97+
//assert
98+
var diagnostic = Assert.Single(diagnostics);
99+
Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic.Descriptor);
100+
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
101+
Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic.GetMessage(CultureInfo.InvariantCulture));
102+
}
103+
104+
[Fact]
105+
public async Task WarnsWhenBuilderLoggingIsNotUsed_OnDifferentLine_Host()
106+
{
107+
//arrange
108+
var source = TestSource.Read(@"
109+
using Microsoft.AspNetCore.Builder;
110+
using Microsoft.Extensions.Hosting;
111+
using Microsoft.Extensions.DependencyInjection;
112+
using Microsoft.Extensions.Logging;
113+
var builder = WebApplication.CreateBuilder(args);
114+
builder.Host.
115+
/*MM*/ConfigureLogging(logging =>
116+
{
117+
logging.AddJsonConsole();
118+
});
119+
");
120+
//act
121+
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
122+
//assert
123+
var diagnostic = Assert.Single(diagnostics);
124+
Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic.Descriptor);
125+
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
126+
Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic.GetMessage(CultureInfo.InvariantCulture));
127+
}
128+
129+
[Fact]
130+
public async Task WarnsWhenBuilderLoggingIsNotUsed_OnDifferentLine_WebHost()
131+
{
132+
//arrange
133+
var source = TestSource.Read(@"
134+
using Microsoft.AspNetCore.Builder;
135+
using Microsoft.Extensions.Hosting;
136+
using Microsoft.Extensions.DependencyInjection;
137+
using Microsoft.Extensions.Logging;
138+
using Microsoft.AspNetCore.Hosting;
139+
var builder = WebApplication.CreateBuilder(args);
140+
builder.WebHost.
141+
/*MM*/ConfigureLogging(logging =>
142+
{
143+
logging.AddJsonConsole();
144+
});
145+
");
146+
//act
147+
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
148+
//assert
149+
var diagnostic = Assert.Single(diagnostics);
150+
Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic.Descriptor);
151+
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
152+
Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic.GetMessage(CultureInfo.InvariantCulture));
153+
}
154+
155+
[Fact]
156+
public async Task WarnsWhenBuilderLoggingIsNotUsed_InMain_Host()
157+
{
158+
//arrange
159+
var source = TestSource.Read(@"
160+
using Microsoft.AspNetCore.Builder;
161+
using Microsoft.Extensions.Hosting;
162+
using Microsoft.Extensions.DependencyInjection;
163+
using Microsoft.Extensions.Logging;
164+
public static class Program
165+
{
166+
public static void Main (string[] args)
167+
{
168+
var builder = WebApplication.CreateBuilder(args);
169+
builder.Host./*MM*/ConfigureLogging(logging =>
170+
{
171+
logging.AddJsonConsole();
172+
});
173+
}
174+
}
175+
public class Startup { }
176+
");
177+
//act
178+
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
179+
//assert
180+
var diagnostic = Assert.Single(diagnostics);
181+
Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic.Descriptor);
182+
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
183+
Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic.GetMessage(CultureInfo.InvariantCulture));
184+
}
185+
186+
[Fact]
187+
public async Task WarnsWhenBuilderLoggingIsNotUsed_InMain_WebHost()
188+
{
189+
//arrange
190+
var source = TestSource.Read(@"
191+
using Microsoft.AspNetCore.Builder;
192+
using Microsoft.Extensions.Hosting;
193+
using Microsoft.Extensions.DependencyInjection;
194+
using Microsoft.Extensions.Logging;
195+
using Microsoft.AspNetCore.Hosting;
196+
public static class Program
197+
{
198+
public static void Main (string[] args)
199+
{
200+
var builder = WebApplication.CreateBuilder(args);
201+
builder.WebHost./*MM*/ConfigureLogging(logging =>
202+
{
203+
logging.AddJsonConsole();
204+
});
205+
}
206+
}
207+
public class Startup { }
208+
");
209+
//act
210+
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
211+
//assert
212+
var diagnostic = Assert.Single(diagnostics);
213+
Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic.Descriptor);
214+
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
215+
Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic.GetMessage(CultureInfo.InvariantCulture));
216+
}
217+
218+
[Fact]
219+
public async Task WarnsWhenBuilderLoggingIsNotUsed_WhenChained_WebHost()
220+
{
221+
//arrange
222+
var source = TestSource.Read(@"
223+
using Microsoft.AspNetCore.Builder;
224+
using Microsoft.Extensions.Hosting;
225+
using Microsoft.Extensions.DependencyInjection;
226+
using Microsoft.Extensions.Logging;
227+
using Microsoft.AspNetCore.Hosting;
228+
var builder = WebApplication.CreateBuilder(args);
229+
builder.WebHost.
230+
/*MM*/ConfigureLogging(logging => { })
231+
.ConfigureServices(services => { });
232+
");
233+
//act
234+
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
235+
//assert
236+
var diagnostic = Assert.Single(diagnostics);
237+
Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic.Descriptor);
238+
AnalyzerAssert.DiagnosticLocation(source.DefaultMarkerLocation, diagnostic.Location);
239+
Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic.GetMessage(CultureInfo.InvariantCulture));
240+
}
241+
242+
[Fact]
243+
public async Task WarnsTwiceWhenBuilderLoggingIsNotUsed_Host()
244+
{
245+
//arrange
246+
var source = TestSource.Read(@"
247+
using Microsoft.AspNetCore.Builder;
248+
using Microsoft.Extensions.Hosting;
249+
using Microsoft.Extensions.DependencyInjection;
250+
using Microsoft.Extensions.Logging;
251+
var builder = WebApplication.CreateBuilder(args);
252+
builder.Host./*MM1*/ConfigureLogging(logging =>
253+
{
254+
logging.AddJsonConsole();
255+
});
256+
builder.Host./*MM2*/ConfigureLogging(logging =>
257+
{
258+
logging.AddJsonConsole();
259+
});
260+
");
261+
//act
262+
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);
263+
//assert
264+
Assert.Equal(2, diagnostics.Length);
265+
var diagnostic1 = diagnostics[0];
266+
var diagnostic2 = diagnostics[1];
267+
268+
Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic1.Descriptor);
269+
AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MM1"], diagnostic1.Location);
270+
Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic1.GetMessage(CultureInfo.InvariantCulture));
271+
272+
Assert.Same(DiagnosticDescriptors.DoNotUseHostConfigureLogging, diagnostic2.Descriptor);
273+
AnalyzerAssert.DiagnosticLocation(source.MarkerLocations["MM2"], diagnostic2.Location);
274+
Assert.Equal("Suggest using builder.Logging instead of ConfigureLogging", diagnostic2.GetMessage(CultureInfo.InvariantCulture));
275+
}
276+
277+
}
278+

0 commit comments

Comments
 (0)