Skip to content

Commit 7d73c91

Browse files
engyebrahimEvangelink
engyebrahim
and
Evangelink
authored
MSTEST0032: Always pass assertions (#3238)
Co-authored-by: Amaury Levé <[email protected]>
1 parent 8799e8d commit 7d73c91

20 files changed

+1395
-0
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@ MSTEST0026 | Usage | Info | AssertionArgsShouldAvoidConditionalAccessRuleId, [Do
99
MSTEST0029 | Design | Disabled | PublicMethodShouldBeTestMethodAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0029)
1010
MSTEST0030 | Usage | Info | TypeContainingTestMethodShouldBeATestClassAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0030)
1111
MSTEST0031 | Usage | Info | DoNotUseSystemDescriptionAttributeAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0031)
12+
MSTEST0032 | Usage | Info | ReviewAlwaysTrueAssertConditionAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0032)

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,5 @@ internal static class DiagnosticIds
3535
public const string PublicMethodShouldBeTestMethodRuleId = "MSTEST0029";
3636
public const string TypeContainingTestMethodShouldBeATestClassRuleId = "MSTEST0030";
3737
public const string DoNotUseSystemDescriptionAttributeRuleId = "MSTEST0031";
38+
public const string ReviewAlwaysTrueAssertConditionAnalyzerRuleId = "MSTEST0032";
3839
}

‎src/Analyzers/MSTest.Analyzers/PublicAPI.Unshipped.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ MSTest.Analyzers.PreferConstructorOverTestInitializeAnalyzer
3131
MSTest.Analyzers.PreferConstructorOverTestInitializeAnalyzer.PreferConstructorOverTestInitializeAnalyzer() -> void
3232
MSTest.Analyzers.PreferDisposeOverTestCleanupAnalyzer
3333
MSTest.Analyzers.PreferDisposeOverTestCleanupAnalyzer.PreferDisposeOverTestCleanupAnalyzer() -> void
34+
MSTest.Analyzers.ReviewAlwaysTrueAssertConditionAnalyzer
35+
MSTest.Analyzers.ReviewAlwaysTrueAssertConditionAnalyzer.ReviewAlwaysTrueAssertConditionAnalyzer() -> void
3436
MSTest.Analyzers.PreferTestCleanupOverDisposeAnalyzer
3537
MSTest.Analyzers.PreferTestCleanupOverDisposeAnalyzer.PreferTestCleanupOverDisposeAnalyzer() -> void
3638
MSTest.Analyzers.PreferTestInitializeOverConstructorAnalyzer
@@ -91,6 +93,8 @@ override MSTest.Analyzers.PreferConstructorOverTestInitializeAnalyzer.Initialize
9193
override MSTest.Analyzers.PreferConstructorOverTestInitializeAnalyzer.SupportedDiagnostics.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.DiagnosticDescriptor!>
9294
override MSTest.Analyzers.PreferDisposeOverTestCleanupAnalyzer.Initialize(Microsoft.CodeAnalysis.Diagnostics.AnalysisContext! context) -> void
9395
override MSTest.Analyzers.PreferDisposeOverTestCleanupAnalyzer.SupportedDiagnostics.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.DiagnosticDescriptor!>
96+
override MSTest.Analyzers.ReviewAlwaysTrueAssertConditionAnalyzer.Initialize(Microsoft.CodeAnalysis.Diagnostics.AnalysisContext! context) -> void
97+
override MSTest.Analyzers.ReviewAlwaysTrueAssertConditionAnalyzer.SupportedDiagnostics.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.DiagnosticDescriptor!>
9498
override MSTest.Analyzers.PreferTestCleanupOverDisposeAnalyzer.Initialize(Microsoft.CodeAnalysis.Diagnostics.AnalysisContext! context) -> void
9599
override MSTest.Analyzers.PreferTestCleanupOverDisposeAnalyzer.SupportedDiagnostics.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.DiagnosticDescriptor!>
96100
override MSTest.Analyzers.PreferTestInitializeOverConstructorAnalyzer.Initialize(Microsoft.CodeAnalysis.Diagnostics.AnalysisContext! context) -> void

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

Lines changed: 18 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/Analyzers/MSTest.Analyzers/Resources.resx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,4 +454,10 @@
454454
<data name="DoNotUseSystemDescriptionAttributeDescription" xml:space="preserve">
455455
<value>'System.ComponentModel.DescriptionAttribute' has no effect in the context of tests and you likely wanted to use 'Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute' instead.</value>
456456
</data>
457+
<data name="ReviewAlwaysTrueAssertConditionAnalyzerTitle" xml:space="preserve">
458+
<value>Assertion condition is always true</value>
459+
</data>
460+
<data name="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat" xml:space="preserve">
461+
<value>Review or remove the assertion as its condition is known to be always true</value>
462+
</data>
457463
</root>

‎src/Analyzers/MSTest.Analyzers/ReviewAlwaysTrueAssertConditionAnalyzer.cs

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
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+
using MSTest.Analyzers.RoslynAnalyzerHelpers;
14+
15+
namespace MSTest.Analyzers;
16+
17+
/// <summary>
18+
/// MSTEST0032: <inheritdoc cref="Resources.ReviewAlwaysTrueAssertConditionAnalyzerTitle"/>.
19+
/// </summary>
20+
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
21+
public sealed class ReviewAlwaysTrueAssertConditionAnalyzer : DiagnosticAnalyzer
22+
{
23+
private enum EqualityStatus
24+
{
25+
Unknown,
26+
Equal,
27+
NotEqual,
28+
}
29+
30+
private const string ExpectedParameterName = "expected";
31+
private const string NotExpectedParameterName = "notExpected";
32+
private const string ActualParameterName = "actual";
33+
private const string ConditionParameterName = "condition";
34+
private const string ValueParameterName = "value";
35+
36+
private static readonly LocalizableResourceString Title = new(nameof(Resources.ReviewAlwaysTrueAssertConditionAnalyzerTitle), Resources.ResourceManager, typeof(Resources));
37+
private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat), Resources.ResourceManager, typeof(Resources));
38+
39+
internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
40+
DiagnosticIds.ReviewAlwaysTrueAssertConditionAnalyzerRuleId,
41+
Title,
42+
MessageFormat,
43+
null,
44+
Category.Design,
45+
DiagnosticSeverity.Info,
46+
isEnabledByDefault: true);
47+
48+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
49+
= ImmutableArray.Create(Rule);
50+
51+
public override void Initialize(AnalysisContext context)
52+
{
53+
context.EnableConcurrentExecution();
54+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
55+
56+
context.RegisterCompilationStartAction(context =>
57+
{
58+
Compilation compilation = context.Compilation;
59+
INamedTypeSymbol? assertSymbol = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingAssert);
60+
INamedTypeSymbol? nullableSymbol = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemNullable);
61+
if (assertSymbol is not null)
62+
{
63+
context.RegisterOperationAction(context => AnalyzeOperation(context, assertSymbol, nullableSymbol), OperationKind.Invocation);
64+
}
65+
});
66+
}
67+
68+
private static void AnalyzeOperation(OperationAnalysisContext context, INamedTypeSymbol assertSymbol, INamedTypeSymbol? nullableSymbol)
69+
{
70+
var operation = (IInvocationOperation)context.Operation;
71+
if (assertSymbol.Equals(operation.TargetMethod.ContainingType, SymbolEqualityComparer.Default) &&
72+
IsAlwaysTrue(operation, nullableSymbol))
73+
{
74+
context.ReportDiagnostic(operation.CreateDiagnostic(Rule));
75+
}
76+
}
77+
78+
private static bool IsAlwaysTrue(IInvocationOperation operation, INamedTypeSymbol? nullableSymbol)
79+
=> operation.TargetMethod.Name switch
80+
{
81+
"IsTrue" => GetConditionArgument(operation) is { Value.ConstantValue: { HasValue: true, Value: true } },
82+
"IsFalse" => GetConditionArgument(operation) is { Value.ConstantValue: { HasValue: true, Value: false } },
83+
"AreEqual" => GetEqualityStatus(operation, ExpectedParameterName) == EqualityStatus.Equal,
84+
"AreNotEqual" => GetEqualityStatus(operation, NotExpectedParameterName) == EqualityStatus.NotEqual,
85+
"IsNull" => GetValueArgument(operation) is { Value.ConstantValue: { HasValue: true, Value: null } },
86+
"IsNotNull" => GetValueArgument(operation) is { } valueArgumentOperation && IsNotNullableType(valueArgumentOperation, nullableSymbol),
87+
_ => false,
88+
};
89+
90+
private static bool IsNotNullableType(IArgumentOperation valueArgumentOperation, INamedTypeSymbol? nullableSymbol)
91+
{
92+
ITypeSymbol? valueArgType = valueArgumentOperation.Value.GetReferencedMemberOrLocalOrParameter().GetReferencedMemberOrLocalOrParameter();
93+
return valueArgType is not null
94+
&& valueArgType.NullableAnnotation == NullableAnnotation.NotAnnotated
95+
&& !SymbolEqualityComparer.IncludeNullability.Equals(valueArgType.OriginalDefinition, nullableSymbol);
96+
}
97+
98+
private static IArgumentOperation? GetArgumentWithName(IInvocationOperation operation, string name)
99+
=> operation.Arguments.FirstOrDefault(arg => arg.Parameter?.Name == name);
100+
101+
private static IArgumentOperation? GetConditionArgument(IInvocationOperation operation)
102+
=> GetArgumentWithName(operation, ConditionParameterName);
103+
104+
private static IArgumentOperation? GetValueArgument(IInvocationOperation operation)
105+
=> GetArgumentWithName(operation, ValueParameterName);
106+
107+
private static EqualityStatus GetEqualityStatus(IInvocationOperation operation, string expectedOrNotExpectedParameterName)
108+
{
109+
if (GetArgumentWithName(operation, expectedOrNotExpectedParameterName) is { } expectedOrNotExpectedArgument &&
110+
GetArgumentWithName(operation, ActualParameterName) is { } actualArgument &&
111+
expectedOrNotExpectedArgument.Value.ConstantValue.HasValue &&
112+
actualArgument.Value.ConstantValue.HasValue)
113+
{
114+
return Equals(expectedOrNotExpectedArgument.Value.ConstantValue.Value, actualArgument.Value.ConstantValue.Value) ? EqualityStatus.Equal : EqualityStatus.NotEqual;
115+
}
116+
117+
// We are not sure about the equality status
118+
return EqualityStatus.Unknown;
119+
}
120+
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,16 @@
236236
<target state="translated">Místo trvalého neúspěšného vyhodnocovacího výrazu použijte „Assert.Fail“.</target>
237237
<note />
238238
</trans-unit>
239+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
240+
<source>Review or remove the assertion as its condition is known to be always true</source>
241+
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
242+
<note />
243+
</trans-unit>
244+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
245+
<source>Assertion condition is always true</source>
246+
<target state="new">Assertion condition is always true</target>
247+
<note />
248+
</trans-unit>
239249
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
240250
<source>Prefer constructors over TestInitialize methods</source>
241251
<target state="translated">Upřednostňovat konstruktory před metodami TestInitialize</target>

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,16 @@
234234
<target state="translated">Verwenden Sie „Assert.Fail“ anstelle einer Assert-Anweisung, bei der immer ein Fehler auftritt.</target>
235235
<note />
236236
</trans-unit>
237+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
238+
<source>Review or remove the assertion as its condition is known to be always true</source>
239+
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
240+
<note />
241+
</trans-unit>
242+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
243+
<source>Assertion condition is always true</source>
244+
<target state="new">Assertion condition is always true</target>
245+
<note />
246+
</trans-unit>
237247
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
238248
<source>Prefer constructors over TestInitialize methods</source>
239249
<target state="translated">Konstruktoren gegenüber TestInitialize-Methoden bevorzugen</target>

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,16 @@
234234
<target state="translated">Usar "Assert.Fail" en lugar de una aserción que siempre tiene errores</target>
235235
<note />
236236
</trans-unit>
237+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
238+
<source>Review or remove the assertion as its condition is known to be always true</source>
239+
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
240+
<note />
241+
</trans-unit>
242+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
243+
<source>Assertion condition is always true</source>
244+
<target state="new">Assertion condition is always true</target>
245+
<note />
246+
</trans-unit>
237247
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
238248
<source>Prefer constructors over TestInitialize methods</source>
239249
<target state="translated">Preferir constructores en lugar de métodos TestInitialize</target>

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,16 @@
234234
<target state="translated">Utilisez « Assert.Fail » à la place d’une assertion toujours en échec</target>
235235
<note />
236236
</trans-unit>
237+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
238+
<source>Review or remove the assertion as its condition is known to be always true</source>
239+
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
240+
<note />
241+
</trans-unit>
242+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
243+
<source>Assertion condition is always true</source>
244+
<target state="new">Assertion condition is always true</target>
245+
<note />
246+
</trans-unit>
237247
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
238248
<source>Prefer constructors over TestInitialize methods</source>
239249
<target state="translated">Préférer les constructeurs aux méthodes TestInitialize</target>

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,16 @@
234234
<target state="translated">Usare 'Assert.Fail' invece di un'asserzione che ha sempre esito negativo</target>
235235
<note />
236236
</trans-unit>
237+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
238+
<source>Review or remove the assertion as its condition is known to be always true</source>
239+
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
240+
<note />
241+
</trans-unit>
242+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
243+
<source>Assertion condition is always true</source>
244+
<target state="new">Assertion condition is always true</target>
245+
<note />
246+
</trans-unit>
237247
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
238248
<source>Prefer constructors over TestInitialize methods</source>
239249
<target state="translated">Preferisci costruttori rispetto ai metodi TestInitialize</target>

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,16 @@
234234
<target state="translated">常に失敗しているアサートの代わりに 'Assert.Fail' を使用する</target>
235235
<note />
236236
</trans-unit>
237+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
238+
<source>Review or remove the assertion as its condition is known to be always true</source>
239+
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
240+
<note />
241+
</trans-unit>
242+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
243+
<source>Assertion condition is always true</source>
244+
<target state="new">Assertion condition is always true</target>
245+
<note />
246+
</trans-unit>
237247
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
238248
<source>Prefer constructors over TestInitialize methods</source>
239249
<target state="translated">TestInitialize メソッドよりもコンストラクターを優先する</target>

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,16 @@
234234
<target state="translated">항상 실패하는 어설션 대신 'Assert.Fail' 사용</target>
235235
<note />
236236
</trans-unit>
237+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
238+
<source>Review or remove the assertion as its condition is known to be always true</source>
239+
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
240+
<note />
241+
</trans-unit>
242+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
243+
<source>Assertion condition is always true</source>
244+
<target state="new">Assertion condition is always true</target>
245+
<note />
246+
</trans-unit>
237247
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
238248
<source>Prefer constructors over TestInitialize methods</source>
239249
<target state="translated">TestInitialize 메서드보다 생성자 선호</target>

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,16 @@
234234
<target state="translated">Użyj trybu „Assert.Fail” zamiast kończącej się zawsze niepowodzeniem instrukcji asercji</target>
235235
<note />
236236
</trans-unit>
237+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
238+
<source>Review or remove the assertion as its condition is known to be always true</source>
239+
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
240+
<note />
241+
</trans-unit>
242+
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
243+
<source>Assertion condition is always true</source>
244+
<target state="new">Assertion condition is always true</target>
245+
<note />
246+
</trans-unit>
237247
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
238248
<source>Prefer constructors over TestInitialize methods</source>
239249
<target state="translated">Preferowanie konstruktorów niż metod TestInitialize</target>

0 commit comments

Comments
 (0)