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

MSTEST0032: Always pass assertions #3238

Merged
merged 40 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
3d76129
add notnull to 0025
engyebrahim Jul 9, 2024
9db6a2b
fix
engyebrahim Jul 9, 2024
902207e
Merge branch 'main' into Enji/fix-0025
engyebrahim Jul 9, 2024
0891a57
fix
engyebrahim Jul 9, 2024
76f453a
add analyzer
engyebrahim Jul 9, 2024
6b17618
fix
engyebrahim Jul 9, 2024
2cb4f5e
add to api
engyebrahim Jul 9, 2024
4d52546
Merge branch 'main' into Enji/fix-0025
engyebrahim Jul 10, 2024
769723b
update helpers
engyebrahim Jul 10, 2024
42b73b2
refactor
engyebrahim Jul 10, 2024
f8e1a57
fix
engyebrahim Jul 10, 2024
d7f7cda
Merge branch 'Enji/fix-0025' of https://github.com/microsoft/testfx i…
engyebrahim Jul 10, 2024
d63a499
add field tests
engyebrahim Jul 10, 2024
41ef2b5
fix build
engyebrahim Jul 10, 2024
17e57d9
add annotatin tests
engyebrahim Jul 10, 2024
5ac5a01
testbase class
engyebrahim Jul 10, 2024
05a82d1
make var local
engyebrahim Jul 10, 2024
b6e2514
Merge branch 'main' into Enji/fix-0025
engyebrahim Jul 10, 2024
5f4e875
Merge branch 'Enji/fix-0025' into Enji/always-pass-0032
engyebrahim Jul 10, 2024
d6e9643
remove c ode
engyebrahim Jul 10, 2024
38ad546
fix it
engyebrahim Jul 10, 2024
1484e41
spaces
engyebrahim Jul 10, 2024
fd00d00
uupdate code
engyebrahim Jul 10, 2024
cc852b6
use nyllable context
engyebrahim Jul 10, 2024
e5ba818
fix
engyebrahim Jul 11, 2024
3ca1df5
add more test
engyebrahim Jul 11, 2024
aae4e66
Merge branch 'main' into Enji/fix-0025
engyebrahim Jul 11, 2024
8ff813b
Merge branch 'Enji/fix-0025' into Enji/always-pass-0032
engyebrahim Jul 11, 2024
0fc59b9
fix analyzer
engyebrahim Jul 11, 2024
53fac98
Update src/Analyzers/MSTest.Analyzers/RoslynAnalyzerHelpers/ISymbolEx…
Evangelink Jul 11, 2024
98abc89
Merge branch 'Enji/fix-0025' into Enji/always-pass-0032
engyebrahim Jul 11, 2024
cb51736
added tests
engyebrahim Jul 11, 2024
6fecdf4
Merge branch 'Enji/always-pass-0032' of https://github.com/microsoft/…
engyebrahim Jul 11, 2024
6ca9459
add link
engyebrahim Jul 11, 2024
df3f22f
fix naming
engyebrahim Jul 11, 2024
08ba886
Merge branch 'main' into Enji/always-pass-0032
engyebrahim Jul 11, 2024
fea46f3
rename
engyebrahim Jul 11, 2024
d1c39a0
update naming
engyebrahim Jul 11, 2024
16a311e
update messages
engyebrahim Jul 11, 2024
a878825
Merge branch 'main' into Enji/always-pass-0032
engyebrahim Jul 11, 2024
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
Expand Up @@ -9,3 +9,4 @@ MSTEST0026 | Usage | Info | AssertionArgsShouldAvoidConditionalAccessRuleId, [Do
MSTEST0029 | Design | Disabled | PublicMethodShouldBeTestMethodAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0029)
MSTEST0030 | Usage | Info | TypeContainingTestMethodShouldBeATestClassAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0030)
MSTEST0031 | Usage | Info | DoNotUseSystemDescriptionAttributeAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0031)
MSTEST0032 | Usage | Info | ReviewAlwaysTrueAssertConditionAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0032)
1 change: 1 addition & 0 deletions src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,5 @@ internal static class DiagnosticIds
public const string PublicMethodShouldBeTestMethodRuleId = "MSTEST0029";
public const string TypeContainingTestMethodShouldBeATestClassRuleId = "MSTEST0030";
public const string DoNotUseSystemDescriptionAttributeRuleId = "MSTEST0031";
public const string ReviewAlwaysTrueAssertConditionAnalyzerRuleId = "MSTEST0032";
}
4 changes: 4 additions & 0 deletions src/Analyzers/MSTest.Analyzers/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ MSTest.Analyzers.PreferConstructorOverTestInitializeAnalyzer
MSTest.Analyzers.PreferConstructorOverTestInitializeAnalyzer.PreferConstructorOverTestInitializeAnalyzer() -> void
MSTest.Analyzers.PreferDisposeOverTestCleanupAnalyzer
MSTest.Analyzers.PreferDisposeOverTestCleanupAnalyzer.PreferDisposeOverTestCleanupAnalyzer() -> void
MSTest.Analyzers.ReviewAlwaysTrueAssertConditionAnalyzer
MSTest.Analyzers.ReviewAlwaysTrueAssertConditionAnalyzer.ReviewAlwaysTrueAssertConditionAnalyzer() -> void
MSTest.Analyzers.PreferTestCleanupOverDisposeAnalyzer
MSTest.Analyzers.PreferTestCleanupOverDisposeAnalyzer.PreferTestCleanupOverDisposeAnalyzer() -> void
MSTest.Analyzers.PreferTestInitializeOverConstructorAnalyzer
Expand Down Expand Up @@ -91,6 +93,8 @@ override MSTest.Analyzers.PreferConstructorOverTestInitializeAnalyzer.Initialize
override MSTest.Analyzers.PreferConstructorOverTestInitializeAnalyzer.SupportedDiagnostics.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.DiagnosticDescriptor!>
override MSTest.Analyzers.PreferDisposeOverTestCleanupAnalyzer.Initialize(Microsoft.CodeAnalysis.Diagnostics.AnalysisContext! context) -> void
override MSTest.Analyzers.PreferDisposeOverTestCleanupAnalyzer.SupportedDiagnostics.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.DiagnosticDescriptor!>
override MSTest.Analyzers.ReviewAlwaysTrueAssertConditionAnalyzer.Initialize(Microsoft.CodeAnalysis.Diagnostics.AnalysisContext! context) -> void
override MSTest.Analyzers.ReviewAlwaysTrueAssertConditionAnalyzer.SupportedDiagnostics.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.DiagnosticDescriptor!>
override MSTest.Analyzers.PreferTestCleanupOverDisposeAnalyzer.Initialize(Microsoft.CodeAnalysis.Diagnostics.AnalysisContext! context) -> void
override MSTest.Analyzers.PreferTestCleanupOverDisposeAnalyzer.SupportedDiagnostics.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.DiagnosticDescriptor!>
override MSTest.Analyzers.PreferTestInitializeOverConstructorAnalyzer.Initialize(Microsoft.CodeAnalysis.Diagnostics.AnalysisContext! context) -> void
Expand Down
18 changes: 18 additions & 0 deletions src/Analyzers/MSTest.Analyzers/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/Analyzers/MSTest.Analyzers/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -454,4 +454,10 @@
<data name="DoNotUseSystemDescriptionAttributeDescription" xml:space="preserve">
<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>
</data>
<data name="ReviewAlwaysTrueAssertConditionAnalyzerTitle" xml:space="preserve">
<value>Assertion condition is always true</value>
</data>
<data name="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat" xml:space="preserve">
<value>Review or remove the assertion as its condition is known to be always true</value>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
// 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 Analyzer.Utilities.Extensions;

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

using MSTest.Analyzers.Helpers;
using MSTest.Analyzers.RoslynAnalyzerHelpers;

namespace MSTest.Analyzers;

/// <summary>
/// MSTEST0032: <inheritdoc cref="Resources.ReviewAlwaysTrueAssertConditionAnalyzerTitle"/>.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed class ReviewAlwaysTrueAssertConditionAnalyzer : DiagnosticAnalyzer
{
private enum EqualityStatus
{
Unknown,
Equal,
NotEqual,
}

private const string ExpectedParameterName = "expected";
private const string NotExpectedParameterName = "notExpected";
private const string ActualParameterName = "actual";
private const string ConditionParameterName = "condition";
private const string ValueParameterName = "value";

private static readonly LocalizableResourceString Title = new(nameof(Resources.ReviewAlwaysTrueAssertConditionAnalyzerTitle), Resources.ResourceManager, typeof(Resources));
private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat), Resources.ResourceManager, typeof(Resources));

internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
DiagnosticIds.ReviewAlwaysTrueAssertConditionAnalyzerRuleId,
Title,
MessageFormat,
null,
Category.Design,
DiagnosticSeverity.Info,
isEnabledByDefault: true);

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

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);

context.RegisterCompilationStartAction(context =>
{
Compilation compilation = context.Compilation;
INamedTypeSymbol? assertSymbol = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingAssert);
INamedTypeSymbol? nullableSymbol = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemNullable);
if (assertSymbol is not null)
{
context.RegisterOperationAction(context => AnalyzeOperation(context, assertSymbol, nullableSymbol), OperationKind.Invocation);
}
});
}

private static void AnalyzeOperation(OperationAnalysisContext context, INamedTypeSymbol assertSymbol, INamedTypeSymbol? nullableSymbol)
{
var operation = (IInvocationOperation)context.Operation;
if (assertSymbol.Equals(operation.TargetMethod.ContainingType, SymbolEqualityComparer.Default) &&
IsAlwaysTrue(operation, nullableSymbol))
{
context.ReportDiagnostic(operation.CreateDiagnostic(Rule));
}
}

private static bool IsAlwaysTrue(IInvocationOperation operation, INamedTypeSymbol? nullableSymbol)
=> operation.TargetMethod.Name switch
{
"IsTrue" => GetConditionArgument(operation) is { Value.ConstantValue: { HasValue: true, Value: true } },
"IsFalse" => GetConditionArgument(operation) is { Value.ConstantValue: { HasValue: true, Value: false } },
"AreEqual" => GetEqualityStatus(operation, ExpectedParameterName) == EqualityStatus.Equal,
"AreNotEqual" => GetEqualityStatus(operation, NotExpectedParameterName) == EqualityStatus.NotEqual,
"IsNull" => GetValueArgument(operation) is { Value.ConstantValue: { HasValue: true, Value: null } },
"IsNotNull" => GetValueArgument(operation) is { } valueArgumentOperation && IsNotNullableType(valueArgumentOperation, nullableSymbol),
_ => false,
};

private static bool IsNotNullableType(IArgumentOperation valueArgumentOperation, INamedTypeSymbol? nullableSymbol)
{
ITypeSymbol? valueArgType = valueArgumentOperation.Value.GetReferencedMemberOrLocalOrParameter().GetReferencedMemberOrLocalOrParameter();
return valueArgType is not null
&& valueArgType.NullableAnnotation == NullableAnnotation.NotAnnotated
&& !SymbolEqualityComparer.IncludeNullability.Equals(valueArgType.OriginalDefinition, nullableSymbol);
}

private static IArgumentOperation? GetArgumentWithName(IInvocationOperation operation, string name)
=> operation.Arguments.FirstOrDefault(arg => arg.Parameter?.Name == name);

private static IArgumentOperation? GetConditionArgument(IInvocationOperation operation)
=> GetArgumentWithName(operation, ConditionParameterName);

private static IArgumentOperation? GetValueArgument(IInvocationOperation operation)
=> GetArgumentWithName(operation, ValueParameterName);

private static EqualityStatus GetEqualityStatus(IInvocationOperation operation, string expectedOrNotExpectedParameterName)
{
if (GetArgumentWithName(operation, expectedOrNotExpectedParameterName) is { } expectedOrNotExpectedArgument &&
GetArgumentWithName(operation, ActualParameterName) is { } actualArgument &&
expectedOrNotExpectedArgument.Value.ConstantValue.HasValue &&
actualArgument.Value.ConstantValue.HasValue)
{
return Equals(expectedOrNotExpectedArgument.Value.ConstantValue.Value, actualArgument.Value.ConstantValue.Value) ? EqualityStatus.Equal : EqualityStatus.NotEqual;
}

// We are not sure about the equality status
return EqualityStatus.Unknown;
}
}
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 @@ -236,6 +236,16 @@
<target state="translated">Místo trvalého neúspěšného vyhodnocovacího výrazu použijte „Assert.Fail“.</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
<source>Review or remove the assertion as its condition is known to be always true</source>
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
<source>Assertion condition is always true</source>
<target state="new">Assertion condition is always true</target>
<note />
</trans-unit>
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
<source>Prefer constructors over TestInitialize methods</source>
<target state="translated">Upřednostňovat konstruktory před metodami TestInitialize</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 @@ -234,6 +234,16 @@
<target state="translated">Verwenden Sie „Assert.Fail“ anstelle einer Assert-Anweisung, bei der immer ein Fehler auftritt.</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
<source>Review or remove the assertion as its condition is known to be always true</source>
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
<source>Assertion condition is always true</source>
<target state="new">Assertion condition is always true</target>
<note />
</trans-unit>
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
<source>Prefer constructors over TestInitialize methods</source>
<target state="translated">Konstruktoren gegenüber TestInitialize-Methoden bevorzugen</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 @@ -234,6 +234,16 @@
<target state="translated">Usar "Assert.Fail" en lugar de una aserción que siempre tiene errores</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
<source>Review or remove the assertion as its condition is known to be always true</source>
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
<source>Assertion condition is always true</source>
<target state="new">Assertion condition is always true</target>
<note />
</trans-unit>
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
<source>Prefer constructors over TestInitialize methods</source>
<target state="translated">Preferir constructores en lugar de métodos TestInitialize</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 @@ -234,6 +234,16 @@
<target state="translated">Utilisez « Assert.Fail » à la place d’une assertion toujours en échec</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
<source>Review or remove the assertion as its condition is known to be always true</source>
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
<source>Assertion condition is always true</source>
<target state="new">Assertion condition is always true</target>
<note />
</trans-unit>
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
<source>Prefer constructors over TestInitialize methods</source>
<target state="translated">Préférer les constructeurs aux méthodes TestInitialize</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 @@ -234,6 +234,16 @@
<target state="translated">Usare 'Assert.Fail' invece di un'asserzione che ha sempre esito negativo</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
<source>Review or remove the assertion as its condition is known to be always true</source>
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
<source>Assertion condition is always true</source>
<target state="new">Assertion condition is always true</target>
<note />
</trans-unit>
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
<source>Prefer constructors over TestInitialize methods</source>
<target state="translated">Preferisci costruttori rispetto ai metodi TestInitialize</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.ja.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,16 @@
<target state="translated">常に失敗しているアサートの代わりに 'Assert.Fail' を使用する</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
<source>Review or remove the assertion as its condition is known to be always true</source>
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
<source>Assertion condition is always true</source>
<target state="new">Assertion condition is always true</target>
<note />
</trans-unit>
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
<source>Prefer constructors over TestInitialize methods</source>
<target state="translated">TestInitialize メソッドよりもコンストラクターを優先する</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.ko.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,16 @@
<target state="translated">항상 실패하는 어설션 대신 'Assert.Fail' 사용</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
<source>Review or remove the assertion as its condition is known to be always true</source>
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
<source>Assertion condition is always true</source>
<target state="new">Assertion condition is always true</target>
<note />
</trans-unit>
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
<source>Prefer constructors over TestInitialize methods</source>
<target state="translated">TestInitialize 메서드보다 생성자 선호</target>
Expand Down
10 changes: 10 additions & 0 deletions src/Analyzers/MSTest.Analyzers/xlf/Resources.pl.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,16 @@
<target state="translated">Użyj trybu „Assert.Fail” zamiast kończącej się zawsze niepowodzeniem instrukcji asercji</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerMessageFormat">
<source>Review or remove the assertion as its condition is known to be always true</source>
<target state="new">Review or remove the assertion as its condition is known to be always true</target>
<note />
</trans-unit>
<trans-unit id="ReviewAlwaysTrueAssertConditionAnalyzerTitle">
<source>Assertion condition is always true</source>
<target state="new">Assertion condition is always true</target>
<note />
</trans-unit>
<trans-unit id="PreferConstructorOverTestInitializeMessageFormat">
<source>Prefer constructors over TestInitialize methods</source>
<target state="translated">Preferowanie konstruktorów niż metod TestInitialize</target>
Expand Down
Loading
Loading