Skip to content

Commit

Permalink
Enable nullable for Platform Services (#1366)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Evangelink
Evangelink committed Nov 3, 2022
1 parent a03fb56 commit 425048f
Show file tree
Hide file tree
Showing 63 changed files with 1,130 additions and 1,168 deletions.
2 changes: 2 additions & 0 deletions .editorconfig
View file
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
View file
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
9 changes: 5 additions & 4 deletions src/Adapter/MSTestAdapter.PlatformServices/Data/CsvDataConnection.cs
View file
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
16 changes: 9 additions & 7 deletions src/Adapter/MSTestAdapter.PlatformServices/Data/OdbcDataConnection.cs
View file
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

0 comments on commit 425048f

Please sign in to comment.