roslyn: IDE0050 - Convert to tuple should not apply in expression trees

I eventually end up in a valid state, but I don’t want to be blocked from these quick transforms just because an intermediary state is temporarily invalid.

Well, this is hardly a “temporarily invalid intermediate” state when your code got transformed into little cul-de-sacs (and you’re lucky to notice at compile-time; as mentioned above, any rewrite inside expression trees can cause runtime errors) and the only way to get out is to make a U-turn and undo the “change” (because the word “fix” is apparently sensitive in this context). I do agree there are cases where successive transforms are needed to get to a valid state, but in this case I’m dealing with a checkmate.

The point of the issue being that a code fix yields invalid code that doesn’t compile because of inherent restrictions in the language.

Note that I’m talking about inherent restrictions to the language. Things I can’t do anything about. And, no, I can’t change my APIs to not take expression trees to move to my next valid state. Quotation is a strong intent, often forced upon me by query providers and the like.

We should not suggest to make these changes actively with a visual annotation in the text itself.

Whether it’s “suggesting” or not seems a bit mood to me now that we have toggles like:

dotnet_analyzer_diagnostic.category-Style.severity = warning

and

<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>

which I’m experimenting with right now (knowing that the support for this itself is “experimental”) and these effectively become warnings:

...\DataSerializerTests.cs(552,28): warning IDE0050: Convert to tuple

And surely my IDE correctly reflects my preferences (hooray!) so I can burn down the list of warnings:

Maybe with these flags you can argue that I’m telling you to now suggest more things, complain about more things, etc. and I’m only to blame myself. I’m surely opting in to another level of pain, but I’ve evaluated it’s worth the time. Except…

For 95% of the time, I want the changes, because I want to keep my code base modern and clean. For 4% of the time, I’m happy to suppress things where I disagree (either by tweaking a rule’s behavior, by disabling a rule altogether, by using #pragma in a few places, or some other means to suppress selectively). For the remainder 1% of the time, I’m frustrated. In this case, I want to convert to tuples in a bunch of places. It’s making my code better, so I don’t want to disable the rule. But for cases like the one shown in this issue, I have to suppress the warning, and a #pragma is my weapon of choice because I can keep the scope as small as possible and have it travel with the code in case of refactorings. All I have to do is come up with a snarky comment to justify the suppression right there:

It just makes me cringe having to write a justification for something that’s just unfixable because of inherent limitations in the language that the analyzer seems to have forgotten about. Another example of this was https://github.com/dotnet/roslyn-analyzers/issues/4651 where the suggested-or-not-but-still-shown-to-me transform yielded invalid C# code that merely screams “we don’t know what we’re doing”. Whether it’s “hey, make your indexer static” (oh, wait, the language says there’s no such thing, so I can’t) or “hey, use modern stuff in your expression tree” (oh, wait, expression trees are still living in the C# 3.0 days, so I can’t), they’re all non-actionable, corner me into a dead end, and I have to throw in a suppression to make it go away.

In short, what’s the stance on analyzers offering up code transforms that yield invalid C# (e.g. static indexers, pattern matching in expression trees, operator overloads with insufficient parameters, inserting a cast to var)? Some, like the impossible cast to var, are easy to address and small fit-n-finish annoyances. But for others, suppression is the only resort.

Then consider the simplest of LINQ 101 queries using an anonymous type as a projection:

static void Demo(IQueryable<Product> products)
{
    var res = from p in products
                select new { p.Name, p.Price };

    foreach (var item in res)
    {
    }
}

The suggested fix yields invalid code here as well, simply because the select clause binds to an expression-tree based Select method:

with the following error:

Of course the better thing would be that expression trees get some renewed love from the language designers and don’t have to reject anything > C# 3.0, but that’s a whole different story 😃.

The point of the issue being that a code fix yields invalid code that doesn’t compile because of inherent restrictions in the language. I don’t think it should even suggest to make such a change.