Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 26, 2025

Developers commonly write assertions inside catch blocks intending to verify exception behavior. This pattern is flawed: if no exception is thrown, the test passes silently since the catch block never executes.

// Anti-pattern - test passes even if DoSomething() doesn't throw
try
{
    DoSomething();
}
catch (InvalidOperationException ex)
{
    Assert.AreEqual("expected", ex.Message);  // ℹ️ MSTEST0058
}

// Correct approach
var ex = Assert.ThrowsException<InvalidOperationException>(() => DoSomething());
Assert.AreEqual("expected", ex.Message);

Changes

  • Analyzer: AvoidAssertsInCatchBlocksAnalyzer detects Assert, StringAssert, and CollectionAssert calls within catch blocks by walking the IOperation tree to find ICatchClauseOperation ancestors
  • Tests: 10 test cases covering edge cases (try blocks, finally blocks, nested catches, specific exception types)
  • Localization: Added resources for all 13 supported languages
  • Diagnostic: MSTEST0058, Category: Usage, Severity: Info

Diagnostic Details

ID: MSTEST0058
Severity: Info (informational diagnostic, not a warning)
Message: "Do not use asserts in catch blocks because they may not fail the test if no exception is thrown"
Recommendation: Use Assert.ThrowsException or Assert.ThrowsExceptionAsync to verify exceptions, then assert on the returned exception object outside the try-catch block

[!WARNING]

Original prompt

This section details on the original issue you should resolve

<filter_complete></filter_complete>

<issue_title>Analyzer suggestion: Don't assert in catch blocks</issue_title>
<issue_description>I have seen developers doing this pattern a lot.

try
{
    // code that throws
}
catch
{
    // do some asserts.
}

The developer intent is usually "assert that the code throws, and assert some stuff later" (assertions may or may not use the exception object).

The reality is, this doesn't really assert that the code throws. If the code didn't throw at all, the test will still pass.

Such patterns make the tests very hard to read and understand, and can easily lead to bugs in production that are not caught by tests.

FYI @stan-sz :)</issue_description>

Comments on the Issue (you are @copilot in this section)

Original prompt

This section details on the original issue you should resolve

<issue_title>Analyzer suggestion: Don't assert in catch blocks</issue_title>
<issue_description>I have seen developers doing this pattern a lot.

try
{
    // code that throws
}
catch
{
    // do some asserts.
}

The developer intent is usually "assert that the code throws, and assert some stuff later" (assertions may or may not use the exception object).

The reality is, this doesn't really assert that the code throws. If the code didn't throw at all, the test will still pass.

Such patterns make the tests very hard to read and understand, and can easily lead to bugs in production that are not caught by tests.

FYI @stan-sz :)</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Refactor analyzer to disallow asserts in catch blocks Add analyzer to detect assertions in catch blocks (MSTEST0058) Oct 26, 2025
Copilot AI requested a review from Evangelink October 26, 2025 10:01
Copilot finished work on behalf of Evangelink October 26, 2025 10:01
@Evangelink Evangelink marked this pull request as ready for review October 26, 2025 10:38
@Evangelink Evangelink enabled auto-merge (squash) October 27, 2025 05:37
Copilot AI requested a review from Evangelink October 27, 2025 12:43
Copilot finished work on behalf of Evangelink October 27, 2025 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Analyzer suggestion: Don't assert in catch blocks

4 participants