Fix the LINQ optimization rules on predicates, casts, ordering and collection interfaces

[meziantou]

What

Several bugs in OptimizeLinqUsageAnalyzer / OptimizeLinqUsageFixer, most of them code fixes that produce code that doesn't compile or that behaves differently at runtime.

MA0029 (Combine LINQ methods)

• The fix dropped both predicates when they were not delegate creations: items.Where(f).Any(g) (with Func<int, bool> f, g) became items.Any(). Expected: items.Any(x => f(x) && g(x)).
• items.Where(x => x > 10).FirstOrDefault(5) was reported and fixed to items.FirstOrDefault(). FirstOrDefault/LastOrDefault/SingleOrDefault(defaultValue) is not a predicate overload. Expected: no diagnostic.
• The fix hardcoded x for a method group: items.Where(Filter).Any(item => item > 0) became items.Any(item => Filter(x) && item > 0). Expected: items.Any(item => Filter(item) && item > 0).
• Only the inner Where was checked for the indexed overload: items.Where(x => x > 0).Where((x, i) => i % 2 == 0) was reported and fixed to broken code. Expected: no diagnostic.
• GetParentLinqOperation followed any argument, so seqs.FirstOrDefault(items.Where(x => x > 0)) reported "Combine 'Where' with 'FirstOrDefault'". Expected: no diagnostic.
• The fix could capture a variable: bool M(int[] items, int x) => items.Where(x => x > 0).Any(y => y > x) became items.Any(x => x > 0 && x > x). It also produced invalid code for block-bodied lambdas.

MA0030 (Remove useless OrderBy call)

• items.OrderBy(x => -x).Order() was fixed to items.OrderBy(x => -x).ThenBy(), which has no key selector. Expected: ThenBy(x => x) (and ThenByDescending(x => x) for OrderDescending).

MA0159 (Use Order)

• The fix removed the source in the static-call form: Enumerable.OrderBy(items, x => x) became Enumerable.Order(x => x). Expected: Enumerable.Order(items).

MA0020 (Use direct methods)

• The fix broke the static-call form: Enumerable.Count(items) (with int[] items) became Enumerable.Length. Expected: items.Length.
• list?.Count() registered a fix that did nothing. Expected: list?.Count.

MA0112 (Use Count > 0 instead of Any())

• The fix emitted .Count on arrays, where it is an explicit implementation: int[] items; items.Any() became items.Count != 0 (CS1061). Expected: items.Length != 0. The doc example had the same mistake.
• A type that implements ICollection<T>.Count explicitly was reported, and the fix couldn't compile.
• The static-call form Enumerable.Any(items) wasn't fixed to items.Count != 0, and items?.Any() got a fix that produced invalid code.

MA0078 (Use Cast instead of Select)

• items.Select(x => (long?)x) and, with enum E : byte, items.Select(x => (E)x) on int[] items were reported and fixed to Cast<long?>() / Cast<E>(), which throw InvalidCastException.

MA0098 / MA0020 on interface-typed receivers

• IList<int> a; ICollection<int> b; IReadOnlyList<int> c; a.First(); b.Count(); c.Last(); a.Count(); gave no diagnostic. AllInterfaces doesn't include the type itself, and GetMembers("Count") doesn't return inherited members (so a class deriving from List<T> wasn't reported either). Expected: 4 diagnostics.

Fix

Analyzer:

• MA0029: new IsPredicateOverload check, based on the original definition of the method, for both the Where and the next method. The next method must take only the source, or the source and a non-indexed predicate.
• GetParentLinqOperation only follows the first (this) argument of an extension method.
• MA0078: CanReplaceByCast uses the C# conversion classification. It accepts identity, reference, boxing and unboxing conversions, and nullable/enum conversions only when the unboxing types match (the enum underlying type, without Nullable<T>). These are the conversions that Cast<T> (unbox/castclass of the boxed value) supports.
• MA0020/MA0098/MA0112: the receiver itself counts as one of the interfaces (GetAllInterfacesIncludingSelf). Count and the int indexer are resolved with SemanticModel.LookupSymbols, which handles inherited and base-interface members and excludes explicit implementations. A diagnostic is reported only when the member lookup finds exactly one accessible member. This also rules out ambiguous interfaces such as interface IMyList : IList<int>, IReadOnlyList<int>. Arrays still use Length.

Fixer (every precondition is now checked before RegisterCodeFix):

• MA0029: the combined lambda is built from the lambda expression bodies, from method groups (Filter(x)), or from delegate locals, parameters and fields (f(x)). The parameter name is the first lambda parameter name that doesn't conflict with an identifier of the other predicate. Otherwise it's x, x1, … not in scope. No fix is offered for block-bodied lambdas or for delegates/instances that can't be safely evaluated once per element (e.g. Where(GetFilter())).
• MA0030: ThenBy(x => x) / ThenByDescending(x => x) when the second call is Order()/OrderDescending(), with a parameter name that isn't in scope.
• MA0159: removes the key selector argument (parameter ordinal 1) instead of the first syntactic argument.
• MA0020/MA0112: a shared CreatePropertyAccess handles items.Count(), Enumerable.Count(items) and items?.Count(). For MA0112, items?.Any() is not fixed, and arrays use Length.

Docs: fixed the MA0112 example. MA0029, MA0030 and MA0078 now describe the new behavior.

Tests

Added regression tests to OptimizeLinqUsageAnalyzerTests (code-fix tests for every sub-bug, plus no-diagnostic/no-fix tests). Existing tests were not changed. The full OptimizeLinqUsageAnalyzerTests class passes on roslyn5.9 (203/203) and roslyn4.8 (203/203). The code fixers build without warnings for roslyn4.14, 5.0 and 5.6. DocumentationGenerator reports no pending change.