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

Add nullable annotations to MSBuildTask project #26622

Closed
wants to merge 2 commits into from
Closed
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
13 changes: 13 additions & 0 deletions src/Compilers/Core/CommandLine/BuildProtocol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,21 @@ public BuildRequest(uint protocolVersion,
}
}

#if USES_ANNOTATIONS
Copy link
Member

@sharwell sharwell May 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❔ If this is in a branch anyway, why have the preprocessor like this? It makes it hard to see the true impact 😦

Edit: Probably a linked file and nullable reference types is only enabled in one of the locations. #Resolved

Copy link
Member Author

@jcouv jcouv May 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. I'm setting LangVersion in MSBuildTask first, then other projects will follow. #Resolved

public static BuildRequest Create(RequestLanguage language,
string workingDirectory,
string tempDirectory,
IList<string> args,
string? keepAlive = null,
string? libDirectory = null)
#else
public static BuildRequest Create(RequestLanguage language,
string workingDirectory,
string tempDirectory,
IList<string> args,
string keepAlive = null,
string libDirectory = null)
#endif
{
Log("Creating BuildRequest");
Log($"Working directory: {workingDirectory}");
Expand Down Expand Up @@ -110,7 +119,11 @@ public static BuildRequest CreateShutdown()
/// The total request size must be less than 1MB.
/// </summary>
/// <returns>null if the Request was too large, the Request otherwise.</returns>
#if USES_ANNOTATIONS
public static async Task<BuildRequest?> ReadAsync(Stream inStream, CancellationToken cancellationToken)
#else
public static async Task<BuildRequest> ReadAsync(Stream inStream, CancellationToken cancellationToken)
#endif
{
// Read the length of the request
var lengthBuffer = new byte[4];
Expand Down
12 changes: 12 additions & 0 deletions src/Compilers/Core/CommandLine/CompilerServerLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ internal class CompilerServerLogger
// Environment variable, if set, to enable logging and set the file to log to.
private const string environmentVariable = "RoslynCommandLineLogFile";

#if USES_ANNOTATIONS
private static readonly Stream? s_loggingStream;
#else
private static readonly Stream s_loggingStream;
#endif
private static string s_prefix = "---";

/// <summary>
Expand Down Expand Up @@ -115,9 +119,17 @@ public static void Log(string message)

// Because multiple processes might be logging to the same file, we always seek to the end,
// write, and flush.
#if USES_ANNOTATIONS
// PROTOTYPE(NullableDogfood): There should be no warning on de-referencing s_loggingStream since checked above
// https://github.com/dotnet/roslyn/issues/26651
s_loggingStream!.Seek(0, SeekOrigin.End);
s_loggingStream!.Write(bytes, 0, bytes.Length);
s_loggingStream!.Flush();
#else
s_loggingStream.Seek(0, SeekOrigin.End);
s_loggingStream.Write(bytes, 0, bytes.Length);
s_loggingStream.Flush();
#endif
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/Compilers/Core/CommandLine/NativeMethods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,11 @@ internal static class NativeMethods
[return: MarshalAs(UnmanagedType.Bool)]
internal static extern bool CreateProcess
(
#if USES_ANNOTATIONS
string? lpApplicationName,
#else
string lpApplicationName,
#endif
[In, Out]StringBuilder lpCommandLine,
IntPtr lpProcessAttributes,
IntPtr lpThreadAttributes,
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/Core/MSBuildTask/AssemblyResolution.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public static void Install()
// empty, just to trigger static ctor
}

internal static Assembly ResolveAssembly(string assemblyDisplayName, Assembly requestingAssemblyOpt)
internal static Assembly? ResolveAssembly(string assemblyDisplayName, Assembly? requestingAssemblyOpt)
{
var name = new AssemblyName(assemblyDisplayName);
try
Expand Down Expand Up @@ -115,8 +115,8 @@ private static bool TryRedirect(AssemblyName name, byte[] token, int major, int
return false;
}

private static readonly string s_assemblyLocation = Utilities.TryGetAssemblyPath(typeof(AssemblyResolution).GetTypeInfo().Assembly);
private static Assembly TryRedirectToRuntimesDir(AssemblyName name)
private static readonly string? s_assemblyLocation = Utilities.TryGetAssemblyPath(typeof(AssemblyResolution).GetTypeInfo().Assembly);
private static Assembly? TryRedirectToRuntimesDir(AssemblyName name)
{
CompilerServerLogger.Log($"Loading with redirect {name.Name}");
if (s_assemblyLocation == null)
Expand Down
10 changes: 5 additions & 5 deletions src/Compilers/Core/MSBuildTask/CanonicalError.cs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ internal Parts()
/// <summary>
/// Name of the file or tool (not localized)
/// </summary>
internal string origin;
internal string? origin;

/// <summary>
/// The line number.
Expand Down Expand Up @@ -203,17 +203,17 @@ internal Parts()
/// <summary>
/// The sub category (localized)
/// </summary>
internal string subcategory;
internal string? subcategory;

/// <summary>
/// The error code (not localized)
/// </summary>
internal string code;
internal string? code;

/// <summary>
/// The error message text (localized)
/// </summary>
internal string text;
internal string? text;

#if NEVER
internal new string ToString()
Expand Down Expand Up @@ -263,7 +263,7 @@ private static int ConvertToIntWithDefault(string value)
/// <owner>JomoF</owner>
/// <param name="message"></param>
/// <returns>Decomposed canonical message, or null.</returns>
internal static Parts Parse(string message)
internal static Parts? Parse(string message)
{
// An unusually long string causes pathologically slow Regex back-tracking.
// To avoid that, only scan the first 400 characters. That's enough for
Expand Down
18 changes: 9 additions & 9 deletions src/Compilers/Core/MSBuildTask/CommandLineBuilderExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal void AppendWhenTrue
string parameterName
)
{
object obj = bag[parameterName];
object? obj = bag[parameterName];
// If the switch isn't set, don't add it to the command line.
if (obj != null)
{
Expand All @@ -47,7 +47,7 @@ internal void AppendPlusOrMinusSwitch
string parameterName
)
{
object obj = bag[parameterName];
object? obj = bag[parameterName];
// If the switch isn't set, don't add it to the command line.
if (obj != null)
{
Expand All @@ -70,7 +70,7 @@ internal void AppendByChoiceSwitch
string choice2
)
{
object obj = bag[parameterName];
object? obj = bag[parameterName];
// If the switch isn't set, don't add it to the command line.
if (obj != null)
{
Expand All @@ -89,7 +89,7 @@ internal void AppendSwitchWithInteger
string parameterName
)
{
object obj = bag[parameterName];
object? obj = bag[parameterName];
// If the switch isn't set, don't add it to the command line.
if (obj != null)
{
Expand Down Expand Up @@ -140,7 +140,7 @@ protected string GetQuotedText(string unquotedText)
internal void AppendSwitchIfNotNull
(
string switchName,
ITaskItem[] parameters,
ITaskItem[]? parameters,
string[] attributes
)
{
Expand All @@ -151,7 +151,7 @@ string[] attributes
/// Append a switch if 'parameter' is not null.
/// Split on the characters provided.
/// </summary>
internal void AppendSwitchWithSplitting(string switchName, string parameter, string delimiter, params char[] splitOn)
internal void AppendSwitchWithSplitting(string switchName, string? parameter, string delimiter, params char[] splitOn)
{
if (parameter != null)
{
Expand All @@ -170,7 +170,7 @@ internal void AppendSwitchWithSplitting(string switchName, string parameter, str
/// even if it contains the separators and white space only
/// Split on the characters provided.
/// </summary>
internal static bool IsParameterEmpty(string parameter, params char[] splitOn)
internal static bool IsParameterEmpty(string? parameter, params char[] splitOn)
{
if (parameter != null)
{
Expand All @@ -197,9 +197,9 @@ internal static bool IsParameterEmpty(string parameter, params char[] splitOn)
internal void AppendSwitchIfNotNull
(
string switchName,
ITaskItem[] parameters,
ITaskItem[]? parameters,
string[] metadataNames,
bool[] treatAsFlags // May be null. In this case no metadata are treated as flags.
bool[]? treatAsFlags // May be null. In this case no metadata are treated as flags.
)
{
Debug.Assert(treatAsFlags == null
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/Core/MSBuildTask/CopyRefAssembly.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ static CopyRefAssembly()
public CopyRefAssembly()
{
TaskResources = ErrorString.ResourceManager;
SourcePath = null!; // PROTOTYPE(NullableDogfood): MSBuild required
DestinationPath = null!; // PROTOTYPE(NullableDogfood): MSBuild required
}

public override bool Execute()
Expand Down
Loading