Skip to content

Parenthesize the expressions produced by the MA0112 and MA0128 fixers - #1385

Merged
meziantou merged 1 commit into
mainfrom
feature/ma0112-ma0128-parentheses-54a171
Sep 6, 2026
Merged

meziantou merged 1 commit into
mainfrom
feature/ma0112-ma0128-parentheses-54a171

Conversation

@meziantou

Copy link
Copy Markdown
Owner

Problem

OptimizeLinqUsageAnalyzer (MA0112) and UseIsPatternInsteadOfSequenceEqualAnalyzer (MA0128) both report on the inner IInvocationOperation, so their fixers get the invocation node and editor.ReplaceNode splices the replacement straight under the invocation's parent. The replacements — an != comparison for MA0112, an is pattern for MA0128 — bind looser than the invocation they replace, so under a unary operator or a member access the fix changed the meaning of the code and produced CS0023:

if (!list.Any()) { }               // -> if (!list.Count != 0) { }
if (!str.SequenceEqual("bar")) { } // -> if (!str is "bar") { }
_ = list.Any().ToString();         // -> _ = list.Count != 0.ToString();

!x.Any() is the most common way Any() is written, so this was the majority case of MA0112's fix rather than an edge case. Both fixers use WellKnownFixAllProviders.BatchFixer, so "Fix all in document" broke every site at once. It went unnoticed because every existing fix test used the bare _ = expr; form, where the replacement happens to need no parentheses.

Fix

Wrap both replacements with Parenthesize() (the Meziantou.Framework.Roslyn helper), which annotates the parentheses with Simplifier.Annotation so the code-fix post-processing removes the ones the final document doesn't need. The existing tests that expect _ = collection.Count != 0; and _ = str is "bar"; still pass unchanged.

Both fixers also dropped the trailing trivia of the invocation, so it is now carried over to the new expression.

Note for the reviewer

WithTriviaFrom is not usable here, even though sibling fixers such as SimplifyNegatedBooleanExpressionFixer use it. Both of these fixers reuse the receiver syntax of the invocation (invocation.Arguments[0].Syntax / operation.Arguments[0].Value.Syntax), which already carries the leading trivia, so copying it onto the parenthesized node would emit it twice — /* c */(/* c */ x is "bar"). Only the trailing trivia, which sits on the invocation's closing parenthesis, was actually being lost, so only that is copied. The KeepsTrivia tests cover both ends.

Tests

Six tests added to the existing test files, covering the negated, member-access and trivia shapes for each rule. Each was run against the unfixed code first and reproduced the exact broken output above.

  • MA0112: Any_List_Negated_CodeFix, Any_List_MemberAccess_CodeFix, Any_List_KeepsTrivia_CodeFix
  • MA0128: ReadOnlySpanChar_SequenceEqual_Negated, ReadOnlySpanChar_SequenceEqual_MemberAccess, ReadOnlySpanChar_SequenceEqual_KeepsTrivia

Verification

  • The two rules' tests pass on all five Roslyn versions (4.8, 4.14, 5.0, 5.6, 5.9), 156 tests each.
  • Full suite: 19123 passed, 0 failed.
  • dotnet run --project src/DocumentationGenerator exits 0 with no markdown changes — this is a behavior-only fix to the code fixers, the rule documentation is unaffected.

Both rules report on the inner invocation, so the fixers replace that node
directly under its parent. The replacements are an '!=' comparison (MA0112)
and an 'is' pattern (MA0128), which bind looser than the invocation they
replace, so splicing them under a unary operator or a member access changed
the meaning of the code and produced CS0023:

    if (!list.Any()) { }        ->  if (!list.Count != 0) { }
    if (!str.SequenceEqual("b"))  ->  if (!str is "b")
    list.Any().ToString()       ->  list.Count != 0.ToString()

Wrap both replacements with Parenthesize(), which annotates the parentheses
with Simplifier.Annotation so the post-processing removes the ones the fixed
document doesn't need. Both fixers also lost the trailing trivia of the
invocation, so carry it over to the new expression.

Note that WithTriviaFrom is not usable here: both fixers reuse the receiver
syntax of the invocation, which already carries the leading trivia, so
copying it again would duplicate it.
@meziantou
meziantou merged commit b12d6d6 into main Sep 6, 2026
13 checks passed
@meziantou
meziantou deleted the feature/ma0112-ma0128-parentheses-54a171 branch September 6, 2026 02:50
This was referenced Sep 6, 2026
This was referenced Sep 24, 2026
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.

1 participant