Skip to content

Commit 26b5323

Browse files
pranavkmpranavkm
authored
Fix more null refs in Api Analyzers (#10375)
* Fix more null refs in Api Analyzers * Make attributes used by analyzer public * Compile analyzer tests against ref assemblies * Turn on Nullable reference types and harden some null-checks Fixes #8686
1 parent e925df6 commit 26b5323

24 files changed

+486
-167
lines changed

‎src/Mvc/Mvc.Analyzers/src/AttributesShouldNotBeAppliedToPageModelAnalyzer.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using System;
54
using System.Collections.Immutable;
65
using Microsoft.CodeAnalysis;
76
using Microsoft.CodeAnalysis.Diagnostics;
@@ -122,7 +121,7 @@ private static void ReportFilterDiagnostic(ref SymbolAnalysisContext symbolAnaly
122121
}
123122
}
124123

125-
private static AttributeData GetAttribute(ISymbol symbol, INamedTypeSymbol attributeType)
124+
private static AttributeData? GetAttribute(ISymbol symbol, INamedTypeSymbol attributeType)
126125
{
127126
foreach (var attribute in symbol.GetAttributes())
128127
{

‎src/Mvc/Mvc.Analyzers/src/CodeAnalysisExtensions.cs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,10 @@ public static IEnumerable<AttributeData> GetAttributes(this IMethodSymbol method
3131
Debug.Assert(methodSymbol != null);
3232
Debug.Assert(attribute != null);
3333

34-
while (methodSymbol != null)
34+
IMethodSymbol? current = methodSymbol;
35+
while (current != null)
3536
{
36-
foreach (var attributeData in GetAttributes(methodSymbol, attribute))
37+
foreach (var attributeData in GetAttributes(current, attribute))
3738
{
3839
yield return attributeData;
3940
}
@@ -43,7 +44,7 @@ public static IEnumerable<AttributeData> GetAttributes(this IMethodSymbol method
4344
break;
4445
}
4546

46-
methodSymbol = methodSymbol.IsOverride ? methodSymbol.OverriddenMethod : null;
47+
current = current.IsOverride ? current.OverriddenMethod : null;
4748
}
4849
}
4950

@@ -76,14 +77,15 @@ public static bool HasAttribute(this IPropertySymbol propertySymbol, ITypeSymbol
7677
return HasAttribute(propertySymbol, attribute);
7778
}
7879

79-
while (propertySymbol != null)
80+
IPropertySymbol? current = propertySymbol;
81+
while (current != null)
8082
{
81-
if (propertySymbol.HasAttribute(attribute))
83+
if (current.HasAttribute(attribute))
8284
{
8385
return true;
8486
}
8587

86-
propertySymbol = propertySymbol.IsOverride ? propertySymbol.OverriddenProperty : null;
88+
current = current.IsOverride ? current.OverriddenProperty : null;
8789
}
8890

8991
return false;

‎src/Mvc/Mvc.Analyzers/src/Microsoft.AspNetCore.Mvc.Analyzers.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
<IncludeBuildOutput>false</IncludeBuildOutput>
1010
<GenerateDocumentationFile>false</GenerateDocumentationFile>
1111
<NuspecFile>$(MSBuildProjectName).nuspec</NuspecFile>
12+
<NullableContextOptions>enable</NullableContextOptions>
13+
<Nullable>enable</Nullable>
1214
</PropertyGroup>
1315

1416
<ItemGroup>

‎src/Mvc/Mvc.Analyzers/src/TopLevelParameterNameAnalyzer.cs

Lines changed: 92 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44

55
using System;
6-
using System.Collections.Generic;
76
using System.Collections.Immutable;
87
using System.Linq;
98
using Microsoft.CodeAnalysis;
@@ -24,8 +23,7 @@ public override void Initialize(AnalysisContext context)
2423

2524
context.RegisterCompilationStartAction(compilationStartAnalysisContext =>
2625
{
27-
var typeCache = new SymbolCache(compilationStartAnalysisContext.Compilation);
28-
if (typeCache.ControllerAttribute == null || typeCache.ControllerAttribute.TypeKind == TypeKind.Error)
26+
if (!SymbolCache.TryCreate(compilationStartAnalysisContext.Compilation, out var typeCache))
2927
{
3028
// No-op if we can't find types we care about.
3129
return;
@@ -185,20 +183,100 @@ internal static bool SpecifiesModelType(in SymbolCache symbolCache, IParameterSy
185183

186184
internal readonly struct SymbolCache
187185
{
188-
public SymbolCache(Compilation compilation)
186+
public SymbolCache(
187+
INamedTypeSymbol bindAttribute,
188+
INamedTypeSymbol controllerAttribute,
189+
INamedTypeSymbol fromBodyAttribute,
190+
INamedTypeSymbol apiBehaviorMetadata,
191+
INamedTypeSymbol binderTypeProviderMetadata,
192+
INamedTypeSymbol modelNameProvider,
193+
INamedTypeSymbol nonControllerAttribute,
194+
INamedTypeSymbol nonActionAttribute,
195+
IMethodSymbol disposableDispose)
189196
{
190-
BindAttribute = compilation.GetTypeByMetadataName(SymbolNames.BindAttribute);
191-
ControllerAttribute = compilation.GetTypeByMetadataName(SymbolNames.ControllerAttribute);
192-
FromBodyAttribute = compilation.GetTypeByMetadataName(SymbolNames.FromBodyAttribute);
193-
IApiBehaviorMetadata = compilation.GetTypeByMetadataName(SymbolNames.IApiBehaviorMetadata);
194-
IBinderTypeProviderMetadata = compilation.GetTypeByMetadataName(SymbolNames.IBinderTypeProviderMetadata);
195-
IModelNameProvider = compilation.GetTypeByMetadataName(SymbolNames.IModelNameProvider);
196-
NonControllerAttribute = compilation.GetTypeByMetadataName(SymbolNames.NonControllerAttribute);
197-
NonActionAttribute = compilation.GetTypeByMetadataName(SymbolNames.NonActionAttribute);
197+
BindAttribute = bindAttribute;
198+
ControllerAttribute = controllerAttribute;
199+
FromBodyAttribute = fromBodyAttribute;
200+
IApiBehaviorMetadata = apiBehaviorMetadata;
201+
IBinderTypeProviderMetadata = binderTypeProviderMetadata;
202+
IModelNameProvider = modelNameProvider;
203+
NonControllerAttribute = nonControllerAttribute;
204+
NonActionAttribute = nonActionAttribute;
205+
IDisposableDispose = disposableDispose;
206+
}
207+
208+
public static bool TryCreate(Compilation compilation, out SymbolCache symbolCache)
209+
{
210+
symbolCache = default;
211+
212+
if (!TryGetType(SymbolNames.BindAttribute, out var bindAttribute))
213+
{
214+
return false;
215+
}
216+
217+
218+
if (!TryGetType(SymbolNames.ControllerAttribute, out var controllerAttribute))
219+
{
220+
return false;
221+
}
222+
223+
224+
if (!TryGetType(SymbolNames.FromBodyAttribute, out var fromBodyAttribute))
225+
{
226+
return false;
227+
}
228+
229+
if (!TryGetType(SymbolNames.IApiBehaviorMetadata, out var apiBehaviorMetadata))
230+
{
231+
return false;
232+
}
233+
234+
if (!TryGetType(SymbolNames.IBinderTypeProviderMetadata, out var iBinderTypeProviderMetadata))
235+
{
236+
return false;
237+
}
238+
239+
if (!TryGetType(SymbolNames.IModelNameProvider, out var iModelNameProvider))
240+
{
241+
return false;
242+
}
243+
244+
if (!TryGetType(SymbolNames.NonControllerAttribute, out var nonControllerAttribute))
245+
{
246+
return false;
247+
}
248+
249+
if (!TryGetType(SymbolNames.NonActionAttribute, out var nonActionAttribute))
250+
{
251+
return false;
252+
}
198253

199254
var disposable = compilation.GetSpecialType(SpecialType.System_IDisposable);
200-
var members = disposable.GetMembers(nameof(IDisposable.Dispose));
201-
IDisposableDispose = members.Length == 1 ? (IMethodSymbol)members[0] : null;
255+
var members = disposable?.GetMembers(nameof(IDisposable.Dispose));
256+
var idisposableDispose = (IMethodSymbol?)members?[0];
257+
if (idisposableDispose == null)
258+
{
259+
return false;
260+
}
261+
262+
symbolCache = new SymbolCache(
263+
bindAttribute,
264+
controllerAttribute,
265+
fromBodyAttribute,
266+
apiBehaviorMetadata,
267+
iBinderTypeProviderMetadata,
268+
iModelNameProvider,
269+
nonControllerAttribute,
270+
nonActionAttribute,
271+
idisposableDispose);
272+
273+
return true;
274+
275+
bool TryGetType(string typeName, out INamedTypeSymbol typeSymbol)
276+
{
277+
typeSymbol = compilation.GetTypeByMetadataName(typeName);
278+
return typeSymbol != null && typeSymbol.TypeKind != TypeKind.Error;
279+
}
202280
}
203281

204282
public INamedTypeSymbol BindAttribute { get; }

‎src/Mvc/Mvc.Analyzers/test/Mvc.Analyzers.Test.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
<ItemGroup>
2323
<ProjectReference Include="..\..\Mvc.Analyzers\src\Microsoft.AspNetCore.Mvc.Analyzers.csproj" />
24-
<Reference Include="Microsoft.AspNetCore.Mvc" />
24+
<ProjectReference Include="..\..\Mvc\ref\Microsoft.AspNetCore.Mvc.csproj" />
2525
<Reference Include="Microsoft.AspNetCore.Analyzer.Testing" />
2626
<Reference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" />
2727
</ItemGroup>

‎src/Mvc/Mvc.Analyzers/test/TopLevelParameterNameAnalyzerTest.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ private async Task<bool> IsProblematicParameterTest([CallerMemberName] string te
134134
var method = (IMethodSymbol)modelType.GetMembers("ActionMethod").First();
135135
var parameter = method.Parameters[0];
136136

137-
var symbolCache = new TopLevelParameterNameAnalyzer.SymbolCache(compilation);
137+
Assert.True(TopLevelParameterNameAnalyzer.SymbolCache.TryCreate(compilation, out var symbolCache));
138138

139139
var result = TopLevelParameterNameAnalyzer.IsProblematicParameter(symbolCache, parameter);
140140
return result;
@@ -145,7 +145,7 @@ public async Task GetName_ReturnsValueFromFirstAttributeWithValue()
145145
{
146146
var methodName = nameof(GetNameTests.SingleAttribute);
147147
var compilation = await GetCompilationForGetName();
148-
var symbolCache = new TopLevelParameterNameAnalyzer.SymbolCache(compilation);
148+
Assert.True(TopLevelParameterNameAnalyzer.SymbolCache.TryCreate(compilation, out var symbolCache));
149149

150150
var type = compilation.GetTypeByMetadataName(typeof(GetNameTests).FullName);
151151
var method = (IMethodSymbol)type.GetMembers(methodName).First();
@@ -161,7 +161,7 @@ public async Task GetName_ReturnsName_IfNoAttributesAreSpecified()
161161
{
162162
var methodName = nameof(GetNameTests.NoAttribute);
163163
var compilation = await GetCompilationForGetName();
164-
var symbolCache = new TopLevelParameterNameAnalyzer.SymbolCache(compilation);
164+
Assert.True(TopLevelParameterNameAnalyzer.SymbolCache.TryCreate(compilation, out var symbolCache));
165165

166166
var type = compilation.GetTypeByMetadataName(typeof(GetNameTests).FullName);
167167
var method = (IMethodSymbol)type.GetMembers(methodName).First();
@@ -177,7 +177,7 @@ public async Task GetName_ReturnsName_IfAttributeDoesNotSpecifyName()
177177
{
178178
var methodName = nameof(GetNameTests.SingleAttributeWithoutName);
179179
var compilation = await GetCompilationForGetName();
180-
var symbolCache = new TopLevelParameterNameAnalyzer.SymbolCache(compilation);
180+
Assert.True(TopLevelParameterNameAnalyzer.SymbolCache.TryCreate(compilation, out var symbolCache));
181181

182182
var type = compilation.GetTypeByMetadataName(typeof(GetNameTests).FullName);
183183
var method = (IMethodSymbol)type.GetMembers(methodName).First();
@@ -193,7 +193,7 @@ public async Task GetName_ReturnsFirstName_IfMultipleAttributesAreSpecified()
193193
{
194194
var methodName = nameof(GetNameTests.MultipleAttributes);
195195
var compilation = await GetCompilationForGetName();
196-
var symbolCache = new TopLevelParameterNameAnalyzer.SymbolCache(compilation);
196+
Assert.True(TopLevelParameterNameAnalyzer.SymbolCache.TryCreate(compilation, out var symbolCache));
197197

198198
var type = compilation.GetTypeByMetadataName(typeof(GetNameTests).FullName);
199199
var method = (IMethodSymbol)type.GetMembers(methodName).First();
@@ -221,7 +221,7 @@ public async Task SpecifiesModelType_ReturnsFalse_IfModelBinderDoesNotSpecifyTyp
221221
var project = DiagnosticProject.Create(GetType().Assembly, new[] { testSource.Source });
222222

223223
var compilation = await project.GetCompilationAsync();
224-
var symbolCache = new TopLevelParameterNameAnalyzer.SymbolCache(compilation);
224+
Assert.True(TopLevelParameterNameAnalyzer.SymbolCache.TryCreate(compilation, out var symbolCache));
225225

226226
var type = compilation.GetTypeByMetadataName(typeof(SpecifiesModelTypeTests).FullName);
227227
var method = (IMethodSymbol)type.GetMembers(testMethod).First();
@@ -239,7 +239,7 @@ public async Task SpecifiesModelType_ReturnsTrue_IfModelBinderSpecifiesTypeFromC
239239
var project = DiagnosticProject.Create(GetType().Assembly, new[] { testSource.Source });
240240

241241
var compilation = await project.GetCompilationAsync();
242-
var symbolCache = new TopLevelParameterNameAnalyzer.SymbolCache(compilation);
242+
Assert.True(TopLevelParameterNameAnalyzer.SymbolCache.TryCreate(compilation, out var symbolCache));
243243

244244
var type = compilation.GetTypeByMetadataName(typeof(SpecifiesModelTypeTests).FullName);
245245
var method = (IMethodSymbol)type.GetMembers(testMethod).First();
@@ -257,7 +257,7 @@ public async Task SpecifiesModelType_ReturnsTrue_IfModelBinderSpecifiesTypeFromP
257257
var project = DiagnosticProject.Create(GetType().Assembly, new[] { testSource.Source });
258258

259259
var compilation = await project.GetCompilationAsync();
260-
var symbolCache = new TopLevelParameterNameAnalyzer.SymbolCache(compilation);
260+
Assert.True(TopLevelParameterNameAnalyzer.SymbolCache.TryCreate(compilation, out var symbolCache));
261261

262262
var type = compilation.GetTypeByMetadataName(typeof(SpecifiesModelTypeTests).FullName);
263263
var method = (IMethodSymbol)type.GetMembers(testMethod).First();

‎src/Mvc/Mvc.Api.Analyzers/src/ActualApiResponseMetadata.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4+
using System;
45
using Microsoft.CodeAnalysis;
56
using Microsoft.CodeAnalysis.CSharp.Syntax;
67

@@ -17,7 +18,7 @@ public ActualApiResponseMetadata(ReturnStatementSyntax returnStatement, ITypeSym
1718
_statusCode = null;
1819
}
1920

20-
public ActualApiResponseMetadata(ReturnStatementSyntax returnStatement, int statusCode, ITypeSymbol returnType)
21+
public ActualApiResponseMetadata(ReturnStatementSyntax returnStatement, int statusCode, ITypeSymbol? returnType)
2122
{
2223
ReturnStatement = returnStatement;
2324
_statusCode = statusCode;
@@ -26,10 +27,10 @@ public ActualApiResponseMetadata(ReturnStatementSyntax returnStatement, int stat
2627

2728
public ReturnStatementSyntax ReturnStatement { get; }
2829

29-
public int StatusCode => _statusCode.Value;
30+
public int StatusCode => _statusCode ?? throw new ArgumentException("Status code is not available when IsDefaultResponse is true");
3031

3132
public bool IsDefaultResponse => _statusCode == null;
3233

33-
public ITypeSymbol ReturnType { get; }
34+
public ITypeSymbol? ReturnType { get; }
3435
}
3536
}

‎src/Mvc/Mvc.Api.Analyzers/src/ActualApiResponseMetadataFactory.cs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,13 @@ internal static bool TryGetActualResponseMetadata(
8787
.FirstOrDefault();
8888

8989
var statusCode = GetDefaultStatusCode(defaultStatusCodeAttribute);
90-
ITypeSymbol returnType = null;
90+
ITypeSymbol? returnType = null;
9191
switch (returnExpression)
9292
{
9393
case InvocationExpressionSyntax invocation:
9494
{
9595
// Covers the 'return StatusCode(200)' case.
96-
var result = InspectMethodArguments(symbolCache, semanticModel, invocation.Expression, invocation.ArgumentList, cancellationToken);
96+
var result = InspectMethodArguments(semanticModel, invocation.Expression, invocation.ArgumentList, cancellationToken);
9797
statusCode = result.statusCode ?? statusCode;
9898
returnType = result.returnType;
9999
break;
@@ -102,7 +102,7 @@ internal static bool TryGetActualResponseMetadata(
102102
case ObjectCreationExpressionSyntax creation:
103103
{
104104
// Read values from 'return new StatusCodeResult(200) case.
105-
var result = InspectMethodArguments(symbolCache, semanticModel, creation, creation.ArgumentList, cancellationToken);
105+
var result = InspectMethodArguments(semanticModel, creation, creation.ArgumentList, cancellationToken);
106106
statusCode = result.statusCode ?? statusCode;
107107
returnType = result.returnType;
108108

@@ -123,14 +123,14 @@ internal static bool TryGetActualResponseMetadata(
123123
return new ActualApiResponseMetadata(returnStatementSyntax, statusCode.Value, returnType);
124124
}
125125

126-
private static (int? statusCode, ITypeSymbol returnType) InspectInitializers(
126+
private static (int? statusCode, ITypeSymbol? returnType) InspectInitializers(
127127
in ApiControllerSymbolCache symbolCache,
128128
SemanticModel semanticModel,
129129
InitializerExpressionSyntax initializer,
130130
CancellationToken cancellationToken)
131131
{
132132
int? statusCode = null;
133-
ITypeSymbol typeSymbol = null;
133+
ITypeSymbol? typeSymbol = null;
134134

135135
for (var i = 0; initializer != null && i < initializer.Expressions.Count; i++)
136136
{
@@ -162,15 +162,14 @@ private static (int? statusCode, ITypeSymbol returnType) InspectInitializers(
162162
return (statusCode, typeSymbol);
163163
}
164164

165-
private static (int? statusCode, ITypeSymbol returnType) InspectMethodArguments(
166-
in ApiControllerSymbolCache symbolCache,
165+
private static (int? statusCode, ITypeSymbol? returnType) InspectMethodArguments(
167166
SemanticModel semanticModel,
168167
ExpressionSyntax expression,
169168
BaseArgumentListSyntax argumentList,
170169
CancellationToken cancellationToken)
171170
{
172171
int? statusCode = null;
173-
ITypeSymbol typeSymbol = null;
172+
ITypeSymbol? typeSymbol = null;
174173

175174
var symbolInfo = semanticModel.GetSymbolInfo(expression, cancellationToken);
176175

0 commit comments

Comments
 (0)