Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,264 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Collections.Immutable;
using System.Composition;

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Editing;

using MSTest.Analyzers.Helpers;

namespace MSTest.Analyzers;

/// <summary>
/// Code fix for MSTEST0023: Do not negate boolean assertions.
/// </summary>
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(DoNotNegateBooleanAssertionFixer))]
[Shared]
public sealed class DoNotNegateBooleanAssertionFixer : CodeFixProvider
{
/// <inheritdoc />
public override ImmutableArray<string> FixableDiagnosticIds { get; }
= ImmutableArray.Create(DiagnosticIds.DoNotNegateBooleanAssertionRuleId);

/// <inheritdoc />
public override FixAllProvider GetFixAllProvider()
=> FixAll.Instance;

/// <inheritdoc />
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
Diagnostic diagnostic = context.Diagnostics[0];
SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);

if (root is null)
{
return;
}

SyntaxNode node = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
if (node is not InvocationExpressionSyntax invocation)
{
return;
}

// Get the proper assert method name from diagnostic properties
if (!diagnostic.Properties.TryGetValue(DoNotNegateBooleanAssertionAnalyzer.ProperAssertMethodNameKey, out string? properAssertMethodName) ||
properAssertMethodName is null)
{
return;
}

string title = string.Format(System.Globalization.CultureInfo.InvariantCulture, Resources.DoNotNegateBooleanAssertionFix, properAssertMethodName);

context.RegisterCodeFix(
CodeAction.Create(
title,
cancellationToken => FixNegatedAssertionAsync(context.Document, invocation, properAssertMethodName, cancellationToken),
equivalenceKey: nameof(DoNotNegateBooleanAssertionFixer)),
diagnostic);
}

private static async Task<Document> FixNegatedAssertionAsync(
Document document,
InvocationExpressionSyntax invocation,
string properAssertMethodName,
CancellationToken cancellationToken)
{
DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);

// Get the member access expression (Assert.IsTrue or Assert.IsFalse)
if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess)
{
return document;
}

// Get the argument list
if (invocation.ArgumentList.Arguments.Count == 0)
{
return document;
}

// Find the 'condition' argument (should be the first one or named 'condition')
ArgumentSyntax? conditionArgument = null;
int conditionArgumentIndex = -1;

for (int i = 0; i < invocation.ArgumentList.Arguments.Count; i++)
{
ArgumentSyntax arg = invocation.ArgumentList.Arguments[i];
if (arg.NameColon?.Name.Identifier.ValueText == "condition" || (arg.NameColon == null && i == 0))
{
conditionArgument = arg;
conditionArgumentIndex = i;
break;
}
}

if (conditionArgument == null || conditionArgumentIndex == -1)
{
return document;
}

ExpressionSyntax argumentExpression = conditionArgument.Expression;

// Remove ALL negations (for double negation in one step)
ExpressionSyntax unnegatedExpression = RemoveAllNegations(argumentExpression);

if (unnegatedExpression == argumentExpression)
{
// No negation found, nothing to fix
return document;
}

// Always swap the method name
IdentifierNameSyntax newMethodName = SyntaxFactory.IdentifierName(properAssertMethodName);
MemberAccessExpressionSyntax newMemberAccess = memberAccess.WithName(newMethodName);
ArgumentSyntax newArgument = conditionArgument.WithExpression(unnegatedExpression);

SeparatedSyntaxList<ArgumentSyntax> newArguments = invocation.ArgumentList.Arguments.Replace(
invocation.ArgumentList.Arguments[conditionArgumentIndex],
newArgument);
ArgumentListSyntax newArgumentList = invocation.ArgumentList.WithArguments(newArguments);

InvocationExpressionSyntax newInvocation = invocation
.WithExpression(newMemberAccess)
.WithArgumentList(newArgumentList);

editor.ReplaceNode(invocation, newInvocation);
return editor.GetChangedDocument();
}

private static ExpressionSyntax RemoveAllNegations(ExpressionSyntax expression)
{
// Walk down parentheses and negations until we find the core expression
ExpressionSyntax current = expression;

while (true)
{
// Remove parentheses
if (current is ParenthesizedExpressionSyntax parenthesized)
{
current = parenthesized.Expression;
continue;
}

// Remove negation
if (current is PrefixUnaryExpressionSyntax { OperatorToken.RawKind: (int)SyntaxKind.ExclamationToken } prefixUnary)
{
current = prefixUnary.Operand;
continue;
}

// No more parentheses or negations
break;
}

return current;
}

private sealed class FixAll : DocumentBasedFixAllProvider
{
public static readonly FixAll Instance = new();

protected override async Task<Document?> FixAllAsync(
FixAllContext fixAllContext,
Document document,
ImmutableArray<Diagnostic> diagnostics)
{
SyntaxNode? root = await document.GetSyntaxRootAsync(fixAllContext.CancellationToken).ConfigureAwait(false);
if (root is null)
{
return document;
}

// Collect all invocations that need to be replaced
var nodesToReplace = new Dictionary<InvocationExpressionSyntax, InvocationExpressionSyntax>();

foreach (Diagnostic diagnostic in diagnostics)
{
SyntaxNode node = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
if (node is not InvocationExpressionSyntax invocation)
{
continue;
}

if (!diagnostic.Properties.TryGetValue(DoNotNegateBooleanAssertionAnalyzer.ProperAssertMethodNameKey, out string? properAssertMethodName) ||
properAssertMethodName is null)
{
continue;
}

// Get the member access expression
if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess)
{
continue;
}

if (invocation.ArgumentList.Arguments.Count == 0)
{
continue;
}

ArgumentSyntax? conditionArgument = null;
int conditionArgumentIndex = -1;

for (int i = 0; i < invocation.ArgumentList.Arguments.Count; i++)
{
ArgumentSyntax arg = invocation.ArgumentList.Arguments[i];
if (arg.NameColon?.Name.Identifier.ValueText == "condition" || (arg.NameColon == null && i == 0))
{
conditionArgument = arg;
conditionArgumentIndex = i;
break;
}
}

if (conditionArgument == null || conditionArgumentIndex == -1)
{
continue;
}

ExpressionSyntax argumentExpression = conditionArgument.Expression;
ExpressionSyntax unnegatedExpression = RemoveAllNegations(argumentExpression);

if (unnegatedExpression == argumentExpression)
{
continue;
}

// Create the new method name identifier
IdentifierNameSyntax newMethodName = SyntaxFactory.IdentifierName(properAssertMethodName);
MemberAccessExpressionSyntax newMemberAccess = memberAccess.WithName(newMethodName);
ArgumentSyntax newArgument = conditionArgument.WithExpression(unnegatedExpression);

SeparatedSyntaxList<ArgumentSyntax> newArguments = invocation.ArgumentList.Arguments.Replace(
invocation.ArgumentList.Arguments[conditionArgumentIndex],
newArgument);
ArgumentListSyntax newArgumentList = invocation.ArgumentList.WithArguments(newArguments);

InvocationExpressionSyntax newInvocation = invocation
.WithExpression(newMemberAccess)
.WithArgumentList(newArgumentList);

nodesToReplace[invocation] = newInvocation;
}

// Apply all replacements at once
if (nodesToReplace.Count > 0)
{
root = root.ReplaceNodes(
nodesToReplace.Keys,
(originalNode, _) => nodesToReplace[originalNode]);

return document.WithSyntaxRoot(root);
}

return document;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public sealed class DoNotNegateBooleanAssertionAnalyzer : DiagnosticAnalyzer
private static readonly LocalizableResourceString Title = new(nameof(Resources.DoNotNegateBooleanAssertionTitle), Resources.ResourceManager, typeof(Resources));
private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.DoNotNegateBooleanAssertionMessageFormat), Resources.ResourceManager, typeof(Resources));

internal const string ProperAssertMethodNameKey = nameof(ProperAssertMethodNameKey);

internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
DiagnosticIds.DoNotNegateBooleanAssertionRuleId,
Title,
Expand Down Expand Up @@ -64,7 +66,17 @@ private static void AnalyzeOperation(OperationAnalysisContext context, INamedTyp
if (conditionArgument != null
&& conditionArgument.Value.WalkDownConversion() is IUnaryOperation { OperatorKind: UnaryOperatorKind.Not })
{
context.ReportDiagnostic(invocationOperation.CreateDiagnostic(Rule));
// Determine the proper assert method name (swap IsTrue <-> IsFalse)
string currentMethodName = invocationOperation.TargetMethod.Name;
string properAssertMethodName = currentMethodName == "IsTrue" ? "IsFalse" : "IsTrue";

// Add the proper method name to diagnostic properties for the code fix
ImmutableDictionary<string, string?>.Builder properties = ImmutableDictionary.CreateBuilder<string, string?>();
properties.Add(ProperAssertMethodNameKey, properAssertMethodName);

context.ReportDiagnostic(invocationOperation.CreateDiagnostic(
Rule,
properties: properties.ToImmutable()));
}
}
}
6 changes: 6 additions & 0 deletions src/Analyzers/MSTest.Analyzers/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -684,4 +684,10 @@ The type declaring these methods should also respect the following rules:
<data name="IgnoreStringMethodReturnValueDescription" xml:space="preserve">
<value>Methods like Contains, StartsWith, and EndsWith return boolean values that indicate whether the condition was met. Ignoring these return values is likely a mistake.</value>
</data>
<data name="DoNotNegateBooleanAssertionFix" xml:space="preserve">
<value>Use 'Assert.{0}' instead</value>
</data>
<data name="DoNotNegateBooleanAssertionFixAllTitle" xml:space="preserve">
<value>Use proper Assert methods for all negated assertions</value>
</data>
</root>
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,16 @@ Typ deklarující tyto metody by měl také respektovat následující pravidla:
<target state="translated">Argument DataRow by měl být platný</target>
<note />
</trans-unit>
<trans-unit id="DoNotNegateBooleanAssertionFix">
<source>Use 'Assert.{0}' instead</source>
<target state="new">Use 'Assert.{0}' instead</target>
<note />
</trans-unit>
<trans-unit id="DoNotNegateBooleanAssertionFixAllTitle">
<source>Use proper Assert methods for all negated assertions</source>
<target state="new">Use proper Assert methods for all negated assertions</target>
<note />
</trans-unit>
<trans-unit id="DoNotNegateBooleanAssertionMessageFormat">
<source>Do not negate boolean assertions, instead use the opposite assertion</source>
<target state="translated">Nenegujte logické kontrolní výrazy – použijte místo nich opačný kontrolní výraz</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,16 @@ Der Typ, der diese Methoden deklariert, sollte auch die folgenden Regeln beachte
<target state="translated">DataRow muss gültig sein.</target>
<note />
</trans-unit>
<trans-unit id="DoNotNegateBooleanAssertionFix">
<source>Use 'Assert.{0}' instead</source>
<target state="new">Use 'Assert.{0}' instead</target>
<note />
</trans-unit>
<trans-unit id="DoNotNegateBooleanAssertionFixAllTitle">
<source>Use proper Assert methods for all negated assertions</source>
<target state="new">Use proper Assert methods for all negated assertions</target>
<note />
</trans-unit>
<trans-unit id="DoNotNegateBooleanAssertionMessageFormat">
<source>Do not negate boolean assertions, instead use the opposite assertion</source>
<target state="translated">Boolesche Assertionen nicht negiert, sondern die entgegengesetzte Assertion verwenden</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,16 @@ El tipo que declara estos métodos también debe respetar las reglas siguientes:
<target state="translated">DataRow debe ser válido</target>
<note />
</trans-unit>
<trans-unit id="DoNotNegateBooleanAssertionFix">
<source>Use 'Assert.{0}' instead</source>
<target state="new">Use 'Assert.{0}' instead</target>
<note />
</trans-unit>
<trans-unit id="DoNotNegateBooleanAssertionFixAllTitle">
<source>Use proper Assert methods for all negated assertions</source>
<target state="new">Use proper Assert methods for all negated assertions</target>
<note />
</trans-unit>
<trans-unit id="DoNotNegateBooleanAssertionMessageFormat">
<source>Do not negate boolean assertions, instead use the opposite assertion</source>
<target state="translated">No negar las aserciones booleanas, usar en su lugar la aserción opuesta</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,16 @@ Le type doit être une classe
<target state="translated">DataRow doit être valide</target>
<note />
</trans-unit>
<trans-unit id="DoNotNegateBooleanAssertionFix">
<source>Use 'Assert.{0}' instead</source>
<target state="new">Use 'Assert.{0}' instead</target>
<note />
</trans-unit>
<trans-unit id="DoNotNegateBooleanAssertionFixAllTitle">
<source>Use proper Assert methods for all negated assertions</source>
<target state="new">Use proper Assert methods for all negated assertions</target>
<note />
</trans-unit>
<trans-unit id="DoNotNegateBooleanAssertionMessageFormat">
<source>Do not negate boolean assertions, instead use the opposite assertion</source>
<target state="translated">Ne pas annuler les assertions booléennes, mais utiliser l'assertion opposée</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.it.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,16 @@ Anche il tipo che dichiara questi metodi deve rispettare le regole seguenti:
<target state="translated">DataRow deve essere valido</target>
<note />
</trans-unit>
<trans-unit id="DoNotNegateBooleanAssertionFix">
<source>Use 'Assert.{0}' instead</source>
<target state="new">Use 'Assert.{0}' instead</target>
<note />
</trans-unit>
<trans-unit id="DoNotNegateBooleanAssertionFixAllTitle">
<source>Use proper Assert methods for all negated assertions</source>
<target state="new">Use proper Assert methods for all negated assertions</target>
<note />
</trans-unit>
<trans-unit id="DoNotNegateBooleanAssertionMessageFormat">
<source>Do not negate boolean assertions, instead use the opposite assertion</source>
<target state="translated">Non negare asserzioni booleane, usare l'asserzione opposta</target>
Expand Down
Loading