CA2246 Warning for unobvious assignment

[maxkoshevoi]

This is related to issue #2527

I've done what I can here. This is my first time contributing to large repository, so I would love to hear any feedback.

I think name of the analyzer could be shorter and more understandable.

Here's how warning currently looks (C is class and S is struct):

Thanks!

[dnfclas]

All CLA requirements met.

[mavasani]

Overall, looks good. @maxkoshevoi would you be able to address the review comments?

Also tagging @dotnet/roslyn-analysis for an additional review of the PR and proposed rule itself.

[maxkoshevoi]

Overall, looks good. @maxkoshevoi would you be able to address the review comments?

Yes, I'll do it in the next couple of days

[gewarren]

I suggested a new description - let me know if I got the logic correct.

[mavasani]

@maxkoshevoi I can merge the PR once you fix up the resource strings and xlf files.

[maxkoshevoi]

All done. Thanks a lot for your help!

[mavasani]

@maxkoshevoi There seem to be merge conflicts in src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MicrosoftQualityGuidelinesAnalyzersResources.resx. Please fix them and rebuild to regenerate xlf files.

[maxkoshevoi]

It's strange. I don't see and conflicts from my end:

PS: I've rebuild solution after merge and still there were no changes to commit

[mavasani]

@maxkoshevoi Did you sync the latest sources from master?

[maxkoshevoi]

Yes, I did (and I've merged my branch with remote master to be sure)

…

On Sat, Sep 7, 2019, 15:56 Manish Vasani ***@***.***> wrote: @maxkoshevoi <https://github.com/maxkoshevoi> Did you sync the latest sources from master? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2717?email_source=notifications&email_token=ABSNYCMPJIXAP7BJMICIUZTQIOQHBA5CNFSM4IHWSBBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6EYHUA#issuecomment-529105872>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABSNYCIYMRDKAUGMW2ZHSATQIOQHBANCNFSM4IHWSBBA> .

[mavasani]

@maxkoshevoi I figured out the reason for your merge conflicts. Your PR is updating src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MicrosoftQualityGuidelinesAnalyzersResources.resx , but that resource file no longer exists in master. The resource strings from that resx file have been moved to https://github.com/dotnet/roslyn-analyzers/blob/master/src/Microsoft.CodeQuality.Analyzers/Core/MicrosoftCodeQualityAnalyzersResources.resx

Can you merge latest master sources and move your added resource strings to the new resx file? Thanks!

[maxkoshevoi]

Oh, now I see it. For some reason forked repository didn't see master from this repository (or something like that, I don't really know). I'm able to perform merge by switching to this request's brunch:

[maxkoshevoi]

I'll try to address the issue today, but it could take a bit longer since I didn't worked with VB before.

…

On Thu, Sep 12, 2019, 04:12 Manish Vasani ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/AssigningSymbolAndItsMemberInSameStatement.cs <#2717 (comment)> : > @@ -0,0 +1,97 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Immutable; +using Analyzer.Utilities; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.CodeQuality.Analyzers.QualityGuidelines +{ + /// <summary> + /// CA2246: Prevent objects from being referenced in statements where they are reassigned + /// </summary> + [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)] @maxkoshevoi <https://github.com/maxkoshevoi> I can merge the PR once you address this comment. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2717?email_source=notifications&email_token=ABSNYCMHZP4WHZG7QEI6GFTQJGJQ5A5CNFSM4IHWSBBKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCEOY5RA#discussion_r323521311>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABSNYCM44DJIAGXQ4ZTLFO3QJGJQ5ANCNFSM4IHWSBBA> .

[mavasani]

@maxkoshevoi Can you make the analyzer itself be C# only then?

[mavasani]

Thanks for the contribution!

[mavasani]

Filed https://github.com/MicrosoftDocs/visualstudio-docs/issues/3916 to track adding documentation for the rule and #2847 to track updating analyzer's helpLinkUri once the documentation has been added.