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.

Sign up for GitHub

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

Remove nullable disable warnings #48846

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
594dd03
progress
jaredpar May 7, 2025
3d58d3f
progress
jaredpar May 7, 2025
28a929e
progress
jaredpar May 7, 2025
d7a7b0f
Merge remote-tracking branch 'origin/main' into nrt2
jaredpar May 9, 2025
1cfd469
more
jaredpar May 9, 2025
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
Next Next commit
progress
  • Loading branch information
@jaredpar
jaredpar committed May 7, 2025
commit 28a929eedc319dc767ac967fff1339c5f6996853
4 changes: 2 additions & 2 deletions src/Cli/dotnet/CommandFactory/CommandSpec.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
namespace Microsoft.DotNet.Cli.CommandFactory;

public class CommandSpec(
string? path,
string path,
string? args,
Dictionary<string, string>? environmentVariables = null)
{
public string? Path { get; } = path;
public string Path { get; } = path;

public string? Args { get; } = args;
Copy link
Member Author

Choose a reason for hiding this comment

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

Having Args be potentially null seems suspicious, I considered changing this to the following but don't know enough about the code base to say if that is okay

public string Args { get; } = args ?? "";


Expand Down
2 changes: 0 additions & 2 deletions src/Cli/dotnet/Commands/New/MSBuildEvaluation/ProjectContextSymbolSource.cs
View file
Open in desktop
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.Extensions.Logging;
using Microsoft.TemplateEngine.Abstractions;
using Microsoft.TemplateEngine.Abstractions.Components;
Expand Down
2 changes: 0 additions & 2 deletions src/Cli/dotnet/Commands/New/PostActions/DotnetRestorePostActionProcessor.cs
View file
Open in desktop
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;
using Microsoft.TemplateEngine.Abstractions;
using Microsoft.TemplateEngine.Cli.PostActionProcessors;
Expand Down
6 changes: 2 additions & 4 deletions src/Cli/dotnet/Commands/Run/CommonRunHelpers.cs
View file
Open in desktop
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.Diagnostics;
using Microsoft.DotNet.Cli.Utils;

Expand All @@ -13,7 +11,7 @@ internal static class CommonRunHelpers
/// <param name="globalProperties">
/// Should have <see cref="StringComparer.OrdinalIgnoreCase"/>.
/// </param>
public static void AddUserPassedProperties(Dictionary<string, string> globalProperties, string[]? args)
public static void AddUserPassedProperties(Dictionary<string, string> globalProperties, string[] args)
{
Debug.Assert(globalProperties.Comparer == StringComparer.OrdinalIgnoreCase);

Expand All @@ -36,7 +34,7 @@ public static void AddUserPassedProperties(Dictionary<string, string> globalProp
}
}

public static Dictionary<string, string> GetGlobalPropertiesFromArgs(string[]? args)
public static Dictionary<string, string> GetGlobalPropertiesFromArgs(string[] args)
{
var globalProperties = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
{
Expand Down
8 changes: 3 additions & 5 deletions src/Cli/dotnet/Commands/Run/LaunchSettings/LaunchSettingsApplyResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
// 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

namespace Microsoft.DotNet.Cli.Commands.Run.LaunchSettings;

public class LaunchSettingsApplyResult(bool success, string? failureReason, ProjectLaunchSettingsModel launchSettings = null)
public class LaunchSettingsApplyResult(bool success, string? failureReason, ProjectLaunchSettingsModel? launchSettings = null)
{
public bool Success { get; } = success;

public string FailureReason { get; } = failureReason;
public string? FailureReason { get; } = failureReason;

public ProjectLaunchSettingsModel LaunchSettings { get; } = launchSettings;
public ProjectLaunchSettingsModel? LaunchSettings { get; } = launchSettings;
}
2 changes: 0 additions & 2 deletions src/Cli/dotnet/Commands/Run/LaunchSettings/LaunchSettingsManager.cs
View file
Open in desktop
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.Diagnostics.CodeAnalysis;
using System.Text.Json;
using Microsoft.DotNet.Cli.Utils;
Expand Down
2 changes: 0 additions & 2 deletions src/Cli/dotnet/Commands/Run/LaunchSettings/ProjectLaunchSettingsProvider.cs
View file
Open in desktop
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.Text.Json;

namespace Microsoft.DotNet.Cli.Commands.Run.LaunchSettings;
Expand Down
2 changes: 1 addition & 1 deletion src/Cli/dotnet/Commands/Run/RunCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ private ICommand GetTargetCommand(Func<ProjectCollection, ProjectInstance>? proj
var command = CreateCommandFromRunProperties(project, runProperties);
return command;

static ProjectInstance EvaluateProject(string? projectFilePath, Func<ProjectCollection, ProjectInstance>? projectFactory, string[]? restoreArgs, ILogger? binaryLogger)
static ProjectInstance EvaluateProject(string? projectFilePath, Func<ProjectCollection, ProjectInstance>? projectFactory, string[] restoreArgs, ILogger? binaryLogger)
{
Debug.Assert(projectFilePath is not null || projectFactory is not null);

Expand Down
2 changes: 1 addition & 1 deletion src/Cli/dotnet/Commands/Run/RunProperties.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace Microsoft.DotNet.Cli.Commands.Run;

internal record RunProperties(string? RunCommand, string? RunArguments, string? RunWorkingDirectory)
internal record RunProperties(string RunCommand, string? RunArguments, string? RunWorkingDirectory)
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 another place where having arguments, specifically RunArguments, be potentially null feels potentially off and "" may work.

{
internal static RunProperties FromProjectAndApplicationArguments(ProjectInstance project, string[] applicationArgs, bool fallbackToTargetPath)
{
Expand Down
2 changes: 0 additions & 2 deletions src/Cli/dotnet/Commands/Sdk/Check/RuntimeOutputWriter.cs
View file
Open in desktop
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.Deployment.DotNet.Releases;
using Microsoft.DotNet.Cli.Utils;
using Microsoft.DotNet.NativeWrapper;
Expand Down
2 changes: 0 additions & 2 deletions src/Cli/dotnet/Commands/Sdk/Check/SdkOutputWriter.cs
View file
Open in desktop
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.Deployment.DotNet.Releases;
using Microsoft.DotNet.Cli.Utils;
using Microsoft.DotNet.NativeWrapper;
Expand Down
17 changes: 9 additions & 8 deletions src/Cli/dotnet/Commands/Solution/Add/SolutionAddCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
// 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.Diagnostics;
using Microsoft.Build.Construction;
using Microsoft.Build.Exceptions;
using Microsoft.Build.Execution;
Expand Down Expand Up @@ -39,7 +38,7 @@ private static bool IsSolutionFolderPathInDirectoryScope(string relativePath)

public SolutionAddCommand(ParseResult parseResult) : base(parseResult)
{
_fileOrDirectory = parseResult.GetValue(SolutionCommandParser.SlnArgument);
_fileOrDirectory = parseResult.GetValue(SolutionCommandParser.SlnArgument)!;
Copy link
Member Author

Choose a reason for hiding this comment

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

The ParseResult type in general does not work well with nullable analysis because GetValue is hard coded to return T?. One possible solution is add a GetValueOrThrow extension method. Happy to make that change if it's desired.

Copy link
Member

Choose a reason for hiding this comment

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

We chatted about this briefly but the main gap here is that the S.CL library enforces the arity of an option/argument but there's no way for that Arity to influence the return type of this method.

_projects = (IReadOnlyCollection<string>)(parseResult.GetValue(SolutionAddCommandParser.ProjectPathArgument) ?? []);
_inRoot = parseResult.GetValue(SolutionAddCommandParser.InRootOption);
_solutionFolderPath = parseResult.GetValue(SolutionAddCommandParser.SolutionFolderOption);
Expand Down Expand Up @@ -75,12 +74,13 @@ public override int Execute()
return null;
}

string relativeSolutionFolderPath = string.Empty;
string? relativeSolutionFolderPath = string.Empty;

if (string.IsNullOrEmpty(_solutionFolderPath))
{
// Generate the solution folder path based on the project path
relativeSolutionFolderPath = Path.GetDirectoryName(relativeProjectPath);
Debug.Assert(relativeSolutionFolderPath is not null);

// If the project is in a folder with the same name as the project, we need to go up one level
if (relativeSolutionFolderPath.Split(Path.DirectorySeparatorChar).LastOrDefault() == Path.GetFileNameWithoutExtension(relativeProjectPath))
Expand Down Expand Up @@ -108,6 +108,7 @@ public override int Execute()
private async Task AddProjectsToSolutionAsync(IEnumerable<string> projectPaths, CancellationToken cancellationToken)
{
SolutionModel solution = SlnFileFactory.CreateFromFileOrDirectory(_solutionFileFullPath);
Debug.Assert(solution.SerializerExtension is not null);
ISolutionSerializer serializer = solution.SerializerExtension.Serializer;

// set UTF8 BOM encoding for .sln
Expand Down Expand Up @@ -138,9 +139,9 @@ private async Task AddProjectsToSolutionAsync(IEnumerable<string> projectPaths,
await serializer.SaveAsync(_solutionFileFullPath, solution, cancellationToken);
}

private void AddProject(SolutionModel solution, string fullProjectPath, ISolutionSerializer serializer = null)
private void AddProject(SolutionModel solution, string fullProjectPath, ISolutionSerializer? serializer = null)
{
string solutionRelativeProjectPath = Path.GetRelativePath(Path.GetDirectoryName(_solutionFileFullPath), fullProjectPath);
string solutionRelativeProjectPath = Path.GetRelativePath(Path.GetDirectoryName(_solutionFileFullPath)!, fullProjectPath);

// Open project instance to see if it is a valid project
ProjectRootElement projectRootElement;
Expand Down Expand Up @@ -193,14 +194,14 @@ private void AddProject(SolutionModel solution, string fullProjectPath, ISolutio
foreach (var solutionPlatform in solution.Platforms)
{
var projectPlatform = projectInstancePlatforms.FirstOrDefault(
platform => platform.Replace(" ", string.Empty) == solutionPlatform.Replace(" ", string.Empty), projectInstancePlatforms.FirstOrDefault());
platform => platform.Replace(" ", string.Empty) == solutionPlatform.Replace(" ", string.Empty), projectInstancePlatforms.FirstOrDefault()!);
Comment on lines 201 to +202
Copy link
Member Author

Choose a reason for hiding this comment

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

Lacking the ! on the default value (last argument) the compiler will end up inferring FirstOrDefault<string?> not FirstOrDefault<string>. That is because the default value directly infers the value for T and FirstOrDefault is null returning. Don't know enough about the semantics here to suggest a more null friendly pattern.

project.AddProjectConfigurationRule(new ConfigurationRule(BuildDimension.Platform, "*", solutionPlatform, projectPlatform));
}

foreach (var solutionBuildType in solution.BuildTypes)
{
var projectBuildType = projectInstanceBuildTypes.FirstOrDefault(
buildType => buildType.Replace(" ", string.Empty) == solutionBuildType.Replace(" ", string.Empty), projectInstanceBuildTypes.FirstOrDefault());
buildType => buildType.Replace(" ", string.Empty) == solutionBuildType.Replace(" ", string.Empty), projectInstanceBuildTypes.FirstOrDefault()!);
Copy link
Member Author

Choose a reason for hiding this comment

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

Same FirstOrDefault pattern as above.

project.AddProjectConfigurationRule(new ConfigurationRule(BuildDimension.BuildType, solutionBuildType, "*", projectBuildType));
}

Expand Down
2 changes: 0 additions & 2 deletions src/Cli/dotnet/Commands/Test/IPC/Serializers/BaseSerializer.cs
View file
Open in desktop
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
using System.Buffers;
#endif
Expand Down
2 changes: 0 additions & 2 deletions src/Cli/dotnet/Extensions/ProjectInstanceExtensions.cs
View file
Open in desktop
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

using Microsoft.Build.Execution;

namespace Microsoft.DotNet.Cli.Extensions;
Expand Down
8 changes: 4 additions & 4 deletions test/dotnet.Tests/CommandFactoryTests/GivenALocalToolsCommandResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void WhenResolveStrictItCanFindToolExecutable()

result.Should().NotBeNull();

var commandPath = result.Args.Trim('"');
var commandPath = result.Args?.Trim('"') ?? "";
_fileSystem.File.Exists(commandPath).Should().BeTrue("the following path exists: " + commandPath);
commandPath.Should().Be(fakeExecutable.Value);
}
Expand All @@ -63,7 +63,7 @@ public void WhenResolveItCanFindToolExecutable(string toolCommand)

result.Should().NotBeNull();

var commandPath = result.Args.Trim('"');
var commandPath = result.Args?.Trim('"') ?? "";
_fileSystem.File.Exists(commandPath).Should().BeTrue("the following path exists: " + commandPath);
commandPath.Should().Be(fakeExecutable.Value);
}
Expand Down Expand Up @@ -217,12 +217,12 @@ [new RestoredCommandIdentifier(
localToolsCommandResolver.Resolve(new CommandResolverArguments()
{
CommandName = "dotnet-a",
}).Args.Trim('"').Should().Be(fakeExecutableA.Value);
}).Args!.Trim('"').Should().Be(fakeExecutableA.Value);

localToolsCommandResolver.Resolve(ne 5C6A w CommandResolverArguments()
{
CommandName = "dotnet-dotnet-a",
}).Args.Trim('"').Should().Be(fakeExecutableDotnetA.Value);
}).Args!.Trim('"').Should().Be(fakeExecutableDotnetA.Value);
}

private string _jsonContent =
Expand Down
Loading
0