Skip to content

Commit ef3b985

Browse files
Youssef1313Evangelink
Youssef1313
and
Evangelink
authoredJan 9, 2025··
Fix ExpectedException codefix to handle few edge cases (#4560)
Co-authored-by: Amaury Levé <amauryleve@microsoft.com>
·
v3.9.0-preview.25167.10v3.8.0
1 parent b9eda74 commit ef3b985

File tree

2 files changed

+457
-13
lines changed

2 files changed

+457
-13
lines changed
 

‎src/Analyzers/MSTest.Analyzers.CodeFixes/AvoidExpectedExceptionAttributeFixer.cs

+77-8
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,67 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
8686
diagnostic);
8787
}
8888

89+
private static (SyntaxNode ExpressionOrStatement, SyntaxNode NodeToReplace)? TryGetExpressionOfInterestAndNodeToFromBlockSyntax(BlockSyntax? block)
90+
{
91+
if (block is null)
92+
{
93+
return null;
94+
}
95+
96+
for (int i = block.Statements.Count - 1; i >= 0; i--)
97+
{
98+
StatementSyntax statement = block.Statements[i];
99+
100+
if (statement is LockStatementSyntax lockStatement)
101+
{
102+
if (lockStatement.Statement is BlockSyntax lockBlock)
103+
{
104+
if (TryGetExpressionOfInterestAndNodeToFromBlockSyntax(lockBlock) is { } resultFromLock)
105+
{
106+
return resultFromLock;
107+
}
108+
109+
continue;
110+
}
111+
112+
statement = lockStatement.Statement;
113+
}
114+
115+
if (statement is LocalFunctionStatementSyntax or EmptyStatementSyntax)
116+
{
117+
continue;
118+
}
119+
else if (statement is BlockSyntax nestedBlock)
120+
{
121+
if (TryGetExpressionOfInterestAndNodeToFromBlockSyntax(nestedBlock) is { } expressionFromNestedBlock)
122+
{
123+
return expressionFromNestedBlock;
124+
}
125+
126+
// The BlockSyntax doesn't have any meaningful statements/expressions.
127+
// Ignore it.
128+
continue;
129+
}
130+
else if (statement is ExpressionStatementSyntax expressionStatement)
131+
{
132+
return (expressionStatement.Expression, statement);
133+
}
134+
else if (statement is LocalDeclarationStatementSyntax localDeclarationStatementSyntax &&
135+
localDeclarationStatementSyntax.Declaration.Variables.Count == 1 &&
136+
localDeclarationStatementSyntax.Declaration.Variables[0].Initializer is { } initializer)
137+
{
138+
return (initializer.Value, statement);
139+
}
140+
141+
return (statement, statement);
142+
}
143+
144+
return null;
145+
}
146+
147+
private static (SyntaxNode ExpressionOrStatement, SyntaxNode NodeToReplace)? TryGetExpressionOfInterestAndNodeToFromExpressionBody(MethodDeclarationSyntax method)
148+
=> method.ExpressionBody is null ? null : (method.ExpressionBody.Expression, method.ExpressionBody.Expression);
149+
89150
private static async Task<Document> WrapLastStatementWithAssertThrowsExceptionAsync(
90151
Document document,
91152
MethodDeclarationSyntax methodDeclaration,
@@ -97,20 +158,28 @@ private static async Task<Document> WrapLastStatementWithAssertThrowsExceptionAs
97158
DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
98159
editor.RemoveNode(attributeSyntax);
99160

100-
SyntaxNode? oldStatement = (SyntaxNode?)methodDeclaration.Body?.Statements.LastOrDefault() ?? methodDeclaration.ExpressionBody?.Expression;
101-
if (oldStatement is null)
161+
(SyntaxNode ExpressionOrStatement, SyntaxNode NodeToReplace)? expressionAndNodeToReplace = TryGetExpressionOfInterestAndNodeToFromBlockSyntax(methodDeclaration.Body)
162+
?? TryGetExpressionOfInterestAndNodeToFromExpressionBody(methodDeclaration);
163+
164+
if (expressionAndNodeToReplace is null)
102165
{
103166
return editor.GetChangedDocument();
104167
}
105168

106-
SyntaxNode newLambdaExpression = oldStatement switch
169+
SyntaxGenerator generator = editor.Generator;
170+
SyntaxNode expressionToUseInLambda = expressionAndNodeToReplace.Value.ExpressionOrStatement;
171+
172+
expressionToUseInLambda = expressionToUseInLambda switch
107173
{
108-
ExpressionStatementSyntax oldLambdaExpression => oldLambdaExpression.Expression,
109-
_ => oldStatement,
174+
ThrowStatementSyntax { Expression: not null } throwStatement => generator.ThrowExpression(throwStatement.Expression),
175+
// This is the case when the last statement of the method body is a loop for example (e.g, for, foreach, while, do while).
176+
// It can also happen for using statement, or switch statement.
177+
// In that case, we need to wrap in a block syntax (i.e, curly braces)
178+
StatementSyntax expressionToUseAsStatement => SyntaxFactory.Block(expressionToUseAsStatement),
179+
_ => expressionToUseInLambda.WithoutTrivia(),
110180
};
111181

112-
SyntaxGenerator generator = editor.Generator;
113-
newLambdaExpression = generator.VoidReturningLambdaExpression(newLambdaExpression);
182+
SyntaxNode newLambdaExpression = generator.VoidReturningLambdaExpression(expressionToUseInLambda);
114183

115184
bool containsAsyncCode = newLambdaExpression.DescendantNodesAndSelf().Any(n => n is AwaitExpressionSyntax);
116185
if (containsAsyncCode)
@@ -142,7 +211,7 @@ private static async Task<Document> WrapLastStatementWithAssertThrowsExceptionAs
142211
newStatement = generator.ExpressionStatement(newStatement);
143212
}
144213

145-
editor.ReplaceNode(oldStatement, newStatement);
214+
editor.ReplaceNode(expressionAndNodeToReplace.Value.NodeToReplace, newStatement.WithTriviaFrom(expressionAndNodeToReplace.Value.NodeToReplace));
146215
return editor.GetChangedDocument();
147216
}
148217
}

‎test/UnitTests/MSTest.Analyzers.UnitTests/AvoidExpectedExceptionAttributeAnalyzerTests.cs

+380-5
Original file line numberDiff line numberDiff line change
@@ -450,11 +450,10 @@ public class TestClass
450450
public async Task TestMethod()
451451
{
452452
Console.WriteLine("Hello, world!");
453-
await Assert.ThrowsExactlyAsync<Exception>(async () =>
454-
// In ideal world, it's best if the codefix can separate await M() to a
455-
// variable, then only wrap M(someVariable) in Assert.ThrowsException
456-
// Let's also have this comment serve as a test for trivia ;)
457-
M(await M()));
453+
// In ideal world, it's best if the codefix can separate await M() to a
454+
// variable, then only wrap M(someVariable) in Assert.ThrowsException
455+
// Let's also have this comment serve as a test for trivia ;)
456+
await Assert.ThrowsExactlyAsync<Exception>(async () => M(await M()));
458457
}
459458
460459
private static Task<int> M() => Task.FromResult(0);
@@ -465,4 +464,380 @@ await Assert.ThrowsExactlyAsync<Exception>(async () =>
465464

466465
await VerifyCS.VerifyCodeFixAsync(code, fixedCode);
467466
}
467+
468+
[TestMethod]
469+
public async Task When_BlockEndsWithLocalFunctions_Should_ConsiderPreviousStatements()
470+
{
471+
string code = """
472+
using System;
473+
using Microsoft.VisualStudio.TestTools.UnitTesting;
474+
475+
[TestClass]
476+
public class TestClass
477+
{
478+
[ExpectedException(typeof(System.Exception))]
479+
[TestMethod]
480+
public void [|TestMethod|]()
481+
{
482+
M1();
483+
M2();
484+
485+
void M1() { }
486+
void M2() { }
487+
}
488+
}
489+
""";
490+
491+
string fixedCode = """
492+
using System;
493+
using Microsoft.VisualStudio.TestTools.UnitTesting;
494+
495+
[TestClass]
496+
public class TestClass
497+
{
498+
[TestMethod]
499+
public void TestMethod()
500+
{
501+
M1();
502+
Assert.ThrowsExactly<Exception>(() => M2());
503+
504+
void M1() { }
505+
void M2() { }
506+
}
507+
}
508+
""";
509+
510+
await VerifyCS.VerifyCodeFixAsync(code, fixedCode);
511+
}
512+
513+
[TestMethod]
514+
public async Task When_BlockEndsWithLocalVariableDeclaration_Should_NotCrash()
515+
{
516+
string code = """
517+
using System;
518+
using Microsoft.VisualStudio.TestTools.UnitTesting;
519+
520+
[TestClass]
521+
public class TestClass
522+
{
523+
[ExpectedException(typeof(System.Exception))]
524+
[TestMethod]
525+
public void [|TestMethod|]()
526+
{
527+
var x = Console.ReadLine();
528+
}
529+
}
530+
""";
531+
532+
string fixedCode = """
533+
using System;
534+
using Microsoft.VisualStudio.TestTools.UnitTesting;
535+
536+
[TestClass]
537+
public class TestClass
538+
{
539+
[TestMethod]
540+
public void TestMethod()
541+
{
542+
Assert.ThrowsExactly<Exception>(() => Console.ReadLine());
543+
}
544+
}
545+
""";
546+
547+
await VerifyCS.VerifyCodeFixAsync(code, fixedCode);
548+
}
549+
550+
[TestMethod]
551+
public async Task When_BlockEndsWithAssignment_Should_NotCrash()
552+
{
553+
string code = """
554+
using System;
555+
using Microsoft.VisualStudio.TestTools.UnitTesting;
556+
557+
[TestClass]
558+
public class TestClass
559+
{
560+
[ExpectedException(typeof(System.Exception))]
561+
[TestMethod]
562+
public void [|TestMethod|]()
563+
{
564+
object x;
565+
x = Console.ReadLine();
566+
}
567+
568+
[ExpectedException(typeof(System.Exception))]
569+
[TestMethod]
570+
public void [|TestMethod2|]()
571+
{
572+
_ = Console.ReadLine();
573+
}
574+
}
575+
""";
576+
577+
string fixedCode = """
578+
using System;
579+
using Microsoft.VisualStudio.TestTools.UnitTesting;
580+
581+
[TestClass]
582+
public class TestClass
583+
{
584+
[TestMethod]
585+
public void TestMethod()
586+
{
587+
object x;
588+
Assert.ThrowsExactly<Exception>(() => x = Console.ReadLine());
589+
}
590+
591+
[TestMethod]
592+
public void TestMethod2()
593+
{
594+
Assert.ThrowsExactly<Exception>(() => _ = Console.ReadLine());
595+
}
596+
}
597+
""";
598+
599+
await VerifyCS.VerifyCodeFixAsync(code, fixedCode);
600+
}
601+
602+
[TestMethod]
603+
public async Task When_BlockEndsWithNestedBlocks_Should_NotCrash()
604+
{
605+
string code = """
606+
using System;
607+
using Microsoft.VisualStudio.TestTools.UnitTesting;
608+
609+
[TestClass]
610+
public class TestClass
611+
{
612+
[ExpectedException(typeof(System.Exception))]
613+
[TestMethod]
614+
public void [|TestMethod|]()
615+
{
616+
object x;
617+
618+
{
619+
{
620+
}
621+
{
622+
x = Console.ReadLine();
623+
}
624+
{
625+
}
626+
}
627+
}
628+
}
629+
""";
630+
631+
string fixedCode = """
632+
using System;
633+
using Microsoft.VisualStudio.TestTools.UnitTesting;
634+
635+
[TestClass]
636+
public class TestClass
637+
{
638+
[TestMethod]
639+
public void TestMethod()
640+
{
641+
object x;
642+
643+
{
644+
{
645+
}
646+
{
647+
Assert.ThrowsExactly<Exception>(() => x = Console.ReadLine());
648+
}
649+
{
650+
}
651+
}
652+
}
653+
}
654+
""";
655+
656+
await VerifyCS.VerifyCodeFixAsync(code, fixedCode);
657+
}
658+
659+
[TestMethod]
660+
public async Task When_BlockEndsWithEmptyStatement_Should_BeIgnored()
661+
{
662+
string code = """
663+
using System;
664+
using Microsoft.VisualStudio.TestTools.UnitTesting;
665+
666+
[TestClass]
667+
public class TestClass
668+
{
669+
[ExpectedException(typeof(System.Exception))]
670+
[TestMethod]
671+
public void [|TestMethod|]()
672+
{
673+
Console.WriteLine();;
674+
}
675+
}
676+
""";
677+
678+
string fixedCode = """
679+
using System;
680+
using Microsoft.VisualStudio.TestTools.UnitTesting;
681+
682+
[TestClass]
683+
public class TestClass
684+
{
685+
[TestMethod]
686+
public void TestMethod()
687+
{
688+
Assert.ThrowsExactly<Exception>(() => Console.WriteLine()); ;
689+
}
690+
}
691+
""";
692+
693+
await VerifyCS.VerifyCodeFixAsync(code, fixedCode);
694+
}
695+
696+
[TestMethod]
697+
public async Task When_BlockEndsWithForEach_Should_NotCrash()
698+
{
699+
string code = """
700+
using System;
701+
using Microsoft.VisualStudio.TestTools.UnitTesting;
702+
703+
[TestClass]
704+
public class TestClass
705+
{
706+
[ExpectedException(typeof(System.Exception))]
707+
[TestMethod]
708+
public void [|TestMethod|]()
709+
{
710+
foreach (var x in new[] { 1, 2, 3 })
711+
{
712+
Console.WriteLine(x);
713+
}
714+
}
715+
}
716+
""";
717+
718+
// TODO: Formatting is so broken here.
719+
// See https://github.com/microsoft/testfx/issues/4570
720+
string fixedCode = """
721+
using System;
722+
using Microsoft.VisualStudio.TestTools.UnitTesting;
723+
724+
[TestClass]
725+
public class TestClass
726+
{
727+
[TestMethod]
728+
public void TestMethod()
729+
{
730+
Assert.ThrowsExactly<Exception>(() =>
731+
{
732+
foreach (var x in new[] { 1, 2, 3 })
733+
{
734+
Console.WriteLine(x);
735+
}
736+
});
737+
}
738+
}
739+
""";
740+
741+
await VerifyCS.VerifyCodeFixAsync(code, fixedCode);
742+
}
743+
744+
[TestMethod]
745+
public async Task When_BlockEndsWithThrowStatement_Should_NotBeWrappedInBlock()
746+
{
747+
string code = """
748+
using System;
749+
using Microsoft.VisualStudio.TestTools.UnitTesting;
750+
751+
[TestClass]
752+
public class TestClass
753+
{
754+
[ExpectedException(typeof(System.Exception))]
755+
[TestMethod]
756+
public void [|TestMethod|]()
757+
{
758+
throw new Exception();
759+
}
760+
}
761+
""";
762+
763+
string fixedCode = """
764+
using System;
765+
using Microsoft.VisualStudio.TestTools.UnitTesting;
766+
767+
[TestClass]
768+
public class TestClass
769+
{
770+
[TestMethod]
771+
public void TestMethod()
772+
{
773+
Assert.ThrowsExactly<Exception>(() => throw new Exception());
774+
}
775+
}
776+
""";
777+
778+
await VerifyCS.VerifyCodeFixAsync(code, fixedCode);
779+
}
780+
781+
[TestMethod]
782+
public async Task When_BlockEndsWithNestedLockStatements_Should_NotCrash()
783+
{
784+
string code = """
785+
using System;
786+
using Microsoft.VisualStudio.TestTools.UnitTesting;
787+
788+
[TestClass]
789+
public class TestClass
790+
{
791+
[ExpectedException(typeof(System.Exception))]
792+
[TestMethod]
793+
public void [|TestMethod|]()
794+
{
795+
object x = new();
796+
lock (x)
797+
{
798+
lock (x)
799+
{
800+
}
801+
lock (x)
802+
{
803+
Console.WriteLine();
804+
}
805+
lock (x)
806+
{
807+
}
808+
}
809+
}
810+
}
811+
""";
812+
813+
string fixedCode = """
814+
using System;
815+
using Microsoft.VisualStudio.TestTools.UnitTesting;
816+
817+
[TestClass]
818+
public class TestClass
819+
{
820+
[TestMethod]
821+
public void TestMethod()
822+
{
823+
object x = new();
824+
lock (x)
825+
{
826+
lock (x)
827+
{
828+
}
829+
lock (x)
830+
{
831+
Assert.ThrowsExactly<Exception>(() => Console.WriteLine());
832+
}
833+
lock (x)
834+
{
835+
}
836+
}
837+
}
838+
}
839+
""";
840+
841+
await VerifyCS.VerifyCodeFixAsync(code, fixedCode);
842+
}
468843
}

0 commit comments

Comments
 (0)
Please sign in to comment.