8000 Remove nullable disable warnings by jaredpar · Pull Request #48846 · dotnet/sdk · GitHub
[go: up one dir, main page]

Skip to content

Remove nullable disable warnings #48846

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

Merged
merged 5 commits into from
May 12, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
more
  • Loading branch information
jaredpar committed May 9, 2025
commit 1cfd469c41e48ca68d192d1f25b1a2727eaf6a6f
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable warnings

using MSBuildProject = Microsoft.Build.Evaluation.Project;

namespace Microsoft.DotNet.Cli.Commands.New.MSBuildEvaluation;
Expand Down
2 changes: 1 addition & 1 deletion src/Cli/dotnet/Commands/Solution/Add/SolutionAddCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ private void AddProject(SolutionModel solution, string fullProjectPath, ISolutio

// Get referencedprojects from the project instance
var referencedProjectsFullPaths = projectInstance.GetItems("ProjectReference")
.Select(item => Path.GetFullPath(item.EvaluatedInclude, Path.GetDirectoryName(fullProjectPath)));
.Select(item => Path.GetFullPath(item.EvaluatedInclude, Path.GetDirectoryName(fullProjectPath)!));

if (_includeReferences)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable warnings

#if NETCOREAPP
#endif

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable warnings

#if NETCOREAPP
#endif

Expand Down
2 changes: 0 additions & 2 deletions src/Cli/dotnet/Commands/Test/Terminal/AnsiDetector.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

#nullable disable warnings

// Portions of the code in this file were ported from the spectre.console by Patrik Svensson, Phil Scott, Nils Andresen
// https://github.com/spectreconsole/spectre.console/blob/main/src/Spectre.Console/Internal/Backends/Ansi/AnsiDetector.cs
// and from the supports-ansi project by Qingrong Ke
Expand Down
2 changes: 0 additions & 2 deletions src/Cli/dotnet/Commands/Test/Terminal/SystemConsole.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

#nullable disable warnings

namespace Microsoft.DotNet.Cli.Commands.Test.Terminal;

internal sealed class SystemConsole : IConsole
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

#nullable disable warnings

using System.Globalization;

namespace Microsoft.DotNet.Cli.Commands.Test.Terminal;
Expand Down
2 changes: 0 additions & 2 deletions src/Cli/dotnet/Commands/Test/Terminal/TestNodeResultsState.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

#nullable disable warnings

using System.Collections.Concurrent;
using System.Globalization;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

#nullable disable warnings

namespace Microsoft.DotNet.Cli.Commands.Test.Terminal;

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable warnings

using System.CommandLine;
using System.CommandLine.Parsing;
using Microsoft.DotNet.Cli.Commands.Tool.Common;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable warnings

using System.CommandLine;
using Microsoft.DotNet.Cli.Commands.Tool.Common;
using Microsoft.DotNet.Cli.Commands.Tool.Install;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable warnings

using System.CommandLine;
using Microsoft.Deployment.DotNet.Releases;
using Microsoft.DotNet.Cli.Commands.Workload.Install;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable warnings

using System.CommandLine;
using Microsoft.Deployment.DotNet.Releases;
using Microsoft.DotNet.Cli.Commands.Workload.Install;
Expand Down Expand Up @@ -55,7 +53,7 @@ public override int Execute()
// It seems that the parser doesn't give us a good way to do that, however
if (_hasUpdateMode)
{
string globalJsonPath = SdkDirectoryWorkloadManifestProvider.GetGlobalJsonPath(Environment.CurrentDirectory);
string? globalJsonPath = SdkDirectoryWorkloadManifestProvider.GetGlobalJsonPath(Environment.CurrentDirectory);
var globalJsonVersion = SdkDirectoryWorkloadManifestProvider.GlobalJsonReader.GetWorkloadVersionFromGlobalJson(globalJsonPath, out bool? shouldUseWorkloadSets);
shouldUseWorkloadSets ??= string.IsNullOrWhiteSpace(globalJsonVersion) ? null : true;
if (WorkloadConfigCommandParser.UpdateMode_WorkloadSet.Equals(_updateMode, StringComparison.InvariantCultureIgnoreCase))
Expand Down
2 changes: 0 additions & 2 deletions src/Cli/dotnet/Commands/Workload/Install/NullReporter.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable warnings

using Microsoft.DotNet.Cli.Utils;

namespace Microsoft.DotNet.Cli.Commands.Workload.Install;
Expand Down
37 changes: 22 additions & 15 deletions src/Cli/dotnet/Commands/Workload/WorkloadCommandBase.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable warnings

using System.CommandLine;
using Microsoft.DotNet.Cli.Commands.Workload.Install;
using Microsoft.DotNet.Cli.Extensions;
Expand Down Expand Up @@ -87,10 +85,10 @@ protected bool VerifySignatures
/// <param name="nugetPackageDownloader">The package downloader to use for acquiring NuGet packages.</param>
public WorkloadCommandBase(
ParseResult parseResult,
Option<VerbosityOptions> verbosityOptions = null,
Option<VerbosityOptions>? verbosityOptions = null,
IReporter? reporter = null,
string tempDirPath = null,
INuGetPackageDownloader nugetPackageDownloader = null) : base(parseResult)
string? tempDirPath = null,
INuGetPackageDownloader? nugetPackageDownloader = null) : base(parseResult)
{
VerifySignatures = ShouldVerifySignatures(parseResult);

Expand All @@ -107,20 +105,29 @@ public WorkloadCommandBase(
TempDirectoryPath = !string.IsNullOrWhiteSpace(tempDirPath)
? tempDirPath
: !string.IsNullOrWhiteSpace(parseResult.GetValue(WorkloadInstallCommandParser.TempDirOption))
? parseResult.GetValue(WorkloadInstallCommandParser.TempDirOption)
? parseResult.GetValue(WorkloadInstallCommandParser.TempDirOption)!
: PathUtilities.CreateTempSubdirectory();

TempPackagesDirectory = new DirectoryPath(Path.Combine(TempDirectoryPath, "dotnet-sdk-advertising-temp"));

IsPackageDownloaderProvided = nugetPackageDownloader != null;
PackageDownloader = IsPackageDownloaderProvided ? nugetPackageDownloader : new NuGetPackageDownloader.NuGetPackageDownloader(TempPackagesDirectory,
filePermissionSetter: null,
new FirstPartyNuGetPackageSigningVerifier(),
nugetLogger,
Reporter,
restoreActionConfig: RestoreActionConfiguration,
verifySignatures: VerifySignatures,
shouldUsePackageSourceMapping: true);
if (nugetPackageDownloader is not null)
{
IsPackageDownloaderProvided = true;
PackageDownloader = nugetPackageDownloader;
}
else
{
IsPackageDownloaderProvided = false;
PackageDownloader = new NuGetPackageDownloader.NuGetPackageDownloader(
TempPackagesDirectory,
filePermissionSetter: null,
new FirstPartyNuGetPackageSigningVerifier(),
nugetLogger,
Reporter,
restoreActionConfig: RestoreActionConfiguration,
verifySignatures: VerifySignatures,
shouldUsePackageSourceMapping: true);
}
Comment on lines +113 to +130
Copy link
Member Author

Choose a reason for hiding this comment

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

This is just a simple expansion of the original code that is more NRT friendly. Could also change this to be a tuple return but this seemed more readable.

}

/// <summary>
Expand Down
26 changes: 13 additions & 13 deletions src/Cli/dotnet/CommonOptions.cs
B41A
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable warnings

using System.CommandLine;
using System.CommandLine.Completions;
using System.CommandLine.Parsing;
Expand Down Expand Up @@ -69,14 +67,14 @@ public static IEnumerable<string> RuntimeArgFunc(string rid)
new DynamicForwardedOption<string>("--runtime", "-r")
{
HelpName = RuntimeArgName
}.ForwardAsMany(RuntimeArgFunc)
}.ForwardAsMany(RuntimeArgFunc!)
.AddCompletions(CliCompletion.RunTimesFromProjectFile);

public static Option<string> LongFormRuntimeOption =
new DynamicForwardedOption<string>("--runtime")
{
HelpName = RuntimeArgName
}.ForwardAsMany(RuntimeArgFunc)
}.ForwardAsMany(RuntimeArgFunc!)
Copy link
Member

Choose a reason for hiding this comment

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

why did you need the ! for these? The static function is known, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

public static Option<T> ForwardAsMany<T>(this ForwardedOption<T> option, Func<T?, IEnumerable<string>> format);

This requires that the delegate have a string? parameter, presumably more fallout of ParseResult design. The RuntimeArgFunc though depends on having a non-null parameter. The parameter is immediately passed to GetArchFromRid and that throws on null.

.AddCompletions(CliCompletion.RunTimesFromProjectFile);

public static Option<bool> CurrentRuntimeOption(string description) =>
Expand Down Expand Up @@ -162,7 +160,7 @@ public static ForwardedOption<bool> InteractiveOption(bool acceptArgument = fals
HelpName = CliStrings.ArchArgumentName
}.SetForwardingFunction(ResolveArchOptionToRuntimeIdentifier);

internal static string ArchOptionValue(ParseResult parseResult) =>
internal static string? ArchOptionValue(ParseResult parseResult) =>
string.IsNullOrEmpty(parseResult.GetValue(ArchitectureOption)) ?
parseResult.GetValue(LongFormArchitectureOption) :
parseResult.GetValue(ArchitectureOption);
Expand Down Expand Up @@ -254,7 +252,7 @@ public static void ValidateSelfContainedOptions(bool hasSelfContainedOption, boo
}
}

internal static IEnumerable<string> ResolveArchOptionToRuntimeIdentifier(string arg, ParseResult parseResult)
internal static IEnumerable<string> ResolveArchOptionToRuntimeIdentifier(string? arg, ParseResult parseResult)
{
if ((parseResult.GetResult(RuntimeOption) ?? parseResult.GetResult(LongFormRuntimeOption)) is not null)
{
Expand All @@ -270,7 +268,7 @@ internal static IEnumerable<string> ResolveArchOptionToRuntimeIdentifier(string
return ResolveRidShorthandOptions(null, arg);
}

internal static IEnumerable<string> ResolveOsOptionToRuntimeIdentifier(string arg, ParseResult parseResult)
internal static IEnumerable<string> ResolveOsOptionToRuntimeIdentifier(string? arg, ParseResult parseResult)
{
if ((parseResult.GetResult(RuntimeOption) ?? parseResult.GetResult(LongFormRuntimeOption)) is not null)
{
Expand All @@ -281,10 +279,10 @@ internal static IEnumerable<string> ResolveOsOptionToRuntimeIdentifier(string ar
return ResolveRidShorthandOptions(arg, arch);
}

private static IEnumerable<string> ResolveRidShorthandOptions(string os, string arch) =>
private static IEnumerable<string> ResolveRidShorthandOptions(string? os, string? arch) =>
[$"-property:RuntimeIdentifier={ResolveRidShorthandOptionsToRuntimeIdentifier(os, arch)}"];

internal static string ResolveRidShorthandOptionsToRuntimeIdentifier(string os, string arch)
internal static string ResolveRidShorthandOptionsToRuntimeIdentifier(string? os, string? arch)
{
var currentRid = GetCurrentRuntimeId();
arch = arch == "amd64" ? "x64" : arch;
Expand All @@ -296,15 +294,17 @@ internal static string ResolveRidShorthandOptionsToRuntimeIdentifier(string os,
public static string GetCurrentRuntimeId()
{
// Get the dotnet directory, while ignoring custom msbuild resolvers
string dotnetRootPath = NativeWrapper.EnvironmentProvider.GetDotnetExeDirectory(key =>
string? dotnetRootPath = NativeWrapper.EnvironmentProvider.GetDotnetExeDirectory(key =>
key.Equals("DOTNET_MSBUILD_SDK_RESOLVER_CLI_DIR", StringComparison.InvariantCultureIgnoreCase)
? null
: Environment.GetEnvironmentVariable(key));
57AE var ridFileName = "NETCoreSdkRuntimeIdentifierChain.txt";
var sdkPath = dotnetRootPath is not null ? Path.Combine(dotnetRootPath, "sdk") : "sdk";
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a case where the NRT analysis seems to have spotted a legitimate bug in the code. In the case that dotnetRootpath is null then the Path.Combine calls below will throw. I changed this to just use "sdk" as a relative path but possible that a throws here is a better idea.


// When running under test the Product.Version might be empty or point to version not installed in dotnetRootPath.
string runtimeIdentifierChainPath = string.IsNullOrEmpty(Product.Version) || !Directory.Exists(Path.Combine(dotnetRootPath, "sdk", Product.Version)) ?
Path.Combine(Directory.GetDirectories(Path.Combine(dotnetRootPath, "sdk"))[0], ridFileName) :
Path.Combine(dotnetRootPath, "sdk", Product.Version, ridFileName);
string runtimeIdentifierChainPath = string.IsNullOrEmpty(Product.Version) || !Directory.Exists(Path.Combine(sdkPath, Product.Version)) ?
Path.Combine(Directory.GetDirectories(sdkPath)[0], ridFileName) :
Path.Combine(sdkPath, Product.Version, ridFileName);
string[] currentRuntimeIdentifiers = File.Exists(runtimeIdentifierChainPath) ? [.. File.ReadAllLines(runtimeIdentifierChainPath).Where(l => !string.IsNullOrEmpty(l))] : [];
if (currentRuntimeIdentifiers == null || !currentRuntimeIdentifiers.Any() || !currentRuntimeIdentifiers[0].Contains("-"))
{
Expand Down
4 changes: 2 additions & 2 deletions src/Tasks/Microsoft.NET.Build.Tasks/GetPackagesToPrune.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable warnings
#nullable disable
Copy link
Member Author

Choose a reason for hiding this comment

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

For most of the build tasks there was a single ? annotation. Instead of fully annotating the build tasks, which is hundreds of changes, I just removed the errant annotation and disable NRT in the file. Seems much better to approach those as a separate change where a good deal of thought is put into how NRT should be structured in tasks.


using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;
Expand Down Expand Up @@ -44,7 +44,7 @@ class CacheKey
public string TargetFrameworkVersion { get; set; }
public HashSet<string> FrameworkReferences { get; set; }

public override bool Equals(object? obj) => obj is CacheKey key &&
public override bool Equals(object obj) => obj is CacheKey key &&
TargetFrameworkIdentifier == key.TargetFrameworkIdentifier &&
TargetFrameworkVersion == key.TargetFrameworkVersion &&
FrameworkReferences.SetEquals(key.FrameworkReferences);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable warnings
#nullable disable

using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;
Expand Down Expand Up @@ -787,7 +787,7 @@ private ToolPackSupport AddToolPack(
packVersion = RuntimeFrameworkVersion;
}

TaskItem? runtimePackToDownload = null;
TaskItem runtimePackToDownload = null;

// Crossgen and ILCompiler have RID-specific bits.
if (toolPackType is ToolPackType.Crossgen2 or ToolPackType.ILCompiler)
Expand Down
10 changes: 5 additions & 5 deletions src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable warnings
#nullable disable

using System.Diagnostics;
using System.Security.Cryptography;
Expand Down Expand Up @@ -956,10 +956,10 @@ private void WriteAnalyzers()
private class AnalyzerResolver
{
private readonly CacheWriter _cacheWriter;
private readonly string? _compilerNameSearchString;
private readonly Version? _compilerVersion;
private Dictionary<(string, NuGetVersion), LockFileTargetLibrary>? _targetLibraries;
private List<(string, LockFileLibrary, Version)>? _potentialAnalyzers;
private readonly string _compilerNameSearchString;
private readonly Version _compilerVersion;
private Dictionary<(string, NuGetVersion), LockFileTargetLibrary> _targetLibraries;
private List<(string, LockFileLibrary, Version)> _potentialAnalyzers;
private Version _maxApplicableVersion;

private Dictionary<(string, NuGetVersion), LockFileTargetLibrary> TargetLibraries =>
Expand Down
4 changes: 2 additions & 2 deletions src/Tasks/Microsoft.NET.Build.Tasks/ShowMissingWorkloads.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable warnings
#nullable disable

using System.Globalization;
using Microsoft.Build.Framework;
Expand Down Expand Up @@ -38,7 +38,7 @@ protected override void ExecuteCore()
{
if (MissingWorkloadPacks.Any())
{
string? userProfileDir = CliFolderPathCalculatorCore.GetDotnetUserProfileFolderPath();
string userProfileDir = CliFolderPathCalculatorCore.GetDotnetUserProfileFolderPath();

// When running MSBuild tasks, the current directory is always the project directory, so we can use that as the
// starting point to search for global.json
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable disable warnings
#nullable disable

using System.Runtime.Versioning;
using Microsoft.Build.Framework;
Expand Down Expand Up @@ -55,7 +55,7 @@ protected override void ExecuteCore()
// We can only access TargetFrameworks and NearestTargetFramework to find the referenced project "TargetFramework".
// We rely on the nearest one because it will pick the lowest 'most-compatible' tfm for the referencer and referencee projects.
// Since 'younger' TFMs are the ones that would error and are generally also what gets picked as 'most-copmaptible,' we can use it.
FrameworkName? referencedProjectTargetFramework = null;
FrameworkName referencedProjectTargetFramework = null;
var targetFrameworkMonikerIndex = Array.FindIndex(project.GetMetadata("TargetFrameworks").Split(';'), targetFramework => targetFramework == nearestTargetFramework);

// Since TargetFrameworks can have aliases that aren't the real TFM, we need to uncover the potential alias to the raw TargetFramework in the TFMs by using its index
Expand Down
0