Fixer for CA1826

[TKharaishvili]

Addresses the issue - #1932

The changes in this PR are obviously not ideal, some of the issues being:

1. lack of tests
2. not being implemented for VB
3. the fixer window showing up for other linq methods too even though it's only implemented for Enumerable.First

but hey it does work for C# and hopefully you guys can help me deal with the rough edges.

[mavasani]

Thanks for the PR! Overall, you need to be much more defensive in code fixers about what nodes/operations are present in the tree. Additionally, it is hard to review the correctness of a PR without unit tests. I would request adding unit tests prior to requesting second round of review. Thanks!

[TKharaishvili]

@mavasani just pushed a new commit with requested fixes as well as unit tests.

The bottom two unit tests named CA1826FixEnumerableFirstMethodChainCallWrongFormattingCSharp and CA1826FixEnumerableFirstProblematicSyntaxCallCSharp document problematic cases I have a little trouble figuring out and will probably need some help with.

As for:

Yes, fixers should not assume anything about the analyzer and/or the diagnostic location or properties.

I added a properties dictionary to the analyzer part. This serves two purposes:

1. It makes sure that the fixer window only shows up for Enumerable.First and not for the other methods.
2. The fixer now has more guarantees as to which analyzer invoked it, therefore doesn't require as much defensive code.

If you still think that the fixer should contain those guards regardless, let me know and I'll add them in.

Thanks for your help once again.

[TKharaishvili]

@mavasani just pushed the changes addressing the 2nd code review. I'll resolve those conversations now.

[mavasani]

LGTM. Primary pending request is for adding a VisualBasic unit test.

Thanks for the contribution!

[mavasani]

Marking as request changes as we have a layering violation in the code fixer implementation, that must be fixed prior to merging this in.

[TKharaishvili]

@mavasani I tried to make it work for VB too but wasn't able to. I also opened up a review, please have a look at that as well. Thanks.

[mavasani]

Thanks!

[TKharaishvili]

@mavasani thanks to you too! Couldn't have done this without your guidance.

[mavasani]

@TKharaishvili Can you resolve the merge conflicts? I'd start by just resolving the conflict in src/Microsoft.NetCore.Analyzers/Core/Runtime/SystemRuntimeAnalyzersResources.resx and then doing a rebuild to regenerate the xlf files.

[TKharaishvili]

@mavasani I resolved that file in the github ui. But I'm not sure how to trigger rebuild from here...

[mavasani]

I resolved that file in the github ui. But I'm not sure how to trigger rebuild from here...

Not sure if that is supported. You'll probably have to do it locally by deleting the xlf folder on your machine and rebuilding and pushing it.

[TKharaishvili]

@mavasani I think it's all finished