Skip to content

Commit ff4ab79

Browse files
GillesTourreauGillesTourreau
committed
Refactoring of the last fix and improve to detect wrong usage of "params object[]" when instantiate Mock<T>.
1 parent 282dce7 commit ff4ab79

File tree

4 files changed

+164
-39
lines changed

4 files changed

+164
-39
lines changed

‎src/Moq.Analyzers/Analyzers/ConstructorArgumentsAnalyzer.cs‎

Lines changed: 68 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,11 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
118118
}
119119

120120
// Gets the list of the constructor arguments
121-
var constructorArguments = new List<ArgumentSyntax>();
121+
var constructorArguments = new List<ExpressionSyntax>();
122122

123123
if (objectCreation.ArgumentList is not null)
124124
{
125-
constructorArguments.AddRange(objectCreation.ArgumentList.Arguments);
125+
constructorArguments.AddRange(objectCreation.ArgumentList.Arguments.Select(a => a.Expression));
126126
}
127127

128128
// Gets the first argument, check if it is MockBehavior argument and skip it.
@@ -158,6 +158,18 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
158158
{
159159
matchedConstructor.Try(constructor);
160160

161+
// Special case, if we have only one argument and it is an array of object.
162+
// Use the array of object as the arguments
163+
if (constructorArguments.Count == 1)
164+
{
165+
var objectArrayElements = ExpandObjectArrayElements(constructorArguments[0], context);
166+
167+
if (objectArrayElements is not null)
168+
{
169+
constructorArguments = objectArrayElements;
170+
}
171+
}
172+
161173
// If the number of arguments is different, check the next constructor definition.
162174
if (constructor.Parameters.Length != constructorArguments.Count)
163175
{
@@ -167,7 +179,7 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
167179

168180
for (var i = 0; i < constructorArguments.Count; i++)
169181
{
170-
if (constructorArguments[i].Expression.IsKind(SyntaxKind.NullLiteralExpression))
182+
if (constructorArguments[i].IsKind(SyntaxKind.NullLiteralExpression))
171183
{
172184
// Null parameter, just check the parameter type is a reference type.
173185
if (!constructor.Parameters[i].Type.IsReferenceType)
@@ -179,38 +191,18 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
179191
continue;
180192
}
181193

182-
if (constructorArguments[i].Expression.IsKind(SyntaxKind.DefaultLiteralExpression))
194+
if (constructorArguments[i].IsKind(SyntaxKind.DefaultLiteralExpression))
183195
{
184196
// Default parameter, skip the parameter.
185197
continue;
186198
}
187199

188-
var constructorArgumentSymbol = context.SemanticModel.GetTypeInfo(constructorArguments[i].Expression, context.CancellationToken);
189-
190-
if (constructorArgumentSymbol.Type is null)
191-
{
192-
// Try to test if there is not an implicit conversion
193-
var conversion = context.Compilation.ClassifyConversion(constructor.Parameters[i].Type, constructorArgumentSymbol.ConvertedType!);
194-
195-
if (!conversion.IsImplicit)
196-
{
197-
matchedConstructor.Cancel();
198-
break;
199-
}
200-
201-
continue;
202-
}
200+
var constructorArgumentSymbol = context.SemanticModel.GetTypeInfo(constructorArguments[i], context.CancellationToken);
203201

204202
if (!constructorArgumentSymbol.Type.IsOrInheritFrom(constructor.Parameters[i].Type))
205203
{
206-
// Try to test if there is not an implicit conversion
207-
var conversion = context.Compilation.ClassifyConversion(constructor.Parameters[i].Type, constructorArgumentSymbol.Type);
208-
209-
if (!conversion.IsImplicit)
210-
{
211-
matchedConstructor.Cancel();
212-
break;
213-
}
204+
matchedConstructor.Cancel();
205+
break;
214206
}
215207
}
216208

@@ -246,6 +238,55 @@ private static void Analyze(SyntaxNodeAnalysisContext context)
246238
}
247239
}
248240

241+
private static List<ExpressionSyntax>? ExpandObjectArrayElements(ExpressionSyntax argument, SyntaxNodeAnalysisContext context)
242+
{
243+
var argumentType = context.SemanticModel.GetTypeInfo(argument, context.CancellationToken);
244+
245+
var type = argumentType.Type;
246+
247+
if (argumentType.Type is null)
248+
{
249+
type = argumentType.ConvertedType;
250+
}
251+
252+
if (type is not IArrayTypeSymbol arrayTypeSymbol)
253+
{
254+
return null;
255+
}
256+
257+
if (arrayTypeSymbol.ElementType.SpecialType != SpecialType.System_Object)
258+
{
259+
return null;
260+
}
261+
262+
// It is an object[] array, try to extract the arguments of the array creation
263+
if (argument is ArrayCreationExpressionSyntax arrayCreationExpressionSyntax)
264+
{
265+
if (arrayCreationExpressionSyntax.Initializer is not null)
266+
{
267+
return arrayCreationExpressionSyntax.Initializer.Expressions.ToList();
268+
}
269+
}
270+
else if (argument is CollectionExpressionSyntax collectionExpressionSyntax)
271+
{
272+
var expressions = new List<ExpressionSyntax>(collectionExpressionSyntax.Elements.Count);
273+
274+
foreach (var element in collectionExpressionSyntax.Elements)
275+
{
276+
if (element is not ExpressionElementSyntax elementExpression)
277+
{
278+
return null;
279+
}
280+
281+
expressions.Add(elementExpression.Expression);
282+
}
283+
284+
return expressions;
285+
}
286+
287+
return null;
288+
}
289+
249290
private struct MatchedConstructor
250291
{
251292
public MatchedConstructor()

‎src/Moq.Analyzers/Moq.Analyzers.csproj‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,8 @@
127127
</PropertyGroup>
128128

129129
<ItemGroup>
130-
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.0.1" />
131-
<PackageReference Include="Microsoft.CodeAnalysis.Workspaces.Common" Version="4.0.1" />
130+
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.7.0" />
131+
<PackageReference Include="Microsoft.CodeAnalysis.Workspaces.Common" Version="4.7.0" />
132132
<PackageReference Include="Microsoft.VisualStudio.Threading.Analyzers" Version="17.14.15">
133133
<PrivateAssets>all</PrivateAssets>
134134
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>

‎src/Moq.Analyzers/MoqExpressionAnalyzer.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ public bool IsMockCreationStrictBehavior(ObjectCreationExpressionSyntax mockCrea
226226
argument = mockCreation.ArgumentList.Arguments[1];
227227
}
228228

229-
return this.IsStrictBehaviorArgument(argument, cancellationToken);
229+
return this.IsStrictBehaviorArgument(argument.Expression, cancellationToken);
230230
}
231231

232232
public bool IsMockOfStrictBehavior(ArgumentListSyntax? argumentList, CancellationToken cancellationToken)
@@ -244,15 +244,15 @@ public bool IsMockOfStrictBehavior(ArgumentListSyntax? argumentList, Cancellatio
244244
return false;
245245
}
246246

247-
return this.IsStrictBehaviorArgument(lastArgument, cancellationToken);
247+
return this.IsStrictBehaviorArgument(lastArgument.Expression, cancellationToken);
248248
}
249249

250-
public bool IsStrictBehaviorArgument(ArgumentSyntax argument, out MemberAccessExpressionSyntax? memberAccessExpression, CancellationToken cancellationToken)
250+
public bool IsStrictBehaviorArgument(ExpressionSyntax argument, out MemberAccessExpressionSyntax? memberAccessExpression, CancellationToken cancellationToken)
251251
{
252252
memberAccessExpression = null;
253253

254254
// Check it is a MemberAccessExpressionSyntax (because we searching for MockBehavior.XXXXX).
255-
if (argument.Expression is not MemberAccessExpressionSyntax expression)
255+
if (argument is not MemberAccessExpressionSyntax expression)
256256
{
257257
return false;
258258
}
@@ -621,7 +621,7 @@ private bool IsMockSetupMethod(InvocationExpressionSyntax invocationExpression,
621621
return true;
622622
}
623623

624-
private bool IsStrictBehaviorArgument(ArgumentSyntax argument, CancellationToken cancellationToken)
624+
private bool IsStrictBehaviorArgument(ExpressionSyntax argument, CancellationToken cancellationToken)
625625
{
626626
if (!this.IsStrictBehaviorArgument(argument, out var memberAccessExpression, cancellationToken))
627627
{

‎tests/Moq.Analyzers.Tests/Analyzers/ConstructorArgumentsAnalyzerTest.cs‎

Lines changed: 89 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -212,15 +212,18 @@ private C()
212212
}
213213

214214
[Theory]
215+
[InlineData("1")]
215216
[InlineData("1, \"B\"")]
216217
[InlineData("1, null")]
217218
[InlineData("default, \"B\"")]
218219
[InlineData("(int)default, default")]
219220
[InlineData("default, null, 1234")]
220221
[InlineData("1, \"An object\", 3, null")]
221222
[InlineData("1, \"An object\", 3, new System.IO.MemoryStream()")]
222-
[InlineData("[\"A\", \"B\"]")]
223-
[InlineData("new object[] { \"A\", \"B\" }")]
223+
[InlineData("[new string[] { \"A\", \"B\" }]")]
224+
[InlineData("new object[] { new string[] { \"A\", \"B\" } }")]
225+
[InlineData("[1]")]
226+
[InlineData("[1, \"A\"]")]
224227
public async Task Arguments_Match(string parameters)
225228
{
226229
var source = @"
@@ -346,8 +349,8 @@ public void TestMethod()
346349
[InlineData("1, \"B\"")]
347350
[InlineData("1, \"An object\", 3, null")]
348351
[InlineData("1, \"An object\", 3, new System.IO.MemoryStream()")]
349-
[InlineData("[\"A\", \"B\"]")]
350-
[InlineData("new object[] { \"A\", \"B\" }")]
352+
[InlineData("[1]")]
353+
[InlineData("[1, \"A\"]")]
351354
public async Task Arguments_Match_WithMockBehavior(string parameters)
352355
{
353356
var source = @"
@@ -421,6 +424,7 @@ public void TestMethod()
421424
[InlineData("\"The string\", 2")]
422425
[InlineData("1, 2, 3, \"The string\"")]
423426
[InlineData("new int[] { 1, 2 }, 1000")]
427+
[InlineData("new object[] { \"A\", \"B\" }, 1000")]
424428
public async Task Arguments_NotMatch(string parameters)
425429
{
426430
var source = @"
@@ -463,6 +467,46 @@ public C(string[] array, int b)
463467
await Verifier.VerifyAnalyzerAsync(source);
464468
}
465469

470+
[Theory]
471+
[InlineData("[]")]
472+
public async Task Arguments_NotMatch_EmptyArray(string parameters)
473+
{
474+
var source = @"
475+
namespace ConsoleApplication1
476+
{
477+
using Moq;
478+
479+
public class TestClass
480+
{
481+
public void TestMethod()
482+
{
483+
var mock = new Mock<C>{|PosInfoMoq2005:(" + parameters + @")|};
484+
}
485+
}
486+
487+
public class C
488+
{
489+
public C(int a)
490+
{
491+
}
492+
493+
public C(int a, string b)
494+
{
495+
}
496+
497+
public C(int a, object c)
498+
{
499+
}
500+
501+
public C(string[] array, int b)
502+
{
503+
}
504+
}
505+
}";
506+
507+
await Verifier.VerifyAnalyzerAsync(source);
508+
}
509+
466510
[Fact]
467511
public async Task Arguments_WithDefaultParameters()
468512
{
@@ -491,7 +535,7 @@ public C(int a = 0, int b = 1, int c = 2, int d = 3)
491535
}
492536

493537
[Fact]
494-
public async Task Arguments_NotMatch_WithNoContructor()
538+
public async Task Arguments_NotMatch_WithNoConstructor()
495539
{
496540
var source = @"
497541
namespace ConsoleApplication1
@@ -556,6 +600,46 @@ public C(string[] array, int b)
556600
await Verifier.VerifyAnalyzerAsync(source);
557601
}
558602

603+
[Theory]
604+
[InlineData("[]")]
605+
public async Task Arguments_NotMatch_WithMockBehavior_EmptyArray(string parameters)
606+
{
607+
var source = @"
608+
namespace ConsoleApplication1
609+
{
610+
using Moq;
611+
612+
public class TestClass
613+
{
614+
public void TestMethod()
615+
{
616+
var mock = new Mock<C>{|PosInfoMoq2005:(MockBehavior.Strict, " + parameters + @")|};
617+
}
618+
}
619+
620+
public class C
621+
{
622+
public C(int a)
623+
{
624+
}
625+
626+
public C(int a, string b)
627+
{
628+
}
629+
630+
public C(int a, object c)
631+
{
632+
}
633+
634+
public C(string[] array, int b)
635+
{
636+
}
637+
}
638+
}";
639+
640+
await Verifier.VerifyAnalyzerAsync(source);
641+
}
642+
559643
[Fact]
560644
public async Task Arguments_NotMatch_WithMockBehavior_WithNoConstructor()
561645
{

0 commit comments

Comments
 (0)