Merge pull request #3306 from calumgrant/cs/extraction-nullability

C#: Enable nullability in Semmle.Extraction project

Author: hvitved

csharp/autobuilder/Semmle.Autobuild.Tests/BuildScripts.cs

@@ -44,7 +44,7 @@ bool IBuildActions.FileExists(string file)
         public IDictionary<string, string> RunProcessOut = new Dictionary<string, string>();
         public IDictionary<string, string> RunProcessWorkingDirectory = new Dictionary<string, string>();
 
-        int IBuildActions.RunProcess(string cmd, string args, string workingDirectory, IDictionary<string, string> env, out IList<string> stdOut)
+        int IBuildActions.RunProcess(string cmd, string args, string? workingDirectory, IDictionary<string, string>? env, out IList<string> stdOut)
         {
             var pattern = cmd + " " + args;
             RunProcessIn.Add(pattern);
@@ -60,7 +60,7 @@ int IBuildActions.RunProcess(string cmd, string args, string workingDirectory, I
             throw new ArgumentException("Missing RunProcess " + pattern);
         }
 
-        int IBuildActions.RunProcess(string cmd, string args, string workingDirectory, IDictionary<string, string> env)
+        int IBuildActions.RunProcess(string cmd, string args, string? workingDirectory, IDictionary<string, string>? env)
         {
             var pattern = cmd + " " + args;
             RunProcessIn.Add(pattern);
@@ -90,16 +90,16 @@ bool IBuildActions.DirectoryExists(string dir)
             throw new ArgumentException("Missing DirectoryExists " + dir);
         }
 
-        public IDictionary<string, string> GetEnvironmentVariable = new Dictionary<string, string>();
+        public IDictionary<string, string?> GetEnvironmentVariable = new Dictionary<string, string?>();
 
-        string IBuildActions.GetEnvironmentVariable(string name)
+        string? IBuildActions.GetEnvironmentVariable(string name)
         {
             if (GetEnvironmentVariable.TryGetValue(name, out var ret))
                 return ret;
             throw new ArgumentException("Missing GetEnvironmentVariable " + name);
         }
 
-        public string GetCurrentDirectory;
+        public string GetCurrentDirectory = "";
 
         string IBuildActions.GetCurrentDirectory()
         {
@@ -334,10 +334,10 @@ public void TestTry()
         }
 
         Autobuilder CreateAutoBuilder(string lgtmLanguage, bool isWindows,
-            string buildless = null, string solution = null, string buildCommand = null, string ignoreErrors = null,
-            string msBuildArguments = null, string msBuildPlatform = null, string msBuildConfiguration = null, string msBuildTarget = null,
-            string dotnetArguments = null, string dotnetVersion = null, string vsToolsVersion = null,
-            string nugetRestore = null, string allSolutions = null,
+            string? buildless = null, string? solution = null, string? buildCommand = null, string? ignoreErrors = null,
+            string? msBuildArguments = null, string? msBuildPlatform = null, string? msBuildConfiguration = null, string? msBuildTarget = null,
+            string? dotnetArguments = null, string? dotnetVersion = null, string? vsToolsVersion = null,
+            string? nugetRestore = null, string? allSolutions = null,
             string cwd = @"C:\Project")
         {
             Actions.GetEnvironmentVariable["CODEQL_AUTOBUILDER_CSHARP_NO_INDEXING"] = "false";

csharp/autobuilder/Semmle.Autobuild.Tests/Semmle.Autobuild.Tests.csproj

@@ -5,6 +5,7 @@
     <TargetFramework>netcoreapp3.0</TargetFramework>
     <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
     <RuntimeIdentifiers>win-x64;linux-x64;osx-x64</RuntimeIdentifiers>
+    <Nullable>enable</Nullable>
   </PropertyGroup>
 
   <ItemGroup>

csharp/autobuilder/Semmle.Autobuild/BuildScript.cs

@@ -61,7 +61,7 @@ class BuildCommand : BuildScript
             /// <param name="silent">Whether this command should run silently.</param>
             /// <param name="workingDirectory">The working directory (<code>null</code> for current directory).</param>
             /// <param name="environment">Additional environment variables.</param>
-            public BuildCommand(string exe, string argumentsOpt, bool silent, string? workingDirectory = null, IDictionary<string, string>? environment = null)
+            public BuildCommand(string exe, string? argumentsOpt, bool silent, string? workingDirectory = null, IDictionary<string, string>? environment = null)
             {
                 this.exe = exe;
                 this.arguments = argumentsOpt ?? "";
@@ -183,7 +183,7 @@ public override int Run(IBuildActions actions, Action<string, bool> startCallbac
         /// <param name="silent">Whether the executable should run silently.</param>
         /// <param name="workingDirectory">The working directory (<code>null</code> for current directory).</param>
         /// <param name="environment">Additional environment variables.</param>
-        public static BuildScript Create(string exe, string argumentsOpt, bool silent, string? workingDirectory, IDictionary<string, string>? environment) =>
+        public static BuildScript Create(string exe, string? argumentsOpt, bool silent, string? workingDirectory, IDictionary<string, string>? environment) =>
             new BuildCommand(exe, argumentsOpt, silent, workingDirectory, environment);
 
         /// <summary>

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

@@ -202,7 +202,7 @@ public override bool Equals(object obj)
             return obj != null && obj.GetType() == typeof(VarargsType);
         }
 
-        public static VarargsType Create(Context cx) => VarargsTypeFactory.Instance.CreateEntity(cx, null);
+        public static VarargsType Create(Context cx) => VarargsTypeFactory.Instance.CreateNullableEntity(cx, null);
 
         class VarargsTypeFactory : ICachedEntityFactory<string, VarargsType>
         {

csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NullType.cs

@@ -27,7 +27,7 @@ public override bool Equals(object obj)
             return obj != null && obj.GetType() == typeof(NullType);
         }
 
-        public static AnnotatedType Create(Context cx) => new AnnotatedType(NullTypeFactory.Instance.CreateEntity(cx, null), NullableAnnotation.None);
+        public static AnnotatedType Create(Context cx) => new AnnotatedType(NullTypeFactory.Instance.CreateNullableEntity(cx, null), NullableAnnotation.None);
 
         class NullTypeFactory : ICachedEntityFactory<ITypeSymbol, NullType>
         {

csharp/extractor/Semmle.Extraction/CommentProcessing.cs

@@ -26,10 +26,9 @@ public void AddComment(ICommentLine comment)
 
         private readonly Dictionary<Label, Key> duplicationGuardKeys = new Dictionary<Label, Key>();
 
-        private Key GetDuplicationGuardKey(Label label)
+        private Key? GetDuplicationGuardKey(Label label)
         {
-            Key duplicationGuardKey;
-            if (duplicationGuardKeys.TryGetValue(label, out duplicationGuardKey))
+            if (duplicationGuardKeys.TryGetValue(label, out var duplicationGuardKey))
                 return duplicationGuardKey;
             return null;
         }
@@ -60,7 +59,7 @@ 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;
@@ -257,19 +256,24 @@ bool GenerateBindings(
             CommentBindingCallback cb
             )
         {
-            CommentBlock block = new CommentBlock();
+            CommentBlock? block = null;
 
             // Iterate comments until the commentEnumerator has gone past nextElement
             while (nextElement == null || Compare(commentEnumerator.Current.Value.Location, nextElement.Value.Key) < 0)
             {
+                if(block is null)
+                    block = new CommentBlock(commentEnumerator.Current.Value);
+
                 if (!block.CombinesWith(commentEnumerator.Current.Value))
                 {
                     // Start of a new block, so generate the bindings for the old block first.
                     GenerateBindings(block, elementStack, nextElement, cb);
-                    block = new CommentBlock();
+                    block = new CommentBlock(commentEnumerator.Current.Value);
+                }
+                else
+                {
+                    block.AddCommentLine(commentEnumerator.Current.Value);
                 }
-
-                block.AddCommentLine(commentEnumerator.Current.Value);
 
                 // Get the next comment.
                 if (!commentEnumerator.MoveNext())
@@ -280,7 +284,9 @@ CommentBindingCallback cb
                 }
             }
 
-            GenerateBindings(block, elementStack, nextElement, cb);
+            if(!(block is null))
+                GenerateBindings(block, elementStack, nextElement, cb);
+
             return true;
         }
 
@@ -332,12 +338,18 @@ public void GenerateBindings(CommentBindingCallback cb)
 
     class CommentBlock : ICommentBlock
     {
-        private readonly List<ICommentLine> lines = new List<ICommentLine>();
+        private readonly List<ICommentLine> lines;
 
         public IEnumerable<ICommentLine> CommentLines => lines;
 
         public Location Location { get; private set; }
 
+        public CommentBlock(ICommentLine firstLine)
+        {
+            lines = new List<ICommentLine> { firstLine };
+            Location = firstLine.Location;
+        }
+
         /// <summary>
         ///     Determine whether commentlines should be merged.
         /// </summary>

csharp/extractor/Semmle.Extraction/Comments.cs

@@ -74,7 +74,7 @@ public interface ICommentBlock
     /// <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>
-    public delegate void CommentBindingCallback(Label elementLabel, Key duplicationGuardKey, ICommentBlock commentBlock, CommentBinding binding);
+    public delegate void CommentBindingCallback(Label elementLabel, Key? duplicationGuardKey, ICommentBlock commentBlock, CommentBinding binding);
 
     /// <summary>
     /// Computes the binding information between comments and program elements.
@@ -88,7 +88,7 @@ public interface ICommentGenerator
         /// <param name="elementLabel">Label of the element.</param>
         /// <param name="duplicationGuardKey">The duplication guard key of the element, if any.</param>
         /// <param name="location">Location of the element.</param>
-        void AddElement(Label elementLabel, Key duplicationGuardKey, Location location);
+        void AddElement(Label elementLabel, Key? duplicationGuardKey, Location location);
 
         /// <summary>
         /// Registers a line of comment.

csharp/extractor/Semmle.Extraction/Context.cs

@@ -34,7 +34,7 @@ public SemanticModel GetModel(SyntaxNode node)
             return cachedModel;
         }
 
-        private SemanticModel cachedModel;
+        private SemanticModel? cachedModel;
 
         /// <summary>
         /// Access to the trap file.
@@ -49,7 +49,31 @@ public SemanticModel GetModel(SyntaxNode node)
         /// <param name="factory">The entity factory.</param>
         /// <param name="init">The initializer for the entity.</param>
         /// <returns>The new/existing entity.</returns>
-        public Entity CreateEntity<Type, Entity>(ICachedEntityFactory<Type, Entity> factory, Type init) where Entity : ICachedEntity
+        public Entity CreateEntity<Type, Entity>(ICachedEntityFactory<Type, Entity> factory, Type init) where Entity : ICachedEntity where Type:struct
+        {
+            return CreateNonNullEntity(factory, init);
+        }
+
+        /// <summary>
+        /// Creates a new entity using the factory.
+        /// </summary>
+        /// <param name="factory">The entity factory.</param>
+        /// <param name="init">The initializer for the entity.</param>
+        /// <returns>The new/existing entity.</returns>
+        public Entity CreateNullableEntity<Type, Entity>(ICachedEntityFactory<Type, Entity> factory, Type init) where Entity : ICachedEntity
+        {
+            return init == null ? CreateEntity2(factory, init) : CreateNonNullEntity(factory, init);
+        }
+
+        /// <summary>
+        /// Creates a new entity using the factory.
+        /// </summary>
+        /// <param name="factory">The entity factory.</param>
+        /// <param name="init">The initializer for the entity.</param>
+        /// <returns>The new/existing entity.</returns>
+        public Entity CreateEntityFromSymbol<Type, Entity>(ICachedEntityFactory<Type, Entity> factory, Type init)
+            where Entity : ICachedEntity
+            where Type: ISymbol
         {
             return init == null ? CreateEntity2(factory, init) : CreateNonNullEntity(factory, init);
         }
@@ -135,8 +159,11 @@ private void CheckEntityHasUniqueLabel(string id, ICachedEntity entity)
 
         public Label GetNewLabel() => new Label(GetNewId());
 
-        private Entity CreateNonNullEntity<Type, Entity>(ICachedEntityFactory<Type, Entity> factory, Type init) where Entity : ICachedEntity
+        public Entity CreateNonNullEntity<Type, Entity>(ICachedEntityFactory<Type, Entity> factory, Type init)
+            where Entity : ICachedEntity
         {
+            if (init is null) throw new ArgumentException("Unexpected null value", nameof(init));
+
             if (objectEntityCache.TryGetValue(init, out var cached))
                 return (Entity)cached;
 
@@ -360,7 +387,7 @@ public void EmitTrap(TextWriter trapFile)
         /// <param name="optionalSymbol">Symbol for reporting errors.</param>
         /// <param name="entity">The entity to populate.</param>
         /// <exception cref="InternalError">Thrown on invalid trap stack behaviour.</exception>
-        public void Populate(ISymbol optionalSymbol, ICachedEntity entity)
+        public void Populate(ISymbol? optionalSymbol, ICachedEntity entity)
         {
             if (WritingLabel)
             {
@@ -453,9 +480,9 @@ public void BindComments(IEntity entity, Microsoft.CodeAnalysis.Location l)
         /// <param name="message">The error message.</param>
         /// <param name="entityText">A textual representation of the failed entity.</param>
         /// <param name="location">The location of the error.</param>
-        /// <param name="stackTrace">An optional stack trace of the error, or an empty string.</param>
+        /// <param name="stackTrace">An optional stack trace of the error, or null.</param>
         /// <param name="severity">The severity of the error.</param>
-        public void ExtractionError(string message, string entityText, Entities.Location location, string stackTrace = "", Severity severity = Severity.Error)
+        public void ExtractionError(string message, string entityText, Entities.Location location, string? stackTrace = null, Severity severity = Severity.Error)
         {
             var msg = new Message(message, entityText, location, stackTrace, severity);
             ExtractionError(msg);
@@ -467,7 +494,7 @@ public void ExtractionError(string message, string entityText, Entities.Location
         /// <param name="message">The text of the message.</param>
         /// <param name="optionalSymbol">The symbol of the error, or null.</param>
         /// <param name="optionalEntity">The entity of the error, or null.</param>
-        public void ExtractionError(string message, ISymbol optionalSymbol, IEntity optionalEntity)
+        public void ExtractionError(string message, ISymbol? optionalSymbol, IEntity optionalEntity)
         {
             if (!(optionalSymbol is null))
             {
@@ -539,7 +566,7 @@ static public void ModelError(this Context cx, string msg)
         /// <param name="node">Optional syntax node for error reporting.</param>
         /// <param name="symbol">Optional symbol for error reporting.</param>
         /// <param name="a">The action to perform.</param>
-        static public void Try(this Context context, SyntaxNode node, ISymbol symbol, Action a)
+        static public void Try(this Context context, SyntaxNode? node, ISymbol? symbol, Action a)
         {
             try
             {

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

@@ -8,7 +8,7 @@ public class Assembly : Location
         readonly string assemblyPath;
         readonly IAssemblySymbol assembly;
 
-        Assembly(Context cx, Microsoft.CodeAnalysis.Location init)
+        Assembly(Context cx, Microsoft.CodeAnalysis.Location? init)
             : base(cx, init)
         {
             if (init == null)
@@ -19,7 +19,7 @@ public class Assembly : Location
             }
             else
             {
-                assembly = symbol.MetadataModule.ContainingAssembly;
+                assembly = init.MetadataModule.ContainingAssembly;
                 var identity = assembly.Identity;
                 var idString = identity.Name + " " + identity.Version;
                 assemblyPath = cx.Extractor.GetAssemblyFile(idString);
@@ -30,7 +30,7 @@ public override void Populate(TextWriter trapFile)
         {
             if (assemblyPath != null)
             {
-                trapFile.assemblies(this, File.Create(Context, assemblyPath), assembly.ToString(),
+                trapFile.assemblies(this, File.Create(Context, assemblyPath), assembly.ToString() ?? "",
                     assembly.Identity.Name, assembly.Identity.Version.ToString());
             }
         }
@@ -41,7 +41,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)
         {
             var other = obj as Assembly;
             if (other == null || other.GetType() != typeof(Assembly))
@@ -50,20 +50,20 @@ public override bool Equals(object obj)
             return Equals(symbol, other.symbol);
         }
 
-        public new static Location Create(Context cx, Microsoft.CodeAnalysis.Location loc) => AssemblyConstructorFactory.Instance.CreateEntity(cx, loc);
+        public new static Location Create(Context cx, Microsoft.CodeAnalysis.Location? loc) => AssemblyConstructorFactory.Instance.CreateNullableEntity(cx, loc);
 
-        class AssemblyConstructorFactory : ICachedEntityFactory<Microsoft.CodeAnalysis.Location, Assembly>
+        class AssemblyConstructorFactory : ICachedEntityFactory<Microsoft.CodeAnalysis.Location?, Assembly>
         {
             public static readonly AssemblyConstructorFactory Instance = new AssemblyConstructorFactory();
 
-            public Assembly Create(Context cx, Microsoft.CodeAnalysis.Location init) => new Assembly(cx, init);
+            public Assembly Create(Context cx, Microsoft.CodeAnalysis.Location? init) => new Assembly(cx, init);
         }
 
         public static Location CreateOutputAssembly(Context cx)
         {
             if (cx.Extractor.OutputPath == null)
                 throw new InternalError("Attempting to create the output assembly in standalone extraction mode");
-            return AssemblyConstructorFactory.Instance.CreateEntity(cx, null);
+            return AssemblyConstructorFactory.Instance.CreateNullableEntity(cx, null);
         }
 
         public override void WriteId(System.IO.TextWriter trapFile)

csharp/extractor/Semmle.Extraction/Entities/ExtractionError.cs

@@ -14,7 +14,7 @@ public ExtractionMessage(Context cx, Message msg) : base(cx)
 
         protected override void Populate(TextWriter trapFile)
         {
-           trapFile.extractor_messages(this, msg.Severity, "C# extractor", msg.Text, msg.EntityText, msg.Location, msg.StackTrace);
+           trapFile.extractor_messages(this, msg.Severity, "C# extractor", msg.Text, msg.EntityText, msg.Location ?? GeneratedLocation.Create(cx), msg.StackTrace);
         }
 
         public override TrapStackBehaviour TrapStackBehaviour => TrapStackBehaviour.NoLabel;