Skip to content

Do not report MA0106 when the factory writes the captured variable - #1488

Merged
meziantou merged 1 commit into
mainfrom
feature/ma0106-captured-variable-writes-8cd1d5
Sep 12, 2026
Merged

meziantou merged 1 commit into
mainfrom
feature/ma0106-captured-variable-writes-8cd1d5

Conversation

@meziantou

Copy link
Copy Markdown
Owner

Problem

MA0106 was reported on factories that write the captured variable, and its code fix redirected those writes to the factoryArgument parameter, which is a copy of the value:

int counter = 0;
var map = new ConcurrentDictionary<string, int>();

map.GetOrAdd("key", _ => ++counter);              // counter is 1 after the call
map.GetOrAdd("key", (_, arg) => ++arg, counter);  // after the code fix: counter is still 0

Both versions compile, so the change is silent. A captured variable is a shared storage; the factoryArgument parameter is not, so the closure cannot be removed when the factory writes the variable.

Changes

Analyzer — DetectClosure excludes the captured symbols that appear in WrittenInside (which covers the writes made by a lambda nested in the factory) and reports only when a read-only captured symbol remains. The message lists those symbols instead of dataFlow.Captured. DetectPotentialUsageOfLambdaParameter (MA0105) already did the same.

Code fix — the factoryArgument fix now validates everything before registering itself, instead of returning an unchanged document from the code action:

  • GetFactoriesToUpdate returns the lambdas the fix rewrites (GetOrAdd with 2 arguments → the value factory; AddOrUpdate with 3 arguments → both factories), or null for the shapes the fix does not support.
  • TryGetFactoryArgumentSymbol requires every rewritten lambda to be a simple or parenthesized lambda, requires the captured symbol to be a local or a parameter, and rejects the symbol when any rewritten factory writes it. That last check is what the analyzer change alone does not cover: the diagnostic can be reported on the add value factory while the update value factory writes the variable, and the fix rewrites both.
  • GetCapturedSymbols skips the written symbols too, so the candidate is never one of them.
  • The two near-duplicate Create*InvocationWithFactoryArg methods are replaced by a single loop, and AddParameterToLambda is no longer nullable now that its preconditions are guaranteed.

Documentation — docs/Rules/MA0106.md documents the written-variable case as "ok".

Tests

Three tests added: the ++counter reproduction, a write made by a lambda nested in the factory, and an AddOrUpdate whose update value factory writes the variable. The last one sets FixedCode to the unchanged source, so an applied code fix fails the test. All three fail without the production changes.

One existing case in GetOrAdd_IsValid (a factory assigning key) lost its {|MA0106:...|} markup, as that closure is now correctly not reported.

The test class passes on Roslyn 4.8, 4.14, 5.0, 5.6 and 5.9 (28 tests each), the full test project passes on the default version (4383 tests), the solution builds without warnings, and dotnet run --project src/DocumentationGenerator reports no further markdown change.

A captured variable is a shared storage, whereas the 'factoryArgument'
parameter is a copy of its value. When the factory writes the variable,
the code fix redirected the writes to the copy, so the mutation was no
longer visible outside of the factory:

    int counter = 0;
    map.GetOrAdd("key", _ => ++counter); // counter is 1 after the call
    map.GetOrAdd("key", (_, arg) => ++arg, counter); // counter is still 0

The analyzer now excludes the captured variables written inside the
lambda, which includes the writes made by a nested lambda.

The code fix also validates everything before registering itself, as the
diagnostic can be reported on the add value factory while the update
value factory writes the variable, and both are rewritten.
@meziantou
meziantou merged commit 6896438 into main Sep 12, 2026
13 checks passed
@meziantou
meziantou deleted the feature/ma0106-captured-variable-writes-8cd1d5 branch September 12, 2026 03:39
meziantou added a commit that referenced this pull request Sep 12, 2026
main rewrote the 'Use factoryArgument overload' code fix in #1488: the
factories are now collected by GetFactoriesToUpdate and rewritten in a
loop, and the conditions are validated before registering the fix.

Kept that structure and layered the explicitly typed parameter on top of
it: CreateFactoryArgumentParameters builds the parameter of every factory
before the fix is registered, so the fix is not offered when one of them
cannot get the parameter. The refactoring of the 'Use lambda parameters'
branch is dropped, as main did not adopt it and it is unrelated to the
CS0748 fix.
This was referenced Sep 12, 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