C#: Enable nullability in csharp extraction project v1

Author: tamasvajk

csharp/extractor/Semmle.Extraction.CSharp/Analyser.cs

@@ -10,22 +10,24 @@
 using Semmle.Util.Logging;
 using Semmle.Util;
 
+#nullable disable warnings
+
 namespace Semmle.Extraction.CSharp
 {
     /// <summary>
     /// Encapsulates a C# analysis task.
     /// </summary>
     public sealed class Analyser : IDisposable
     {
-        private Extraction.Extractor extractor;
-        private CSharpCompilation compilation;
-        private Layout layout;
+        private Extraction.Extractor? extractor;
+        private CSharpCompilation? compilation;
+        private Layout? layout;
         private bool init;
         private readonly object progressMutex = new object();
         private int taskCount = 0;
-        private CommonOptions options;
-        private Entities.Compilation compilationEntity;
-        private IDisposable compilationTrapFile;
+        private CommonOptions? options;
+        private Entities.Compilation? compilationEntity;
+        private IDisposable? compilationTrapFile;
 
         private readonly Stopwatch stopWatch = new Stopwatch();
 
@@ -359,13 +361,13 @@ private void DoExtractTree(SyntaxTree tree)
 
                 var projectLayout = layout.LookupProjectOrNull(transformedSourcePath);
                 var excluded = projectLayout == null;
-                var trapPath = excluded ? "" : projectLayout.GetTrapPath(Logger, transformedSourcePath, options.TrapCompression);
+                var trapPath = excluded ? "" : projectLayout!.GetTrapPath(Logger, transformedSourcePath, options.TrapCompression);
                 var upToDate = false;
 
                 if (!excluded)
                 {
                     // compilation.Clone() is used to allow symbols to be garbage collected.
-                    using var trapWriter = projectLayout.CreateTrapWriter(Logger, transformedSourcePath, options.TrapCompression, discardDuplicates: false);
+                    using var trapWriter = projectLayout!.CreateTrapWriter(Logger, transformedSourcePath, options.TrapCompression, discardDuplicates: false);
 
                     upToDate = options.Fast && FileIsUpToDate(sourcePath, trapWriter.TrapFile);
 
@@ -571,3 +573,5 @@ public interface IProgressMonitor
         void MissingSummary(int types, int namespaces);
     }
 }
+
+#nullable restore warnings

csharp/extractor/Semmle.Extraction.CSharp/Comments/CommentProcessor.cs

@@ -26,7 +26,7 @@ public void AddComment(CommentLine comment)
 
         private readonly Dictionary<Label, Key> duplicationGuardKeys = new Dictionary<Label, Key>();
 
-        private Key GetDuplicationGuardKey(Label label)
+        private Key? GetDuplicationGuardKey(Label label)
         {
             if (duplicationGuardKeys.TryGetValue(label, out var duplicationGuardKey))
                 return duplicationGuardKey;
@@ -35,7 +35,7 @@ private Key GetDuplicationGuardKey(Label label)
 
         private class LocationComparer : IComparer<Location>
         {
-            public int Compare(Location l1, Location l2) => CommentProcessor.Compare(l1, l2);
+            public int Compare(Location? l1, Location? l2) => CommentProcessor.Compare(l1, l2);
         }
 
         /// <summary>
@@ -44,7 +44,7 @@ private class LocationComparer : IComparer<Location>
         /// <param name="l1">First location</param>
         /// <param name="l2">Second location</param>
         /// <returns>&lt;0 if l1 before l2, &gt;0 if l1 after l2, else 0.</returns>
-        private static int Compare(Location l1, Location l2)
+        private static int Compare(Location? l1, Location? l2)
         {
             if (object.ReferenceEquals(l1, l2))
                 return 0;
@@ -68,7 +68,7 @@ private static int Compare(Location l1, Location l2)
         /// <param name="elementLabel">The label of the element in the trap file.</param>
         /// <param name="duplicationGuardKey">The duplication guard key of the element, if any.</param>
         /// <param name="loc">The location of the element.</param>
-        public void AddElement(Label elementLabel, Key duplicationGuardKey, Location loc)
+        public void AddElement(Label elementLabel, Key? duplicationGuardKey, Location? loc)
         {
             if (loc != null && loc.IsInSource)
                 elements[loc] = elementLabel;
@@ -265,7 +265,7 @@ private bool GenerateBindings(
             CommentBindingCallback cb
             )
         {
-            Comments.CommentBlock block = null;
+            Comments.CommentBlock? block = null;
 
             // Iterate comments until the commentEnumerator has gone past nextElement
             while (nextElement == null || Compare(commentEnumerator.Current.Value.Location, nextElement.Value.Key) < 0)
@@ -350,5 +350,5 @@ public void GenerateBindings(CommentBindingCallback cb)
     /// <param name="duplicationGuardKey">The duplication guard key of the element, if any</param>
     /// <param name="commentBlock">The comment block associated with the element</param>
     /// <param name="binding">The relationship between the commentblock and the element</param>
-    internal delegate void CommentBindingCallback(Label elementLabel, Key duplicationGuardKey, Comments.CommentBlock commentBlock, CommentBinding binding);
+    internal delegate void CommentBindingCallback(Label elementLabel, Key? duplicationGuardKey, Comments.CommentBlock commentBlock, CommentBinding binding);
 }

csharp/extractor/Semmle.Extraction.CSharp/CompilerVersion.cs

@@ -14,12 +14,12 @@ namespace Semmle.Extraction.CSharp
     public class CompilerVersion
     {
         private const string csc_rsp = "csc.rsp";
-        private readonly string specifiedFramework = null;
+        private readonly string? specifiedFramework = null;
 
         /// <summary>
         /// The value specified by --compiler, or null.
         /// </summary>
-        public string SpecifiedCompiler
+        public string? SpecifiedCompiler
         {
             get;
             private set;
@@ -28,12 +28,20 @@ public string SpecifiedCompiler
         /// <summary>
         /// Why was the candidate exe rejected as a compiler?
         /// </summary>
-        public string SkipReason
+        public string? SkipReason
         {
             get;
             private set;
         }
 
+        private static readonly Dictionary<string, string> knownCompilerNames = new Dictionary<string, string>
+        {
+            { "csc.exe", "Microsoft" },
+            { "csc2.exe", "Microsoft" },
+            { "csc.dll", "Microsoft" },
+            { "mcs.exe", "Novell" }
+        };
+
         /// <summary>
         /// Probes the compiler (if specified).
         /// </summary>
@@ -52,23 +60,21 @@ public CompilerVersion(Options options)
                 }
 
                 // Reads the file details from the .exe
-                var versionInfo = FileVersionInfo.GetVersionInfo(SpecifiedCompiler);
-
                 var compilerDir = Path.GetDirectoryName(SpecifiedCompiler);
-                var knownCompilerNames = new Dictionary<string, string>
+                if (compilerDir is null)
                 {
-                    { "csc.exe", "Microsoft" },
-                    { "csc2.exe", "Microsoft" },
-                    { "csc.dll", "Microsoft" },
-                    { "mcs.exe", "Novell" }
-                };
+                    SkipExtractionBecause("the compiler directory could not be retrieved");
+                    return;
+                }
+
                 var mscorlibExists = File.Exists(Path.Combine(compilerDir, "mscorlib.dll"));
 
                 if (specifiedFramework == null && mscorlibExists)
                 {
                     specifiedFramework = compilerDir;
                 }
 
+                var versionInfo = FileVersionInfo.GetVersionInfo(SpecifiedCompiler);
                 if (!knownCompilerNames.TryGetValue(versionInfo.OriginalFilename, out var vendor))
                 {
                     SkipExtractionBecause("the compiler name is not recognised");
@@ -114,7 +120,7 @@ public bool SkipExtraction
         /// <summary>
         /// Gets additional reference directories - the compiler directory.
         /// </summary>
-        public string AdditionalReferenceDirectories => SpecifiedCompiler != null ? Path.GetDirectoryName(SpecifiedCompiler) : null;
+        public string? AdditionalReferenceDirectories => SpecifiedCompiler != null ? Path.GetDirectoryName(SpecifiedCompiler) : null;
 
         /// <summary>
         /// Adds @csc.rsp to the argument list to mimic csc.exe.
@@ -134,6 +140,6 @@ private static bool SuppressDefaultResponseFile(IEnumerable<string> args)
             return args.Any(arg => new[] { "/noconfig", "-noconfig" }.Contains(arg.ToLowerInvariant()));
         }
 
-        public IEnumerable<string> ArgsWithResponse { get; }
+        public IEnumerable<string> ArgsWithResponse { get; } = Enumerable.Empty<string>();
     }
 }

csharp/extractor/Semmle.Extraction.CSharp/Context.cs

@@ -29,7 +29,7 @@ public SemanticModel GetModel(SyntaxNode node)
             return cachedModel;
         }
 
-        private SemanticModel cachedModel;
+        private SemanticModel? cachedModel;
 
         /// <summary>
         /// The current compilation unit.
@@ -51,7 +51,7 @@ public Context(Extraction.Extractor e, Compilation c, TrapWriter trapWriter, IEx
 
         public bool IsAssemblyScope => scope is AssemblyScope;
 
-        private SyntaxTree SourceTree => scope is SourceScope sc ? sc.SourceTree : null;
+        private SyntaxTree? SourceTree => scope is SourceScope sc ? sc.SourceTree : null;
 
         /// <summary>
         ///     Whether the given symbol needs to be defined in this context.
@@ -85,7 +85,7 @@ public override Extraction.Entities.Location CreateLocation()
                 : CreateLocation(Microsoft.CodeAnalysis.Location.Create(SourceTree, Microsoft.CodeAnalysis.Text.TextSpan.FromBounds(0, 0)));
         }
 
-        public override Extraction.Entities.Location CreateLocation(Microsoft.CodeAnalysis.Location location)
+        public override Extraction.Entities.Location CreateLocation(Microsoft.CodeAnalysis.Location? location)
         {
             return (location == null || location.Kind == LocationKind.None)
                 ? GeneratedLocation.Create(this)
@@ -100,13 +100,13 @@ public override Extraction.Entities.Location CreateLocation(Microsoft.CodeAnalys
         /// <param name="cx">Extractor context.</param>
         /// <param name="entity">Program entity.</param>
         /// <param name="l">Location of the entity.</param>
-        public void BindComments(Entity entity, Microsoft.CodeAnalysis.Location l)
+        public void BindComments(Entity entity, Microsoft.CodeAnalysis.Location? l)
         {
             var duplicationGuardKey = GetCurrentTagStackKey();
             CommentGenerator.AddElement(entity.Label, duplicationGuardKey, l);
         }
 
-        protected override bool IsEntityDuplicationGuarded(IEntity entity, [NotNullWhen(true)] out Extraction.Entities.Location loc)
+        protected override bool IsEntityDuplicationGuarded(IEntity entity, [NotNullWhen(true)] out Extraction.Entities.Location? loc)
         {
             if (CreateLocation(entity.ReportingLocation) is Entities.NonGeneratedSourceLocation l)
             {

csharp/extractor/Semmle.Extraction.CSharp/Entities/Accessor.cs

@@ -37,7 +37,7 @@ public override void Populate(TextWriter trapFile)
         {
             PopulateMethod(trapFile);
             PopulateModifiers(trapFile);
-            ContainingType.PopulateGenerics();
+            ContainingType!.PopulateGenerics();
 
             var prop = PropertySymbol;
             if (prop == null)
@@ -52,12 +52,12 @@ public override void Populate(TextWriter trapFile)
             if (SymbolEqualityComparer.Default.Equals(Symbol, prop.GetMethod))
             {
                 kind = 1;
-                unboundAccessor = Create(Context, prop.OriginalDefinition.GetMethod);
+                unboundAccessor = Create(Context, prop.OriginalDefinition.GetMethod!);
             }
             else if (SymbolEqualityComparer.Default.Equals(Symbol, prop.SetMethod))
             {
                 kind = 2;
-                unboundAccessor = Create(Context, prop.OriginalDefinition.SetMethod);
+                unboundAccessor = Create(Context, prop.OriginalDefinition.SetMethod!);
             }
             else
             {

csharp/extractor/Semmle.Extraction.CSharp/Entities/Assembly.cs

@@ -12,7 +12,7 @@ internal class Assembly : Extraction.Entities.Location
         private readonly string assemblyPath;
         private readonly IAssemblySymbol assembly;
 
-        private Assembly(Context cx, Microsoft.CodeAnalysis.Location init)
+        private Assembly(Context cx, Microsoft.CodeAnalysis.Location? init)
             : base(cx, init)
         {
             if (init == null)
@@ -44,7 +44,7 @@ public override void Populate(TextWriter trapFile)
         public override int GetHashCode() =>
             Symbol == null ? 91187354 : Symbol.GetHashCode();
 
-        public override bool Equals(object obj)
+        public override bool Equals(object? obj)
         {
             if (obj is Assembly other && other.GetType() == typeof(Assembly))
                 return Equals(Symbol, other.Symbol);
@@ -54,11 +54,11 @@ public override bool Equals(object obj)
 
         public static Extraction.Entities.Location Create(Context cx, Microsoft.CodeAnalysis.Location loc) => AssemblyConstructorFactory.Instance.CreateEntity(cx, loc, loc);
 
-        private class AssemblyConstructorFactory : CachedEntityFactory<Microsoft.CodeAnalysis.Location, Assembly>
+        private class AssemblyConstructorFactory : CachedEntityFactory<Microsoft.CodeAnalysis.Location?, Assembly>
         {
             public static AssemblyConstructorFactory Instance { get; } = new AssemblyConstructorFactory();
 
-            public override Assembly Create(Context cx, Microsoft.CodeAnalysis.Location init) => new Assembly(cx, init);
+            public override Assembly Create(Context cx, Microsoft.CodeAnalysis.Location? init) => new Assembly(cx, init);
         }
 
         private static readonly object outputAssemblyCacheKey = new object();

csharp/extractor/Semmle.Extraction.CSharp/Entities/Attribute.cs

@@ -10,7 +10,7 @@ internal class Attribute : CachedEntity<AttributeData>, IExpressionParentEntity
     {
         bool IExpressionParentEntity.IsTopLevelParent => true;
 
-        private readonly AttributeSyntax attributeSyntax;
+        private readonly AttributeSyntax? attributeSyntax;
         private readonly IEntity entity;
 
         private Attribute(Context cx, AttributeData attributeData, IEntity entity)
@@ -93,7 +93,7 @@ private void ExtractArguments(TextWriter trapFile)
             {
                 var expr = CreateExpressionFromArgument(
                     namedArgument.Value,
-                    attributeSyntax?.ArgumentList.Arguments.Single(a => a.NameEquals?.Name?.Identifier.Text == namedArgument.Key).Expression,
+                    attributeSyntax?.ArgumentList?.Arguments.Single(a => a.NameEquals?.Name?.Identifier.Text == namedArgument.Key).Expression,
                     this,
                     childIndex++);
 
@@ -104,7 +104,7 @@ private void ExtractArguments(TextWriter trapFile)
             }
         }
 
-        private Expression CreateExpressionFromArgument(TypedConstant constant, ExpressionSyntax syntax, IExpressionParentEntity parent,
+        private Expression? CreateExpressionFromArgument(TypedConstant constant, ExpressionSyntax? syntax, IExpressionParentEntity parent,
             int childIndex)
         {
             return syntax is null
@@ -114,11 +114,14 @@ private Expression CreateExpressionFromArgument(TypedConstant constant, Expressi
 
         public override TrapStackBehaviour TrapStackBehaviour => TrapStackBehaviour.OptionalLabel;
 
-        public override Microsoft.CodeAnalysis.Location ReportingLocation => attributeSyntax?.Name.GetLocation();
+        public override Microsoft.CodeAnalysis.Location? ReportingLocation => attributeSyntax?.Name.GetLocation();
+
+        private Semmle.Extraction.Entities.Location? location;
 
-        private Semmle.Extraction.Entities.Location location;
         private Semmle.Extraction.Entities.Location Location =>
-            location ?? (location = Context.CreateLocation(attributeSyntax is null ? entity.ReportingLocation : attributeSyntax.Name.GetLocation()));
+            location ??= Context.CreateLocation(attributeSyntax is null
+                ? entity.ReportingLocation
+                : attributeSyntax.Name.GetLocation());
 
         public override bool NeedsPopulation => true;
 

csharp/extractor/Semmle.Extraction.CSharp/Entities/CachedEntity.cs

@@ -1,11 +1,12 @@
 namespace Semmle.Extraction.CSharp.Entities
 {
-    internal abstract class CachedEntity<T> : Extraction.CachedEntity<T>
+    internal abstract class CachedEntity<T> : Extraction.CachedEntity<T> where T : class
     {
         // todo: this can be changed to an override after the .NET 5 upgrade
         protected new Context Context => (Context)base.Context;
 
-        protected CachedEntity(Context context, T symbol) : base(context, symbol)
+        protected CachedEntity(Context context, T symbol)
+            : base(context, symbol)
         {
         }
     }

csharp/extractor/Semmle.Extraction.CSharp/Entities/CachedSymbol.cs

@@ -9,17 +9,16 @@
 
 namespace Semmle.Extraction.CSharp.Entities
 {
-    internal abstract class CachedSymbol<T> : CachedEntity<T> where T : ISymbol
+    internal abstract class CachedSymbol<T> : CachedEntity<T> where T : class, ISymbol
     {
-        // todo: this can be changed to an override after the .NET 5 upgrade
-        protected new Context Context => (Context)base.Context;
-
-        protected CachedSymbol(Context cx, T init)
+#nullable disable warnings
+        protected CachedSymbol(Context cx, T? init)
             : base(cx, init)
         {
         }
+#nullable restore warnings
 
-        public virtual Type ContainingType => Symbol.ContainingType != null ? Type.Create(Context, Symbol.ContainingType) : null;
+        public virtual Type? ContainingType => Symbol.ContainingType != null ? Type.Create(Context, Symbol.ContainingType) : null;
 
         public void PopulateModifiers(TextWriter trapFile)
         {
@@ -68,12 +67,12 @@ protected void ExtractCompilerGenerated(TextWriter trapFile)
         /// The location which is stored in the database and is used when highlighing source code.
         /// It's generally short, e.g. a method name.
         /// </summary>
-        public override Microsoft.CodeAnalysis.Location ReportingLocation => Symbol.Locations.FirstOrDefault();
+        public override Microsoft.CodeAnalysis.Location? ReportingLocation => Symbol.Locations.FirstOrDefault();
 
         /// <summary>
         /// The full text span of the entity, e.g. for binding comments.
         /// </summary>
-        public virtual Microsoft.CodeAnalysis.Location FullLocation => Symbol.Locations.FirstOrDefault();
+        public virtual Microsoft.CodeAnalysis.Location? FullLocation => Symbol.Locations.FirstOrDefault();
 
         public virtual IEnumerable<Extraction.Entities.Location> Locations
         {
@@ -139,7 +138,7 @@ protected void PopulateMetadataHandle(TextWriter trapFile)
                 trapFile.metadata_handle(this, Location, MetadataTokens.GetToken(handle.Value));
         }
 
-        private static System.Reflection.PropertyInfo GetPropertyInfo(object o, string name)
+        private static System.Reflection.PropertyInfo? GetPropertyInfo(object o, string name)
         {
             return o.GetType().GetProperty(name, System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.GetProperty);
         }