Annotate System.IO.FileSystem.Watcher for nullable reference types (#456

)

src/libraries/Common/src/System/IO/PathInternal.Windows.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 #nullable enable
+using System.Diagnostics.CodeAnalysis;
 using System.Runtime.CompilerServices;
 
 namespace System.IO
@@ -74,7 +75,7 @@ private static bool EndsWithPeriodOrSpace(string path)
         /// away from paths during normalization, but if we see such a path at this point it should be
         /// normalized and has retained the final characters. (Typically from one of the *Info classes)
         /// </summary>
-        /// TODO: add atribute [return: NotNullIfNotNull("path")]
+        [return: NotNullIfNotNull("path")]
         internal static string? EnsureExtendedPrefixIfNeeded(string? path)
         {
             if (path != null && (path.Length >= MaxShortPath || EndsWithPeriodOrSpace(path)))

src/libraries/Common/src/System/Text/ValueUtf8Converter.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+#nullable enable
 using System.Buffers;
 
 namespace System.Text
@@ -13,7 +14,7 @@ namespace System.Text
     /// </summary>
     internal ref struct ValueUtf8Converter
     {
-        private byte[] _arrayToReturnToPool;
+        private byte[]? _arrayToReturnToPool;
         private Span<byte> _bytes;
 
         public ValueUtf8Converter(Span<byte> initialBuffer)
@@ -40,7 +41,7 @@ public Span<byte> ConvertAndTerminateString(ReadOnlySpan<char> value)
 
         public void Dispose()
         {
-            byte[] toReturn = _arrayToReturnToPool;
+            byte[]? toReturn = _arrayToReturnToPool;
             if (toReturn != null)
             {
                 _arrayToReturnToPool = null;

src/libraries/System.IO.FileSystem.Watcher/ref/System.IO.FileSystem.Watcher.cs

@@ -15,10 +15,10 @@ public ErrorEventArgs(System.Exception exception) { }
     public delegate void ErrorEventHandler(object sender, System.IO.ErrorEventArgs e);
     public partial class FileSystemEventArgs : System.EventArgs
     {
-        public FileSystemEventArgs(System.IO.WatcherChangeTypes changeType, string directory, string name) { }
+        public FileSystemEventArgs(System.IO.WatcherChangeTypes changeType, string directory, string? name) { }
         public System.IO.WatcherChangeTypes ChangeType { get { throw null; } }
         public string FullPath { get { throw null; } }
-        public string Name { get { throw null; } }
+        public string? Name { get { throw null; } }
     }
     public delegate void FileSystemEventHandler(object sender, System.IO.FileSystemEventArgs e);
     public partial class FileSystemWatcher : System.ComponentModel.Component, System.ComponentModel.ISupportInitialize
@@ -33,13 +33,13 @@ public FileSystemWatcher(string path, string filter) { }
         public int InternalBufferSize { get { throw null; } set { } }
         public System.IO.NotifyFilters NotifyFilter { get { throw null; } set { } }
         public string Path { get { throw null; } set { } }
-        public override System.ComponentModel.ISite Site { get { throw null; } set { } }
-        public System.ComponentModel.ISynchronizeInvoke SynchronizingObject { get { throw null; } set { } }
-        public event System.IO.FileSystemEventHandler Changed { add { } remove { } }
-        public event System.IO.FileSystemEventHandler Created { add { } remove { } }
-        public event System.IO.FileSystemEventHandler Deleted { add { } remove { } }
-        public event System.IO.ErrorEventHandler Error { add { } remove { } }
-        public event System.IO.RenamedEventHandler Renamed { add { } remove { } }
+        public override System.ComponentModel.ISite? Site { get { throw null; } set { } }
+        public System.ComponentModel.ISynchronizeInvoke? SynchronizingObject { get { throw null; } set { } }
+        public event System.IO.FileSystemEventHandler? Changed { add { } remove { } }
+        public event System.IO.FileSystemEventHandler? Created { add { } remove { } }
+        public event System.IO.FileSystemEventHandler? Deleted { add { } remove { } }
+        public event System.IO.ErrorEventHandler? Error { add { } remove { } }
+        public event System.IO.RenamedEventHandler? Renamed { add { } remove { } }
         public void BeginInit() { }
         protected override void Dispose(bool disposing) { }
         public void EndInit() { }
@@ -55,8 +55,8 @@ public partial class InternalBufferOverflowException : System.SystemException
     {
         public InternalBufferOverflowException() { }
         protected InternalBufferOverflowException(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { }
-        public InternalBufferOverflowException(string message) { }
-        public InternalBufferOverflowException(string message, System.Exception inner) { }
+        public InternalBufferOverflowException(string? message) { }
+        public InternalBufferOverflowException(string? message, System.Exception? inner) { }
     }
     [System.FlagsAttribute]
     public enum NotifyFilters
@@ -72,18 +72,18 @@ public enum NotifyFilters
     }
     public partial class RenamedEventArgs : System.IO.FileSystemEventArgs
     {
-        public RenamedEventArgs(System.IO.WatcherChangeTypes changeType, string directory, string name, string oldName) : base (default(System.IO.WatcherChangeTypes), default(string), default(string)) { }
+        public RenamedEventArgs(System.IO.WatcherChangeTypes changeType, string directory, string? name, string? oldName) : base (default(System.IO.WatcherChangeTypes), default(string), default(string)) { }
         public string OldFullPath { get { throw null; } }
-        public string OldName { get { throw null; } }
+        public string? OldName { get { throw null; } }
     }
     public delegate void RenamedEventHandler(object sender, System.IO.RenamedEventArgs e);
     public partial struct WaitForChangedResult
     {
         private object _dummy;
         private int _dummyPrimitive;
         public System.IO.WatcherChangeTypes ChangeType { readonly get { throw null; } set { } }
-        public string Name { readonly get { throw null; } set { } }
-        public string OldName { readonly get { throw null; } set { } }
+        public string? Name { readonly get { throw null; } set { } }
+        public string? OldName { readonly get { throw null; } set { } }
         public bool TimedOut { readonly get { throw null; } set { } }
     }
     [System.FlagsAttribute]

src/libraries/System.IO.FileSystem.Watcher/ref/System.IO.FileSystem.Watcher.csproj

@@ -1,5 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
+    <Nullable>enable</Nullable>
     <Configurations>netcoreapp-Debug;netcoreapp-Release</Configurations>
   </PropertyGroup>
   <ItemGroup>

src/libraries/System.IO.FileSystem.Watcher/src/System.IO.FileSystem.Watcher.csproj

@@ -1,6 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <Nullable>enable</Nullable>
     <Configurations>netcoreapp-FreeBSD-Debug;netcoreapp-FreeBSD-Release;netcoreapp-Linux-Debug;netcoreapp-Linux-Release;netcoreapp-OSX-Debug;netcoreapp-OSX-Release;netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release</Configurations>
   </PropertyGroup>
   <ItemGroup>

src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemEventArgs.cs

@@ -10,13 +10,13 @@ namespace System.IO
     public class FileSystemEventArgs : EventArgs
     {
         private readonly WatcherChangeTypes _changeType;
-        private readonly string _name;
+        private readonly string? _name;
         private readonly string _fullPath;
 
         /// <devdoc>
         /// Initializes a new instance of the <see cref='System.IO.FileSystemEventArgs'/> class.
         /// </devdoc>
-        public FileSystemEventArgs(WatcherChangeTypes changeType, string directory, string name)
+        public FileSystemEventArgs(WatcherChangeTypes changeType, string directory, string? name)
         {
             _changeType = changeType;
             _name = name;
@@ -31,7 +31,7 @@ public FileSystemEventArgs(WatcherChangeTypes changeType, string directory, stri
         /// This is like Path.Combine, except without argument validation,
         /// and a separator is used even if the name argument is empty.
         /// </remarks>
-        internal static string Combine(string directoryPath, string name)
+        internal static string Combine(string directoryPath, string? name)
         {
             bool hasSeparator = false;
             if (directoryPath.Length > 0)
@@ -71,7 +71,7 @@ public string FullPath
         /// <devdoc>
         ///       Gets the name of the affected file or directory.
         /// </devdoc>
-        public string Name
+        public string? Name
         {
             get
             {

src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Linux.cs

@@ -40,7 +40,7 @@ private void StartRaisingEvents()
                 switch (error.Error)
                 {
                     case Interop.Error.EMFILE:
-                        string maxValue = ReadMaxUserLimit(MaxUserInstancesPath);
+                        string? maxValue = ReadMaxUserLimit(MaxUserInstancesPath);
                         string message = !string.IsNullOrEmpty(maxValue) ?
                             SR.Format(SR.IOException_INotifyInstanceUserLimitExceeded_Value, maxValue) :
                             SR.IOException_INotifyInstanceUserLimitExceeded;
@@ -125,12 +125,12 @@ private void FinalizeDispose()
         /// Cancellation for the currently running watch operation.
         /// This is non-null if an operation has been started and null if stopped.
         /// </summary>
-        private CancellationTokenSource _cancellation;
+        private CancellationTokenSource? _cancellation;
 
         /// <summary>Reads the value of a max user limit path from procfs.</summary>
         /// <param name="path">The path to read.</param>
         /// <returns>The value read, or "0" if a failure occurred.</returns>
-        private static string ReadMaxUserLimit(string path)
+        private static string? ReadMaxUserLimit(string path)
         {
             try { return File.ReadAllText(path).Trim(); }
             catch { return null; }
@@ -301,7 +301,7 @@ internal RunningInstance(
             internal void Start()
             {
                 // Schedule a task to read from the inotify queue and process the events.
-                Task.Factory.StartNew(obj => ((RunningInstance)obj).ProcessEvents(),
+                Task.Factory.StartNew(obj => ((RunningInstance)obj!).ProcessEvents(),
                     this, CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default);
 
                 // PERF: As needed, we can look into making this use async I/O rather than burning
@@ -337,7 +337,7 @@ private void AddDirectoryWatch(WatchedDirectory parent, string directoryName)
             /// <summary>Adds a watch on a directory to the existing inotify handle.</summary>
             /// <param name="parent">The parent directory entry.</param>
             /// <param name="directoryName">The new directory path to monitor, relative to the root.</param>
-            private void AddDirectoryWatchUnlocked(WatchedDirectory parent, string directoryName)
+            private void AddDirectoryWatchUnlocked(WatchedDirectory? parent, string directoryName)
             {
                 string fullPath = parent != null ? parent.GetPath(false, directoryName) : directoryName;
 
@@ -363,7 +363,7 @@ private void AddDirectoryWatchUnlocked(WatchedDirectory parent, string directory
                     Exception exc;
                     if (error.Error == Interop.Error.ENOSPC)
                     {
-                        string maxValue = ReadMaxUserLimit(MaxUserWatchesPath);
+                        string? maxValue = ReadMaxUserLimit(MaxUserWatchesPath);
                         string message = !string.IsNullOrEmpty(maxValue) ?
                             SR.Format(SR.IOException_INotifyWatchesUserLimitExceeded_Value, maxValue) :
                             SR.IOException_INotifyWatchesUserLimitExceeded;
@@ -374,8 +374,7 @@ private void AddDirectoryWatchUnlocked(WatchedDirectory parent, string directory
                         exc = Interop.GetExceptionForIoErrno(error, fullPath);
                     }
 
-                    FileSystemWatcher watcher;
-                    if (_weakWatcher.TryGetTarget(out watcher))
+                    if (_weakWatcher.TryGetTarget(out FileSystemWatcher? watcher))
                     {
                         watcher.OnError(new ErrorEventArgs(exc));
                     }
@@ -384,7 +383,7 @@ private void AddDirectoryWatchUnlocked(WatchedDirectory parent, string directory
                 }
 
                 // Then store the path information into our map.
-                WatchedDirectory directoryEntry;
+                WatchedDirectory? directoryEntry;
                 bool isNewDirectory = false;
                 if (_wdToPathMap.TryGetValue(wd, out directoryEntry))
                 {
@@ -396,10 +395,7 @@ private void AddDirectoryWatchUnlocked(WatchedDirectory parent, string directory
                     // of the world, but there's little that can be done about that.)
                     if (directoryEntry.Parent != parent)
                     {
-                        if (directoryEntry.Parent != null)
-                        {
-                            directoryEntry.Parent.Children.Remove (directoryEntry);
-                        }
+                        directoryEntry.Parent?.Children!.Remove (directoryEntry);
                         directoryEntry.Parent = parent;
                         if (parent != null)
                         {
@@ -452,10 +448,7 @@ private void RemoveWatchedDirectory(WatchedDirectory directoryEntry, bool remove
                 Debug.Assert (_includeSubdirectories);
                 lock (SyncObj)
                 {
-                    if (directoryEntry.Parent != null)
-                    {
-                        directoryEntry.Parent.Children.Remove (directoryEntry);
-                    }
+                    directoryEntry.Parent?.Children!.Remove(directoryEntry);
                     RemoveWatchedDirectoryUnlocked (directoryEntry, removeInotify);
                 }
             }
@@ -519,12 +512,12 @@ private void ProcessEvents()
                 // When cancellation is requested, clear out all watches.  This should force any active or future reads
                 // on the inotify handle to return 0 bytes read immediately, allowing us to wake up from the blocking call
                 // and exit the processing loop and clean up.
-                var ctr = _cancellationToken.UnsafeRegister(obj => ((RunningInstance)obj).CancellationCallback(), this);
+                var ctr = _cancellationToken.UnsafeRegister(obj => ((RunningInstance)obj!).CancellationCallback(), this);
                 try
                 {
                     // Previous event information
                     ReadOnlySpan<char> previousEventName = ReadOnlySpan<char>.Empty;
-                    WatchedDirectory previousEventParent = null;
+                    WatchedDirectory? previousEventParent = null;
                     uint previousEventCookie = 0;
 
                     // Process events as long as we're not canceled and there are more to read...
@@ -535,15 +528,15 @@ private void ProcessEvents()
                         // so as to avoid a rooted cycle that would prevent our processing loop from ever ending
                         // if the watcher is dropped by the user without being disposed. If we can't get the watcher,
                         // there's nothing more to do (we can't raise events), so bail.
-                        FileSystemWatcher watcher;
+                        FileSystemWatcher? watcher;
                         if (!_weakWatcher.TryGetTarget(out watcher))
                         {
                             break;
                         }
 
                         uint mask = nextEvent.mask;
                         ReadOnlySpan<char> expandedName = ReadOnlySpan<char>.Empty;
-                        WatchedDirectory associatedDirectoryEntry = null;
+                        WatchedDirectory? associatedDirectoryEntry = null;
 
                         // An overflow event means that we can't trust our state without restarting since we missed events and
                         // some of those events could be a directory create, meaning we wouldn't have added the directory to the
@@ -729,8 +722,7 @@ private void ProcessEvents()
                 }
                 catch (Exception exc)
                 {
-                    FileSystemWatcher watcher;
-                    if (_weakWatcher.TryGetTarget(out watcher))
+                    if (_weakWatcher.TryGetTarget(out FileSystemWatcher? watcher))
                     {
                         watcher.OnError(new ErrorEventArgs(exc));
                     }
@@ -852,32 +844,22 @@ private sealed class WatchedDirectory
             {
                 /// <summary>A StringBuilder cached on the current thread to avoid allocations when possible.</summary>
                 [ThreadStatic]
-                private static StringBuilder t_builder;
+                private static StringBuilder? t_builder;
 
                 /// <summary>The parent directory.</summary>
-                internal WatchedDirectory Parent;
+                internal WatchedDirectory? Parent;
 
                 /// <summary>The watch descriptor associated with this directory.</summary>
                 internal int WatchDescriptor;
 
                 /// <summary>The filename of this directory.</summary>
-                internal string Name;
+                internal string? Name;
 
                 /// <summary>Child directories of this directory for which we added explicit watches.</summary>
-                internal List<WatchedDirectory> Children;
+                internal List<WatchedDirectory>? Children;
 
                 /// <summary>Child directories of this directory for which we added explicit watches.  This is the same as Children, but ensured to be initialized as non-null.</summary>
-                internal List<WatchedDirectory> InitializedChildren
-                {
-                    get
-                    {
-                        if (Children == null)
-                        {
-                            Children = new List<WatchedDirectory> ();
-                        }
-                        return Children;
-                    }
-                }
+                internal List<WatchedDirectory> InitializedChildren => Children ??= new List<WatchedDirectory>();
 
                 // PERF: Work is being done here proportionate to depth of watch directories.
                 // If this becomes a bottleneck, we'll need to come up with another mechanism
@@ -894,14 +876,10 @@ internal List<WatchedDirectory> InitializedChildren
                 /// <param name="relativeToRoot">Whether to get a path relative to the root directory being watched, or a full path.</param>
                 /// <param name="additionalName">An additional name to include in the path, relative to this directory.</param>
                 /// <returns>The computed path.</returns>
-                internal string GetPath(bool relativeToRoot, string additionalName = null)
+                internal string GetPath(bool relativeToRoot, string? additionalName = null)
                 {
                     // Use our cached builder
-                    StringBuilder builder = t_builder;
-                    if (builder == null)
-                    {
-                        t_builder = builder = new StringBuilder();
-                    }
+                    StringBuilder builder = (t_builder ??= new StringBuilder());
                     builder.Clear();
 
                     // Write the directory's path.  Then if an additional filename was supplied, append it