Skip to content

Commit

Permalink
[GH-1] - taking into account null values in substitute initializers
Browse files Browse the repository at this point in the history
  • Loading branch information
@tpodolak
tpodolak committed May 30, 2018
1 parent aaac82b commit ff3d0dc
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 78 deletions.
73 changes: 20 additions & 53 deletions src/NSubstitute.Analyzers/DiagnosticAnalyzers/SubstituteAnalysis.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,28 @@
using Microsoft.CodeAnalysis;
using NSubstitute.Analyzers.Extensions;
#if CSHARP
#endif

#if VISUAL_BASIC
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
#elif VISUAL_BASIC
using Microsoft.CodeAnalysis.VisualBasic;
using Microsoft.CodeAnalysis.VisualBasic.Syntax;

#endif

namespace NSubstitute.Analyzers.DiagnosticAnalyzers
{
// TODO remove duplication
public static class SubstituteAnalysis
{
public static InvocationInfo GetInfocationInfo(SubstituteAnalyzer.SubstituteContext substituteContext)
public static IList<TypeInfo> GetInvocationInfo(SubstituteAnalyzer.SubstituteContext substituteContext)
{
var infos = substituteContext.MethodSymbol.IsGenericMethod
? GetGenericInvocationArgumentTypes(substituteContext)
: GetNonGenericInvocationArgumentTypes(substituteContext);

return new InvocationInfo(infos);
return infos;
}

private static IList<TypeInfo> GetGenericInvocationArgumentTypes(
SubstituteAnalyzer.SubstituteContext substituteContext)
private static IList<TypeInfo> GetGenericInvocationArgumentTypes(SubstituteAnalyzer.SubstituteContext substituteContext)
{
if (substituteContext.InvocationExpression.ArgumentList == null)
{
Expand All @@ -52,19 +50,7 @@ public static InvocationInfo GetInfocationInfo(SubstituteAnalyzer.SubstituteCont
possibleParamsArgument.ConvertedType is IArrayTypeSymbol arrayTypeSymbol &&
arrayTypeSymbol.ElementType.Equals(substituteContext.SyntaxNodeAnalysisContext.Compilation.ObjectType))
{
if (possibleParamsArgument.Type == null)
{
return new List<TypeInfo>();
}

var parameterExpressionsFromArrayArgument = arguments.First().GetArgumentExpression()
.GetParameterExpressionsFromArrayArgument();

var types = parameterExpressionsFromArrayArgument
.Select(exp => GetTypeInfo(substituteContext, exp))
.ToList();

return types;
return GetArgumentTypeInfo(substituteContext, arguments.First());
}

return typeInfos;
Expand All @@ -79,25 +65,29 @@ private static IList<TypeInfo> GetNonGenericInvocationArgumentTypes(SubstituteAn
return null;
}

// Substitute.For(new [] { typeof(T) }, null) // means we dont pass any arguments
var typeInfo =
substituteContext.SyntaxNodeAnalysisContext.SemanticModel.GetTypeInfo(arrayArgument.DescendantNodes()
.First());
if (typeInfo.ConvertedType != null && typeInfo.ConvertedType.TypeKind == TypeKind.Array &&
return GetArgumentTypeInfo(substituteContext, arrayArgument);
}

private static IList<TypeInfo> GetArgumentTypeInfo(SubstituteAnalyzer.SubstituteContext substituteContext, ArgumentSyntax arrayArgument)
{
var typeInfo = substituteContext.SyntaxNodeAnalysisContext.SemanticModel.GetTypeInfo(arrayArgument.DescendantNodes().First());

if (typeInfo.ConvertedType != null &&
typeInfo.ConvertedType.TypeKind == TypeKind.Array &&
typeInfo.Type == null)
{
return new List<TypeInfo>();
}

// Substitute.For(new [] { typeof(T)}, new object[] { }); // means we dont pass any arguments
// new object[] { }; // means we dont pass any arguments
var parameterExpressionsFromArrayArgument =
arrayArgument.GetArgumentExpression().GetParameterExpressionsFromArrayArgument();
if (parameterExpressionsFromArrayArgument.Count == 0)
if (parameterExpressionsFromArrayArgument == null)
{
return new List<TypeInfo>();
return null;
}

// Substitute.For(new [] { typeof(T)}, new object[] { 1, 2, 3}); // means we pass arguments
// new object[] { 1, 2, 3}); // means we pass arguments
var types = parameterExpressionsFromArrayArgument
.Select(exp => GetTypeInfo(substituteContext, exp))
.ToList();
Expand All @@ -109,28 +99,5 @@ private static TypeInfo GetTypeInfo(SubstituteAnalyzer.SubstituteContext substit
{
return substituteContext.SyntaxNodeAnalysisContext.SemanticModel.GetTypeInfo(syntax);
}

public class InvocationInfo
{
private IList<ITypeSymbol> _typeSymbols;

public IList<ITypeSymbol> CapturedSymbols
{
get
{
return _typeSymbols = _typeSymbols ??
TypeInfos?.Select(info => info.Type).Where(type => type != null).ToList();
}
}

public IList<TypeInfo> TypeInfos { get; }

public bool AllTypesCapured => TypeInfos != null && TypeInfos.Count == CapturedSymbols.Count;

public InvocationInfo(IList<TypeInfo> typeInfos)
{
TypeInfos = typeInfos;
}
}
}
}
24 changes: 13 additions & 11 deletions src/NSubstitute.Analyzers/DiagnosticAnalyzers/SubstituteAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using NSubstitute.Analyzers.Extensions;
using static NSubstitute.Analyzers.DiagnosticAnalyzers.SubstituteAnalysis;
#if CSHARP
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
#elif VISUAL_BASIC
using Microsoft.CodeAnalysis.VisualBasic;
using Microsoft.CodeAnalysis.VisualBasic.Syntax;

#endif

namespace NSubstitute.Analyzers.DiagnosticAnalyzers
Expand Down Expand Up @@ -189,6 +191,11 @@ private ImmutableArray<ITypeSymbol> GetProxySymbols(SubstituteContext substitute
.GetArgumentExpression()
.GetParameterExpressionsFromArrayArgument();

if (arrayParameters == null)
{
return ImmutableArray<ITypeSymbol>.Empty;
}

var proxyTypes = arrayParameters.OfType<TypeOfExpressionSyntax>()
.Select(exp =>
substituteContext.SyntaxNodeAnalysisContext.SemanticModel
Expand All @@ -210,10 +217,10 @@ private bool AnalyzeConstructorInvocation(SubstituteContext substituteContext, I
var possibleConstructors = GetPossibleConstructors(substituteContext, typeSymbol);
var invocationInfo = GetInvocationInfo(substituteContext);

if (invocationInfo.AllTypesCapured && possibleConstructors.All(ctor =>
SubstituteConstructorMatcher.MatchesInvocation(
substituteContext.SyntaxNodeAnalysisContext.SemanticModel.Compilation, ctor, invocationInfo.CapturedSymbols) ==
false))
if (invocationInfo != null && possibleConstructors.All(ctor =>
SubstituteConstructorMatcher.MatchesInvocation(
substituteContext.SyntaxNodeAnalysisContext.SemanticModel.Compilation, ctor, invocationInfo) ==
false))
{
var diagnostic = Diagnostic.Create(
DiagnosticDescriptors.SubstituteConstructorMismatch,
Expand All @@ -228,7 +235,7 @@ private bool AnalyzeConstructorInvocation(SubstituteContext substituteContext, I

private bool AnalyzeConstructorParametersCount(SubstituteContext substituteContext, ITypeSymbol typeSymbol)
{
var invocationArgumentTypes = GetInvocationInfo(substituteContext).TypeInfos?.Count;
var invocationArgumentTypes = GetInvocationInfo(substituteContext)?.Count;
switch (typeSymbol.TypeKind)
{
case TypeKind.Interface when invocationArgumentTypes > 0:
Expand Down Expand Up @@ -268,18 +275,13 @@ private bool AnalyzeConstructorParametersCount(SubstituteContext substituteConte

private IEnumerable<IMethodSymbol> GetPossibleConstructors(SubstituteContext substituteContext, ITypeSymbol typeSymbol)
{
var count = GetInvocationInfo(substituteContext).TypeInfos?.Count;
var count = GetInvocationInfo(substituteContext)?.Count;

return count.HasValue
? GetAccessibleConstructors(typeSymbol).Where(ctor => ctor.Parameters.Length == count)
: null;
}

private SubstituteAnalysis.InvocationInfo GetInvocationInfo(SubstituteContext substituteContext)
{
return SubstituteAnalysis.GetInfocationInfo(substituteContext);
}

private static bool AnalyzeTypeKind(SubstituteContext substituteContext, ITypeSymbol proxyType)
{
if (proxyType.TypeKind == TypeKind.Interface || proxyType.TypeKind == TypeKind.Delegate)
Expand Down
6 changes: 3 additions & 3 deletions src/NSubstitute.Analyzers/DiagnosticAnalyzers/SubstituteConstructorMatcher.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,19 @@ static SubstituteConstructorMatcher()
IntegerTypes = ((SpecialType[])Enum.GetValues(typeof(SpecialType))).Where(type => (int)type >= 11 && (int)type <= 16).ToArray();
}

public static bool MatchesInvocation(Compilation compilation, IMethodSymbol methodSymbol, IList<ITypeSymbol> argumentTypes)
public static bool MatchesInvocation(Compilation compilation, IMethodSymbol methodSymbol, IList<TypeInfo> argumentTypes)
{
if (methodSymbol.Parameters.Length != argumentTypes.Count)
{
return false;
}

return methodSymbol.Parameters.Length == 0 || methodSymbol.Parameters.Where((symbol, index) => IsConvertible(compilation, argumentTypes[index], symbol.Type)).Any();
return methodSymbol.Parameters.Length == 0 || methodSymbol.Parameters.Where((symbol, index) => IsConvertible(compilation, argumentTypes[index].Type, symbol.Type)).Any();
}

private static bool IsConvertible(Compilation compilation, ITypeSymbol source, ITypeSymbol destination)
{
if (source.Equals(destination))
if (source == null || source.Equals(destination))
{
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion src/NSubstitute.Analyzers/Extensions/ExpressionSyntaxExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public static IList<ExpressionSyntax> GetParameterExpressionsFromArrayArgument(t
initializer = ((ImplicitArrayCreationExpressionSyntax)expression).Initializer;
break;
default:
return EmptyExpressionList;
return null;
}

if (initializer == null)
Expand Down
14 changes: 12 additions & 2 deletions ...s.Test.CSharp/DiagnosticAnalyzerTests/SubstituteAnalyzersTests/ForAsGenericMethodTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -534,8 +534,8 @@ public void Test()
[InlineData("int x", "1D")]
[InlineData("int x", "1D")]
[InlineData("List<int> x", "new List<int>().AsReadOnly()")]
public override async Task ReturnsDiagnostic_WhenConstructorArgumentsRequireExplicitConversion(
string ctorValues, string invocationValues)
[InlineData("int x", "new [] { 1 }")]
public override async Task ReturnsDiagnostic_WhenConstructorArgumentsRequireExplicitConversion(string ctorValues, string invocationValues)
{
var source = $@"using NSubstitute;
using System.Collections.Generic;
Expand Down Expand Up @@ -577,10 +577,20 @@ public void Test()
[InlineData("IEnumerable<int> x", "new List<int>()")]
[InlineData("IEnumerable<int> x", "new List<int>().AsReadOnly()")]
[InlineData("IEnumerable<char> x", @"""value""")]
[InlineData("int x", @"new object[] { 1 }")]
[InlineData("int[] x", @"new int[] { 1 }")]
[InlineData("object[] x , int y", @"new object[] { 1 }, 1")]
[InlineData("int[] x , int y", @"new int[] { 1 }, 1")]
[InlineData("", @"new object[] { }")]
[InlineData("", "new object[] { 1, 2 }.ToArray()")] // actual values known at runtime only so constructor analysys skipped
[InlineData("int x", "new object[] { null }")] // even though we pass null as first arg, this works fine with NSubstitute
[InlineData("int x, int y", "new object[] { null, null }")] // even though we pass null as first arg, this works fine with NSubstitute
[InlineData("int x, int y", "new object[] { 1, null }")] // even though we pass null as first arg, this works fine with NSubstitute
public override async Task ReturnsNoDiagnostic_WhenConstructorArgumentsAreImplicitlyConvertible(string ctorValues, string invocationValues)
{
var source = $@"using NSubstitute;
using System.Collections.Generic;
using System.Linq;
namespace MyNamespace
{{
public class Foo
Expand Down
19 changes: 11 additions & 8 deletions ...est.CSharp/DiagnosticAnalyzerTests/SubstituteAnalyzersTests/ForAsNonGenericMethodTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -620,17 +620,20 @@ public void Test()
await VerifyCSharpDiagnostic(source, expectedDiagnostic);
}

[InlineData("int x", "1")]
[InlineData("float x", "'c'")]
[InlineData("int x", "'c'")]
[InlineData("IList<int> x", "new List<int>()")]
[InlineData("IEnumerable<int> x", "new List<int>()")]
[InlineData("IEnumerable<int> x", "new List<int>().AsReadOnly()")]
[InlineData("IEnumerable<char> x", @"""value""")]
[InlineData("int x", "new object [] { 1 }")]
[InlineData("float x", "new object [] { 'c' }")]
[InlineData("int x", "new object [] { 'c' }")]
[InlineData("IList<int> x", "new object [] { new List<int>() }")]
[InlineData("IEnumerable<int> x", "new object [] { new List<int>() }")]
[InlineData("IEnumerable<int> x", "new object [] { new List<int>().AsReadOnly() }")]
[InlineData("IEnumerable<char> x", @"new object [] { ""value"" }")]
[InlineData("", @"new object[] { }")]
[InlineData("", "new object[] { 1, 2 }.ToArray()")] // actual values known at runtime only so constructor analysys skipped
public override async Task ReturnsNoDiagnostic_WhenConstructorArgumentsAreImplicitlyConvertible(string ctorValues, string invocationValues)
{
var source = $@"using NSubstitute;
using System.Collections.Generic;
using System.Linq;
namespace MyNamespace
{{
public class Foo
Expand All @@ -644,7 +647,7 @@ public class FooTests
{{
public void Test()
{{
var substitute = NSubstitute.Substitute.For(new [] {{ typeof(Foo) }}, new object[] {{{invocationValues}}});
var substitute = NSubstitute.Substitute.For(new [] {{ typeof(Foo) }}, {invocationValues});
}}
}}
}}";
Expand Down
7 changes: 7 additions & 0 deletions ...ers.Test.CSharp/DiagnosticAnalyzerTests/SubstituteAnalyzersTests/ForPartsOfMethodTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,10 +381,17 @@ public void Test()
[InlineData("IEnumerable<int> x", "new List<int>()")]
[InlineData("IEnumerable<int> x", "new List<int>().AsReadOnly()")]
[InlineData("IEnumerable<char> x", @"""value""")]
[InlineData("int x", @"new object[] { 1 }")]
[InlineData("int[] x", @"new int[] { 1 }")]
[InlineData("object[] x , int y", @"new object[] { 1 }, 1")]
[InlineData("int[] x , int y", @"new int[] { 1 }, 1")]
[InlineData("", @"new object[] { }")]
[InlineData("", "new object[] { 1, 2 }.ToArray()")] // actual values known at runtime only
public override async Task ReturnsNoDiagnostic_WhenConstructorArgumentsAreImplicitlyConvertible(string ctorValues, string invocationValues)
{
var source = $@"using NSubstitute;
using System.Collections.Generic;
using System.Linq;
namespace MyNamespace
{{
public class Foo
Expand Down

0 comments on commit ff3d0dc

Please sign in to comment.