From 5a0b78c9c06d719dffba830a665ad93aac9aa6e5 Mon Sep 17 00:00:00 2001 From: Jared Parsons Date: Thu, 30 Jan 2020 09:20:54 -0800 Subject: [PATCH] Fixup nullable annotation Had to clean up a few nullable annotations now that we are compiling agaist `netcoreapp3.1` and hence get the full value of the framework annotations. This is also problematic though because there are now two places where the compiler can see nullable attributes that are directly used by the developer. For example `NotNullWhenAttribute`. This is both defined in our assemblies for non-netcoreapp target frameworks and provided by the SDK when targeting `netcoreapp3.1`. This causes a problem for assemblies which have the following characteristics: 1. Target `netcoreapp3.1` 1. Reference an assembly targeting `netstandard2.0` which uses our nullable attributes definition 1. Has IVT into (2) above These properties essentially define all of our unit test assemblies. In that environment it's not possible to use nullable attributes in code because the compiler can't disambiguate which definition of `NotNullWhenAttribute` to use. This meant I had to temporarily remove a few attributes until we can complete #40766. --- .../Core/CommandLine/CompilerServerLogger.cs | 8 ++++---- .../Core/MSBuildTask/ManagedCompiler.cs | 19 ++++++++++++++----- .../Core/MSBuildTask/MapSourceRoots.cs | 2 +- .../Microsoft.Build.Tasks.CodeAnalysis.csproj | 8 +++++++- src/Compilers/Core/MSBuildTask/MvidReader.cs | 6 +++--- src/Compilers/Core/MSBuildTask/Utilities.cs | 4 ++-- .../Core/MSBuildTask/ValidateBootstrap.cs | 4 ++-- .../InternalUtilities/NullableAttributes.cs | 5 +---- src/Compilers/Shared/BuildServerConnection.cs | 12 ++++++------ src/Compilers/Shared/RuntimeHostInfo.cs | 4 ++-- 10 files changed, 42 insertions(+), 30 deletions(-) diff --git a/src/Compilers/Core/CommandLine/CompilerServerLogger.cs b/src/Compilers/Core/CommandLine/CompilerServerLogger.cs index 0efc3761e05..74352c0d31c 100644 --- a/src/Compilers/Core/CommandLine/CompilerServerLogger.cs +++ b/src/Compilers/Core/CommandLine/CompilerServerLogger.cs @@ -41,7 +41,7 @@ static CompilerServerLogger() try { // Check if the environment - string loggingFileName = Environment.GetEnvironmentVariable(environmentVariable); + string? loggingFileName = Environment.GetEnvironmentVariable(environmentVariable); if (loggingFileName != null) { @@ -75,15 +75,15 @@ public static void Initialize(string outputPrefix) /// /// Log an exception. Also logs information about inner exceptions. /// - public static void LogException(Exception e, string reason) + public static void LogException(Exception exception, string reason) { if (s_loggingStream != null) { - Log("Exception '{0}' occurred during '{1}'. Stack trace:\r\n{2}", e.Message, reason, e.StackTrace); + Log("Exception '{0}' occurred during '{1}'. Stack trace:\r\n{2}", exception.Message, reason, exception.StackTrace); int innerExceptionLevel = 0; - e = e.InnerException; + Exception? e = exception.InnerException; while (e != null) { Log("Inner exception[{0}] '{1}'. Stack trace: \r\n{1}", innerExceptionLevel, e.Message, e.StackTrace); diff --git a/src/Compilers/Core/MSBuildTask/ManagedCompiler.cs b/src/Compilers/Core/MSBuildTask/ManagedCompiler.cs index 4470b8ec286..617b695dd32 100644 --- a/src/Compilers/Core/MSBuildTask/ManagedCompiler.cs +++ b/src/Compilers/Core/MSBuildTask/ManagedCompiler.cs @@ -343,7 +343,12 @@ public bool SkipCompilerExecution public string? TargetType { - set { _store[nameof(TargetType)] = CultureInfo.InvariantCulture.TextInfo.ToLower(value); } + set + { + _store[nameof(TargetType)] = value != null + ? CultureInfo.InvariantCulture.TextInfo.ToLower(value) + : null; + } get { return (string?)_store[nameof(TargetType)]; } } @@ -467,11 +472,11 @@ protected override int ExecuteTool(string pathToTool, string responseFileCommand try { var workingDir = CurrentDirectoryToUse(); - string tempDir = BuildServerConnection.GetTempPath(workingDir); + string? tempDir = BuildServerConnection.GetTempPath(workingDir); if (!UseSharedCompilation || HasToolBeenOverridden || - !BuildServerConnection.IsCompilerServerSupported(tempDir)) + !BuildServerConnection.IsCompilerServerSupported) { return base.ExecuteTool(pathToTool, responseFileCommands, commandLineCommands); } @@ -483,6 +488,10 @@ protected override int ExecuteTool(string pathToTool, string responseFileCommand CompilerServerLogger.Log($"BuildResponseFile = '{responseFileCommands}'"); var clientDir = Path.GetDirectoryName(PathToManagedTool); + if (clientDir is null || tempDir is null) + { + return base.ExecuteTool(pathToTool, responseFileCommands, commandLineCommands); + } // Note: we can't change the "tool path" printed to the console when we run // the Csc/Vbc task since MSBuild logs it for us before we get here. Instead, @@ -567,10 +576,10 @@ private string CurrentDirectoryToUse() /// /// Get the "LIB" environment variable, or NULL if none. /// - private string LibDirectoryToUse() + private string? LibDirectoryToUse() { // First check the real environment. - string libDirectory = Environment.GetEnvironmentVariable("LIB"); + string? libDirectory = Environment.GetEnvironmentVariable("LIB"); // Now go through additional environment variables. string[] additionalVariables = EnvironmentVariables; diff --git a/src/Compilers/Core/MSBuildTask/MapSourceRoots.cs b/src/Compilers/Core/MSBuildTask/MapSourceRoots.cs index 5611691b804..6d83946e482 100644 --- a/src/Compilers/Core/MSBuildTask/MapSourceRoots.cs +++ b/src/Compilers/Core/MSBuildTask/MapSourceRoots.cs @@ -89,7 +89,7 @@ private static bool EndsWithDirectorySeparator(string path) private void MergeSourceRootMetadata(ITaskItem left, ITaskItem right) { - foreach (string metadataName in right.MetadataNames) + foreach (string? metadataName in right.MetadataNames) { var leftValue = left.GetMetadata(metadataName); var rightValue = right.GetMetadata(metadataName); diff --git a/src/Compilers/Core/MSBuildTask/Microsoft.Build.Tasks.CodeAnalysis.csproj b/src/Compilers/Core/MSBuildTask/Microsoft.Build.Tasks.CodeAnalysis.csproj index ed371546c2a..1892d067053 100644 --- a/src/Compilers/Core/MSBuildTask/Microsoft.Build.Tasks.CodeAnalysis.csproj +++ b/src/Compilers/Core/MSBuildTask/Microsoft.Build.Tasks.CodeAnalysis.csproj @@ -12,7 +12,7 @@ $(NoWarn);CA1819 - + true Microsoft.CodeAnalysis.Build.Tasks @@ -23,6 +23,12 @@ true + + + + $(NoWarn);8600;8601;8602;8603;8604 + + PreserveNewest diff --git a/src/Compilers/Core/MSBuildTask/MvidReader.cs b/src/Compilers/Core/MSBuildTask/MvidReader.cs index e79fbd28d3c..42135ab83a8 100755 --- a/src/Compilers/Core/MSBuildTask/MvidReader.cs +++ b/src/Compilers/Core/MSBuildTask/MvidReader.cs @@ -99,7 +99,7 @@ private static Guid FindMvidInSections(ushort count, BinaryReader reader) return s_empty; } - if (name.Length == 8 && name[0] == '.' && + if (name!.Length == 8 && name[0] == '.' && name[1] == 'm' && name[2] == 'v' && name[3] == 'i' && name[4] == 'd' && name[5] == '\0') { // Section: VirtualSize (4) @@ -150,7 +150,7 @@ private static Guid ReadMvidSection(BinaryReader reader, uint pointerToMvidSecti return s_empty; } - return new Guid(guidBytes); + return new Guid(guidBytes!); } private static bool ReadUInt16(BinaryReader reader, out ushort output) @@ -177,7 +177,7 @@ private static bool ReadUInt32(BinaryReader reader, out uint output) return true; } - private static bool ReadBytes(BinaryReader reader, int count, [NotNullWhen(true)] out byte[]? output) + private static bool ReadBytes(BinaryReader reader, int count, out byte[]? output) { if (reader.BaseStream.Position + count >= reader.BaseStream.Length) { diff --git a/src/Compilers/Core/MSBuildTask/Utilities.cs b/src/Compilers/Core/MSBuildTask/Utilities.cs index 01ae73908aa..74da0c2bdd3 100644 --- a/src/Compilers/Core/MSBuildTask/Utilities.cs +++ b/src/Compilers/Core/MSBuildTask/Utilities.cs @@ -178,8 +178,8 @@ internal static string GenerateFullPathToTool(string toolName) var assemblyDirectory = Path.GetDirectoryName(assemblyPath); return RuntimeHostInfo.IsDesktopRuntime - ? Path.Combine(assemblyDirectory, toolName) - : Path.Combine(assemblyDirectory, "bincore", toolName); + ? Path.Combine(assemblyDirectory!, toolName) + : Path.Combine(assemblyDirectory!, "bincore", toolName); } } } diff --git a/src/Compilers/Core/MSBuildTask/ValidateBootstrap.cs b/src/Compilers/Core/MSBuildTask/ValidateBootstrap.cs index 33a86738507..2b0a90fd9a9 100644 --- a/src/Compilers/Core/MSBuildTask/ValidateBootstrap.cs +++ b/src/Compilers/Core/MSBuildTask/ValidateBootstrap.cs @@ -36,7 +36,7 @@ public sealed partial class ValidateBootstrap : Task public string? TasksAssemblyFullPath { get { return _tasksAssemblyFullPath; } - set { _tasksAssemblyFullPath = NormalizePath(Path.GetFullPath(value)); } + set { _tasksAssemblyFullPath = NormalizePath(Path.GetFullPath(value!)); } } public ValidateBootstrap() @@ -103,7 +103,7 @@ public override bool Execute() return path; } - private string GetDirectory(Assembly assembly) => Path.GetDirectoryName(Utilities.TryGetAssemblyPath(assembly)); + private string? GetDirectory(Assembly assembly) => Path.GetDirectoryName(Utilities.TryGetAssemblyPath(assembly)); internal static void AddFailedLoad(AssemblyName name) { diff --git a/src/Compilers/Core/Portable/InternalUtilities/NullableAttributes.cs b/src/Compilers/Core/Portable/InternalUtilities/NullableAttributes.cs index 0943bd89d3e..fde55dce4fa 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/NullableAttributes.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/NullableAttributes.cs @@ -5,7 +5,7 @@ // This was copied from https://github.com/dotnet/coreclr/blob/60f1e6265bd1039f023a82e0643b524d6aaf7845/src/System.Private.CoreLib/shared/System/Diagnostics/CodeAnalysis/NullableAttributes.cs // and updated to have the scope of the attributes be internal. -#if NETSTANDARD2_0 || NET472 || NET20 || NETSTANDARD1_3 || NET45 +#if !NETCOREAPP #nullable enable namespace System.Diagnostics.CodeAnalysis @@ -88,7 +88,4 @@ internal sealed class DoesNotReturnIfAttribute : Attribute } } -#elif NETCOREAPP3_1 -#else -#error "Unsupported configuration" #endif \ No newline at end of file diff --git a/src/Compilers/Shared/BuildServerConnection.cs b/src/Compilers/Shared/BuildServerConnection.cs index fcb138d8858..cfd9c9bf1c4 100644 --- a/src/Compilers/Shared/BuildServerConnection.cs +++ b/src/Compilers/Shared/BuildServerConnection.cs @@ -31,7 +31,7 @@ namespace Microsoft.CodeAnalysis.CommandLine /// communication layer which is shared between MSBuild and non-MSBuild components. This is the problem that /// BuildPathsAlt fixes as the type lives with the build server communication code. /// - internal readonly struct BuildPathsAlt + internal sealed class BuildPathsAlt { /// /// The path which contains the compiler binaries and response files. @@ -77,7 +77,7 @@ internal sealed class BuildServerConnection /// /// Determines if the compiler server is supported in this environment. /// - internal static bool IsCompilerServerSupported(string tempPath) => GetPipeNameForPathOpt("") is object; + internal static bool IsCompilerServerSupported => GetPipeNameForPathOpt("") is object; public static Task RunServerCompilation( RequestLanguage language, @@ -85,7 +85,7 @@ internal sealed class BuildServerConnection List arguments, BuildPathsAlt buildPaths, string? keepAlive, - string libEnvVariable, + string? libEnvVariable, CancellationToken cancellationToken) { var pipeNameOpt = sharedCompilationId ?? GetPipeNameForPathOpt(buildPaths.ClientDirectory); @@ -108,7 +108,7 @@ internal sealed class BuildServerConnection BuildPathsAlt buildPaths, string? pipeName, string? keepAlive, - string libEnvVariable, + string? libEnvVariable, int? timeoutOverride, CreateServerFunc createServerFunc, CancellationToken cancellationToken) @@ -588,7 +588,7 @@ internal static string GetClientMutexName(string pipeName) /// is . This function must emulate as /// closely as possible. /// - public static string GetTempPath(string? workingDir) + public static string? GetTempPath(string? workingDir) { if (PlatformInformation.IsUnix) { @@ -656,7 +656,7 @@ internal sealed class FileMutex : IDisposable internal static string GetMutexDirectory() { var tempPath = BuildServerConnection.GetTempPath(null); - var result = Path.Combine(tempPath, ".roslyn"); + var result = Path.Combine(tempPath!, ".roslyn"); Directory.CreateDirectory(result); return result; } diff --git a/src/Compilers/Shared/RuntimeHostInfo.cs b/src/Compilers/Shared/RuntimeHostInfo.cs index d04b6ac8ce4..ce4296e5651 100644 --- a/src/Compilers/Shared/RuntimeHostInfo.cs +++ b/src/Compilers/Shared/RuntimeHostInfo.cs @@ -33,7 +33,7 @@ internal static (string processFilePath, string commandLineArguments, string too if (IsDotNetHost(out string? pathToDotNet)) { commandLineArguments = $@"exec ""{toolFilePath}"" {commandLineArguments}"; - return (pathToDotNet, commandLineArguments, toolFilePath); + return (pathToDotNet!, commandLineArguments, toolFilePath); } else { @@ -58,7 +58,7 @@ internal static bool IsDotNetHost([NotNullWhen(true)] out string? pathToDotNet) private static string DotNetHostPathEnvironmentName = "DOTNET_HOST_PATH"; - private static bool IsDotNetHost([NotNullWhen(true)] out string? pathToDotNet) + private static bool IsDotNetHost(out string? pathToDotNet) { pathToDotNet = GetDotNetPathOrDefault(); return true; -- GitLab