Merge branch 'main' into testcontext-resolution

Directory.Packages.props

@@ -24,7 +24,7 @@
   </PropertyGroup>
   <PropertyGroup Label="Test dependencies">
     <MicrosoftCodeAnalysisAnalyzerTestingVersion>1.1.3-beta1.24423.1</MicrosoftCodeAnalysisAnalyzerTestingVersion>
-    <MSTestVersion>3.8.0-preview.24631.6</MSTestVersion>
+    <MSTestVersion>3.8.0-preview.25052.2</MSTestVersion>
   </PropertyGroup>
   <ItemGroup Label="Analyzers">
     <PackageVersion Include="Microsoft.CodeAnalysis.BannedApiAnalyzers" Version="$(MicrosoftCodeAnalysisBannedApiAnalyzersVersion)" />

eng/Version.Details.xml

@@ -17,9 +17,9 @@
       <Uri>https://dev.azure.com/devdiv/DevDiv/_git/vs-code-coverage</Uri>
       <Sha>d4a113f856a31bcdcbf6e08da8928961c98bb497</Sha>
     </Dependency>
-    <Dependency Name="MSTest.Engine" Version="1.0.0-alpha.24630.3">
+    <Dependency Name="MSTest.Engine" Version="1.0.0-alpha.25053.2">
       <Uri>https://github.com/microsoft/testanywhere</Uri>
-      <Sha>1b35871af094500bb7bf28aa23b61d7135e1fca2</Sha>
+      <Sha>2a22637b3897dea2062a1ca574817772bc9346f5</Sha>
     </Dependency>
     <!-- Intermediate is necessary for source build. -->
     <Dependency Name="Microsoft.SourceBuild.Intermediate.diagnostics" Version="9.0.0-preview.24566.1">

eng/Versions.props

@@ -10,6 +10,6 @@
     <MicrosoftDotNetBuildTasksTemplatingPackageVersion>10.0.0-beta.24604.4</MicrosoftDotNetBuildTasksTemplatingPackageVersion>
     <MicrosoftTestingExtensionsCodeCoverageVersion>17.14.0-preview.24630.1</MicrosoftTestingExtensionsCodeCoverageVersion>
     <!-- comment to facilitate merge conflicts -->
-    <MSTestEngineVersion>1.0.0-alpha.24630.3</MSTestEngineVersion>
+    <MSTestEngineVersion>1.0.0-alpha.25053.2</MSTestEngineVersion>
   </PropertyGroup>
 </Project>

src/Adapter/MSTest.TestAdapter/DynamicDataOperations.cs

@@ -1,4 +1,4 @@
-// Copyright (c) Microsoft Corporation. All rights reserved.
+// 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.VisualStudio.TestTools.UnitTesting;
@@ -19,11 +19,11 @@ public IEnumerable<object[]> GetData(Type? _dynamicDataDeclaringType, DynamicDat
         {
             case DynamicDataSourceType.AutoDetect:
 #pragma warning disable IDE0045 // Convert to conditional expression - it becomes less readable.
-                if (PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredProperty(_dynamicDataDeclaringType, _dynamicDataSourceName) is { } dynamicDataPropertyInfo)
+                if (GetPropertyConsideringInheritance(_dynamicDataDeclaringType, _dynamicDataSourceName) is { } dynamicDataPropertyInfo)
                 {
                     obj = GetDataFromProperty(dynamicDataPropertyInfo);
                 }
-                else if (PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredMethod(_dynamicDataDeclaringType, _dynamicDataSourceName) is { } dynamicDataMethodInfo)
+                else if (GetMethodConsideringInheritance(_dynamicDataDeclaringType, _dynamicDataSourceName) is { } dynamicDataMethodInfo)
                 {
                     obj = GetDataFromMethod(dynamicDataMethodInfo);
                 }
@@ -35,14 +35,14 @@ public IEnumerable<object[]> GetData(Type? _dynamicDataDeclaringType, DynamicDat
 
                 break;
             case DynamicDataSourceType.Property:
-                PropertyInfo property = PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredProperty(_dynamicDataDeclaringType, _dynamicDataSourceName)
+                PropertyInfo property = GetPropertyConsideringInheritance(_dynamicDataDeclaringType, _dynamicDataSourceName)
                     ?? throw new ArgumentNullException($"{DynamicDataSourceType.Property} {_dynamicDataSourceName}");
 
                 obj = GetDataFromProperty(property);
                 break;
 
             case DynamicDataSourceType.Method:
-                MethodInfo method = PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredMethod(_dynamicDataDeclaringType, _dynamicDataSourceName)
+                MethodInfo method = GetMethodConsideringInheritance(_dynamicDataDeclaringType, _dynamicDataSourceName)
                     ?? throw new ArgumentNullException($"{DynamicDataSourceType.Method} {_dynamicDataSourceName}");
 
                 obj = GetDataFromMethod(method);
@@ -251,4 +251,40 @@ private static bool IsTupleOrValueTuple(Type type, out int tupleSize)
         return false;
     }
 #endif
+
+    private static PropertyInfo? GetPropertyConsideringInheritance(Type type, string propertyName)
+    {
+        // NOTE: Don't use GetRuntimeProperty. It considers inheritance only for instance properties.
+        Type? currentType = type;
+        while (currentType is not null)
+        {
+            PropertyInfo? property = PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredProperty(currentType, propertyName);
+            if (property is not null)
+            {
+                return property;
+            }
+
+            currentType = currentType.BaseType;
+        }
+
+        return null;
+    }
+
+    private static MethodInfo? GetMethodConsideringInheritance(Type type, string methodName)
+    {
+        // NOTE: Don't use GetRuntimeMethod. It considers inheritance only for instance methods.
+        Type? currentType = type;
+        while (currentType is not null)
+        {
+            MethodInfo? method = PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredMethod(currentType, methodName);
+            if (method is not null)
+            {
+                return method;
+            }
+
+            currentType = currentType.BaseType;
+        }
+
+        return null;
+    }
 }

src/Adapter/MSTest.TestAdapter/Execution/TestClassInfo.cs

@@ -780,7 +780,7 @@ void DoRun()
     {
         try
         {
-            PropertyInfo? testContextProperty = PlatformServiceProvider.Instance.ReflectionOperations.GetRuntimeProperty(classType, TestContextPropertyName);
+            PropertyInfo? testContextProperty = PlatformServiceProvider.Instance.ReflectionOperations.GetRuntimeProperty(classType, TestContextPropertyName, includeNonPublic: false);
             if (testContextProperty == null)
             {
                 // that's okay may be the property was not defined

src/Adapter/MSTest.TestAdapter/Execution/TypeCache.cs

@@ -771,7 +771,8 @@ private MethodInfo GetMethodInfoForTestMethod(TestMethod testMethod, TestClassIn
         else if (methodBase != null)
         {
             Type[] parameters = methodBase.GetParameters().Select(i => i.ParameterType).ToArray();
-            testMethodInfo = PlatformServiceProvider.Instance.ReflectionOperations.GetRuntimeMethod(methodBase.DeclaringType!, methodBase.Name, parameters);
+            // TODO: Should we pass true for includeNonPublic?
+            testMethodInfo = PlatformServiceProvider.Instance.ReflectionOperations.GetRuntimeMethod(methodBase.DeclaringType!, methodBase.Name, parameters, includeNonPublic: false);
         }
 
         return testMethodInfo is null

src/Adapter/MSTest.TestAdapter/SourceGeneration/SourceGeneratedReflectionOperations.cs

@@ -61,22 +61,33 @@ public PropertyInfo[] GetDeclaredProperties(Type type)
         => ReflectionDataProvider.TypeProperties[type];
 
     public PropertyInfo? GetDeclaredProperty(Type type, string propertyName)
-        => GetRuntimeProperty(type, propertyName);
+        => GetRuntimeProperty(type, propertyName, includeNonPublic: true);
 
     public Type[] GetDefinedTypes(Assembly assembly)
         => ReflectionDataProvider.Types;
 
     public MethodInfo[] GetRuntimeMethods(Type type)
         => ReflectionDataProvider.TypeMethods[type];
 
-    public MethodInfo? GetRuntimeMethod(Type declaringType, string methodName, Type[] parameters) => throw new NotImplementedException();
+    public MethodInfo? GetRuntimeMethod(Type declaringType, string methodName, Type[] parameters, bool includeNonPublic)
+    {
+        IEnumerable<MethodInfo> runtimeMethods = GetRuntimeMethods(declaringType)
+            .Where(
+                m => m.Name == methodName &&
+                m.GetParameters().Select(pi => pi.ParameterType).SequenceEqual(parameters) &&
+                (includeNonPublic || m.IsPublic));
+        return runtimeMethods.SingleOrDefault();
+    }
 
-    public PropertyInfo? GetRuntimeProperty(Type classType, string propertyName)
+    public PropertyInfo? GetRuntimeProperty(Type classType, string propertyName, bool includeNonPublic)
     {
         Dictionary<string, PropertyInfo> type = ReflectionDataProvider.TypePropertiesByName[classType];
 
         // We as asking for TestContext here, it may not be there.
-        return type.TryGetValue(propertyName, out PropertyInfo? propertyInfo) ? propertyInfo : null;
+        PropertyInfo? property = type.TryGetValue(propertyName, out PropertyInfo? propertyInfo) ? propertyInfo : null;
+        return !includeNonPublic && (property?.GetMethod?.IsPublic == true || property?.SetMethod?.IsPublic == true)
+            ? null
+            : property;
     }
 
     public Type? GetType(string typeName)

src/Adapter/MSTestAdapter.PlatformServices/Interfaces/IReflectionOperations2.cs

@@ -19,9 +19,9 @@ internal interface IReflectionOperations2 : IReflectionOperations
 
     MethodInfo[] GetRuntimeMethods(Type type);
 
-    MethodInfo? GetRuntimeMethod(Type declaringType, string methodName, Type[] parameters);
+    MethodInfo? GetRuntimeMethod(Type declaringType, string methodName, Type[] parameters, bool includeNonPublic);
 
-    PropertyInfo? GetRuntimeProperty(Type classType, string propertyName);
+    PropertyInfo? GetRuntimeProperty(Type classType, string propertyName, bool includeNonPublic);
 
     Type? GetType(string typeName);
 

src/Adapter/MSTestAdapter.PlatformServices/Services/ReflectionOperations2.cs

@@ -45,11 +45,15 @@ public Type[] GetDefinedTypes(Assembly assembly)
     public MethodInfo[] GetRuntimeMethods(Type type)
         => type.GetMethods(Everything);
 
-    public MethodInfo? GetRuntimeMethod(Type declaringType, string methodName, Type[] parameters)
-        => declaringType.GetRuntimeMethod(methodName, parameters);
-
-    public PropertyInfo? GetRuntimeProperty(Type classType, string testContextPropertyName)
-        => classType.GetProperty(testContextPropertyName);
+    public MethodInfo? GetRuntimeMethod(Type declaringType, string methodName, Type[] parameters, bool includeNonPublic)
+        => includeNonPublic
+            ? declaringType.GetMethod(methodName, Everything, null, parameters, null)
+            : declaringType.GetMethod(methodName, parameters);
+
+    public PropertyInfo? GetRuntimeProperty(Type classType, string testContextPropertyName, bool includeNonPublic)
+        => includeNonPublic
+            ? classType.GetProperty(testContextPropertyName, Everything)
+            : classType.GetProperty(testContextPropertyName);
 
     public Type? GetType(string typeName)
         => Type.GetType(typeName);

src/Analyzers/MSTest.Analyzers.CodeFixes/AvoidExpectedExceptionAttributeFixer.cs

@@ -42,10 +42,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
             return;
         }
 
-        if (diagnostic.Properties.ContainsKey(DiagnosticDescriptorHelper.CannotFixPropertyKey))
-        {
-            return;
-        }
+        bool allowDerivedTypes = diagnostic.Properties.ContainsKey(AvoidExpectedExceptionAttributeAnalyzer.AllowDerivedTypesKey);
 
         // Find the method declaration identified by the diagnostic.
         MethodDeclarationSyntax methodDeclaration = syntaxToken.Parent.AncestorsAndSelf().OfType<MethodDeclarationSyntax>().First();
@@ -84,7 +81,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
         context.RegisterCodeFix(
             CodeAction.Create(
                 title: CodeFixResources.UseAssertThrowsExceptionOnLastStatementFix,
-                createChangedDocument: c => WrapLastStatementWithAssertThrowsExceptionAsync(context.Document, methodDeclaration, attributeSyntax, exceptionTypeSymbol, c),
+                createChangedDocument: c => WrapLastStatementWithAssertThrowsExceptionAsync(context.Document, methodDeclaration, attributeSyntax, exceptionTypeSymbol, allowDerivedTypes, c),
                 equivalenceKey: nameof(AvoidExpectedExceptionAttributeFixer)),
             diagnostic);
     }
@@ -94,6 +91,7 @@ private static async Task<Document> WrapLastStatementWithAssertThrowsExceptionAs
         MethodDeclarationSyntax methodDeclaration,
         SyntaxNode attributeSyntax,
         ITypeSymbol exceptionTypeSymbol,
+        bool allowDerivedTypes,
         CancellationToken cancellationToken)
     {
         DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
@@ -123,7 +121,14 @@ private static async Task<Document> WrapLastStatementWithAssertThrowsExceptionAs
         SyntaxNode newStatement = generator.InvocationExpression(
                 generator.MemberAccessExpression(
                     generator.IdentifierName("Assert"),
-                    generator.GenericName(containsAsyncCode ? "ThrowsExceptionAsync" : "ThrowsException", [exceptionTypeSymbol])),
+                    generator.GenericName(
+                        (containsAsyncCode, allowDerivedTypes) switch
+                        {
+                            (false, false) => "ThrowsExactly",
+                            (false, true) => "Throws",
+                            (true, false) => "ThrowsExactlyAsync",
+                            (true, true) => "ThrowsAsync",
+                        }, [exceptionTypeSymbol])),
                 newLambdaExpression);
 
         if (containsAsyncCode)

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

@@ -1,2 +1,7 @@
-; Unshipped analyzer release
+; Unshipped analyzer release
 ; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md
+### New Rules
+
+Rule ID | Category | Severity | Notes
+--------|----------|----------|-------
+MSTEST0038 | `Usage` | Warning | AvoidAssertAreSameWithValueTypesAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0038)

src/Analyzers/MSTest.Analyzers/AvoidAssertAreSameWithValueTypesAnalyzer.cs

@@ -0,0 +1,78 @@
+// 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>
+/// MSTEST0025: <inheritdoc cref="Resources.AvoidAssertAreSameWithValueTypesTitle"/>.
+/// </summary>
+[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
+public sealed class AvoidAssertAreSameWithValueTypesAnalyzer : DiagnosticAnalyzer
+{
+    private static readonly LocalizableResourceString Title = new(nameof(Resources.AvoidAssertAreSameWithValueTypesTitle), Resources.ResourceManager, typeof(Resources));
+    private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.AvoidAssertAreSameWithValueTypesMessageFormat), Resources.ResourceManager, typeof(Resources));
+    private static readonly LocalizableResourceString Description = new(nameof(Resources.AvoidAssertAreSameWithValueTypesDescription), Resources.ResourceManager, typeof(Resources));
+
+    internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
+        DiagnosticIds.AvoidAssertAreSameWithValueTypesRuleId,
+        Title,
+        MessageFormat,
+        Description,
+        Category.Usage,
+        DiagnosticSeverity.Warning,
+        isEnabledByDefault: true);
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
+        = ImmutableArray.Create(Rule);
+
+    public override void Initialize(AnalysisContext context)
+    {
+        context.EnableConcurrentExecution();
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+
+        context.RegisterCompilationStartAction(context =>
+        {
+            Compilation compilation = context.Compilation;
+            INamedTypeSymbol? assertSymbol = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingAssert);
+            if (assertSymbol is not null)
+            {
+                context.RegisterOperationAction(context => AnalyzeOperation(context, assertSymbol), OperationKind.Invocation);
+            }
+        });
+    }
+
+    private static void AnalyzeOperation(OperationAnalysisContext context, INamedTypeSymbol assertSymbol)
+    {
+        var operation = (IInvocationOperation)context.Operation;
+        IMethodSymbol targetMethod = operation.TargetMethod;
+        if (targetMethod.Name != "AreSame" ||
+            !assertSymbol.Equals(operation.TargetMethod.ContainingType, SymbolEqualityComparer.Default))
+        {
+            return;
+        }
+
+        IArgumentOperation? argExpected = operation.Arguments.FirstOrDefault(arg => arg.Parameter?.Ordinal == 0);
+        IArgumentOperation? argActual = operation.Arguments.FirstOrDefault(arg => arg.Parameter?.Ordinal == 1);
+        if (argExpected is null || argActual is null)
+        {
+            return;
+        }
+
+        if (argExpected.Value.WalkDownConversion().Type?.IsValueType == true ||
+            argActual.Value.WalkDownConversion().Type?.IsValueType == true)
+        {
+            context.ReportDiagnostic(operation.CreateDiagnostic(Rule));
+        }
+    }
+}

src/Analyzers/MSTest.Analyzers/AvoidExpectedExceptionAttributeAnalyzer.cs

@@ -22,6 +22,8 @@ public sealed class AvoidExpectedExceptionAttributeAnalyzer : DiagnosticAnalyzer
     private static readonly LocalizableResourceString Description = new(nameof(Resources.AvoidExpectedExceptionAttributeDescription), Resources.ResourceManager, typeof(Resources));
     private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.AvoidExpectedExceptionAttributeMessageFormat), Resources.ResourceManager, typeof(Resources));
 
+    internal const string AllowDerivedTypesKey = nameof(AllowDerivedTypesKey);
+
     internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
         DiagnosticIds.AvoidExpectedExceptionAttributeRuleId,
         Title,
@@ -59,7 +61,7 @@ private static void AnalyzeSymbol(SymbolAnalysisContext context, INamedTypeSymbo
             // Assert.ThrowsException checks the exact Exception type. So, we cannot offer a fix to ThrowsException if the user sets AllowDerivedTypes to true.
             context.ReportDiagnostic(
                 allowsDerivedTypes
-                ? methodSymbol.CreateDiagnostic(Rule, properties: DiagnosticDescriptorHelper.CannotFixProperties)
+                ? methodSymbol.CreateDiagnostic(Rule, properties: ImmutableDictionary<string, string?>.Empty.Add(AllowDerivedTypesKey, null))
                 : methodSymbol.CreateDiagnostic(Rule));
         }
     }