Skip to content

Commit 5639216

Browse files
Add PreferAssertFailOverAlwaysFalseConditionsAnalyzer (#2799)
Co-authored-by: Amaury Levé <[email protected]>
1 parent 61a2963 commit 5639216

20 files changed

+1274
-3
lines changed

src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md

+1
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,4 @@ MSTEST0021 | Design | Disabled | PreferDisposeOverTestCleanupAnalyzer, [Document
1111
MSTEST0022 | Design | Disabled | PreferTestCleanupOverDisposeAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0022)
1212
MSTEST0023 | Usage | Info | DoNotNegateBooleanAssertionAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0023)
1313
MSTEST0024 | Usage | Info | DoNotStoreStaticTestContextAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0024)
14+
MSTEST0025 | Usage | Info | PreferAssertFailOverAlwaysFalseConditionsAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0025)

src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs

+1
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,5 @@ internal static class DiagnosticIds
2828
public const string PreferTestCleanupOverDisposeRuleId = "MSTEST0022";
2929
public const string DoNotNegateBooleanAssertionRuleId = "MSTEST0023";
3030
public const string DoNotStoreStaticTestContextAnalyzerRuleId = "MSTEST0024";
31+
public const string PreferAssertFailOverAlwaysFalseConditionsRuleId = "MSTEST0025";
3132
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using System.Collections.Immutable;
5+
6+
using Analyzer.Utilities.Extensions;
7+
8+
using Microsoft.CodeAnalysis;
9+
using Microsoft.CodeAnalysis.Diagnostics;
10+
using Microsoft.CodeAnalysis.Operations;
11+
12+
using MSTest.Analyzers.Helpers;
13+
14+
namespace MSTest.Analyzers;
15+
16+
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
17+
public sealed class PreferAssertFailOverAlwaysFalseConditionsAnalyzer : DiagnosticAnalyzer
18+
{
19+
private enum EqualityStatus
20+
{
21+
Unknown,
22+
Equal,
23+
NotEqual,
24+
}
25+
26+
private const string ExpectedParameterName = "expected";
27+
private const string NotExpectedParameterName = "notExpected";
28+
private const string ActualParameterName = "actual";
29+
private const string ConditionParameterName = "condition";
30+
private const string ValueParameterName = "value";
31+
32+
private static readonly LocalizableResourceString Title = new(nameof(Resources.PreferAssertFailOverAlwaysFalseConditionsTitle), Resources.ResourceManager, typeof(Resources));
33+
private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.PreferAssertFailOverAlwaysFalseConditionsMessageFormat), Resources.ResourceManager, typeof(Resources));
34+
35+
internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
36+
DiagnosticIds.PreferAssertFailOverAlwaysFalseConditionsRuleId,
37+
Title,
38+
MessageFormat,
39+
null,
40+
Category.Design,
41+
DiagnosticSeverity.Info,
42+
isEnabledByDefault: true);
43+
44+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
45+
= ImmutableArray.Create(Rule);
46+
47+
public override void Initialize(AnalysisContext context)
48+
{
49+
context.EnableConcurrentExecution();
50+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
51+
52+
context.RegisterCompilationStartAction(context =>
53+
{
54+
Compilation compilation = context.Compilation;
55+
INamedTypeSymbol? assertSymbol = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingAssert);
56+
if (assertSymbol is not null)
57+
{
58+
context.RegisterOperationAction(context => AnalyzeOperation(context, assertSymbol), OperationKind.Invocation);
59+
}
60+
});
61+
}
62+
63+
private static void AnalyzeOperation(OperationAnalysisContext context, INamedTypeSymbol assertSymbol)
64+
{
65+
var operation = (IInvocationOperation)context.Operation;
66+
if (assertSymbol.Equals(operation.TargetMethod?.ContainingType, SymbolEqualityComparer.Default) &&
67+
IsAlwaysFalse(operation))
68+
{
69+
context.ReportDiagnostic(operation.CreateDiagnostic(Rule, operation.TargetMethod.Name));
70+
}
71+
}
72+
73+
private static bool IsAlwaysFalse(IInvocationOperation operation)
74+
=> operation.TargetMethod.Name switch
75+
{
76+
"IsTrue" => GetConditionArgument(operation) is { Value.ConstantValue: { HasValue: true, Value: false } },
77+
"IsFalse" => GetConditionArgument(operation) is { Value.ConstantValue: { HasValue: true, Value: true } },
78+
"AreEqual" => GetEqualityStatus(operation, ExpectedParameterName) == EqualityStatus.NotEqual,
79+
"AreNotEqual" => GetEqualityStatus(operation, NotExpectedParameterName) == EqualityStatus.Equal,
80+
"IsNotNull" => GetValueArgument(operation) is { Value.ConstantValue: { HasValue: true, Value: null } },
81+
_ => false,
82+
};
83+
84+
private static IArgumentOperation? GetArgumentWithName(IInvocationOperation operation, string name)
85+
=> operation.Arguments.FirstOrDefault(arg => arg.Parameter?.Name == name);
86+
87+
private static IArgumentOperation? GetConditionArgument(IInvocationOperation operation)
88+
=> GetArgumentWithName(operation, ConditionParameterName);
89+
90+
private static IArgumentOperation? GetValueArgument(IInvocationOperation operation)
91+
=> GetArgumentWithName(operation, ValueParameterName);
92+
93+
private static EqualityStatus GetEqualityStatus(IInvocationOperation operation, string expectedOrNotExpectedParameterName)
94+
{
95+
if (GetArgumentWithName(operation, expectedOrNotExpectedParameterName) is { } expectedOrNotExpectedArgument &&
96+
GetArgumentWithName(operation, ActualParameterName) is { } actualArgument &&
97+
expectedOrNotExpectedArgument.Value.ConstantValue.HasValue &&
98+
actualArgument.Value.ConstantValue.HasValue)
99+
{
100+
return Equals(expectedOrNotExpectedArgument.Value.ConstantValue.Value, actualArgument.Value.ConstantValue.Value) ? EqualityStatus.Equal : EqualityStatus.NotEqual;
101+
}
102+
103+
// We are not sure about the equality status
104+
return EqualityStatus.Unknown;
105+
}
106+
}

src/Analyzers/MSTest.Analyzers/Resources.Designer.cs

+18
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Analyzers/MSTest.Analyzers/Resources.resx

+6
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,12 @@
313313
<data name="DoNotStoreStaticTestContextAnalyzerTitle" xml:space="preserve">
314314
<value>Do not store TestContext in a static member</value>
315315
</data>
316+
<data name="PreferAssertFailOverAlwaysFalseConditionsMessageFormat" xml:space="preserve">
317+
<value>Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</value>
318+
</data>
319+
<data name="PreferAssertFailOverAlwaysFalseConditionsTitle" xml:space="preserve">
320+
<value>Use 'Assert.Fail' instead of an always-failing assert</value>
321+
</data>
316322
<data name="PreferConstructorOverTestInitializeMessageFormat" xml:space="preserve">
317323
<value>Prefer constructors over TestInitialize methods</value>
318324
</data>

src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf

+10
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,16 @@
343343
<target state="translated">Neukládejte TestContext ve statickém členu</target>
344344
<note />
345345
</trans-unit>
346+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsMessageFormat">
347+
<source>Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</source>
348+
<target state="new">Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</target>
349+
<note />
350+
</trans-unit>
351+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsTitle">
352+
<source>Use 'Assert.Fail' instead of an always-failing assert</source>
353+
<target state="new">Use 'Assert.Fail' instead of an always-failing assert</target>
354+
<note />
355+
</trans-unit>
346356
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
347357
<source>Prefer constructors over TestInitialize methods</source>
348358
<target state="translated">Upřednostňovat konstruktory před metodami TestInitialize</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf

+10
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,16 @@
339339
<target state="translated">TestContext nicht in einem statischen Member speichern</target>
340340
<note />
341341
</trans-unit>
342+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsMessageFormat">
343+
<source>Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</source>
344+
<target state="new">Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</target>
345+
<note />
346+
</trans-unit>
347+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsTitle">
348+
<source>Use 'Assert.Fail' instead of an always-failing assert</source>
349+
<target state="new">Use 'Assert.Fail' instead of an always-failing assert</target>
350+
<note />
351+
</trans-unit>
342352
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
343353
<source>Prefer constructors over TestInitialize methods</source>
344354
<target state="translated">Konstruktoren gegenüber TestInitialize-Methoden bevorzugen</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf

+10
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,16 @@
339339
<target state="translated">No almacenar TestContext en un miembro estático</target>
340340
<note />
341341
</trans-unit>
342+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsMessageFormat">
343+
<source>Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</source>
344+
<target state="new">Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</target>
345+
<note />
346+
</trans-unit>
347+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsTitle">
348+
<source>Use 'Assert.Fail' instead of an always-failing assert</source>
349+
<target state="new">Use 'Assert.Fail' instead of an always-failing assert</target>
350+
<note />
351+
</trans-unit>
342352
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
343353
<source>Prefer constructors over TestInitialize methods</source>
344354
<target state="translated">Preferir constructores en lugar de métodos TestInitialize</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf

+10
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,16 @@
339339
<target state="translated">Ne pas stocker TestContext dans un membre statique</target>
340340
<note />
341341
</trans-unit>
342+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsMessageFormat">
343+
<source>Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</source>
344+
<target state="new">Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</target>
345+
<note />
346+
</trans-unit>
347+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsTitle">
348+
<source>Use 'Assert.Fail' instead of an always-failing assert</source>
349+
<target state="new">Use 'Assert.Fail' instead of an always-failing assert</target>
350+
<note />
351+
</trans-unit>
342352
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
343353
<source>Prefer constructors over TestInitialize methods</source>
344354
<target state="translated">Préférer les constructeurs aux méthodes TestInitialize</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.it.xlf

+10
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,16 @@
339339
<target state="translated">Non archiviare TestContext in un membro statico</target>
340340
<note />
341341
</trans-unit>
342+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsMessageFormat">
343+
<source>Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</source>
344+
<target state="new">Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</target>
345+
<note />
346+
</trans-unit>
347+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsTitle">
348+
<source>Use 'Assert.Fail' instead of an always-failing assert</source>
349+
<target state="new">Use 'Assert.Fail' instead of an always-failing assert</target>
350+
<note />
351+
</trans-unit>
342352
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
343353
<source>Prefer constructors over TestInitialize methods</source>
344354
<target state="translated">Preferisci costruttori rispetto ai metodi TestInitialize</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.ja.xlf

+10
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,16 @@
339339
<target state="translated">静的メンバーに TestContext を格納しない</target>
340340
<note />
341341
</trans-unit>
342+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsMessageFormat">
343+
<source>Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</source>
344+
<target state="new">Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</target>
345+
<note />
346+
</trans-unit>
347+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsTitle">
348+
<source>Use 'Assert.Fail' instead of an always-failing assert</source>
349+
<target state="new">Use 'Assert.Fail' instead of an always-failing assert</target>
350+
<note />
351+
</trans-unit>
342352
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
343353
<source>Prefer constructors over TestInitialize methods</source>
344354
<target state="translated">TestInitialize メソッドよりもコンストラクターを優先する</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.ko.xlf

+10
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,16 @@
339339
<target state="translated">TestContext를 정적 멤버에 저장하지 마세요</target>
340340
<note />
341341
</trans-unit>
342+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsMessageFormat">
343+
<source>Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</source>
344+
<target state="new">Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</target>
345+
<note />
346+
</trans-unit>
347+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsTitle">
348+
<source>Use 'Assert.Fail' instead of an always-failing assert</source>
349+
<target state="new">Use 'Assert.Fail' instead of an always-failing assert</target>
350+
<note />
351+
</trans-unit>
342352
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
343353
<source>Prefer constructors over TestInitialize methods</source>
344354
<target state="translated">TestInitialize 메서드보다 생성자 선호</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.pl.xlf

+10
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,16 @@
339339
<target state="translated">Nie przechowuj elementu TestContext w statycznym elemencie członkowskim</target>
340340
<note />
341341
</trans-unit>
342+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsMessageFormat">
343+
<source>Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</source>
344+
<target state="new">Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</target>
345+
<note />
346+
</trans-unit>
347+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsTitle">
348+
<source>Use 'Assert.Fail' instead of an always-failing assert</source>
349+
<target state="new">Use 'Assert.Fail' instead of an always-failing assert</target>
350+
<note />
351+
</trans-unit>
342352
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
343353
<source>Prefer constructors over TestInitialize methods</source>
344354
<target state="translated">Preferowanie konstruktorów niż metod TestInitialize</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.pt-BR.xlf

+10
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,16 @@
339339
<target state="translated">Não armazene TestContext em um membro estático</target>
340340
<note />
341341
</trans-unit>
342+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsMessageFormat">
343+
<source>Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</source>
344+
<target state="new">Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</target>
345+
<note />
346+
</trans-unit>
347+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsTitle">
348+
<source>Use 'Assert.Fail' instead of an always-failing assert</source>
349+
<target state="new">Use 'Assert.Fail' instead of an always-failing assert</target>
350+
<note />
351+
</trans-unit>
342352
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
343353
<source>Prefer constructors over TestInitialize methods</source>
344354
<target state="translated">Preferir construtores em vez de métodos TestInitialize</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.ru.xlf

+10
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,16 @@
339339
<target state="translated">Не хранить TestContext в статическом элементе</target>
340340
<note />
341341
</trans-unit>
342+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsMessageFormat">
343+
<source>Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</source>
344+
<target state="new">Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</target>
345+
<note />
346+
</trans-unit>
347+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsTitle">
348+
<source>Use 'Assert.Fail' instead of an always-failing assert</source>
349+
<target state="new">Use 'Assert.Fail' instead of an always-failing assert</target>
350+
<note />
351+
</trans-unit>
342352
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
343353
<source>Prefer constructors over TestInitialize methods</source>
344354
<target state="translated">Предпочитать конструкторы методам TestInitialize</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.tr.xlf

+10
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,16 @@
339339
<target state="translated">TestContext'i statik üyede depolama</target>
340340
<note />
341341
</trans-unit>
342+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsMessageFormat">
343+
<source>Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</source>
344+
<target state="new">Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</target>
345+
<note />
346+
</trans-unit>
347+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsTitle">
348+
<source>Use 'Assert.Fail' instead of an always-failing assert</source>
349+
<target state="new">Use 'Assert.Fail' instead of an always-failing assert</target>
350+
<note />
351+
</trans-unit>
342352
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
343353
<source>Prefer constructors over TestInitialize methods</source>
344354
<target state="translated">Oluşturucuları TestInitialize yöntemlerine tercih et</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hans.xlf

+10
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,16 @@
339339
<target state="translated">不要将 TestContext 存储在静态成员中</target>
340340
<note />
341341
</trans-unit>
342+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsMessageFormat">
343+
<source>Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</source>
344+
<target state="new">Use 'Assert.Fail' instead of an always-failing 'Assert.{0}' assert</target>
345+
<note />
346+
</trans-unit>
347+
<trans-unit id="PreferAssertFailOverAlwaysFalseConditionsTitle">
348+
<source>Use 'Assert.Fail' instead of an always-failing assert</source>
349+
<target state="new">Use 'Assert.Fail' instead of an always-failing assert</target>
350+
<note />
351+
</trans-unit>
342352
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
343353
<source>Prefer constructors over TestInitialize methods</source>
344354
<target state="translated">首选构造函数而不是 TestInitialize 方法</target>

0 commit comments

Comments
 (0)