Use AsParallel() correctly

[terrajobst]

Using .AsParallel() at the end of a LINQ query, e.g. foreach (var item in src.Select(...).Where(...).AsParallel(...)), is a nop and should either be removed or the AsParallel() moved earlier in the query. I've even seen developers write foreach (var item in src.AsParallel()) thinking it parallelizes the foreach loop, which it doesn't... it'd be good to warn about such misuse.

Category: Performance

[stephentoub]

Here's an example of the kinds of things we'd like to catch:
dotnet/sdk#10937

[jeffhandley]

Estimates:

• Analyzer: Medium
• Fixer: Medium

[Mrnikbobjeff]

Would the fixer simply remove the call? I do not think it can add it earlier in the chain as it may cause race conditions depending on the implementation of the earlier linq chain, e.g. list.Select(x => NonThreadSafeMethod(x)).AsParallel() can only be resolved by removing the code. I guess that is the intended fix? Also, I guess the analyzer only triggers when the AsParallel is directly inside a foreach? Otherwise i may add other methods to the linq call chain after assigning the AsParallel() result at the end of the chain.

[stephentoub]

guess the analyzer only triggers when the AsParallel is directly inside a foreach

Yes

can only be resolved by removing the code. I guess that is the intended fix

That's the safe fix, in that it doesn't change the semantics. It'd be fine to offer multiple fixes, though, with a second option being to move the call to the beginning of the chain, and if the developer chooses that, it's their choice.

[Mrnikbobjeff]

I have a working analyzer for the described scenario, as well as the safe fix as you described it. I would still have to integrate it with the analyzer structure in this project but otherwise I would love to work on this if this is available.

[jeffhandley]

@carlossanlop @buyaa-n Can you follow up with @Mrnikbobjeff on this? We would need to bring this in for API Review if we want to move it forward.

[carlossanlop]

This comment in dotnet/roslyn-analyzers#2812 gives a clear example of what to detect in a foreach and what should be suggested as a fix.
This issue gives an example of an enumeration that saves the value into a variable.

• If an AsParallel invocation is found, check if it was added after enumerating invocations like Select or Where.
  • The first case to address should be to detect AsParallel calls inside a foreach.
  • The second case to address should be to detect AsParallel calls anywhere else, like when an enumeration result is saved into a variable.
• If any of those two cases were found, then the fixer should suggest moving the AsParallel to the first position.

As long as we keep those rules in mind, we think this is ready for review. After it gets approved, we will let @Mrnikbobjeff know so he can start working on it.

Category: Reliability Performance
Analyzer size: Medium
Fixer size: Medium
Suggested severity: Warning
Suggested messages:
- Title: Use 'AsParallel()' correctly
- Message: The 'AsParallel()' method should be called before enumerating.
- Description: The 'AsParallel()' method result is only effective when its returned value is consumed by enumerating methods.

cc @buyaa-n