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

Implement MSTestAnalysisMode #4712

Merged
merged 9 commits into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 7 additions & 0 deletions TestFx.sln
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.Testing.Extension
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.Testing.Extensions.Retry", "src\Platform\Microsoft.Testing.Extensions.Retry\Microsoft.Testing.Extensions.Retry.csproj", "{FB4ED3AA-A12E-4192-861F-4B025876AA0F}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "MSTest.GlobalConfigsGenerator", "src\Analyzers\MSTest.GlobalConfigsGenerator\MSTest.GlobalConfigsGenerator.csproj", "{A85AA656-6DB6-4A0B-AA80-CBB4058B3DDB}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -494,6 +496,10 @@ Global
{FB4ED3AA-A12E-4192-861F-4B025876AA0F}.Debug|Any CPU.Build.0 = Debug|Any CPU
{FB4ED3AA-A12E-4192-861F-4B025876AA0F}.Release|Any CPU.ActiveCfg = Release|Any CPU
{FB4ED3AA-A12E-4192-861F-4B025876AA0F}.Release|Any CPU.Build.0 = Release|Any CPU
{A85AA656-6DB6-4A0B-AA80-CBB4058B3DDB}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{A85AA656-6DB6-4A0B-AA80-CBB4058B3DDB}.Debug|Any CPU.Build.0 = Debug|Any CPU
{A85AA656-6DB6-4A0B-AA80-CBB4058B3DDB}.Release|Any CPU.ActiveCfg = Release|Any CPU
{A85AA656-6DB6-4A0B-AA80-CBB4058B3DDB}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down Expand Up @@ -580,6 +586,7 @@ Global
{4A93E1A2-B61E-31B2-33F2-478156A9B5E7} = {E7F15C9C-3928-47AD-8462-64FD29FFCA54}
{53EBA540-F6CF-0715-1F62-241A53F537EC} = {6AEE1440-FDF0-4729-8196-B24D0E333550}
{FB4ED3AA-A12E-4192-861F-4B025876AA0F} = {6AEE1440-FDF0-4729-8196-B24D0E333550}
{A85AA656-6DB6-4A0B-AA80-CBB4058B3DDB} = {E7F15C9C-3928-47AD-8462-64FD29FFCA54}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {31E0F4D5-975A-41CC-933E-545B2201FAF9}
Expand Down
2 changes: 1 addition & 1 deletion eng/verify-nupkgs.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function Confirm-NugetPackages {
"MSTest.TestFramework" = 148
"MSTest.TestAdapter" = 75
"MSTest" = 6
"MSTest.Analyzers" = 50
"MSTest.Analyzers" = 56
}

$packageDirectory = Resolve-Path "$PSScriptRoot/../artifacts/packages/$configuration"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
Expand Down Expand Up @@ -32,7 +32,15 @@
<None Include="$(PackageReadmeFile)" Pack="true" PackagePath="\" />
</ItemGroup>

<Target Name="_AddAnalyzersToOutput">
<Target Name="_GenerateGlobalConfigs">
<PropertyGroup>
<DotNetExecutable Condition="'$(OS)' == 'Windows_NT'">$(DotNetRoot)dotnet.exe</DotNetExecutable>
<DotNetExecutable Condition="'$(DotNetExecutable)' == ''">$(DotNetRoot)dotnet</DotNetExecutable>
</PropertyGroup>
<Exec Command="$(DotNetExecutable) run" WorkingDirectory="$(RepoRoot)src\Analyzers\MSTest.GlobalConfigsGenerator" EnvironmentVariables="OUTPUT_PATH=$(OutputPath)" />
</Target>

<Target Name="_AddAnalyzersToOutput" DependsOnTargets="_GenerateGlobalConfigs">
<ItemGroup>
<TfmSpecificPackageFile Include="$(OutputPath)\MSTest.Analyzers.dll" PackagePath="analyzers/dotnet/cs" />
<TfmSpecificPackageFile Include="$(OutputPath)\MSTest.Analyzers.CodeFixes.dll" PackagePath="analyzers/dotnet/cs" />
Expand All @@ -42,6 +50,9 @@
<!-- NOTE: Currently, code fixes are C# only. -->
<TfmSpecificPackageFile Include="$(OutputPath)\MSTest.Analyzers.dll" PackagePath="analyzers/dotnet/vb" />
<TfmSpecificPackageFile Include="$(OutputPath)\**\MSTest.Analyzers.resources.dll" PackagePath="analyzers/dotnet/vb/" />

<TfmSpecificPackageFile Include="$(OutputPath)\globalconfigs\*.globalconfig" PackagePath="globalconfigs" />
<TfmSpecificPackageFile Include="buildTransitive\*" PackagePath="buildTransitive" />
</ItemGroup>
</Target>

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<Project>
<PropertyGroup>
<MSTestAnalysisMode Condition="'$(MSTestAnalysisMode)' == ''">Default</MSTestAnalysisMode>
</PropertyGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<Project>
<ItemGroup>
<!-- Don't use GlobalAnalyzerConfigFiles here -->
<!-- GlobalAnalyzerConfigFiles are evaluated very early before restore is done -->
<!-- https://github.com/dotnet/roslyn/blob/22afb85fd930deeeb1e0f39851ff0a09f86abe73/src/Compilers/Core/MSBuildTask/Microsoft.Managed.Core.targets#L138 -->
<!-- We use EditorConfigFiles instead which is passed to Csc task. -->
<!-- For the case of Code Quality (CAxxxx) rules, they can use GlobalAnalyzerConfigFiles because it's added from SDK and happens early enough for them -->
<EditorConfigFiles Include="$(MSBuildThisFileDirectory)..\globalconfigs\mstest-$(MSTestAnalysisMode.ToLowerInvariant()).globalconfig" Condition="'$(MSTestAnalysisMode)' != ''" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -22,29 +22,61 @@ public static DiagnosticDescriptor Create(
DiagnosticSeverity defaultSeverity,
bool isEnabledByDefault,
bool isReportedAtCompilationEnd = false,
bool escalateToErrorInRecommended = false,
bool disableInAllMode = false,
params string[] customTags)
=> new(id, title, messageFormat, category.ToString(), defaultSeverity, isEnabledByDefault, description,
$"https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/{id.ToLowerInvariant()}",
CreateCustomTags(isReportedAtCompilationEnd, customTags));
CreateCustomTags(isReportedAtCompilationEnd, escalateToErrorInRecommended, disableInAllMode, customTags));

public static DiagnosticDescriptor WithMessage(this DiagnosticDescriptor diagnosticDescriptor, LocalizableResourceString messageFormat)
=> new(diagnosticDescriptor.Id, diagnosticDescriptor.Title, messageFormat, diagnosticDescriptor.Category, diagnosticDescriptor.DefaultSeverity,
diagnosticDescriptor.IsEnabledByDefault, diagnosticDescriptor.Description, diagnosticDescriptor.HelpLinkUri, diagnosticDescriptor.CustomTags.ToArray());

private static string[] CreateCustomTags(bool isReportedAtCompilationEnd, string[] customTags)
private static string[] CreateCustomTags(bool isReportedAtCompilationEnd, bool escalateToErrorInRecommended, bool disableInAllMode, string[] customTags)
{
if (!isReportedAtCompilationEnd)
int extraTagsCount = 0;
if (isReportedAtCompilationEnd)
{
extraTagsCount++;
}

if (escalateToErrorInRecommended)
{
extraTagsCount++;
}

if (disableInAllMode)
{
extraTagsCount++;
}

if (extraTagsCount == 0)
{
return customTags;
}

string[] tags = new string[customTags.Length + 1];
string[] tags = new string[customTags.Length + extraTagsCount];
if (customTags.Length > 0)
{
customTags.CopyTo(tags, 0);
}

tags[^1] = WellKnownCustomTags.CompilationEnd;
int index = customTags.Length;
if (isReportedAtCompilationEnd)
{
tags[index++] = WellKnownCustomTags.CompilationEnd;
}

if (escalateToErrorInRecommended)
{
tags[index++] = WellKnownCustomTags.EscalateToErrorInRecommended;
}

if (disableInAllMode)
{
tags[index] = WellKnownCustomTags.DisableInAllMode;
}

return tags;
}
Expand Down
2 changes: 2 additions & 0 deletions src/Analyzers/MSTest.Analyzers/Helpers/WellKnownCustomTags.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,6 @@ namespace MSTest.Analyzers.Helpers;
internal static class WellKnownCustomTags
{
public const string CompilationEnd = nameof(CompilationEnd);
public const string EscalateToErrorInRecommended = nameof(EscalateToErrorInRecommended);
public const string DisableInAllMode = nameof(DisableInAllMode);
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ public sealed class PreferConstructorOverTestInitializeAnalyzer : DiagnosticAnal
null,
Category.Design,
DiagnosticSeverity.Info,
isEnabledByDefault: false);
isEnabledByDefault: false,
disableInAllMode: true);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
= ImmutableArray.Create(Rule);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ public sealed class PreferDisposeOverTestCleanupAnalyzer : DiagnosticAnalyzer
null,
Category.Design,
DiagnosticSeverity.Info,
isEnabledByDefault: false);
isEnabledByDefault: false,
disableInAllMode: true);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
= ImmutableArray.Create(Rule);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ public sealed class PreferTestCleanupOverDisposeAnalyzer : DiagnosticAnalyzer
null,
Category.Design,
DiagnosticSeverity.Info,
isEnabledByDefault: false);
isEnabledByDefault: false,
disableInAllMode: true);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
= ImmutableArray.Create(Rule);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ public sealed class PreferTestInitializeOverConstructorAnalyzer : DiagnosticAnal
null,
Category.Design,
DiagnosticSeverity.Info,
isEnabledByDefault: false);
isEnabledByDefault: false,
disableInAllMode: true);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
= ImmutableArray.Create(Rule);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ public sealed class TestMethodShouldBeValidAnalyzer : DiagnosticAnalyzer
Description,
Category.Usage,
DiagnosticSeverity.Warning,
isEnabledByDefault: true);
isEnabledByDefault: true,
escalateToErrorInRecommended: true);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
= ImmutableArray.Create(ValidTestMethodSignatureRule);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.CodeAnalysis;

internal sealed class AnalyzerAssemblyLoader : IAnalyzerAssemblyLoader
{
public static IAnalyzerAssemblyLoader Instance { get; } = new AnalyzerAssemblyLoader();

private AnalyzerAssemblyLoader()
{
}

public void AddDependencyLocation(string fullPath)
{
}

public Assembly LoadFromPath(string fullPath)
=> Assembly.LoadFrom(fullPath);
}
122 changes: 122 additions & 0 deletions src/Analyzers/MSTest.GlobalConfigsGenerator/AnalyzerSeverityDecider.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.CodeAnalysis;

using MSTest.Analyzers.Helpers;

// NOTE for our current matrix.
// We decide whether to include a rule in a specific mode or not based on two factors.
// 1. IsEnabledByDefault (true/false)
// 2. DefaultSeverity (Info, Warn, Error) - note: we currently don't use severity Hidden or Error (but we intend to use Error in future)
// So, we have 6 possible combinations.
// 1. IsEnabledByDefault = true, DefaultSeverity = Error (we include that in All, Recommended, and Default)
// 2. IsEnabledByDefault = true, DefaultSeverity = Warn (we include that in All, Recommended, and Default)
// 3. IsEnabledByDefault = true, DefaultSeverity = Info (we include that in All and Recommended)
// 4. IsEnabledByDefault = false, DefaultSeverity = Error (we don't combine Error with not enabled by default)
// 5. IsEnabledByDefault = false, DefaultSeverity = Warn (we don't combine Warn with not enabled by default)
// 6. IsEnabledByDefault = false, DefaultSeverity = Info (we include that in All only)
//
// In addition to that, there are two custom tags that influence the logic.
// 1. EscalateToErrorInRecommended (makes a specific rule as "error" when using Recommended or All mode)
// 2. DisableInAllMode (Always disable the rule - it's completely opt-in)
internal static class AnalyzerSeverityDecider
{
public static DiagnosticSeverity? GetSeverity(DiagnosticDescriptor rule, MSTestAnalysisMode mode)
{
DiagnosticSeverity? severity = mode switch
{
MSTestAnalysisMode.All => DecideForModeAll(rule),
MSTestAnalysisMode.Recommended => DecideForModeRecommended(rule),
MSTestAnalysisMode.Default => DecideForModeDefault(rule),
MSTestAnalysisMode.None => DecideForModeNone(rule),
_ => throw new ArgumentException($"Unexpected MSTestAnalysisMode '{mode}'.", nameof(mode)),
};

if (rule.CustomTags.Contains(WellKnownCustomTags.DisableInAllMode))
{
if (rule.IsEnabledByDefault || rule.DefaultSeverity > DiagnosticSeverity.Info)
{
throw new InvalidOperationException("Rules with DisableInAllMode custom tag are expected to be disabled by default and have severity Info.");
}
else if (severity != null)
{
throw new InvalidOperationException("Rules with DisableInAllMode custom tag are expected to be disabled.");
}
}

return severity;
}

private static DiagnosticSeverity? DecideForModeAll(DiagnosticDescriptor rule)
{
if (rule.CustomTags.Contains(WellKnownCustomTags.EscalateToErrorInRecommended))
{
if (rule.DefaultSeverity != DiagnosticSeverity.Warning)
{
// It feels odd to escalate a rule to Error in Recommended mode if it's not a warning by default.
throw new InvalidOperationException("Is it intended that default severity is not warning when escalating to error in recommended mode?");
}

return DiagnosticSeverity.Error;
}
else if (rule.CustomTags.Contains(WellKnownCustomTags.DisableInAllMode))
{
return null;
}

// NOTE: If we decided to introduce an analyzer with default severity as Error,
// then analysis mode all shouldn't change that.
return (DiagnosticSeverity)Math.Max((int)DiagnosticSeverity.Warning, (int)rule.DefaultSeverity);
}

private static DiagnosticSeverity? DecideForModeRecommended(DiagnosticDescriptor rule)
{
if (rule.CustomTags.Contains(WellKnownCustomTags.EscalateToErrorInRecommended))
{
if (rule.DefaultSeverity != DiagnosticSeverity.Warning)
{
// It feels odd to escalate a rule to Error in Recommended mode if it's not a warning by default.
throw new InvalidOperationException("Is it intended that default severity is not warning when escalating to error in recommended mode?");
}

return DiagnosticSeverity.Error;
}

if (rule.IsEnabledByDefault && rule.DefaultSeverity >= DiagnosticSeverity.Info)
{
// Recommended mode will elevate info to warning only if the rule is enabled by default.
return DiagnosticSeverity.Warning;
}

if (rule.DefaultSeverity >= DiagnosticSeverity.Warning)
{
throw new InvalidOperationException($"Rule '{rule.Id}' with severity >= Warning is expected to be enabled by default.");
}

// If a rule is enabled by default, Recommended keeps it as Info.
// If a rule is disabled by default, Recommended keeps it disabled.
return rule.IsEnabledByDefault ? DiagnosticSeverity.Info : null;
}

private static DiagnosticSeverity? DecideForModeDefault(DiagnosticDescriptor rule)
{
if (rule.IsEnabledByDefault && rule.DefaultSeverity >= DiagnosticSeverity.Warning)
{
// Default mode will enable warnings only if the rule is enabled by default.
return rule.DefaultSeverity;
}

if (rule.DefaultSeverity >= DiagnosticSeverity.Warning)
{
throw new InvalidOperationException("Rules with severity >= Warning are expected to be enabled by default.");
}

// If a rule is enabled by default, Default keeps it at its original severity.
// If a rule is disabled by default, Default keeps it disabled.
return rule.IsEnabledByDefault ? rule.DefaultSeverity : null;
}

private static DiagnosticSeverity? DecideForModeNone(DiagnosticDescriptor _)
=> null;
}
Loading