Skip to content

Fix more null refs in Api Analyzers #10375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 21, 2019
Merged

Fix more null refs in Api Analyzers #10375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d951a5c
Fix more null refs in Api Analyzers
pranavkm May 19, 2019
351079c
Update how symbol caches is initialized
pranavkm May 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/Mvc/Mvc.Analyzers/src/AttributesShouldNotBeAppliedToPageModelAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
Expand Down Expand Up @@ -122,7 +121,7 @@ private static void ReportFilterDiagnostic(ref SymbolAnalysisContext symbolAnaly
}
}

private static AttributeData GetAttribute(ISymbol symbol, INamedTypeSymbol attributeType)
private static AttributeData? GetAttribute(ISymbol symbol, INamedTypeSymbol attributeType)
{
foreach (var attribute in symbol.GetAttributes())
{
Expand Down
14 changes: 8 additions & 6 deletions src/Mvc/Mvc.Analyzers/src/CodeAnalysisExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ public static IEnumerable<AttributeData> GetAttributes(this IMethodSymbol method
Debug.Assert(methodSymbol != null);
Debug.Assert(attribute != null);

while (methodSymbol != null)
IMethodSymbol? current = methodSymbol;
while (current != null)
{
foreach (var attributeData in GetAttributes(methodSymbol, attribute))
foreach (var attributeData in GetAttributes(current, attribute))
{
yield return attributeData;
}
Expand All @@ -43,7 +44,7 @@ public static IEnumerable<AttributeData> GetAttributes(this IMethodSymbol method
break;
}

methodSymbol = methodSymbol.IsOverride ? methodSymbol.OverriddenMethod : null;
current = current.IsOverride ? current.OverriddenMethod : null;
}
}

Expand Down Expand Up @@ -76,14 +77,15 @@ public static bool HasAttribute(this IPropertySymbol propertySymbol, ITypeSymbol
return HasAttribute(propertySymbol, attribute);
}

while (propertySymbol != null)
IPropertySymbol? current = propertySymbol;
while (current != null)
{
if (propertySymbol.HasAttribute(attribute))
if (current.HasAttribute(attribute))
{
return true;
}

propertySymbol = propertySymbol.IsOverride ? propertySymbol.OverriddenProperty : null;
current = current.IsOverride ? current.OverriddenProperty : null;
}

return false;
Expand Down
2 changes: 2 additions & 0 deletions src/Mvc/Mvc.Analyzers/src/Microsoft.AspNetCore.Mvc.Analyzers.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
<IncludeBuildOutput>false</IncludeBuildOutput>
<GenerateDocumentationFile>false</GenerateDocumentationFile>
<NuspecFile>$(MSBuildProjectName).nuspec</NuspecFile>
<NullableContextOptions>enable</NullableContextOptions>
<Nullable>enable</Nullable>
</PropertyGroup>

<ItemGroup>
Expand Down
106 changes: 92 additions & 14 deletions src/Mvc/Mvc.Analyzers/src/TopLevelParameterNameAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@


using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
Expand All @@ -24,8 +23,7 @@ public override void Initialize(AnalysisContext context)

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

internal readonly struct SymbolCache
{
public SymbolCache(Compilation compilation)
public SymbolCache(
INamedTypeSymbol bindAttribute,
INamedTypeSymbol controllerAttribute,
INamedTypeSymbol fromBodyAttribute,
INamedTypeSymbol apiBehaviorMetadata,
INamedTypeSymbol binderTypeProviderMetadata,
INamedTypeSymbol modelNameProvider,
INamedTypeSymbol nonControllerAttribute,
INamedTypeSymbol nonActionAttribute,
IMethodSymbol disposableDispose)
{
BindAttribute = compilation.GetTypeByMetadataName(SymbolNames.BindAttribute);
ControllerAttribute = compilation.GetTypeByMetadataName(SymbolNames.ControllerAttribute);
FromBodyAttribute = compilation.GetTypeByMetadataName(SymbolNames.FromBodyAttribute);
IApiBehaviorMetadata = compilation.GetTypeByMetadataName(SymbolNames.IApiBehaviorMetadata);
IBinderTypeProviderMetadata = compilation.GetTypeByMetadataName(SymbolNames.IBinderTypeProviderMetadata);
IModelNameProvider = compilation.GetTypeByMetadataName(SymbolNames.IModelNameProvider);
NonControllerAttribute = compilation.GetTypeByMetadataName(SymbolNames.NonControllerAttribute);
NonActionAttribute = compilation.GetTypeByMetadataName(SymbolNames.NonActionAttribute);
BindAttribute = bindAttribute;
ControllerAttribute = controllerAttribute;
FromBodyAttribute = fromBodyAttribute;
IApiBehaviorMetadata = apiBehaviorMetadata;
IBinderTypeProviderMetadata = binderTypeProviderMetadata;
IModelNameProvider = modelNameProvider;
NonControllerAttribute = nonControllerAttribute;
NonActionAttribute = nonActionAttribute;
IDisposableDispose = disposableDispose;
}

public static bool TryCreate(Compilation compilation, out SymbolCache symbolCache)
{
symbolCache = default;

if (!TryGetType(SymbolNames.BindAttribute, out var bindAttribute))
{
return false;
}


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JamesNK

if (!TryGetType(SymbolNames.ControllerAttribute, out var controllerAttribute))
{
return false;
}


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JamesNK

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is aquaman sad?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanos collected the one ring and now aquaman has to leave Hogwarts. Poor aquaman 😢

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll clean up the extra new lines in a follow up.

if (!TryGetType(SymbolNames.FromBodyAttribute, out var fromBodyAttribute))
{
return false;
}

if (!TryGetType(SymbolNames.IApiBehaviorMetadata, out var apiBehaviorMetadata))
{
return false;
}

if (!TryGetType(SymbolNames.IBinderTypeProviderMetadata, out var iBinderTypeProviderMetadata))
{
return false;
}

if (!TryGetType(SymbolNames.IModelNameProvider, out var iModelNameProvider))
{
return false;
}

if (!TryGetType(SymbolNames.NonControllerAttribute, out var nonControllerAttribute))
{
return false;
}

if (!TryGetType(SymbolNames.NonActionAttribute, out var nonActionAttribute))
{
return false;
}

var disposable = compilation.GetSpecialType(SpecialType.System_IDisposable);
var members = disposable.GetMembers(nameof(IDisposable.Dispose));
IDisposableDispose = members.Length == 1 ? (IMethodSymbol)members[0] : null;
var members = disposable?.GetMembers(nameof(IDisposable.Dispose));
var idisposableDispose = (IMethodSymbol?)members?[0];
if (idisposableDispose == null)
{
return false;
}

symbolCache = new SymbolCache(
bindAttribute,
controllerAttribute,
fromBodyAttribute,
apiBehaviorMetadata,
iBinderTypeProviderMetadata,
iModelNameProvider,
nonControllerAttribute,
nonActionAttribute,
idisposableDispose);

return true;

bool TryGetType(string typeName, out INamedTypeSymbol typeSymbol)
{
typeSymbol = compilation.GetTypeByMetadataName(typeName);
return typeSymbol != null && typeSymbol.TypeKind != TypeKind.Error;
}
}

public INamedTypeSymbol BindAttribute { get; }
Expand Down
2 changes: 1 addition & 1 deletion src/Mvc/Mvc.Analyzers/test/Mvc.Analyzers.Test.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

<ItemGroup>
<ProjectReference Include="..\..\Mvc.Analyzers\src\Microsoft.AspNetCore.Mvc.Analyzers.csproj" />
<Reference Include="Microsoft.AspNetCore.Mvc" />
<ProjectReference Include="..\..\Mvc\ref\Microsoft.AspNetCore.Mvc.csproj" />
<Reference Include="Microsoft.AspNetCore.Analyzer.Testing" />
<Reference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" />
</ItemGroup>
Expand Down
16 changes: 8 additions & 8 deletions src/Mvc/Mvc.Analyzers/test/TopLevelParameterNameAnalyzerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ private async Task<bool> IsProblematicParameterTest([CallerMemberName] string te
var method = (IMethodSymbol)modelType.GetMembers("ActionMethod").First();
var parameter = method.Parameters[0];

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

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

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

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

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

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

var compilation = await project.GetCompilationAsync();
var symbolCache = new TopLevelParameterNameAnalyzer.SymbolCache(compilation);
Assert.True(TopLevelParameterNameAnalyzer.SymbolCache.TryCreate(compilation, out var symbolCache));

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

var compilation = await project.GetCompilationAsync();
var symbolCache = new TopLevelParameterNameAnalyzer.SymbolCache(compilation);
Assert.True(TopLevelParameterNameAnalyzer.SymbolCache.TryCreate(compilation, out var symbolCache));

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

var compilation = await project.GetCompilationAsync();
var symbolCache = new TopLevelParameterNameAnalyzer.SymbolCache(compilation);
Assert.True(TopLevelParameterNameAnalyzer.SymbolCache.TryCreate(compilation, out var symbolCache));

var type = compilation.GetTypeByMetadataName(typeof(SpecifiesModelTypeTests).FullName);
var method = (IMethodSymbol)type.GetMembers(testMethod).First();
Expand Down
7 changes: 4 additions & 3 deletions src/Mvc/Mvc.Api.Analyzers/src/ActualApiResponseMetadata.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;

Expand All @@ -17,7 +18,7 @@ public ActualApiResponseMetadata(ReturnStatementSyntax returnStatement, ITypeSym
_statusCode = null;
}

public ActualApiResponseMetadata(ReturnStatementSyntax returnStatement, int statusCode, ITypeSymbol returnType)
public ActualApiResponseMetadata(ReturnStatementSyntax returnStatement, int statusCode, ITypeSymbol? returnType)
{
ReturnStatement = returnStatement;
_statusCode = statusCode;
Expand All @@ -26,10 +27,10 @@ public ActualApiResponseMetadata(ReturnStatementSyntax returnStatement, int stat

public ReturnStatementSyntax ReturnStatement { get; }

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

public bool IsDefaultResponse => _statusCode == null;

public ITypeSymbol ReturnType { get; }
public ITypeSymbol? ReturnType { get; }
}
}
15 changes: 7 additions & 8 deletions src/Mvc/Mvc.Api.Analyzers/src/ActualApiResponseMetadataFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,13 @@ internal static bool TryGetActualResponseMetadata(
.FirstOrDefault();

var statusCode = GetDefaultStatusCode(defaultStatusCodeAttribute);
ITypeSymbol returnType = null;
ITypeSymbol? returnType = null;
switch (returnExpression)
{
case InvocationExpressionSyntax invocation:
{
// Covers the 'return StatusCode(200)' case.
var result = InspectMethodArguments(symbolCache, semanticModel, invocation.Expression, invocation.ArgumentList, cancellationToken);
var result = InspectMethodArguments(semanticModel, invocation.Expression, invocation.ArgumentList, cancellationToken);
statusCode = result.statusCode ?? statusCode;
returnType = result.returnType;
break;
Expand All @@ -102,7 +102,7 @@ internal static bool TryGetActualResponseMetadata(
case ObjectCreationExpressionSyntax creation:
{
// Read values from 'return new StatusCodeResult(200) case.
var result = InspectMethodArguments(symbolCache, semanticModel, creation, creation.ArgumentList, cancellationToken);
var result = InspectMethodArguments(semanticModel, creation, creation.ArgumentList, cancellationToken);
statusCode = result.statusCode ?? statusCode;
returnType = result.returnType;

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

private static (int? statusCode, ITypeSymbol returnType) InspectInitializers(
private static (int? statusCode, ITypeSymbol? returnType) InspectInitializers(
in ApiControllerSymbolCache symbolCache,
SemanticModel semanticModel,
InitializerExpressionSyntax initializer,
CancellationToken cancellationToken)
{
int? statusCode = null;
ITypeSymbol typeSymbol = null;
ITypeSymbol? typeSymbol = null;

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

private static (int? statusCode, ITypeSymbol returnType) InspectMethodArguments(
in ApiControllerSymbolCache symbolCache,
private static (int? statusCode, ITypeSymbol? returnType) InspectMethodArguments(
SemanticModel semanticModel,
ExpressionSyntax expression,
BaseArgumentListSyntax argumentList,
CancellationToken cancellationToken)
{
int? statusCode = null;
ITypeSymbol typeSymbol = null;
ITypeSymbol? typeSymbol = null;

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

Expand Down
Loading