[RFC FS-1060] Nullness checking (applied to codebase)

.fantomasignore

@@ -12,7 +12,7 @@ vsintegration/*
 !vsintegration/tests/FSharp.Editor.Tests
 artifacts/
 
-# For some reason, it tries to format files from remotes (Processing .\.git\refs\remotes\<remote>\FSComp.fsi)
+# For some reason, it tries to format files from remotes (Processing ./.git/refs/remotes/<remote>/FSComp.fsi)
 .git/
 
 # Explicitly unformatted implementation
@@ -101,10 +101,22 @@ src/FSharp.Core/option.fsi
 src/FSharp.Core/option.fs
 src/fsi/console.fs
 src/FSharp.Build/FSharpCommandLineBuilder.fs
+
+src/Compiler/Utilities/Activity.fs
 src/Compiler/Utilities/sformat.fs
 src/Compiler/Utilities/illib.fsi
 src/Compiler/Utilities/illib.fs
 
+
+src/Compiler/Utilities/NullnessShims.fs
+src/Compiler/Utilities/LruCache.fsi
+src/Compiler/Utilities/LruCache.fs
+src/Compiler/Utilities/HashMultiMap.fsi
+src/Compiler/Utilities/HashMultiMap.fs
+src/Compiler/Facilities/AsyncMemoize.fsi
+src/Compiler/Facilities/AsyncMemoize.fs
+src/Compiler/AbstractIL/il.fs
+
 # Fantomas limitations on implementation files  (to investigate)
 
 src/Compiler/AbstractIL/ilwrite.fs

FSharpBuild.Directory.Build.targets

@@ -88,6 +88,14 @@
     </ItemGroup>
   </Target>
 
+      <!-- SDK targets override -->
+  <PropertyGroup Condition="'$(Configuration)' != 'Proto' AND '$(DisableCompilerRedirection)'!='true' AND Exists('$(ProtoOutputPath)')">
+    <FSharpBuildAssemblyFileOverride>$(ProtoOutputPath)\fsc\FSharp.Build.dll</FSharpBuildAssemblyFileOverride>
+  </PropertyGroup>  
+  <UsingTask TaskName="FSharpEmbedResourceText" AssemblyFile="$(FSharpBuildAssemblyFileOverride)" Override="true" Condition="'$(Configuration)' != 'Proto' AND '$(DisableCompilerRedirection)'!='true' AND Exists('$(ProtoOutputPath)')" />
+  <UsingTask TaskName="FSharpEmbedResXSource" AssemblyFile="$(FSharpBuildAssemblyFileOverride)" Override="true" Condition="'$(Configuration)' != 'Proto' AND '$(DisableCompilerRedirection)'!='true' AND Exists('$(ProtoOutputPath)')" />   
+
+
   <Target Name="BeforeResGen"
           Inputs="@(EmbeddedResource->'$(IntermediateOutputPath)%(Filename)%(Extension)')"
           Outputs="@(EmbeddedResource->'$(IntermediateOutputPath)resources\%(Filename)%(Extension)')"

azure-pipelines-PR.yml

@@ -553,6 +553,8 @@ stages:
           - checkout: self
             clean: true
           - script: ./eng/cibuild.sh --configuration $(_BuildConfig) --testcoreclr
+            env:
+              SKIP_NETCURRENT_FSC_BUILD: true
             displayName: Build / Test
           - task: PublishTestResults@2
             displayName: Publish Test Results
@@ -595,6 +597,7 @@ stages:
           - script: ./eng/cibuild.sh --configuration $(_BuildConfig) --testcoreclr
             env:
               COMPlus_DefaultStackSize: 1000000
+              SKIP_NETCURRENT_FSC_BUILD: true
             displayName: Build / Test
           - task: PublishTestResults@2
             displayName: Publish Test Results

docs/release-notes/.FSharp.Compiler.Service/9.0.100.md

@@ -25,6 +25,7 @@
 * Treat `{ new Foo() }` as `SynExpr.ObjExpr` ([PR #17388](https://github.com/dotnet/fsharp/pull/17388))
 * Optimize metadata reading for type members and custom attributes. ([PR #17364](https://github.com/dotnet/fsharp/pull/17364))
 * Enforce `AttributeTargets` on unions. ([PR #17389](https://github.com/dotnet/fsharp/pull/17389))
+* Applied nullable reference types to FSharp.Compiler.Service itself ([PR #15310](https://github.com/dotnet/fsharp/pull/15310))
 * Ensure that isinteractive multi-emit backing fields are not public. ([Issue #17439](https://github.com/dotnet/fsharp/issues/17438)), ([PR #17439](https://github.com/dotnet/fsharp/pull/17439))
 * Enable FSharp 9.0 Language Version ([Issue #17497](https://github.com/dotnet/fsharp/issues/17438)), [PR](https://github.com/dotnet/fsharp/pull/17500)))
 * Better error reporting for unions with duplicated fields. ([PR #17521](https://github.com/dotnet/fsharp/pull/17521))

src/Compiler/AbstractIL/il.fs

@@ -170,7 +170,11 @@ let splitTypeNameRight nm =
 // --------------------------------------------------------------------
 
 /// This is used to store event, property and field maps.
-type LazyOrderedMultiMap<'Key, 'Data when 'Key: equality>(keyf: 'Data -> 'Key, lazyItems: InterruptibleLazy<'Data list>) =
+type LazyOrderedMultiMap<'Key, 'Data when 'Key: equality
+#if !NO_CHECKNULLS
+    and 'Key:not null
+#endif
+    >(keyf: 'Data -> 'Key, lazyItems: InterruptibleLazy<'Data list>) =
 
     let quickMap =
         lazyItems
@@ -515,7 +519,8 @@ type ILAssemblyRef(data) =
 
         let retargetable = aname.Flags = AssemblyNameFlags.Retargetable
 
-        ILAssemblyRef.Create(aname.Name, None, publicKey, retargetable, version, locale)
+        let name = match aname.Name with | null -> aname.FullName | name -> name
+        ILAssemblyRef.Create(name, None, publicKey, retargetable, version, locale)
 
     member aref.QualifiedName =
         let b = StringBuilder(100)
@@ -823,7 +828,7 @@ type ILTypeRef =
     member x.DebugText = x.ToString()
 
     /// For debugging
-    override x.ToString() = x.FullName
+    override x.ToString() : string = x.FullName
 
 and [<StructuralEquality; StructuralComparison; StructuredFormatDisplay("{DebugText}")>] ILTypeSpec =
     {
@@ -875,7 +880,7 @@ and [<StructuralEquality; StructuralComparison; StructuredFormatDisplay("{DebugT
         && (x.GenericArgs = y.GenericArgs)
 
     override x.ToString() =
-        x.TypeRef.ToString() + if isNil x.GenericArgs then "" else "<...>"
+        x.TypeRef.FullName + if isNil x.GenericArgs then "" else "<...>"
 
 and [<RequireQualifiedAccess; StructuralEquality; StructuralComparison; StructuredFormatDisplay("{DebugText}")>] ILType =
     | Void
@@ -1017,8 +1022,9 @@ type ILMethodRef =
     [<DebuggerBrowsable(DebuggerBrowsableState.Never)>]
     member x.DebugText = x.ToString()
 
-    override x.ToString() =
-        x.DeclaringTypeRef.ToString() + "::" + x.Name + "(...)"
+    member x.FullName = x.DeclaringTypeRef.FullName + "::" + x.Name + "(...)"
+
+    override x.ToString() = x.FullName
 
 [<StructuralEquality; StructuralComparison; StructuredFormatDisplay("{DebugText}")>]
 type ILFieldRef =
@@ -1033,7 +1039,7 @@ type ILFieldRef =
     member x.DebugText = x.ToString()
 
     override x.ToString() =
-        x.DeclaringTypeRef.ToString() + "::" + x.Name
+        x.DeclaringTypeRef.FullName + "::" + x.Name
 
 [<StructuralEquality; StructuralComparison; StructuredFormatDisplay("{DebugText}")>]
 type ILMethodSpec =
@@ -1072,7 +1078,7 @@ type ILMethodSpec =
     [<DebuggerBrowsable(DebuggerBrowsableState.Never)>]
     member x.DebugText = x.ToString()
 
-    override x.ToString() = x.MethodRef.ToString() + "(...)"
+    override x.ToString() = x.MethodRef.FullName + "(...)"
 
 [<StructuralEquality; StructuralComparison; StructuredFormatDisplay("{DebugText}")>]
 type ILFieldSpec =
@@ -1213,7 +1219,7 @@ type ILAttribute =
     [<DebuggerBrowsable(DebuggerBrowsableState.Never)>]
     member x.DebugText = x.ToString()
 
-    override x.ToString() = x.Method.ToString() + "(...)"
+    override x.ToString() = x.Method.MethodRef.FullName
 
 [<NoEquality; NoComparison; Struct>]
 type ILAttributes(array: ILAttribute[]) =
@@ -1571,7 +1577,7 @@ type ILFieldInit =
         | ILFieldInit.UInt64 u64 -> box u64
         | ILFieldInit.Single ieee32 -> box ieee32
         | ILFieldInit.Double ieee64 -> box ieee64
-        | ILFieldInit.Null -> (null :> Object)
+        | ILFieldInit.Null -> (null :> objnull)
 
 // --------------------------------------------------------------------
 // Native Types, for marshalling to the native C interface.

src/Compiler/AbstractIL/il.fsi

@@ -237,6 +237,8 @@ type ILTypeRef =
 
     member internal EqualsWithPrimaryScopeRef: ILScopeRef * obj -> bool
 
+    override ToString: unit -> string
+
     interface System.IComparable
 
 /// Type specs and types.
@@ -664,7 +666,7 @@ type ILFieldInit =
     | Double of double
     | Null
 
-    member AsObject: unit -> obj
+    member AsObject: unit -> objnull
 
 [<RequireQualifiedAccess; StructuralEquality; StructuralComparison>]
 type internal ILNativeVariant =

src/Compiler/AbstractIL/ilnativeres.fs

@@ -1000,7 +1000,7 @@ type Directory(name, id) =
     member val ID = id
     member val NumberOfNamedEntries = Unchecked.defaultof<uint16> with get, set
     member val NumberOfIdEntries = Unchecked.defaultof<uint16> with get, set
-    member val Entries = List<obj>()
+    member val Entries = List<objnull>()
 
 type NativeResourceWriter() =
     static member private CompareResources (left: Win32Resource) (right: Win32Resource) =
@@ -1149,7 +1149,12 @@ type NativeResourceWriter() =
                             dataWriter.WriteByte 0uy
 
                         false
-                    | e -> failwithf "Unknown entry %s" (if isNull e then "<NULL>" else e.GetType().FullName)
+                    | e ->
+                        failwithf
+                            "Unknown entry %s"
+                            (match e with
+                             | null -> "<NULL>"
+                             | e -> e.GetType().FullName)
 
                 if id >= 0 then
                     writer.WriteInt32 id

src/Compiler/AbstractIL/ilpars.fsy

@@ -3,6 +3,7 @@
 %{
 
 #nowarn "1182"  // the generated code often has unused variable "parseState"
+#nowarn "3261"  // the generated code would need to properly annotate nulls, e.g. changing System.Object to `obj|null`
 
 open Internal.Utilities.Library
 

src/Compiler/AbstractIL/ilread.fs

@@ -941,11 +941,11 @@ let mkCacheGeneric lowMem _inbase _nm (sz: int) =
         fun f (idx: 'T) ->
             let cache =
                 match cache with
-                | Null ->
+                | null ->
                     let v = ConcurrentDictionary<_, _>(Environment.ProcessorCount, sz)
                     cache <- v
                     v
-                | NonNull v -> v
+                | v -> v
 
             match cache.TryGetValue idx with
             | true, v ->