Skip to content
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.

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

[GH-1] - Checking constructor arguments passed via Substitute.For/Substitute.ForPartsOf #13

Merged
merged 38 commits into from
Jun 18, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
1bc8396
[GH-1] - first draf of substitute analyzers
tpodolak May 24, 2018
b1a225c
Stylecop added
tpodolak May 26, 2018
c2cf8e3
Proper naming for the rulset file
tpodolak May 26, 2018
b6e478e
Excluding resource files from code coverage
tpodolak May 26, 2018
955a052
Removing redundant file
tpodolak May 26, 2018
895c436
Trying to fix codecov navigation issues
tpodolak May 26, 2018
3252668
Going back to coveralls.io
tpodolak May 26, 2018
21358c1
Adding proper build and coverage badges
tpodolak May 26, 2018
bff9bc1
[GH-1] - Fixing stylecop issues
tpodolak May 26, 2018
a582a77
[GH-1] - Refactoring of substitute analyzers, first attempts to write…
tpodolak May 26, 2018
789dc71
[GH-1] - Additional tests for c# implementation
tpodolak May 26, 2018
0c5aa94
[GH-1] - Attempting to analyze non generic version of Substitute.For
tpodolak May 27, 2018
aaac82b
[GH-1] - changing project structure in preparation of CodeFixProviders
tpodolak May 27, 2018
ff3d0dc
[GH-1] - taking into account null values in substitute initializers
tpodolak May 30, 2018
efb21a8
[GH-1] - reducing amount of constructor types calculation to minimum
tpodolak May 31, 2018
11a74c0
Adding test cases for Visual Basic - still problems with implict conv…
tpodolak Jun 1, 2018
37edcaf
[GH-1] - Adding test cases for Visual Basic - still problems with imp…
tpodolak Jun 1, 2018
85cd1c6
Merge branch 'GH-1-SubstituteForAnalyzers' of https://github.com/nsub…
tpodolak Jun 1, 2018
55b7fd4
[GH-1] - first attempts to write code fix providers for wrong usage o…
tpodolak Jun 2, 2018
536c401
[GH-1] - handling well known conversions
tpodolak Jun 9, 2018
4ce8f6d
Merge branch 'dev' into GH-1-SubstituteForAnalyzers
tpodolak Jun 9, 2018
f691234
[GH-1] - adapting to the dev changes - first stage
tpodolak Jun 9, 2018
ff9582a
[GH-1] - adapting to the dev changes - second stage
tpodolak Jun 10, 2018
dcbc78d
[GH-1] - first draft of SubstituteAnalyzer adjusted to dev branch cha…
tpodolak Jun 10, 2018
005a6df
[GH-1] - adding code fix providers adjusted to dev branch changes
tpodolak Jun 10, 2018
6ea7adc
[GH-1] - fixing typos, adding convention tests for code fix providers
tpodolak Jun 10, 2018
d5e381e
[GH-1] - fixing failing tests, better structure of SubstituteAnalyzer
tpodolak Jun 11, 2018
8514bd9
[GH1-] - removing redundant code, removing code duplication
tpodolak Jun 11, 2018
2cbfa26
[GH-1] - adjusting namespaces, using async instead of blocking calls
tpodolak Jun 13, 2018
fddae9d
[GH-1] - adding missing awaits
tpodolak Jun 13, 2018
7133335
[GH-1] - removing overloaping tests
tpodolak Jun 13, 2018
dedc3fe
[GH-1] - Suppressing xUnit warnings on class level rather than globally
tpodolak Jun 13, 2018
554251c
[GH-1] - using constants instead of magic strings in fix providers
tpodolak Jun 14, 2018
10d7c48
[GH-1] - Proper quick-fix name
tpodolak Jun 14, 2018
9806bd4
[GH-1] - ignoring constructor analysis when proxy is type argument
tpodolak Jun 16, 2018
1c3c766
[GH-1] - more descriptive messages for csharp implementation, initial…
tpodolak Jun 16, 2018
c5e106c
[GH-1] - VB messages improved
tpodolak Jun 17, 2018
e1317b4
Minor message tweaks (#14)
dtchepak Jun 18, 2018
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
Prev Previous commit
Next Next commit
[GH-1] - reducing amount of constructor types calculation to minimum
  • Loading branch information
tpodolak committed May 31, 2018
commit efb21a83b17e4afee40439e27a9751f08bb4391e
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
using System.Collections.Generic;
using Microsoft.CodeAnalysis;

namespace NSubstitute.Analyzers.DiagnosticAnalyzers
{
public readonly struct ConstructorContext
{
public IList<IMethodSymbol> AccessibleConstructors { get; }

public IList<IMethodSymbol> PossibleConstructors { get; }

public IList<ITypeSymbol> InvocationParameters { get; }

public ITypeSymbol ConstructorType { get; }

public ConstructorContext(
ITypeSymbol constructorType,
IList<IMethodSymbol> accessibleConstructors,
IList<IMethodSymbol> possibleConstructors,
IList<ITypeSymbol> invocationParameters)
{
ConstructorType = constructorType;
InvocationParameters = invocationParameters;
AccessibleConstructors = accessibleConstructors;
PossibleConstructors = possibleConstructors;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,34 @@ namespace NSubstitute.Analyzers.DiagnosticAnalyzers
// TODO remove duplication
public static class SubstituteAnalysis
{
public static IList<TypeInfo> GetInvocationInfo(SubstituteAnalyzer.SubstituteContext substituteContext)
public static ConstructorContext CollectConstructorContext(SubstituteContext substituteContext, ITypeSymbol proxyTypeSymbol)
{
var accessibleConstructors = GetAccessibleConstructors(proxyTypeSymbol);
var invocationParameterTypes = GetInvocationInfo(substituteContext);
var possibleConstructors = invocationParameterTypes != null && accessibleConstructors != null
? accessibleConstructors.Where(ctor => ctor.Parameters.Length == invocationParameterTypes.Count)
.ToList().AsReadOnly()
: null;

return new ConstructorContext(
proxyTypeSymbol,
accessibleConstructors,
possibleConstructors,
invocationParameterTypes);
}

public static bool InternalsVisibleToProxyGenerator(ISymbol typeSymbol)
{
var internalsVisibleToAttribute = typeSymbol.ContainingAssembly.GetAttributes()
.FirstOrDefault(att =>
att.AttributeClass.ToString() == MetadataNames.InternalsVisibleToAttributeFullTypeName);

return internalsVisibleToAttribute != null &&
internalsVisibleToAttribute.ConstructorArguments.Any(arg =>
arg.Value.ToString() == MetadataNames.CastleDynamicProxyGenAssembly2Name);
}

private static IList<ITypeSymbol> GetInvocationInfo(SubstituteContext substituteContext)
{
var infos = substituteContext.MethodSymbol.IsGenericMethod
? GetGenericInvocationArgumentTypes(substituteContext)
Expand All @@ -24,7 +51,7 @@ public static IList<TypeInfo> GetInvocationInfo(SubstituteAnalyzer.SubstituteCon
return infos;
}

private static IList<TypeInfo> GetGenericInvocationArgumentTypes(SubstituteAnalyzer.SubstituteContext substituteContext)
private static IList<ITypeSymbol> GetGenericInvocationArgumentTypes(SubstituteContext substituteContext)
{
if (substituteContext.InvocationExpression.ArgumentList == null)
{
Expand All @@ -35,7 +62,7 @@ private static IList<TypeInfo> GetGenericInvocationArgumentTypes(SubstituteAnaly

if (arguments.Count == 0)
{
return new List<TypeInfo>();
return new List<ITypeSymbol>();
}

var typeInfos = arguments.Select(arg =>
Expand All @@ -53,10 +80,10 @@ possibleParamsArgument.ConvertedType is IArrayTypeSymbol arrayTypeSymbol &&
return GetArgumentTypeInfo(substituteContext, arguments.First());
}

return typeInfos;
return typeInfos.Select(type => type.Type).ToList();
}

private static IList<TypeInfo> GetNonGenericInvocationArgumentTypes(SubstituteAnalyzer.SubstituteContext substituteContext)
private static IList<ITypeSymbol> GetNonGenericInvocationArgumentTypes(SubstituteContext substituteContext)
{
// Substitute.For(new [] { typeof(T) }, new object[] { 1, 2, 3}) // actual arguments reside in second arg
var arrayArgument = substituteContext.InvocationExpression.ArgumentList?.Arguments.Skip(1).FirstOrDefault();
Expand All @@ -68,15 +95,15 @@ private static IList<TypeInfo> GetNonGenericInvocationArgumentTypes(SubstituteAn
return GetArgumentTypeInfo(substituteContext, arrayArgument);
}

private static IList<TypeInfo> GetArgumentTypeInfo(SubstituteAnalyzer.SubstituteContext substituteContext, ArgumentSyntax arrayArgument)
private static IList<ITypeSymbol> GetArgumentTypeInfo(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>();
return new List<ITypeSymbol>();
}

// new object[] { }; // means we dont pass any arguments
Expand All @@ -89,13 +116,25 @@ private static IList<TypeInfo> GetArgumentTypeInfo(SubstituteAnalyzer.Substitute

// new object[] { 1, 2, 3}); // means we pass arguments
var types = parameterExpressionsFromArrayArgument
.Select(exp => GetTypeInfo(substituteContext, exp))
.Select(exp => GetTypeInfo(substituteContext, exp).Type)
.ToList();

return types;
}

private static TypeInfo GetTypeInfo(SubstituteAnalyzer.SubstituteContext substituteContext, SyntaxNode syntax)
private static IList<IMethodSymbol> GetAccessibleConstructors(ITypeSymbol genericArgument)
{
var internalsVisibleToProxy = InternalsVisibleToProxyGenerator(genericArgument);

return genericArgument.GetMembers().OfType<IMethodSymbol>().Where(symbol =>
symbol.MethodKind == MethodKind.Constructor &&
symbol.IsStatic == false &&
(symbol.DeclaredAccessibility == Accessibility.Protected ||
symbol.DeclaredAccessibility == Accessibility.Public ||
(internalsVisibleToProxy && symbol.DeclaredAccessibility == Accessibility.Internal))).ToList();
}

private static TypeInfo GetTypeInfo(SubstituteContext substituteContext, SyntaxNode syntax)
{
return substituteContext.SyntaxNodeAnalysisContext.SemanticModel.GetTypeInfo(syntax);
}
Expand Down
130 changes: 32 additions & 98 deletions src/NSubstitute.Analyzers/DiagnosticAnalyzers/SubstituteAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#elif VISUAL_BASIC
using Microsoft.CodeAnalysis.VisualBasic;
using Microsoft.CodeAnalysis.VisualBasic.Syntax;

#endif

namespace NSubstitute.Analyzers.DiagnosticAnalyzers
Expand Down Expand Up @@ -98,20 +97,8 @@ private void AnalyzeSubstituteForMethod(SubstituteContext substituteContext)
return;
}

if (AnalyzeConstructorAccessability(substituteContext, proxyType))
{
return;
}

if (AnalyzeConstructorParametersCount(substituteContext, proxyType))
{
return;
}

if (AnalyzeConstructorInvocation(substituteContext, proxyType))
{
return;
}
var constructorContext = CollectConstructorContext(substituteContext, proxyType);
AnalyzeConstructor(substituteContext, constructorContext);
}

private void AnalyzeSubstituteForPartsOf(SubstituteContext substituteContext)
Expand All @@ -138,17 +125,23 @@ private void AnalyzeSubstituteForPartsOf(SubstituteContext substituteContext)
return;
}

if (AnalyzeConstructorAccessability(substituteContext, proxyType))
var constructorContext = CollectConstructorContext(substituteContext, proxyType);
AnalyzeConstructor(substituteContext, constructorContext);
}

private void AnalyzeConstructor(SubstituteContext substituteContext, ConstructorContext constructorContext)
{
if (AnalyzeConstructorAccessability(substituteContext, constructorContext))
{
return;
}

if (AnalyzeConstructorParametersCount(substituteContext, proxyType))
if (AnalyzeConstructorParametersCount(substituteContext, constructorContext))
{
return;
}

if (AnalyzeConstructorInvocation(substituteContext, proxyType))
if (AnalyzeConstructorInvocation(substituteContext, constructorContext))
{
return;
}
Expand Down Expand Up @@ -207,36 +200,10 @@ private ImmutableArray<ITypeSymbol> GetProxySymbols(SubstituteContext substitute
return arrayParameters.Count == proxyTypes.Length ? proxyTypes : ImmutableArray<ITypeSymbol>.Empty;
}

private bool AnalyzeConstructorInvocation(SubstituteContext substituteContext, ITypeSymbol typeSymbol)
{
if (typeSymbol.TypeKind != TypeKind.Class)
{
return false;
}

var possibleConstructors = GetPossibleConstructors(substituteContext, typeSymbol);
var invocationInfo = GetInvocationInfo(substituteContext);

if (invocationInfo != null && possibleConstructors.All(ctor =>
SubstituteConstructorMatcher.MatchesInvocation(
substituteContext.SyntaxNodeAnalysisContext.SemanticModel.Compilation, ctor, invocationInfo) ==
false))
{
var diagnostic = Diagnostic.Create(
DiagnosticDescriptors.SubstituteConstructorMismatch,
substituteContext.InvocationExpression.GetLocation());

substituteContext.SyntaxNodeAnalysisContext.ReportDiagnostic(diagnostic);
return true;
}

return false;
}

private bool AnalyzeConstructorParametersCount(SubstituteContext substituteContext, ITypeSymbol typeSymbol)
private bool AnalyzeConstructorParametersCount(SubstituteContext substituteContext, ConstructorContext constructorContext)
{
var invocationArgumentTypes = GetInvocationInfo(substituteContext)?.Count;
switch (typeSymbol.TypeKind)
var invocationArgumentTypes = constructorContext.InvocationParameters?.Count;
switch (constructorContext.ConstructorType.TypeKind)
{
case TypeKind.Interface when invocationArgumentTypes > 0:
var diagnostic = Diagnostic.Create(
Expand All @@ -258,9 +225,7 @@ private bool AnalyzeConstructorParametersCount(SubstituteContext substituteConte
return false;
}

var possibleConstructors = GetPossibleConstructors(substituteContext, typeSymbol);

if (possibleConstructors != null && possibleConstructors.Any() == false)
if (constructorContext.PossibleConstructors != null && constructorContext.PossibleConstructors.Any() == false)
{
var diagnostic = Diagnostic.Create(
DiagnosticDescriptors.SubstituteForConstructorParametersMismatch,
Expand All @@ -273,15 +238,6 @@ private bool AnalyzeConstructorParametersCount(SubstituteContext substituteConte
return false;
}

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

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

private static bool AnalyzeTypeKind(SubstituteContext substituteContext, ITypeSymbol proxyType)
{
if (proxyType.TypeKind == TypeKind.Interface || proxyType.TypeKind == TypeKind.Delegate)
Expand Down Expand Up @@ -313,18 +269,20 @@ private bool AnalyzeTypeAccessability(SubstituteContext substituteContext, IType
return false;
}

private bool AnalyzeConstructorAccessability(SubstituteContext substituteContext, ITypeSymbol typeSymbol)
private bool AnalyzeConstructorInvocation(SubstituteContext substituteContext, ConstructorContext constructorContext)
{
if (typeSymbol.TypeKind != TypeKind.Class)
if (constructorContext.ConstructorType.TypeKind != TypeKind.Class || constructorContext.InvocationParameters == null || constructorContext.PossibleConstructors == null)
{
return false;
}

var accessibleConstructors = GetAccessibleConstructors(typeSymbol);
if (accessibleConstructors.Any() == false)
if (constructorContext.PossibleConstructors.All(ctor =>
SubstituteConstructorMatcher.MatchesInvocation(
substituteContext.SyntaxNodeAnalysisContext.SemanticModel.Compilation, ctor, constructorContext.InvocationParameters) ==
false))
{
var diagnostic = Diagnostic.Create(
DiagnosticDescriptors.SubstituteForWithoutAccessibleConstructor,
DiagnosticDescriptors.SubstituteConstructorMismatch,
substituteContext.InvocationExpression.GetLocation());

substituteContext.SyntaxNodeAnalysisContext.ReportDiagnostic(diagnostic);
Expand All @@ -334,27 +292,19 @@ private bool AnalyzeConstructorAccessability(SubstituteContext substituteContext
return false;
}

private bool InternalsVisibleToProxyGenerator(ISymbol typeSymbol)
private bool AnalyzeConstructorAccessability(SubstituteContext substituteContext, ConstructorContext constructorContext)
{
var internalsVisibleToAttribute = typeSymbol.ContainingAssembly.GetAttributes()
.FirstOrDefault(att =>
att.AttributeClass.ToString() == MetadataNames.InternalsVisibleToAttributeFullTypeName);
if (constructorContext.ConstructorType.TypeKind == TypeKind.Class && constructorContext.AccessibleConstructors != null && constructorContext.AccessibleConstructors.Any() == false)
{
var diagnostic = Diagnostic.Create(
DiagnosticDescriptors.SubstituteForWithoutAccessibleConstructor,
substituteContext.InvocationExpression.GetLocation());

return internalsVisibleToAttribute != null &&
internalsVisibleToAttribute.ConstructorArguments.Any(arg =>
arg.Value.ToString() == MetadataNames.CastleDynamicProxyGenAssembly2Name);
}
substituteContext.SyntaxNodeAnalysisContext.ReportDiagnostic(diagnostic);
return true;
}

private IEnumerable<IMethodSymbol> GetAccessibleConstructors(ITypeSymbol genericArgument)
{
var internalsVisibleToProxy = InternalsVisibleToProxyGenerator(genericArgument);

return genericArgument.GetMembers().OfType<IMethodSymbol>().Where(symbol =>
symbol.MethodKind == MethodKind.Constructor &&
symbol.IsStatic == false &&
(symbol.DeclaredAccessibility == Accessibility.Protected ||
symbol.DeclaredAccessibility == Accessibility.Public ||
(internalsVisibleToProxy && symbol.DeclaredAccessibility == Accessibility.Internal)));
return false;
}

private static bool IsSubstituteMethod(SyntaxNodeAnalysisContext syntaxNodeContext, SyntaxNode syntax, string memberName)
Expand All @@ -369,21 +319,5 @@ private static bool IsSubstituteMethod(SyntaxNodeAnalysisContext syntaxNodeConte
return symbol.Symbol?.ContainingAssembly?.Name.Equals(MetadataNames.NSubstituteAssemblyName, StringComparison.Ordinal) == true &&
symbol.Symbol?.ContainingType?.ToString().Equals(MetadataNames.NSubstituteSubstituteFullTypeName, StringComparison.Ordinal) == true;
}

public readonly struct SubstituteContext
{
public SyntaxNodeAnalysisContext SyntaxNodeAnalysisContext { get; }

public InvocationExpressionSyntax InvocationExpression { get; }

public IMethodSymbol MethodSymbol { get; }

public SubstituteContext(SyntaxNodeAnalysisContext syntaxNodeAnalysisContext, InvocationExpressionSyntax invocationExpression, IMethodSymbol methodSymbol)
{
SyntaxNodeAnalysisContext = syntaxNodeAnalysisContext;
InvocationExpression = invocationExpression;
MethodSymbol = methodSymbol;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using Microsoft.CodeAnalysis;
#if CSHARP
Expand All @@ -21,14 +22,14 @@ 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<TypeInfo> argumentTypes)
public static bool MatchesInvocation(Compilation compilation, IMethodSymbol methodSymbol, IList<ITypeSymbol> invocationParameters)
{
if (methodSymbol.Parameters.Length != argumentTypes.Count)
if (methodSymbol.Parameters.Length != invocationParameters.Count)
{
return false;
}

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

private static bool IsConvertible(Compilation compilation, ITypeSymbol source, ITypeSymbol destination)
Expand Down
Loading