Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CA2246 Warning for unobvious assignment #2717

Merged
merged 19 commits into from Sep 12, 2019
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -1334,4 +1334,13 @@
<data name="AvoidPropertySelfAssignmentMessage" xml:space="preserve">
<value>The property {0} should not be assigned to itself.</value>
</data>
<data name="AssigningSymbolAndItsMemberInSameStatementDescription" xml:space="preserve">
<value>Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.</value>
</data>
<data name="AssigningSymbolAndItsMemberInSameStatementMessage" xml:space="preserve">
<value>Symbol '{0}' and its member '{1}' are both assigned in the same statement. You are at risk of assigning the member of an unintended object.</value>
</data>
<data name="AssigningSymbolAndItsMemberInSameStatementTitle" xml:space="preserve">
<value>Assigning symbol and its member in the same statement.</value>
</data>
</root>
@@ -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 marked this conversation as resolved.
Show resolved Hide resolved
public sealed class AssigningSymbolAndItsMemberInSameStatement : DiagnosticAnalyzer
{
internal const string RuleId = "CA2246";

private static readonly LocalizableString s_localizableTitle = new LocalizableResourceString(nameof(MicrosoftQualityGuidelinesAnalyzersResources.AssigningSymbolAndItsMemberInSameStatementTitle), MicrosoftQualityGuidelinesAnalyzersResources.ResourceManager, typeof(MicrosoftQualityGuidelinesAnalyzersResources));
private static readonly LocalizableString s_localizableMessage = new LocalizableResourceString(nameof(MicrosoftQualityGuidelinesAnalyzersResources.AssigningSymbolAndItsMemberInSameStatementMessage), MicrosoftQualityGuidelinesAnalyzersResources.ResourceManager, typeof(MicrosoftQualityGuidelinesAnalyzersResources));
private static readonly LocalizableString s_localizableDescription = new LocalizableResourceString(nameof(MicrosoftQualityGuidelinesAnalyzersResources.AssigningSymbolAndItsMemberInSameStatementDescription), MicrosoftQualityGuidelinesAnalyzersResources.ResourceManager, typeof(MicrosoftQualityGuidelinesAnalyzersResources));

internal static DiagnosticDescriptor Rule = new DiagnosticDescriptor(RuleId,
s_localizableTitle,
s_localizableMessage,
DiagnosticCategory.Usage,
DiagnosticHelpers.DefaultDiagnosticSeverity,
DiagnosticHelpers.EnabledByDefaultIfNotBuildingVSIX,
s_localizableDescription);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

public override void Initialize(AnalysisContext context)
{
context.RegisterOperationAction(AnalyzeAssignment, OperationKind.SimpleAssignment);
}

private void AnalyzeAssignment(OperationAnalysisContext context)
{
var assignmentOperation = (ISimpleAssignmentOperation)context.Operation;

// Check if there are more then one assignment in a statement
if (!(assignmentOperation.Target is IMemberReferenceOperation operationTarget))
{
return;
}

// This analyzer makes sense only for reference type objects
if (operationTarget.Instance?.Type.IsValueType == true)
{
return;
}

// Search for object equal to operationTarget.Instance further in assignment chain
bool isViolationFound = false;
if (operationTarget.Instance is ILocalReferenceOperation localInstance)
{
isViolationFound = AnalyzeAssignmentToMember(assignmentOperation, localInstance, (a, b) => a.Local == b.Local);
}
else if (operationTarget.Instance is IMemberReferenceOperation memberInstance)
{
isViolationFound = AnalyzeAssignmentToMember(assignmentOperation, memberInstance, (a, b) => a.Member == b.Member && a.Instance?.Syntax.ToString() == b.Instance?.Syntax.ToString());
}
else if (operationTarget.Instance is IParameterReferenceOperation parameterInstance)
{
isViolationFound = AnalyzeAssignmentToMember(assignmentOperation, parameterInstance, (a, b) => a.Parameter == b.Parameter);
}
else
{
return;
}

if (isViolationFound)
{
var diagnostic = Diagnostic.Create(Rule, operationTarget.Syntax.GetLocation(), operationTarget.Instance.Syntax, operationTarget.Member.Name);
context.ReportDiagnostic(diagnostic);
}
}

private static bool AnalyzeAssignmentToMember<T>(ISimpleAssignmentOperation assignmentOperation, T instance, Func<T, T, bool> equalityComparer) where T : class, IOperation
{
// Check every simple assignments target in a statement for equality to `instance`
while (assignmentOperation.Value.Kind == OperationKind.SimpleAssignment)
{
assignmentOperation = (ISimpleAssignmentOperation)assignmentOperation.Value;

var operationValue = assignmentOperation.Target as T;
if (equalityComparer(instance, operationValue))
{
return true;
}
}
return false;
}
}
}
Expand Up @@ -7,6 +7,21 @@
<target state="translated">Připojit .ConfigureAwait(true)</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementDescription">
<source>Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.</source>
<target state="new">Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementMessage">
<source>Symbol '{0}' and its member '{1}' are both assigned in the same statement. You are at risk of assigning the member of an unintended object.</source>
<target state="new">Symbol '{0}' and its member '{1}' are both assigned in the same statement. You are at risk of assigning the member of an unintended object.</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementTitle">
<source>Assigning symbol and its member in the same statement.</source>
<target state="new">Assigning symbol and its member in the same statement.</target>
<note />
</trans-unit>
<trans-unit id="AvoidAsyncVoidTitle">
<source>Avoid Async Void</source>
<target state="translated">Vyhněte se Async Void.</target>
Expand Down
Expand Up @@ -7,6 +7,21 @@
<target state="translated">"ConfigureAwait(true)" anfügen</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementDescription">
<source>Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.</source>
<target state="new">Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementMessage">
<source>Symbol '{0}' and its member '{1}' are both assigned in the same statement. You are at risk of assigning the member of an unintended object.</source>
<target state="new">Symbol '{0}' and its member '{1}' are both assigned in the same statement. You are at risk of assigning the member of an unintended object.</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementTitle">
<source>Assigning symbol and its member in the same statement.</source>
<target state="new">Assigning symbol and its member in the same statement.</target>
<note />
</trans-unit>
<trans-unit id="AvoidAsyncVoidTitle">
<source>Avoid Async Void</source>
<target state="translated">Async Void vermeiden</target>
Expand Down
Expand Up @@ -7,6 +7,21 @@
<target state="translated">Anexar .ConfigureAwait(true)</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementDescription">
<source>Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.</source>
<target state="new">Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementMessage">
<source>Symbol '{0}' and its member '{1}' are both assigned in the same statement. You are at risk of assigning the member of an unintended object.</source>
<target state="new">Symbol '{0}' and its member '{1}' are both assigned in the same statement. You are at risk of assigning the member of an unintended object.</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementTitle">
<source>Assigning symbol and its member in the same statement.</source>
<target state="new">Assigning symbol and its member in the same statement.</target>
<note />
</trans-unit>
<trans-unit id="AvoidAsyncVoidTitle">
<source>Avoid Async Void</source>
<target state="translated">Evitar async void</target>
Expand Down
Expand Up @@ -7,6 +7,21 @@
<target state="translated">Ajouter .ConfigureAwait(true)</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementDescription">
<source>Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.</source>
<target state="new">Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementMessage">
<source>Symbol '{0}' and its member '{1}' are both assigned in the same statement. You are at risk of assigning the member of an unintended object.</source>
<target state="new">Symbol '{0}' and its member '{1}' are both assigned in the same statement. You are at risk of assigning the member of an unintended object.</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementTitle">
<source>Assigning symbol and its member in the same statement.</source>
<target state="new">Assigning symbol and its member in the same statement.</target>
<note />
</trans-unit>
<trans-unit id="AvoidAsyncVoidTitle">
<source>Avoid Async Void</source>
<target state="translated">Éviter Async Void</target>
Expand Down
Expand Up @@ -7,6 +7,21 @@
<target state="translated">Accoda .ConfigureAwait(true)</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementDescription">
<source>Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.</source>
<target state="new">Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementMessage">
<source>Symbol '{0}' and its member '{1}' are both assigned in the same statement. You are at risk of assigning the member of an unintended object.</source>
<target state="new">Symbol '{0}' and its member '{1}' are both assigned in the same statement. You are at risk of assigning the member of an unintended object.</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementTitle">
<source>Assigning symbol and its member in the same statement.</source>
<target state="new">Assigning symbol and its member in the same statement.</target>
<note />
</trans-unit>
<trans-unit id="AvoidAsyncVoidTitle">
<source>Avoid Async Void</source>
<target state="translated">Evitare metodi asincroni void</target>
Expand Down
Expand Up @@ -7,6 +7,21 @@
<target state="translated">.ConfigureAwait(true) を追加します</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementDescription">
<source>Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.</source>
<target state="new">Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementMessage">
<source>Symbol '{0}' and its member '{1}' are both assigned in the same statement. You are at risk of assigning the member of an unintended object.</source>
<target state="new">Symbol '{0}' and its member '{1}' are both assigned in the same statement. You are at risk of assigning the member of an unintended object.</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementTitle">
<source>Assigning symbol and its member in the same statement.</source>
<target state="new">Assigning symbol and its member in the same statement.</target>
<note />
</trans-unit>
<trans-unit id="AvoidAsyncVoidTitle">
<source>Avoid Async Void</source>
<target state="translated">Async Void を使用しないでください</target>
Expand Down
Expand Up @@ -7,6 +7,21 @@
<target state="translated">.ConfigureAwait(true) 추가</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementDescription">
<source>Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.</source>
<target state="new">Assigning to a symbol and its member (field/property) in the same statement is not recommended. It is not clear if the member access was intended to use symbol's old value prior to the assignment or new value from the assignment in this statement. For clarity, consider splitting the assignments into separate statements.</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementMessage">
<source>Symbol '{0}' and its member '{1}' are both assigned in the same statement. You are at risk of assigning the member of an unintended object.</source>
<target state="new">Symbol '{0}' and its member '{1}' are both assigned in the same statement. You are at risk of assigning the member of an unintended object.</target>
<note />
</trans-unit>
<trans-unit id="AssigningSymbolAndItsMemberInSameStatementTitle">
<source>Assigning symbol and its member in the same statement.</source>
<target state="new">Assigning symbol and its member in the same statement.</target>
<note />
</trans-unit>
<trans-unit id="AvoidAsyncVoidTitle">
<source>Avoid Async Void</source>
<target state="translated">Async Void를 사용하지 마세요.</target>
Expand Down