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

Remove all use of <Nullable>annotations</Nullable> #90401

Merged
merged 4 commits into from
Aug 12, 2023
Merged
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
4 changes: 0 additions & 4 deletions eng/generators.targets
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@
<PropertyGroup>
<EnableLibraryImportGenerator Condition="'$(EnableLibraryImportGenerator)' == '' and
'$(MSBuildProjectName)' == 'System.Private.CoreLib'">true</EnableLibraryImportGenerator>
<!-- Disable the library import generator when the project requires polyfill source files but doesn't have nullable reference types enabled. -->
<EnableLibraryImportGenerator Condition="'$(EnableLibraryImportGenerator)' == '' and
!$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0')) and
('$(Nullable)' == 'disable' or '$(Nullable)' == '')">false</EnableLibraryImportGenerator>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,7 @@
<RuntimeMetadataVersion>v4.0.30319</RuntimeMetadataVersion>
<GenerateTargetFrameworkAttribute>false</GenerateTargetFrameworkAttribute>
<RunAnalyzers>false</RunAnalyzers>
<!-- This assembly has no LibraryImports and is used as a custom corelib, so disable the generator. -->
<EnableLibraryImportGenerator>false</EnableLibraryImportGenerator>
</PropertyGroup>
</Project>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@
<GenerateTargetFrameworkAttribute>false</GenerateTargetFrameworkAttribute>
<RunAnalyzers>false</RunAnalyzers>
<DefineConstants>TYPEEQUIVALENCEASSEMBLY_1</DefineConstants>
<!-- This assembly has no LibraryImports and uses a custom corelib, so disable the generator. -->
<EnableLibraryImportGenerator>false</EnableLibraryImportGenerator>
</PropertyGroup>

<ItemGroup>
<ProjectReference Include="..\CoreTestAssembly\CoreTestAssembly.csproj">
<Name>CoreTestAssembly</Name>
</ProjectReference>
</ItemGroup>
</Project>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@
<GenerateTargetFrameworkAttribute>false</GenerateTargetFrameworkAttribute>
<RunAnalyzers>false</RunAnalyzers>
<DefineConstants>TYPEEQUIVALENCEASSEMBLY_2</DefineConstants>
<!-- This assembly has no LibraryImports and uses a custom corelib, so disable the generator. -->
<EnableLibraryImportGenerator>false</EnableLibraryImportGenerator>
</PropertyGroup>

<ItemGroup>
<ProjectReference Include="..\CoreTestAssembly\CoreTestAssembly.csproj">
<Name>CoreTestAssembly</Name>
</ProjectReference>
</ItemGroup>
</Project>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace Microsoft.NET.HostModel.AppHost
/// Embeds the App Name into the AppHost.exe
/// If an apphost is a single-file bundle, updates the location of the bundle headers.
/// </summary>
public static class HostWriter
public static partial class HostWriter
{
/// <summary>
/// hash value embedded in default apphost executable in a place where the path to the app binary should be stored.
Expand Down Expand Up @@ -232,7 +232,7 @@ void FindBundleHeader()
return headerOffset != 0;
}

[DllImport("libc", SetLastError = true)]
private static extern int chmod(string pathname, int mode);
[LibraryImport("libc", SetLastError = true)]
private static partial int chmod([MarshalAs(UnmanagedType.LPStr)] string pathname, int mode);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This was annotated in #67513. Removing <Nullable> and <NoWarn> from M.E.H.Systemd's ref and src csproj seems to build.

Copy link
Member Author

@stephentoub stephentoub Aug 13, 2023

Choose a reason for hiding this comment

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

seems to build

That doesn't guarantee they're correct, though.

Someone should re-review the annotations to ensure they're correct, submitting a separate PR to flip it to enabled.

I'm surprised, for example, that there's no validation for the loggerFactory argument here:

public SystemdLifetime(IHostEnvironment environment, IHostApplicationLifetime applicationLifetime, ISystemdNotifier systemdNotifier, ILoggerFactory loggerFactory)

It'll null ref if someone passes null, something we try to avoid in general even for non-nullable parameters (and the other args are validated). Logging is generally also optional. Is that supposed to be a nullable factory and the ctor uses a default nop logger if it's null?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

It looks like this library was nullable enabled in the middle of the !! changes.

!! was added on Feb 8, 2022:
3ae8739

Added nullable=enabled on April 18, 2022:
9849af2

!! was removed on April 21, 2022 (and also changed to nullable=annotations):
215b39a

I'll put up a PR to fix Systemd's nullability.

Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.1</TargetFrameworks>
<EnableDefaultItems>true</EnableDefaultItems>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<IsPackable>true</IsPackable>
<PackageDescription>.NET hosting infrastructure for Systemd Services.</PackageDescription>
<Nullable>annotations</Nullable>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'">
Expand Down
5 changes: 4 additions & 1 deletion src/libraries/System.CodeDom/ref/System.CodeDom.csproj
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<ItemGroup>
<Compile Include="System.CodeDom.cs" Condition="'$(TargetFrameworkIdentifier)' != '.NETFramework'" />
<Compile Include="System.CodeDom.netframework.cs" Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'" />
</ItemGroup>
</Project>
</Project>
7 changes: 5 additions & 2 deletions src/libraries/System.CodeDom/src/System.CodeDom.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<DefineConstants>$(DefineConstants);CODEDOM</DefineConstants>
<Nullable>annotations</Nullable>
<IsTrimmable>false</IsTrimmable>
<IsPackable>true</IsPackable>
<PackageDescription>Provides types that can be used to model the structure of a source code document and to output source code for that model in a supported language.
Expand All @@ -13,6 +12,10 @@ System.CodeDom.CodeObject
System.CodeDom.Compiler.CodeDomProvider
Microsoft.CSharp.CSharpCodeProvider
Microsoft.VisualBasic.VBCodeProvider</PackageDescription>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<!-- DesignTimeBuild requires all the TargetFramework Derived Properties to not be present in the first property group. -->
Expand Down Expand Up @@ -141,4 +144,4 @@ Microsoft.VisualBasic.VBCodeProvider</PackageDescription>
<Compile Include="$(CommonPath)System\CSharpHelpers.cs" />
<Compile Include="StringExtensions.cs" />
</ItemGroup>
</Project>
</Project>
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.1</TargetFrameworks>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand All @@ -12,4 +15,4 @@
<ProjectReference Include="$(LibrariesProjectRoot)System.ComponentModel.Composition\ref\System.ComponentModel.Composition.csproj" />
<ProjectReference Include="$(LibrariesProjectRoot)System.Reflection.Context\ref\System.Reflection.Context.csproj" />
</ItemGroup>
</Project>
</Project>
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.1</TargetFrameworks>
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
<IsTrimmable>false</IsTrimmable>
<IsPackable>true</IsPackable>
<AddNETFrameworkPlaceholderFileToPackage>true</AddNETFrameworkPlaceholderFileToPackage>
Expand All @@ -16,6 +14,10 @@ System.ComponentModel.Composition.Registration.PartBuilder&lt;T&gt;
System.ComponentModel.Composition.Registration.ParameterImportBuilder
System.ComponentModel.Composition.Registration.ImportBuilder
System.ComponentModel.Composition.Registration.ExportBuilder</PackageDescription>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
<StrongNameKeyId>Microsoft</StrongNameKeyId>
<IsPackable>true</IsPackable>
<PackageDescription>Provides common attributes used by System.Composition types.
Expand All @@ -11,6 +9,10 @@ Commonly Used Types:
System.Composition.ExportAttribute
System.Composition.ImportAttribute
System.Composition.Convention.AttributedModelProvider</PackageDescription>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
<IsTrimmable>false</IsTrimmable>
<StrongNameKeyId>Microsoft</StrongNameKeyId>
<IsPackable>true</IsPackable>
Expand All @@ -14,6 +12,10 @@ System.Composition.Convention.ExportConventionBuilder
System.Composition.Convention.ImportConventionBuilder
System.Composition.Convention.PartConventionBuilder
System.Composition.Convention.ParameterImportConventionBuilder</PackageDescription>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
<IsTrimmable>false</IsTrimmable>
<EnableAOTAnalyzer>false</EnableAOTAnalyzer>
<StrongNameKeyId>Microsoft</StrongNameKeyId>
Expand All @@ -11,6 +9,10 @@

Commonly Used Types:
System.Composition.Hosting.CompositionHost</PackageDescription>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<RootNamespace>System.Composition</RootNamespace>
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
<EnableAOTAnalyzer>false</EnableAOTAnalyzer>
<StrongNameKeyId>Microsoft</StrongNameKeyId>
<IsPackable>true</IsPackable>
<PackageDescription>Contains runtime components of the Managed Extensibility Framework.

Commonly Used Types:
System.Composition.CompositionContext</PackageDescription>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<RootNamespace>System.Composition</RootNamespace>
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
<IsTrimmable>false</IsTrimmable>
<EnableAOTAnalyzer>false</EnableAOTAnalyzer>
<StrongNameKeyId>Microsoft</StrongNameKeyId>
Expand All @@ -13,6 +11,10 @@
Commonly Used Types:
System.Composition.CompositionContextExtensions
System.Composition.Hosting.ContainerConfiguration</PackageDescription>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
<Project Sdk="Microsoft.Build.NoTargets">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<Nullable>disable</Nullable>
<IsPackable>true</IsPackable>
<StrongNameKeyId>Microsoft</StrongNameKeyId>
<!-- A meta package doesn't have any artifacts per TargetFramework. -->
Expand All @@ -17,6 +16,10 @@ System.Composition.Convention.ConventionBuilder
System.Composition.Hosting.CompositionHost
System.Composition.CompositionContext
System.Composition.CompositionContextExtensions</PackageDescription>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);CS0618</NoWarn>
<IncludeInternalObsoleteAttribute>true</IncludeInternalObsoleteAttribute>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand All @@ -21,4 +24,4 @@
<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'">
<Reference Include="System.Configuration" />
</ItemGroup>
</Project>
</Project>
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);CA2249;nullable</NoWarn>
<NoWarn>$(NoWarn);CA2249</NoWarn>
<!-- opt-out of trimming until it works https://github.com/dotnet/runtime/issues/49062 -->
<IsTrimmable>false</IsTrimmable>
<EnableAOTAnalyzer>false</EnableAOTAnalyzer>
<IncludeInternalObsoleteAttribute>true</IncludeInternalObsoleteAttribute>
<IsPackable>true</IsPackable>
<PackageDescription>Provides types that support using XML configuration files (app.config). This package exists only to support migrating existing .NET Framework code that already uses System.Configuration. When writing new code, use another configuration system instead, such as Microsoft.Extensions.Configuration.</PackageDescription>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<!-- DesignTimeBuild requires all the TargetFramework Derived Properties to not be present in the first property group. -->
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<!-- Set Nullable to empty so Roslyn doesn't emit CodeAnalysis attributes into the assembly. -->
<Nullable></Nullable>
<Nullable>disable</Nullable>
<Win32Resource>EventLogMessages.res</Win32Resource>
<!-- Override the parent Directory.Build.props -->
<SupportedOSPlatforms />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent);$(NetCoreAppPrevious)-windows;$(NetCoreAppPrevious);$(NetCoreAppMinimum)-windows;$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<Nullable>annotations</Nullable>
<IsPackable>true</IsPackable>
<PackageDescription>Provides the System.Diagnostics.EventLog class, which allows the applications to use the Windows event log service.

Commonly Used Types:
System.Diagnostics.EventLog</PackageDescription>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<!-- DesignTimeBuild requires all the TargetFramework Derived Properties to not be present in the first property group. -->
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<Nullable>disable</Nullable>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>

<!-- TODO https://github.com/dotnet/runtime/issues/90400: Annotate for nullable reference types -->
<Nullable>disable</Nullable>
<NoWarn>$(NoWarn);nullable</NoWarn>
</PropertyGroup>

<ItemGroup>
Expand Down
Loading