Ignore parameters that specify a model binder type

This commit is contained in:
Pranav K 2018-08-15 11:13:05 -07:00
parent 200a70bb86
commit af770ede87
9 changed files with 154 additions and 14 deletions

View File

@ -31,6 +31,8 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers
public const string IApiBehaviorMetadata = "Microsoft.AspNetCore.Mvc.Internal.IApiBehaviorMetadata";
public const string IBinderTypeProviderMetadata = "Microsoft.AspNetCore.Mvc.ModelBinding.IBinderTypeProviderMetadata";
public const string IActionResult = "Microsoft.AspNetCore.Mvc.IActionResult";
public const string IConvertToActionResult = "Microsoft.AspNetCore.Mvc.IConvertToActionResult";

View File

@ -91,6 +91,12 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers
return false;
}
if (SpecifiesModelType(symbolCache, parameter))
{
// Ignore parameters that specify a model type.
return false;
}
var parameterName = GetName(symbolCache, parameter);
var type = parameter.Type;
@ -144,6 +150,39 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers
return symbol.Name;
}
internal static bool SpecifiesModelType(in SymbolCache symbolCache, IParameterSymbol parameterSymbol)
{
foreach (var attribute in parameterSymbol.GetAttributes(symbolCache.IBinderTypeProviderMetadata))
{
// Look for a attribute property named BinderType being assigned. This would match
// [ModelBinder(BinderType = typeof(SomeBinder))]
for (var i = 0; i < attribute.NamedArguments.Length; i++)
{
var namedArgument = attribute.NamedArguments[i];
var namedArgumentValue = namedArgument.Value;
if (string.Equals(namedArgument.Key, "BinderType", StringComparison.Ordinal) &&
namedArgumentValue.Kind == TypedConstantKind.Type)
{
return true;
}
}
// Look for the binder type being specified in the constructor. This would match
// [ModelBinder(typeof(SomeBinder))]
var constructorParameters = attribute.AttributeConstructor?.Parameters ?? ImmutableArray<IParameterSymbol>.Empty;
for (var i = 0; i < constructorParameters.Length; i++)
{
if (string.Equals(constructorParameters[i].Name, "binderType", StringComparison.Ordinal))
{
// A constructor that requires binderType was used.
return true;
}
}
}
return false;
}
internal readonly struct SymbolCache
{
public SymbolCache(Compilation compilation)
@ -152,6 +191,7 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers
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);
@ -165,6 +205,7 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers
public INamedTypeSymbol ControllerAttribute { get; }
public INamedTypeSymbol FromBodyAttribute { get; }
public INamedTypeSymbol IApiBehaviorMetadata { get; }
public INamedTypeSymbol IBinderTypeProviderMetadata { get; }
public INamedTypeSymbol IModelNameProvider { get; }
public INamedTypeSymbol NonControllerAttribute { get; }
public INamedTypeSymbol NonActionAttribute { get; }

View File

@ -0,0 +1,9 @@
namespace Microsoft.AspNetCore.Mvc.Analyzers.TopLevelParameterNameAnalyzerTestFiles
{
public class IsProblematicParameter_ReturnsFalse_ForParametersWithCustomModelBinder
{
public string Model { get; set; }
public void ActionMethod([ModelBinder(typeof(object))] IsProblematicParameter_ReturnsFalse_ForParametersWithCustomModelBinder model) { }
}
}

View File

@ -1,9 +1,9 @@
namespace Microsoft.AspNetCore.Mvc.Analyzers.TopLevelParameterNameAnalyzerTestFiles
{
public class IsProblematicParameter_ReturnsFalse_IfModelBinderAttributeIsUsedToRenameParameter
public class IsProblematicParameter_ReturnsFalse_IfBindingSourceAttributeIsUsedToRenameParameter
{
public string Model { get; set; }
public void ActionMethod([FromRoute(Name = "id")] IsProblematicParameter_ReturnsFalse_IfModelBinderAttributeIsUsedToRenameParameter model) { }
public void ActionMethod([FromRoute(Name = "id")] IsProblematicParameter_ReturnsFalse_IfBindingSourceAttributeIsUsedToRenameParameter model) { }
}
}

View File

@ -0,0 +1,10 @@
namespace Microsoft.AspNetCore.Mvc.Analyzers.TopLevelParameterNameAnalyzerTestFiles
{
public class IsProblematicParameter_ReturnsFalse_IfBindingSourceAttributeIsUsedToRenameProperty
{
[FromQuery(Name = "different")]
public string Model { get; set; }
public void ActionMethod(IsProblematicParameter_ReturnsFalse_IfBindingSourceAttributeIsUsedToRenameProperty model) { }
}
}

View File

@ -1,10 +0,0 @@
namespace Microsoft.AspNetCore.Mvc.Analyzers.TopLevelParameterNameAnalyzerTestFiles
{
public class IsProblematicParameter_ReturnsFalse_IfModelBinderAttributeIsUsedToRenameProperty
{
[FromQuery(Name = "different")]
public string Model { get; set; }
public void ActionMethod([Bind(Prefix = nameof(Model))] IsProblematicParameter_ReturnsFalse_IfModelBinderAttributeIsUsedToRenameProperty different) { }
}
}

View File

@ -0,0 +1,9 @@
namespace Microsoft.AspNetCore.Mvc.Analyzers.TopLevelParameterNameAnalyzerTestFiles
{
public class IsProblematicParameter_ReturnsTrue_IfModelBinderAttributeIsUsedToRenameParameter
{
public string Model { get; set; }
public void ActionMethod([ModelBinder(Name = "model")] IsProblematicParameter_ReturnsTrue_IfModelBinderAttributeIsUsedToRenameParameter different) { }
}
}

View File

@ -0,0 +1,11 @@
namespace Microsoft.AspNetCore.Mvc.Analyzers.TopLevelParameterNameAnalyzerTestFiles
{
public class SpecifiesModelTypeTests
{
public void SpecifiesModelType_ReturnsFalse_IfModelBinderDoesNotSpecifyType([ModelBinder(Name = "Name")] object model) { }
public void SpecifiesModelType_ReturnsTrue_IfModelBinderSpecifiesTypeFromConstructor([ModelBinder(typeof(object))] object model) { }
public void SpecifiesModelType_ReturnsTrue_IfModelBinderSpecifiesTypeFromProperty([ModelBinder(BinderType = typeof(object))] object model) { }
}
}

View File

@ -61,14 +61,21 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers
}
[Fact]
public async Task IsProblematicParameter_ReturnsFalse_IfModelBinderAttributeIsUsedToRenameParameter()
public async Task IsProblematicParameter_ReturnsTrue_IfModelBinderAttributeIsUsedToRenameParameter()
{
var result = await IsProblematicParameterTest();
Assert.True(result);
}
[Fact]
public async Task IsProblematicParameter_ReturnsFalse_IfBindingSourceAttributeIsUsedToRenameProperty()
{
var result = await IsProblematicParameterTest();
Assert.False(result);
}
[Fact]
public async Task IsProblematicParameter_ReturnsFalse_IfModelBinderAttributeIsUsedToRenameProperty()
public async Task IsProblematicParameter_ReturnsFalse_IfBindingSourceAttributeIsUsedToRenameParameter()
{
var result = await IsProblematicParameterTest();
Assert.False(result);
@ -81,6 +88,13 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers
Assert.False(result);
}
[Fact]
public async Task IsProblematicParameter_ReturnsFalse_ForParametersWithCustomModelBinder()
{
var result = await IsProblematicParameterTest();
Assert.False(result);
}
[Fact]
public async Task IsProblematicParameter_IgnoresStaticProperties()
{
@ -199,6 +213,60 @@ namespace Microsoft.AspNetCore.Mvc.Analyzers
return compilation;
}
[Fact]
public async Task SpecifiesModelType_ReturnsFalse_IfModelBinderDoesNotSpecifyType()
{
var testMethod = nameof(SpecifiesModelType_ReturnsFalse_IfModelBinderDoesNotSpecifyType);
var testSource = MvcTestSource.Read(GetType().Name, "SpecifiesModelTypeTests");
var project = DiagnosticProject.Create(GetType().Assembly, new[] { testSource.Source });
var compilation = await project.GetCompilationAsync();
var symbolCache = new TopLevelParameterNameAnalyzer.SymbolCache(compilation);
var type = compilation.GetTypeByMetadataName(typeof(SpecifiesModelTypeTests).FullName);
var method = (IMethodSymbol)type.GetMembers(testMethod).First();
var parameter = method.Parameters[0];
var result = TopLevelParameterNameAnalyzer.SpecifiesModelType(symbolCache, parameter);
Assert.False(result);
}
[Fact]
public async Task SpecifiesModelType_ReturnsTrue_IfModelBinderSpecifiesTypeFromConstructor()
{
var testMethod = nameof(SpecifiesModelType_ReturnsTrue_IfModelBinderSpecifiesTypeFromConstructor);
var testSource = MvcTestSource.Read(GetType().Name, "SpecifiesModelTypeTests");
var project = DiagnosticProject.Create(GetType().Assembly, new[] { testSource.Source });
var compilation = await project.GetCompilationAsync();
var symbolCache = new TopLevelParameterNameAnalyzer.SymbolCache(compilation);
var type = compilation.GetTypeByMetadataName(typeof(SpecifiesModelTypeTests).FullName);
var method = (IMethodSymbol)type.GetMembers(testMethod).First();
var parameter = method.Parameters[0];
var result = TopLevelParameterNameAnalyzer.SpecifiesModelType(symbolCache, parameter);
Assert.True(result);
}
[Fact]
public async Task SpecifiesModelType_ReturnsTrue_IfModelBinderSpecifiesTypeFromProperty()
{
var testMethod = nameof(SpecifiesModelType_ReturnsTrue_IfModelBinderSpecifiesTypeFromProperty);
var testSource = MvcTestSource.Read(GetType().Name, "SpecifiesModelTypeTests");
var project = DiagnosticProject.Create(GetType().Assembly, new[] { testSource.Source });
var compilation = await project.GetCompilationAsync();
var symbolCache = new TopLevelParameterNameAnalyzer.SymbolCache(compilation);
var type = compilation.GetTypeByMetadataName(typeof(SpecifiesModelTypeTests).FullName);
var method = (IMethodSymbol)type.GetMembers(testMethod).First();
var parameter = method.Parameters[0];
var result = TopLevelParameterNameAnalyzer.SpecifiesModelType(symbolCache, parameter);
Assert.True(result);
}
private async Task RunNoDiagnosticsAreReturned([CallerMemberName] string testMethod = "")
{
// Arrange