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

Enable Nullable in Main Directory.Build.Props, Add #nullable disable to .cs Files, and Clean Up Redundant Configurations #45522

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

v-wuzhai
Copy link
Member

@v-wuzhai v-wuzhai commented Dec 18, 2024

  1. Centralized Management: Enabling nullable reference types in the main Directory.Build.Props file ensures consistent default behavior across the entire repository.
  2. Gradual Transition: By adding #nullable disable at the top of each .cs file, excluding files in projects where nullable reference types are already enabled, nullable reference types can be gradually enabled without immediately affecting all files.
  3. Cleaning Up Redundant Configurations: Removing all enable and #nullable enable from the src and test directories avoids duplicate configurations, simplifies project files, and ensures consistency.

@v-wuzhai v-wuzhai requested review from a team, tmat, AntonLapounov and vijayrkn as code owners December 18, 2024 09:05
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-dotnet new the item is related to dotnet new command untriaged Request triage from a team member labels Dec 18, 2024
@v-wuzhai v-wuzhai requested a review from marcpopMSFT December 18, 2024 09:06
@v-wuzhai v-wuzhai force-pushed the dev/Jason/enable-nullable-and-cleanup branch from 16dd053 to ff0daa5 Compare December 19, 2024 08:38
@v-wuzhai
Copy link
Member Author

@ViktorHofer @marcpopMSFT To ensure consistent default behavior across the entire repository, I've enabled nullable reference types in the main Directory.Build.Props file. This allows for a gradual transition by adding #nullable disable at the top of each .cs file, excluding files in projects where nullable reference types are already enabled. Additionally, I've cleaned up redundant configurations by removing all <Nullable>enable</Nullable> and #nullable enable from the src and test directories.

Could you help to fix the VMR build failures?

@ViktorHofer
Copy link
Member

The VMR is its own entity and doesn't inherit settings from the dotnet/sdk repo. Therefore you need to set the Nullable=enable default in src/SourceBuild/content/Directory.Build.props as well.

@v-wuzhai
Copy link
Member Author

@ViktorHofer The VMR still seems to be failing, with the following error message:

CSC : error CS8630: Invalid 'nullable' value: 'Enable' for C# 7.3. Please use language version '8.0' or greater. [/Users/runner/work/1/vmr/eng/tools/tasks/Microsoft.DotNet.UnifiedBuild.MSBuildSdkResolver/Microsoft.DotNet.UnifiedBuild.MSBuildSdkResolver.csproj]
##[error]CSC(0,0): error CS8630: Invalid 'nullable' value: 'Enable' for C# 7.3. Please use language version '8.0' or greater.

The default language version for 'netstandard2.0' appears to be 7.3, which seems incompatible with enabling nullable. So, do we need to explicitly set the language version to 8.0 or higher, or should we disable nullable for netstandard2.0?

@ViktorHofer
Copy link
Member

@v-wuzhai I pushed a commit into your branch. We had to set LangVersion explicitly in the VMR orchestrator.

@akoeplinger
Copy link
Member

akoeplinger commented Dec 20, 2024

A couple more nullability errors:

##[error]eng/tools/tasks/Microsoft.DotNet.UnifiedBuild.MSBuildSdkResolver/UnifiedBuildSdkResolver.cs(126,32): error CS8603: Possible null reference return.
##[error]eng/tools/tasks/Microsoft.DotNet.UnifiedBuild.MSBuildSdkResolver/UnifiedBuildSdkResolver.cs(129,33): error CS8600: Converting null literal or possible null value to non-nullable type.
##[error]eng/tools/tasks/Microsoft.DotNet.UnifiedBuild.MSBuildSdkResolver/UnifiedBuildSdkResolver.cs(138,37): error CS8600: Converting null literal or possible null value to non-nullable type.
##[error]eng/tools/tasks/Microsoft.DotNet.UnifiedBuild.MSBuildSdkResolver/UnifiedBuildSdkResolver.cs(147,30): error CS8601: Possible null reference assignment.
##[error]eng/tools/tasks/Microsoft.DotNet.UnifiedBuild.MSBuildSdkResolver/UnifiedBuildSdkResolver.cs(150,34): error CS8601: Possible null reference assignment.
##[error]eng/tools/tasks/Microsoft.DotNet.UnifiedBuild.MSBuildSdkResolver/UnifiedBuildSdkResolver.cs(153,24): error CS8603: Possible null reference return.
##[error]eng/tools/tasks/Microsoft.DotNet.UnifiedBuild.MSBuildSdkResolver/UnifiedBuildSdkResolver.cs(159,27): error CS8618: Non-nullable property 'Group' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
##[error]eng/tools/tasks/Microsoft.DotNet.UnifiedBuild.MSBuildSdkResolver/UnifiedBuildSdkResolver.cs(164,27): error CS8618: Non-nullable property 'Id' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
##[error]eng/tools/tasks/Microsoft.DotNet.UnifiedBuild.MSBuildSdkResolver/UnifiedBuildSdkResolver.cs(169,27): error CS8618: Non-nullable property 'Version' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
##[error]eng/tools/tasks/Microsoft.DotNet.UnifiedBuild.MSBuildSdkResolver/UnifiedBuildSdkResolver.cs(174,27): error CS8618: Non-nullable property 'Dir' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.
##[error]eng/tools/tasks/Microsoft.DotNet.UnifiedBuild.MSBuildSdkResolver/UnifiedBuildSdkResolver.cs(179,27): error CS8618: Non-nullable property 'SdkDir' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the property as nullable.

pushed a fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-dotnet new the item is related to dotnet new command untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants