Enable #nullable enable in SolutionBuilder.cs#11458
Merged
Merged
Conversation
6 tasks
Agent-Logs-Url: https://github.com/dotnet/android/sessions/cab59e12-2042-496b-a394-a730244a44f2 Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Enable nullable enable in SolutionBuilder.cs
Enable May 22, 2026
#nullable enable in SolutionBuilder.cs
Contributor
There was a problem hiding this comment.
Pull request overview
Enables nullable reference type analysis in the SolutionBuilder test helper to surface potential null-reference issues at compile time, and adds runtime guards where SolutionPath is required.
Changes:
- Added
#nullable enableto opt the file into full nullable analysis. - Annotated
SolutionPathas nullable and provided a non-null default forSolutionName. - Added null guards for
SolutionPathinSave()/BuildProject()and tightened cleanup conditions inDispose().
Comments suppressed due to low confidence (3)
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/SolutionBuilder.cs:27
ArgumentNullException.ThrowIfNull (SolutionPath)doesn’t make subsequent uses of the property non-nullable for the compiler (and the property could change between reads). Consider capturing to a local (var solutionPath = SolutionPath; ThrowIfNull (solutionPath);) and using the local for allPath.Combine/WriteAllTextcalls in this method to avoid nullable warnings and double-evaluation.
public void Save ()
{
ArgumentNullException.ThrowIfNull (SolutionPath);
foreach (var p in Projects) {
using (var pb = new ProjectBuilder (Path.Combine (SolutionPath, p.ProjectName))) {
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/SolutionBuilder.cs:72
- Same pattern here: after
ThrowIfNull (SolutionPath), subsequent references to theSolutionPathproperty are stillstring?from the compiler’s perspective. Capture to a local once, validate it, and use the local forPath.Combine/BuildInternalto avoid nullable warnings and ensure a consistent value is used.
public bool BuildProject(XamarinProject project, string target = "Build")
{
ArgumentNullException.ThrowIfNull (SolutionPath);
BuildSucceeded = BuildInternal(Path.Combine (SolutionPath, project.ProjectName, project.ProjectFilePath), target, restore: project.ShouldRestorePackageReferences);
return BuildSucceeded;
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/SolutionBuilder.cs:102
!string.IsNullOrEmpty (SolutionPath)followed byDirectory.Delete (SolutionPath, ...)re-reads the nullable property; this can still produce nullable warnings and allows the value to change between the check and the delete. CaptureSolutionPathto a local variable, check the local, and pass the local intoDirectory.Delete.
protected override void Dispose (bool disposing)
{
if (disposing)
if (BuildSucceeded && !string.IsNullOrEmpty (SolutionPath))
try {
Directory.Delete (SolutionPath, recursive: true);
} catch (Exception) {
Comment on lines
12
to
15
| public IList<XamarinProject> Projects { get; } | ||
| public string SolutionPath { get; set; } | ||
| public string SolutionName { get; set; } | ||
| public string? SolutionPath { get; set; } | ||
| public string SolutionName { get; set; } = ""; | ||
| public bool BuildSucceeded { get; set; } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SolutionBuilder.cslacks nullable annotations, hiding potential null-reference issues at compile time.#nullable enableas the first lineSolutionPath→string?(never set in constructor, always via object initializer)SolutionName→ default= ""(constructor assigns it, default satisfies the compiler)ArgumentNullException.ThrowIfNull(SolutionPath)inSave()andBuildProject()Directory.DeleteinDisposewithstring.IsNullOrEmpty(SolutionPath)!(null-forgiving operator) usedNote: used
string.IsNullOrEmpty()rather than the.IsNullOrEmpty()extension method becauseXamarin.ProjectToolsdoesn't reference theNullableExtensionsclass — consistent with all other usages in this project.