Skip to content

Fix MA0183 false positive and missed diagnostic when named arguments are reordered - #1408

Merged
meziantou merged 1 commit into
mainfrom
feature/string-format-named-args-0258ac
Sep 6, 2026
Merged

meziantou merged 1 commit into
mainfrom
feature/string-format-named-args-0258ac

Conversation

@meziantou

Copy link
Copy Markdown
Owner

What changed

StringFormatShouldBeConstantAnalyzer (MA0183) located the format string by indexing IInvocationOperation.Arguments positionally. That collection is in evaluation/source order, so when named arguments are written out of parameter order the analyzer picked the wrong operand:

_ = string.Format(arg0: "hello", format: "{0}");   // false positive: "hello" treated as the format string
_ = string.Format(arg0: x, provider: c, format: "abc"); // missed diagnostic

The same applied to StringBuilder.AppendFormat and Console.Write/Console.WriteLine.

The analyzer now works with parameter ordinals instead of argument positions:

  • GetFormatArgIndex(operation, …) became GetFormatParameterOrdinal(method, …): the IFormatProvider overload is detected from IMethodSymbol.Parameters, which is always in declaration order, and the method returns a parameter ordinal rather than an argument index.
  • A new GetArgumentForParameter helper resolves the format argument by matching IArgumentOperation.Parameter.Ordinal, replacing operation.Arguments[formatArgumentIndex] and its bounds check.
  • HasFormattingArguments iterates every argument and keeps the ones whose Parameter.Ordinal is greater than the format parameter's ordinal, instead of slicing the array from formatArgumentIndex + 1.

Tests

7 cases added to StringFormatShouldBeConstantAnalyzerTests, covering reordered named arguments for string.Format (with and without provider:), StringBuilder.AppendFormat and Console.WriteLine, in both the false-positive and the missed-diagnostic direction. 4 of them failed before the fix (3 false positives, 1 missed diagnostic).

Verification

  • StringFormatShouldBeConstantAnalyzerTests: 62 tests, green on roslyn4.8, roslyn4.14, roslyn5.0, roslyn5.6 and roslyn5.9.
  • Full suite on the default version (roslyn5.9): 3916/3916 passing.
  • dotnet run --project src/DocumentationGenerator exits 0 with no markdown changes — this is a behavior fix that does not alter the documented rule.

Note for the reviewer

The report that prompted this also flagged the same positional assumption in UseDateTimeUnixEpochAnalyzer.ArgumentsEquals. That no longer applies: it already matches on argument.Parameter.Ordinal and its doc comment states it is order-independent, so it is left untouched here.

IInvocationOperation.Arguments is in evaluation order, so reordered named
arguments made the analyzer pick the wrong operand as the format string.
string.Format(arg0: "hello", format: "{0}") was reported as having no
placeholder, and string.Format(arg0: x, provider: c, format: "abc") was
not reported at all.

Locate the format argument through IArgumentOperation.Parameter.Ordinal
instead of the position in the argument list, and detect the IFormatProvider
overload from IMethodSymbol.Parameters, which is always in declaration order.
HasFormattingArguments now compares parameter ordinals as well.
@meziantou
meziantou enabled auto-merge (squash) September 6, 2026 04:21
@meziantou
meziantou merged commit c3946e5 into main Sep 6, 2026
13 checks passed
@meziantou
meziantou deleted the feature/string-format-named-args-0258ac branch September 6, 2026 04:21
This was referenced Sep 6, 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