Skip to content

Do not offer the MA0044 code fix when removing ToString would drop argument side effects - #1462

Merged
meziantou merged 1 commit into
mainfrom
feature/ma0044-tostring-provider-5d57b7
Sep 12, 2026
Merged

meziantou merged 1 commit into
mainfrom
feature/ma0044-tostring-provider-5d57b7

Conversation

@meziantou

Copy link
Copy Markdown
Owner

Problem

MA0044 reports any ToString call on System.String, including ToString(IFormatProvider). The provider is ignored by that overload, but C# still evaluates the argument. The fixer replaced the whole invocation with its receiver, so side effects of the argument were silently removed:

_ = "text".ToString(Provider()); // Provider() is called
// after the fix
_ = "text";                      // Provider() is no longer called

Changes

  • RemoveUselessToStringFixer now only registers the fix when every argument can be discarded without observable effect: constants and literals, locals, parameters, this, static fields or fields of this, and static CultureInfo properties (so str.ToString(CultureInfo.InvariantCulture) is still fixed). Implicit conversions are looked through unless they call a user-defined operator.
  • The member-access check is moved from the fix action to the registration, per the repository guidance. As a result the fix is no longer offered for value?.ToString(), where it previously returned the document unchanged.
  • The analyzer is unchanged: the call is still useless, so the diagnostic is still reported.
  • docs/Rules/MA0044.md documents when the fix is not offered.

Tests

Added to RemoveUselessToStringAnalyzerTests:

  • no fix for a side-effecting provider call, a property provider, and ?.ToString() (these three fail against the previous fixer)
  • fix applied for CultureInfo.InvariantCulture, null, and a parameter

The test class passes (8/8) on Roslyn 4.8, 4.14, 5.0, 5.6 and 5.9. The full suite was not run locally.

Known gaps (not addressed here)

  • The fix still removes the call when the receiver may be null (s.ToString() throws a NullReferenceException, s does not).
  • Reading a static field may run a static constructor.

…gument side effects

string.ToString(IFormatProvider) ignores its provider, but C# still evaluates the
argument. The fixer replaced the whole invocation with its receiver, so a call such as
"text".ToString(Provider()) lost the evaluation of Provider(). The fix is now only
registered when every argument can be discarded without observable effect (constants,
locals, parameters, this, static fields or fields of this, static CultureInfo properties).

The member-access check is also moved to the registration, so the fix is no longer
offered for value?.ToString(), where it returned the document unchanged.
@meziantou
meziantou merged commit 650ac39 into main Sep 12, 2026
13 checks passed
@meziantou
meziantou deleted the feature/ma0044-tostring-provider-5d57b7 branch September 12, 2026 00:47
This was referenced Sep 12, 2026
This was referenced Sep 17, 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