diff --git a/src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md b/src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md index f2b7fad657..db7d4e4da2 100644 --- a/src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md @@ -1,2 +1,8 @@ ; Unshipped analyzer release ; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md + +### New Rules + +Rule ID | Category | Severity | Notes +--------|----------|----------|------- +MSTEST0058 | Usage | Info | AvoidAssertsInCatchBlocksAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0058) diff --git a/src/Analyzers/MSTest.Analyzers/AvoidAssertsInCatchBlocksAnalyzer.cs b/src/Analyzers/MSTest.Analyzers/AvoidAssertsInCatchBlocksAnalyzer.cs new file mode 100644 index 0000000000..d7316bd9a8 --- /dev/null +++ b/src/Analyzers/MSTest.Analyzers/AvoidAssertsInCatchBlocksAnalyzer.cs @@ -0,0 +1,99 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Collections.Immutable; + +using Analyzer.Utilities.Extensions; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +using MSTest.Analyzers.Helpers; + +namespace MSTest.Analyzers; + +/// +/// MSTEST0058: . +/// +[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)] +public sealed class AvoidAssertsInCatchBlocksAnalyzer : DiagnosticAnalyzer +{ + private static readonly LocalizableResourceString Title = new(nameof(Resources.AvoidAssertsInCatchBlocksTitle), Resources.ResourceManager, typeof(Resources)); + private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.AvoidAssertsInCatchBlocksMessageFormat), Resources.ResourceManager, typeof(Resources)); + private static readonly LocalizableResourceString Description = new(nameof(Resources.AvoidAssertsInCatchBlocksDescription), Resources.ResourceManager, typeof(Resources)); + + internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create( + DiagnosticIds.AvoidAssertsInCatchBlocksRuleId, + Title, + MessageFormat, + Description, + Category.Usage, + DiagnosticSeverity.Info, + isEnabledByDefault: true); + + /// + public override ImmutableArray SupportedDiagnostics { get; } + = ImmutableArray.Create(Rule); + + /// + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + + context.RegisterCompilationStartAction(context => + { + Compilation compilation = context.Compilation; + INamedTypeSymbol? assertSymbol = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingAssert); + INamedTypeSymbol? stringAssertSymbol = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingStringAssert); + INamedTypeSymbol? collectionAssertSymbol = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingCollectionAssert); + + if (assertSymbol is not null || stringAssertSymbol is not null || collectionAssertSymbol is not null) + { + context.RegisterOperationAction(context => AnalyzeOperation(context, assertSymbol, stringAssertSymbol, collectionAssertSymbol), OperationKind.Invocation); + } + }); + } + + private static void AnalyzeOperation( + OperationAnalysisContext context, + INamedTypeSymbol? assertSymbol, + INamedTypeSymbol? stringAssertSymbol, + INamedTypeSymbol? collectionAssertSymbol) + { + var operation = (IInvocationOperation)context.Operation; + INamedTypeSymbol targetType = operation.TargetMethod.ContainingType; + bool isAssertType = + targetType.Equals(assertSymbol, SymbolEqualityComparer.Default) || + targetType.Equals(stringAssertSymbol, SymbolEqualityComparer.Default) || + targetType.Equals(collectionAssertSymbol, SymbolEqualityComparer.Default); + + if (!isAssertType) + { + return; + } + + // Walk up the operation tree to check if we're inside a catch clause + if (IsInsideCatchClause(operation)) + { + context.ReportDiagnostic(operation.CreateDiagnostic(Rule)); + } + } + + private static bool IsInsideCatchClause(IOperation operation) + { + IOperation? current = operation; + while (current is not null) + { + if (current is ICatchClauseOperation) + { + return true; + } + + current = current.Parent; + } + + return false; + } +} diff --git a/src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs b/src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs index d7f58d9e67..15b809366e 100644 --- a/src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs +++ b/src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs @@ -62,4 +62,5 @@ internal static class DiagnosticIds public const string IgnoreStringMethodReturnValueRuleId = "MSTEST0055"; public const string TestMethodAttributeShouldSetDisplayNameCorrectlyRuleId = "MSTEST0056"; public const string TestMethodAttributeShouldPropagateSourceInformationRuleId = "MSTEST0057"; + public const string AvoidAssertsInCatchBlocksRuleId = "MSTEST0058"; } diff --git a/src/Analyzers/MSTest.Analyzers/Resources.resx b/src/Analyzers/MSTest.Analyzers/Resources.resx index 0c8a17d969..85df4da66e 100644 --- a/src/Analyzers/MSTest.Analyzers/Resources.resx +++ b/src/Analyzers/MSTest.Analyzers/Resources.resx @@ -684,4 +684,13 @@ The type declaring these methods should also respect the following rules: Methods like Contains, StartsWith, and EndsWith return boolean values that indicate whether the condition was met. Ignoring these return values is likely a mistake. + + Do not use asserts in catch blocks + + + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + + + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + \ No newline at end of file diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf index ba62836f66..b7b156acca 100644 --- a/src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf +++ b/src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf @@ -989,6 +989,21 @@ Typ deklarující tyto metody by měl také respektovat následující pravidla: Parametr Assert.Throws by měl obsahovat jenom jeden příkaz nebo výraz + + Do not use asserts in catch blocks + Do not use asserts in catch blocks + + + + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + + + + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + + \ No newline at end of file diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf index 76737e3938..5a331eb3c6 100644 --- a/src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf +++ b/src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf @@ -990,6 +990,21 @@ Der Typ, der diese Methoden deklariert, sollte auch die folgenden Regeln beachte Assert.Throws darf nur eine einzelne Anweisung/einen einzelnen Ausdruck enthalten + + Do not use asserts in catch blocks + Do not use asserts in catch blocks + + + + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + + + + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + + \ No newline at end of file diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf index 3e7bb27bc0..b5ad5d9582 100644 --- a/src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf +++ b/src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf @@ -989,6 +989,21 @@ El tipo que declara estos métodos también debe respetar las reglas siguientes: Assert.Throws debe contener solo una única instrucción o expresión + + Do not use asserts in catch blocks + Do not use asserts in catch blocks + + + + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + + + + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + + \ No newline at end of file diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf index 8b30fdd14e..bf6b4b71e9 100644 --- a/src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf +++ b/src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf @@ -989,6 +989,21 @@ Le type doit être une classe Assert.Throws ne doit contenir qu’une seule instruction + + Do not use asserts in catch blocks + Do not use asserts in catch blocks + + + + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + + + + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + + \ No newline at end of file diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.it.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.it.xlf index 06dd23cd42..98b9f2dc46 100644 --- a/src/Analyzers/MSTest.Analyzers/xlf/Resources.it.xlf +++ b/src/Analyzers/MSTest.Analyzers/xlf/Resources.it.xlf @@ -989,6 +989,21 @@ Anche il tipo che dichiara questi metodi deve rispettare le regole seguenti: Assert.Throws deve contenere solo una singola istruzione/espressione + + Do not use asserts in catch blocks + Do not use asserts in catch blocks + + + + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + + + + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + + \ No newline at end of file diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.ja.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.ja.xlf index ab704cbb51..143099764f 100644 --- a/src/Analyzers/MSTest.Analyzers/xlf/Resources.ja.xlf +++ b/src/Analyzers/MSTest.Analyzers/xlf/Resources.ja.xlf @@ -989,6 +989,21 @@ The type declaring these methods should also respect the following rules: Assert.Throws には 1 つのステートメントまたは式のみを含める必要があります + + Do not use asserts in catch blocks + Do not use asserts in catch blocks + + + + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + + + + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + + \ No newline at end of file diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.ko.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.ko.xlf index e1579bc142..ba20940467 100644 --- a/src/Analyzers/MSTest.Analyzers/xlf/Resources.ko.xlf +++ b/src/Analyzers/MSTest.Analyzers/xlf/Resources.ko.xlf @@ -989,6 +989,21 @@ The type declaring these methods should also respect the following rules: Assert.Throws는 단일 문/식만 포함해야 합니다. + + Do not use asserts in catch blocks + Do not use asserts in catch blocks + + + + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + + + + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + + \ No newline at end of file diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.pl.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.pl.xlf index 8b72c8d6e2..0cddbbe3a1 100644 --- a/src/Analyzers/MSTest.Analyzers/xlf/Resources.pl.xlf +++ b/src/Analyzers/MSTest.Analyzers/xlf/Resources.pl.xlf @@ -989,6 +989,21 @@ Typ deklarujący te metody powinien również przestrzegać następujących regu Element Assert.Throws powinien zawierać tylko jedną instrukcję/wyrażenie + + Do not use asserts in catch blocks + Do not use asserts in catch blocks + + + + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + + + + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + + \ No newline at end of file diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.pt-BR.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.pt-BR.xlf index 3e9f3a2913..455d4571c1 100644 --- a/src/Analyzers/MSTest.Analyzers/xlf/Resources.pt-BR.xlf +++ b/src/Analyzers/MSTest.Analyzers/xlf/Resources.pt-BR.xlf @@ -989,6 +989,21 @@ O tipo que declara esses métodos também deve respeitar as seguintes regras: Assert.Throws deve conter apenas uma única declaração/expressão + + Do not use asserts in catch blocks + Do not use asserts in catch blocks + + + + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + + + + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + + \ No newline at end of file diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.ru.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.ru.xlf index 2a32db08a3..fdb2b4f9bb 100644 --- a/src/Analyzers/MSTest.Analyzers/xlf/Resources.ru.xlf +++ b/src/Analyzers/MSTest.Analyzers/xlf/Resources.ru.xlf @@ -1001,6 +1001,21 @@ The type declaring these methods should also respect the following rules: Assert.Throws должен содержать только одну инструкцию/выражение + + Do not use asserts in catch blocks + Do not use asserts in catch blocks + + + + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + + + + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + + \ No newline at end of file diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.tr.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.tr.xlf index 4c959ae0e5..8b5b8691a7 100644 --- a/src/Analyzers/MSTest.Analyzers/xlf/Resources.tr.xlf +++ b/src/Analyzers/MSTest.Analyzers/xlf/Resources.tr.xlf @@ -991,6 +991,21 @@ Bu yöntemleri bildiren tipin ayrıca aşağıdaki kurallara uyması gerekir: Assert.Throws yalnızca tek bir deyim/ifade içermelidir. + + Do not use asserts in catch blocks + Do not use asserts in catch blocks + + + + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + + + + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + + \ No newline at end of file diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hans.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hans.xlf index 2058347afa..2bea1eda8d 100644 --- a/src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hans.xlf +++ b/src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hans.xlf @@ -989,6 +989,21 @@ The type declaring these methods should also respect the following rules: Assert.Throws 应仅包含单个语句/表达式 + + Do not use asserts in catch blocks + Do not use asserts in catch blocks + + + + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + + + + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + + \ No newline at end of file diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hant.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hant.xlf index 228d78acdd..088053f50b 100644 --- a/src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hant.xlf +++ b/src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hant.xlf @@ -989,6 +989,21 @@ The type declaring these methods should also respect the following rules: Assert.Throws 應該只包含單一陳述式/運算式 + + Do not use asserts in catch blocks + Do not use asserts in catch blocks + + + + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + Do not use asserts in catch blocks because they may not fail the test if no exception is thrown + + + + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + Using asserts in catch blocks is problematic because the test will pass even if no exception is thrown and the catch block is never executed. Use 'Assert.Throws', 'Assert.ThrowsExactly', 'Assert.ThrowsAsync' or 'Assert.ThrowsExactlyAsync' to verify that an exception is thrown, and then make additional assertions on the caught exception without using the try-catch block. + + \ No newline at end of file diff --git a/test/UnitTests/MSTest.Analyzers.UnitTests/AvoidAssertsInCatchBlocksAnalyzerTests.cs b/test/UnitTests/MSTest.Analyzers.UnitTests/AvoidAssertsInCatchBlocksAnalyzerTests.cs new file mode 100644 index 0000000000..7ba1a06e1f --- /dev/null +++ b/test/UnitTests/MSTest.Analyzers.UnitTests/AvoidAssertsInCatchBlocksAnalyzerTests.cs @@ -0,0 +1,297 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using VerifyCS = MSTest.Analyzers.Test.CSharpCodeFixVerifier< + MSTest.Analyzers.AvoidAssertsInCatchBlocksAnalyzer, + Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>; + +namespace MSTest.Analyzers.UnitTests; + +[TestClass] +public sealed class AvoidAssertsInCatchBlocksAnalyzerTests +{ + [TestMethod] + public async Task AssertOutsideCatchBlock_NoDiagnostic() + { + string code = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod() + { + Assert.IsTrue(true); + } + } + """; + + await VerifyCS.VerifyCodeFixAsync(code, code); + } + + [TestMethod] + public async Task AssertInTryBlock_NoDiagnostic() + { + string code = """ + using System; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod() + { + try + { + Assert.IsTrue(true); + } + catch + { + } + } + } + """; + + await VerifyCS.VerifyCodeFixAsync(code, code); + } + + [TestMethod] + public async Task AssertInCatchBlock_Diagnostic() + { + string code = """ + using System; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod() + { + try + { + // code that may throw + } + catch + { + [|Assert.Fail("Exception was thrown")|]; + } + } + } + """; + + await VerifyCS.VerifyCodeFixAsync(code, code); + } + + [TestMethod] + public async Task AssertInCatchBlockWithExceptionVariable_Diagnostic() + { + string code = """ + using System; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod() + { + try + { + throw new InvalidOperationException("test"); + } + catch (InvalidOperationException ex) + { + [|Assert.IsNotNull(ex)|]; + } + } + } + """; + + await VerifyCS.VerifyCodeFixAsync(code, code); + } + + [TestMethod] + public async Task StringAssertInCatchBlock_Diagnostic() + { + string code = """ + using System; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod() + { + try + { + throw new InvalidOperationException("test message"); + } + catch (InvalidOperationException ex) + { + [|StringAssert.Contains(ex.Message, "test")|]; + } + } + } + """; + + await VerifyCS.VerifyCodeFixAsync(code, code); + } + + [TestMethod] + public async Task CollectionAssertInCatchBlock_Diagnostic() + { + string code = """ + using System; + using System.Collections.Generic; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod() + { + try + { + throw new InvalidOperationException("test"); + } + catch + { + var list = new List { 1, 2, 3 }; + [|CollectionAssert.AreEqual(new List { 1, 2, 3 }, list)|]; + } + } + } + """; + + await VerifyCS.VerifyCodeFixAsync(code, code); + } + + [TestMethod] + public async Task MultipleAssertsInCatchBlock_MultipleDiagnostics() + { + string code = """ + using System; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod() + { + try + { + throw new InvalidOperationException("test"); + } + catch (InvalidOperationException ex) + { + [|Assert.IsNotNull(ex)|]; + [|Assert.AreEqual("test", ex.Message)|]; + } + } + } + """; + + await VerifyCS.VerifyCodeFixAsync(code, code); + } + + [TestMethod] + public async Task AssertInFinallyBlock_NoDiagnostic() + { + string code = """ + using System; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod() + { + try + { + // code + } + finally + { + Assert.IsTrue(true); + } + } + } + """; + + await VerifyCS.VerifyCodeFixAsync(code, code); + } + + [TestMethod] + public async Task AssertInNestedCatchBlock_Diagnostic() + { + string code = """ + using System; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod() + { + try + { + try + { + throw new InvalidOperationException(); + } + catch + { + [|Assert.Fail("Inner exception")|]; + } + } + catch + { + [|Assert.Fail("Outer exception")|]; + } + } + } + """; + + await VerifyCS.VerifyCodeFixAsync(code, code); + } + + [TestMethod] + public async Task AssertInCatchBlockWithSpecificExceptionType_Diagnostic() + { + string code = """ + using System; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod() + { + try + { + throw new ArgumentNullException("param"); + } + catch (ArgumentNullException ex) + { + [|Assert.IsNotNull(ex)|]; + } + catch (Exception ex) + { + [|Assert.Fail("Wrong exception type")|]; + } + } + } + """; + + await VerifyCS.VerifyCodeFixAsync(code, code); + } +}