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

Enable nullable for Platform Services #1366

Merged
merged 3 commits into from
Nov 3, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ end_of_line = crlf

#### .NET Coding Conventions ####

dotnet_diagnostic.RS0041.severity = none

## Organize usings

dotnet_separate_import_directive_groups = true
Expand Down
93 changes: 48 additions & 45 deletions src/Adapter/MSTestAdapter.PlatformServices/AssemblyResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using System.Security.Permissions;

using Microsoft.VisualStudio.TestPlatform.ObjectModel;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices;

Expand Down Expand Up @@ -54,12 +55,12 @@ public class AssemblyResolver : MarshalByRefObject, IDisposable
/// <summary>
/// Dictionary of Assemblies discovered to date.
/// </summary>
private readonly Dictionary<string, Assembly> _resolvedAssemblies = new();
private readonly Dictionary<string, Assembly?> _resolvedAssemblies = new();

/// <summary>
/// Dictionary of Reflection-Only Assemblies discovered to date.
/// </summary>
private readonly Dictionary<string, Assembly> _reflectionOnlyResolvedAssemblies = new();
private readonly Dictionary<string, Assembly?> _reflectionOnlyResolvedAssemblies = new();

/// <summary>
/// lock for the loaded assemblies cache.
Expand Down Expand Up @@ -123,7 +124,7 @@ public void Dispose()
/// </returns>
[SecurityCritical]
[SecurityPermission(SecurityAction.LinkDemand, Flags = SecurityPermissionFlag.Infrastructure)]
public override object InitializeLifetimeService()
public override object? InitializeLifetimeService()
{
return null;
}
Expand Down Expand Up @@ -154,7 +155,7 @@ public void AddSearchDirectoriesFromRunSetting(List<RecursiveDirectoryPath> recu
/// <param name="sender"> The sender App Domain. </param>
/// <param name="args"> The args. </param>
/// <returns> The <see cref="Assembly"/>. </returns>
internal Assembly ReflectionOnlyOnResolve(object sender, ResolveEventArgs args)
internal Assembly? ReflectionOnlyOnResolve(object sender, ResolveEventArgs args)
{
return OnResolveInternal(sender, args, true);
}
Expand All @@ -165,7 +166,7 @@ internal Assembly ReflectionOnlyOnResolve(object sender, ResolveEventArgs args)
/// <param name="sender"> The sender App Domain. </param>
/// <param name="args"> The args. </param>
/// <returns> The <see cref="Assembly"/>. </returns>
internal Assembly OnResolve(object sender, ResolveEventArgs args)
internal Assembly? OnResolve(object sender, ResolveEventArgs args)
{
return OnResolveInternal(sender, args, false);
}
Expand All @@ -177,8 +178,8 @@ internal Assembly OnResolve(object sender, ResolveEventArgs args)
/// <param name="searchDirectories"> The search Directories. </param>
internal void AddSubdirectories(string path, List<string> searchDirectories)
{
Debug.Assert(!string.IsNullOrEmpty(path), "'path' cannot be null or empty.");
Debug.Assert(searchDirectories != null, "'searchDirectories' cannot be null.");
DebugEx.Assert(!StringEx.IsNullOrEmpty(path), "'path' cannot be null or empty.");
DebugEx.Assert(searchDirectories != null, "'searchDirectories' cannot be null.");

// If the directory exists, get it's subdirectories
if (DoesDirectoryExist(path))
Expand Down Expand Up @@ -264,15 +265,15 @@ protected virtual Assembly ReflectionOnlyLoadAssemblyFrom(string path)
/// <param name="name"> The name. </param>
/// <param name="isReflectionOnly"> Indicates whether this is called under a Reflection Only Load context. </param>
/// <returns> The <see cref="Assembly"/>. </returns>
protected virtual Assembly SearchAssembly(List<string> searchDirectorypaths, string name, bool isReflectionOnly)
protected virtual Assembly? SearchAssembly(List<string> searchDirectorypaths, string name, bool isReflectionOnly)
{
if (searchDirectorypaths == null || searchDirectorypaths.Count == 0)
{
return null;
}

// args.Name is like: "Microsoft.VisualStudio.TestTools.Common, Version=[VersionMajor].0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a".
AssemblyName requestedName = null;
AssemblyName? requestedName = null;

try
{
Expand All @@ -284,37 +285,37 @@ protected virtual Assembly SearchAssembly(List<string> searchDirectorypaths, str
SafeLog(
name,
() =>
{
if (EqtTrace.IsInfoEnabled)
{
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info(
"AssemblyResolver: {0}: Failed to create assemblyName. Reason:{1} ",
name,
ex);
}
});
EqtTrace.Info(
"AssemblyResolver: {0}: Failed to create assemblyName. Reason:{1} ",
name,
ex);
}
});

return null;
}

Debug.Assert(requestedName != null && !string.IsNullOrEmpty(requestedName.Name), "AssemblyResolver.OnResolve: requested is null or name is empty!");
DebugEx.Assert(requestedName != null && !StringEx.IsNullOrEmpty(requestedName.Name), "AssemblyResolver.OnResolve: requested is null or name is empty!");

foreach (var dir in searchDirectorypaths)
{
if (string.IsNullOrEmpty(dir))
if (StringEx.IsNullOrEmpty(dir))
{
continue;
}

SafeLog(
name,
() =>
{
if (EqtTrace.IsVerboseEnabled)
{
if (EqtTrace.IsVerboseEnabled)
{
EqtTrace.Verbose("AssemblyResolver: Searching assembly: {0} in the directory: {1}", requestedName.Name, dir);
}
});
EqtTrace.Verbose("AssemblyResolver: Searching assembly: {0} in the directory: {1}", requestedName.Name, dir);
}
});

foreach (var extension in new string[] { ".dll", ".exe" })
{
Expand All @@ -340,8 +341,8 @@ protected virtual Assembly SearchAssembly(List<string> searchDirectorypaths, str
/// <returns> The <see cref="bool"/>. </returns>
private static bool RequestedAssemblyNameMatchesFound(AssemblyName requestedName, AssemblyName foundName)
{
Debug.Assert(requestedName != null, "requested assembly name should not be null.");
Debug.Assert(foundName != null, "found assembly name should not be null.");
DebugEx.Assert(requestedName != null, "requested assembly name should not be null.");
DebugEx.Assert(foundName != null, "found assembly name should not be null.");

var requestedPublicKey = requestedName.GetPublicKeyToken();
if (requestedPublicKey != null)
Expand Down Expand Up @@ -392,9 +393,9 @@ private void WindowsRuntimeMetadataReflectionOnlyNamespaceResolve(object sender,
/// <returns> The <see cref="Assembly"/>. </returns>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Requirement is to handle all kinds of user exceptions and message appropriately.")]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA1801:ReviewUnusedParameters", MessageId = "senderAppDomain", Justification = "This is an event handler.")]
private Assembly OnResolveInternal(object senderAppDomain, ResolveEventArgs args, bool isReflectionOnly)
private Assembly? OnResolveInternal(object senderAppDomain, ResolveEventArgs args, bool isReflectionOnly)
{
if (string.IsNullOrEmpty(args?.Name))
if (StringEx.IsNullOrEmpty(args?.Name))
{
Debug.Fail("AssemblyResolver.OnResolve: args.Name is null or empty.");
return null;
Expand Down Expand Up @@ -446,23 +447,23 @@ private Assembly OnResolveInternal(object senderAppDomain, ResolveEventArgs args
// instead of loading whole search directory in one time, we are adding directory on the basis of need
var currentNode = _directoryList.Dequeue();

List<string> increamentalSearchDirectory = new();
List<string> incrementalSearchDirectory = new();

if (DoesDirectoryExist(currentNode.DirectoryPath))
{
increamentalSearchDirectory.Add(currentNode.DirectoryPath);
incrementalSearchDirectory.Add(currentNode.DirectoryPath);

if (currentNode.IncludeSubDirectories)
{
// Add all its sub-directory in depth first search order.
AddSubdirectories(currentNode.DirectoryPath, increamentalSearchDirectory);
AddSubdirectories(currentNode.DirectoryPath, incrementalSearchDirectory);
}

// Add this directory list in this.searchDirectories so that when we will try to resolve some other
// assembly, then it will look in this whole directory first.
_searchDirectories.AddRange(increamentalSearchDirectory);
_searchDirectories.AddRange(incrementalSearchDirectory);

assembly = SearchAssembly(increamentalSearchDirectory, assemblyNameToLoad, isReflectionOnly);
assembly = SearchAssembly(incrementalSearchDirectory, assemblyNameToLoad, isReflectionOnly);
}
else
{
Expand Down Expand Up @@ -522,14 +523,14 @@ private Assembly OnResolveInternal(object senderAppDomain, ResolveEventArgs args
catch (Exception ex)
{
SafeLog(
args?.Name,
() =>
{
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("AssemblyResolver: {0}: Failed to load assembly. Reason: {1}", assemblyNameToLoad, ex);
}
});
args?.Name,
() =>
{
if (EqtTrace.IsInfoEnabled)
{
EqtTrace.Info("AssemblyResolver: {0}: Failed to load assembly. Reason: {1}", assemblyNameToLoad, ex);
}
});
}

return assembly;
Expand All @@ -543,7 +544,7 @@ private Assembly OnResolveInternal(object senderAppDomain, ResolveEventArgs args
/// <param name="isReflectionOnly">Indicates if this is a reflection-only context.</param>
/// <param name="assembly"> The assembly. </param>
/// <returns> The <see cref="bool"/>. </returns>
private bool TryLoadFromCache(string assemblyName, bool isReflectionOnly, out Assembly assembly)
private bool TryLoadFromCache(string assemblyName, bool isReflectionOnly, out Assembly? assembly)
{
bool isFoundInCache = false;

Expand Down Expand Up @@ -581,10 +582,12 @@ private bool TryLoadFromCache(string assemblyName, bool isReflectionOnly, out As
/// </summary>
/// <param name="assemblyName">The assembly being resolved.</param>
/// <param name="loggerAction">The logger function.</param>
private static void SafeLog(string assemblyName, Action loggerAction)
private static void SafeLog(string? assemblyName, Action loggerAction)
{
// Logger assembly was in `Microsoft.VisualStudio.TestPlatform.ObjectModel` assembly in legacy versions and we need to omit it as well.
if (!string.IsNullOrEmpty(assemblyName) && !assemblyName.StartsWith(LoggerAssemblyName) && !assemblyName.StartsWith(LoggerAssemblyNameLegacy))
if (!StringEx.IsNullOrEmpty(assemblyName)
&& !assemblyName.StartsWith(LoggerAssemblyName)
&& !assemblyName.StartsWith(LoggerAssemblyNameLegacy))
{
loggerAction.Invoke();
}
Expand All @@ -600,7 +603,7 @@ private static void SafeLog(string assemblyName, Action loggerAction)
/// <returns> The <see cref="Assembly"/>. </returns>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2001:AvoidCallingProblematicMethods", MessageId = "System.Reflection.Assembly.LoadFrom", Justification = "The assembly location is figured out from the configuration that the user passes in.")]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Requirement is to handle all kinds of user exceptions and message appropriately.")]
private Assembly SearchAndLoadAssembly(string assemblyPath, string assemblyName, AssemblyName requestedName, bool isReflectionOnly)
private Assembly? SearchAndLoadAssembly(string assemblyPath, string assemblyName, AssemblyName requestedName, bool isReflectionOnly)
{
try
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using System.Text;

using Microsoft.VisualStudio.TestPlatform.ObjectModel;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Data;

Expand All @@ -32,7 +33,7 @@ internal sealed class CsvDataConnection : TestDataConnection
public CsvDataConnection(string fileName, List<string> dataFolders)
: base(dataFolders)
{
Debug.Assert(!string.IsNullOrEmpty(fileName), "fileName");
DebugEx.Assert(!StringEx.IsNullOrEmpty(fileName), "fileName");
_fileName = fileName;
}

Expand All @@ -55,7 +56,7 @@ public override List<string> GetDataTablesAndViews()
}

[SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Requirement is to handle all kinds of user exceptions and message appropriately.")]
public override List<string> GetColumns(string tableName)
public override List<string>? GetColumns(string tableName)
{
// Somewhat heavy, this could be improved, right now I simply
// read the table in then check the columns...
Expand Down Expand Up @@ -83,7 +84,7 @@ public override List<string> GetColumns(string tableName)

[SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Untested. Leaving as-is.")]
[SuppressMessage("Microsoft.Security", "CA2100:Review SQL queries for security", Justification = "Not passed in from user.")]
public DataTable ReadTable(string tableName, IEnumerable columns, int maxRows)
public DataTable ReadTable(string tableName, IEnumerable? columns, int maxRows)
{
// We specifically use OleDb to read a CSV file...
WriteDiagnostics("ReadTable: {0}", tableName);
Expand Down Expand Up @@ -168,7 +169,7 @@ public DataTable ReadTable(string tableName, IEnumerable columns, int maxRows)
return table;
}

public override DataTable ReadTable(string tableName, IEnumerable columns)
public override DataTable ReadTable(string tableName, IEnumerable? columns)
{
return ReadTable(tableName, columns, -1);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
using System.Data.Odbc;
using System.Diagnostics;

using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Data;

/// <summary>
Expand All @@ -20,7 +22,7 @@ public OdbcDataConnection(string invariantProviderName, string connectionString,
: base(invariantProviderName, FixConnectionString(connectionString, dataFolders), dataFolders)
{
// Need open connection to get Connection.Driver.
Debug.Assert(IsOpen(), "The connection must be open!");
DebugEx.Assert(IsOpen(), "The connection must be open!");

_isMSSql = Connection != null && IsMSSql(Connection.Driver);
}
Expand All @@ -43,7 +45,7 @@ public override void GetQuoteLiterals()
GetQuoteLiteralsHelper();
}

public override string GetDefaultSchema()
public override string? GetDefaultSchema()
{
if (_isMSSql)
{
Expand Down Expand Up @@ -79,13 +81,13 @@ protected override SchemaMetaData[] GetSchemaMetaData()

protected override string QuoteIdentifier(string identifier)
{
Debug.Assert(!string.IsNullOrEmpty(identifier), "identifier");
DebugEx.Assert(!StringEx.IsNullOrEmpty(identifier), "identifier");
return CommandBuilder.QuoteIdentifier(identifier, Connection); // Must pass connection.
}

protected override string UnquoteIdentifier(string identifier)
{
Debug.Assert(!string.IsNullOrEmpty(identifier), "identifier");
DebugEx.Assert(!StringEx.IsNullOrEmpty(identifier), "identifier");
return CommandBuilder.UnquoteIdentifier(identifier, Connection); // Must pass connection.
}

Expand All @@ -100,16 +102,16 @@ private static string FixConnectionString(string connectionString, List<string>
return connectionString;
}

string fileName = builder["dbq"] as string;
string? fileName = builder["dbq"] as string;

if (string.IsNullOrEmpty(fileName))
if (StringEx.IsNullOrEmpty(fileName))
{
return connectionString;
}
else
{
// Fix-up magic file paths
string fixedFilePath = FixPath(fileName, dataFolders);
string? fixedFilePath = FixPath(fileName, dataFolders);
if (fixedFilePath != null)
{
builder["dbq"] = fixedFilePath;
Expand Down