Skip to content

[code-simplifier] Simplify Assert code-fix helpers: defer async root fetch, inline adapter lambda#8860

Closed
Evangelink wants to merge 1 commit into
mainfrom
code-simplifier/assert-fixer-cleanup-668f3ffbb9f050c1
Closed

[code-simplifier] Simplify Assert code-fix helpers: defer async root fetch, inline adapter lambda#8860
Evangelink wants to merge 1 commit into
mainfrom
code-simplifier/assert-fixer-cleanup-668f3ffbb9f050c1

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Code Simplification — 2026-06-05

This PR simplifies code introduced in #8848 (the String/Collection Assert code-fix scaffolding refactor) to improve clarity and code flow while preserving all functionality.

Files Simplified

  • src/Analyzers/MSTest.Analyzers.CodeFixes/AssertToAssertFixerHelpers.cs — defer async root acquisition until after cheap synchronous checks
  • src/Analyzers/MSTest.Analyzers.CodeFixes/StringAssertToAssertFixer.cs — inline adapter lambda, remove obvious comments

Improvements Made

  1. Deferred async work in AssertToAssertFixerHelpers.RegisterCodeFixAsync

    • SyntaxNode root = await context.Document.GetRequiredSyntaxRootAsync(...) was fetched unconditionally at the top of the method, before two cheap Dictionary.TryGetValue checks that can cause early returns.
    • Moving the await after those guards skips the async round-trip when the diagnostic properties are missing, which is consistent with the "synchronous guards first, async work second" pattern.
  2. Removed FixAssertAsync adapter method from StringAssertToAssertFixer

    • The method was a pure bridge: it accepted the shared callback signature (including string? fixKind) but immediately ignored fixKind and forwarded to FixStringAssertAsync.
    • Replaced with an inline lambda (doc, expr, methodName, _, ct) => FixStringAssertAsync(doc, expr, methodName, ct) where the _ discard explicitly signals that fixKind is not used for this fixer. This removes 9 lines without any loss of clarity.
  3. Removed two obvious comments from FixStringAssertAsync

    • // Check if the invocation expression has a member access expression — the pattern match immediately below is self-explanatory.
    • // Replace StringAssert with Assert in the member access expression / // Change StringAssert.MethodName to Assert.ProperMethodName — two lines saying the same thing about obvious code.

Changes Based On

Testing

  • ✅ No behavioral changes — all modifications are purely structural
  • ✅ Lambda matches Func<Document, InvocationExpressionSyntax, string, string?, CancellationToken, Task<Document>> exactly
  • ✅ Root reordering does not affect any code path that passes the guards

Review Focus

Please verify:

  • The _ discard in the lambda is idiomatic in this codebase
  • The early-return order in RegisterCodeFixAsync still makes logical sense

Automated by Code Simplifier Agent

Generated by Code Simplifier · sonnet46 2.7M ·

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/code-simplifier.md@main
  • expires on Jun 6, 2026, 2:15 PM UTC

…ter lambda

- AssertToAssertFixerHelpers.RegisterCodeFixAsync: move SyntaxNode root
  acquisition after the cheap synchronous diagnostic-property checks so
  the async round-trip is skipped on early returns.

- StringAssertToAssertFixer.RegisterCodeFixesAsync: replace the pure
  FixAssertAsync adapter method (which ignored fixKind and just forwarded
  to FixStringAssertAsync) with an inline lambda; the _ discard makes the
  unused fixKind parameter explicit at the call site.

- Remove two obvious comments from FixStringAssertAsync that described
  self-evident code.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 5, 2026 14:15
@Evangelink Evangelink added type/automation Created or maintained by an agentic workflow. type/tech-debt Code health, refactoring, simplification. labels Jun 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR simplifies the MSTest analyzers’ Assert-migration code-fix scaffolding (introduced in #8848) by reducing unnecessary async work and removing a trivial adapter method, while preserving behavior.

Changes:

  • Deferred GetRequiredSyntaxRootAsync in AssertToAssertFixerHelpers.RegisterCodeFixAsync until after cheap diagnostic-property guard checks.
  • Inlined the FixAssertAsync adapter in StringAssertToAssertFixer with a lambda that explicitly discards the unused fixKind parameter.
  • Removed a few redundant comments in FixStringAssertAsync to reduce noise.
Show a summary per file
File Description
src/Analyzers/MSTest.Analyzers.CodeFixes/StringAssertToAssertFixer.cs Removes the adapter method and inlines a non-capturing lambda (with _ discard) while keeping the same rewrite behavior.
src/Analyzers/MSTest.Analyzers.CodeFixes/AssertToAssertFixerHelpers.cs Reorders async syntax-root acquisition to avoid doing it on code paths that return early due to missing diagnostic properties.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

@Evangelink Evangelink marked this pull request as ready for review June 5, 2026 15:01
@Evangelink Evangelink enabled auto-merge (squash) June 5, 2026 15:44
@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

This pull request was automatically closed because it expired on 2026-06-06T14:15:14.790Z.

Closed by Workflow

@github-actions github-actions Bot closed this Jun 6, 2026
auto-merge was automatically disabled June 6, 2026 15:01

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/automation Created or maintained by an agentic workflow. type/tech-debt Code health, refactoring, simplification.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants