Skip to content

Commit

Permalink
Remove all use of <Nullable>annotations</Nullable> (#90401)
Browse files Browse the repository at this point in the history
* Remove all use of <Nullable>annotations</Nullable>

We've inadvertently shipped multiple libraries with incorrect nullable reference type annotations because:
a) Their ref assemblies aren't actually being shipped, and/or
b) Their src projects using `<Nullable>annotations</Nullable>`

`<Nullable>annotations</Nullable>` means "I'm nullable annotated but don't validate them", which means consumers of these libraries see annotations that we haven't thoughtfully added or reviewed.

This removes all use of it and gets us back to a place where we're only shipping nullable annotations for libraries where we've done the work to ensure they're correct.  We can subsequently finish annotating these stragglers.

* Don't disable LibraryImportGenerator when nullable is disabled

* Add #nullable enable to LibraryImportAttribute file for polyfill scenarios.

* Disable LibraryImportGenerator on ILCompiler.TypeSystem.Tests assemblies.
Convert a DllImport in HostWriter that now gets the polyfill experience.

---------

Co-authored-by: Eric StJohn <ericstj@microsoft.com>
Co-authored-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
  • Loading branch information
3 people authored Aug 12, 2023
1 parent 7325cc4 commit 616f758
Show file tree
Hide file tree
Showing 43 changed files with 160 additions and 58 deletions.
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);
}
}
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

0 comments on commit 616f758

Please sign in to comment.