C#: Address review comments.

Add more tests for duplicated entities, and fix some duplicated entities. Update the TupleTypes output - some extraneous results gone so it's probably better.
2026-04-30 03:05:15 +02:00 · 2018-12-20 20:23:12 +00:00
parent 6a54a6d3e5
commit d73b28efe4
6 changed files with 31 additions and 31 deletions
--- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Namespace.cs
+++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Namespace.cs
@@ -31,7 +31,7 @@ namespace Semmle.Extraction.CSharp.Entities
            }
        }

-        public static Namespace Create(Context cx, INamespaceSymbol ns) => NamespaceFactory.Instance.CreateEntity(cx, ns);
+        public static Namespace Create(Context cx, INamespaceSymbol ns) => NamespaceFactory.Instance.CreateEntity2(cx, ns);

        class NamespaceFactory : ICachedEntityFactory<INamespaceSymbol, Namespace>
        {
--- a/csharp/extractor/Semmle.Extraction/Context.cs
+++ b/csharp/extractor/Semmle.Extraction/Context.cs
@@ -61,31 +61,47 @@ namespace Semmle.Extraction
        /// <param name="factory">The entity factory.</param>
        /// <param name="init">The initializer for the entity.</param>
        /// <returns>The new/existing entity.</returns>
-        public Entity CreateEntity2<Type, Entity>(ICachedEntityFactory<Type, Entity> factory, Type type) where Entity : ICachedEntity
+        public Entity CreateEntity2<Type, Entity>(ICachedEntityFactory<Type, Entity> factory, Type init) where Entity : ICachedEntity
        {
            using (StackGuard)
            {
-                var entity = factory.Create(this, type);
+                var entity = factory.Create(this, init);

-                if(entityLabelCache.TryGetValue(entity, out var label))
+                if (entityLabelCache.TryGetValue(entity, out var label))
                {
                    entity.Label = label;
-                    return entity;
                }
                else
                {
                    var id = entity.Id;
+#if DEBUG_LABELS
+                    CheckEntityHasUniqueLabel(id, entity);
+#endif
                    label = new Label(NewId());
                    entity.Label = label;
                    entityLabelCache[entity] = label;
                    DefineLabel(label, id);
                    if (entity.NeedsPopulation)
-                        Populate(null, entity);
-                    return entity;
+                        Populate(init as ISymbol, entity);
                }
+                return entity;
            }
        }

+#if DEBUG_LABELS
+        private void CheckEntityHasUniqueLabel(IId id, ICachedEntity entity)
+        {
+            if (idLabelCache.TryGetValue(id, out var originalEntity))
+            {
+                Extractor.Message(new Message { message = "Label collision for " + id.ToString(), severity = Severity.Warning });
+            }
+            else
+            {
+                idLabelCache[id] = entity;
+            }
+        }
+#endif
+
        private Entity CreateNonNullEntity<Type, Entity>(ICachedEntityFactory<Type, Entity> factory, Type init) where Entity : ICachedEntity
        {
            if (objectEntityCache.TryGetValue(init, out var cached))
@@ -103,14 +119,7 @@ namespace Semmle.Extraction
                DefineLabel(label, id);

 #if DEBUG_LABELS
-                if (idLabelCache.TryGetValue(id, out var originalEntity))
-                {
-                    Extractor.Message(new Message { message = "Label collision for " + id.ToString(), severity = Severity.Warning });
-                }
-                else
-                {
-                    idLabelCache[id] = entity;
-                }
+                CheckEntityHasUniqueLabel(id, entity);
 #endif

                if (entity.NeedsPopulation)
@@ -154,7 +163,9 @@ namespace Semmle.Extraction
            entity.Label = label;
        }

+#if DEBUG_LABELS
        readonly Dictionary<IId, ICachedEntity> idLabelCache = new Dictionary<IId, ICachedEntity>();
+#endif
        readonly Dictionary<object, ICachedEntity> objectEntityCache = new Dictionary<object, ICachedEntity>();
        readonly Dictionary<ICachedEntity, Label> entityLabelCache = new Dictionary<ICachedEntity, Label>();
        readonly HashSet<Label> extractedGenerics = new HashSet<Label>();
--- a/csharp/extractor/Semmle.Extraction/Entities/Folder.cs
+++ b/csharp/extractor/Semmle.Extraction/Entities/Folder.cs
@@ -39,7 +39,7 @@ namespace Semmle.Extraction.Entities
        public override IId Id => new Key(DatabasePath, ";folder");

        public static Folder Create(Context cx, DirectoryInfo folder) =>
-            FolderFactory.Instance.CreateEntity(cx, folder);
+            FolderFactory.Instance.CreateEntity2(cx, folder);

        public override Microsoft.CodeAnalysis.Location ReportingLocation => null;

--- a/csharp/extractor/Semmle.Extraction/Semmle.Extraction.csproj
+++ b/csharp/extractor/Semmle.Extraction/Semmle.Extraction.csproj
@@ -7,6 +7,10 @@
    <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
  </PropertyGroup>

+  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
+    <DefineConstants>TRACE;DEBUG;DEBUG_LABELS</DefineConstants>
+  </PropertyGroup>
+
  <ItemGroup>
    <PackageReference Include="Microsoft.CodeAnalysis" Version="2.8.0" />
    <PackageReference Include="GitInfo" Version="2.0.18" />
--- a/csharp/ql/test/library-tests/assignables/Assignables.expected
+++ b/csharp/ql/test/library-tests/assignables/Assignables.expected
@@ -58,8 +58,6 @@
 | Assignables.cs:87:10:87:17 | Item1 |
 | Assignables.cs:87:10:87:17 | Property |
 | Assignables.cs:87:20:87:26 | Item2 |
-| Assignables.cs:87:32:87:32 | Item1 |
-| Assignables.cs:87:35:87:35 | Item2 |
 | Assignables.cs:88:10:88:10 | Item1 |
 | Assignables.cs:88:10:88:10 | x |
 | Assignables.cs:88:13:88:18 | Item2 |
@@ -69,13 +67,9 @@
 | Assignables.cs:88:17:88:17 | x |
 | Assignables.cs:88:24:88:24 | Item1 |
 | Assignables.cs:88:27:88:36 | Item2 |
-| Assignables.cs:88:28:88:32 | Item1 |
-| Assignables.cs:88:35:88:35 | Item2 |
 | Assignables.cs:89:10:89:10 | Item1 |
 | Assignables.cs:89:10:89:10 | b |
 | Assignables.cs:89:13:89:18 | Item2 |
-| Assignables.cs:89:24:89:27 | Item1 |
-| Assignables.cs:89:30:89:35 | Item2 |
 | Assignables.cs:92:6:92:8 | Item1 |
 | Assignables.cs:92:11:92:24 | Item2 |
 | Assignables.cs:92:12:92:15 | Item1 |
--- a/csharp/ql/test/library-tests/csharp7/TupleTypes.expected
+++ b/csharp/ql/test/library-tests/csharp7/TupleTypes.expected
@@ -25,33 +25,24 @@
 | (Int32,Int32) | (int, int) | ValueTuple<int, int> | 2 | 1 | CSharp7.cs:112:19:112:20 | m5 |
 | (Int32,Int32) | (int, int) | ValueTuple<int, int> | 2 | 1 | CSharp7.cs:114:27:114:32 | m9 |
 | (Int32,Int32,Int32) | (int, int, int) | ValueTuple<int, int, int> | 3 | 0 | CSharp7.cs:75:10:75:10 | x |
-| (Int32,Int32,Int32) | (int, int, int) | ValueTuple<int, int, int> | 3 | 0 | CSharp7.cs:75:28:75:28 | Item1 |
 | (Int32,Int32,Int32) | (int, int, int) | ValueTuple<int, int, int> | 3 | 1 | CSharp7.cs:75:13:75:13 | y |
-| (Int32,Int32,Int32) | (int, int, int) | ValueTuple<int, int, int> | 3 | 1 | CSharp7.cs:75:31:75:31 | Item2 |
 | (Int32,Int32,Int32) | (int, int, int) | ValueTuple<int, int, int> | 3 | 2 | CSharp7.cs:75:16:75:22 | Item3 |
-| (Int32,Int32,Int32) | (int, int, int) | ValueTuple<int, int, int> | 3 | 2 | CSharp7.cs:75:34:75:34 | Item3 |
-| (Int32,String) | (int, string) | ValueTuple<int, string> | 2 | 0 | CSharp7.cs:97:19:97:19 | Item1 |
 | (Int32,String) | (int, string) | ValueTuple<int, string> | 2 | 0 | CSharp7.cs:284:41:284:48 | Key |
 | (Int32,String) | (int, string) | ValueTuple<int, string> | 2 | 0 | CSharp7.cs:286:19:286:23 | a |
-| (Int32,String) | (int, string) | ValueTuple<int, string> | 2 | 1 | CSharp7.cs:97:22:97:37 | Item2 |
 | (Int32,String) | (int, string) | ValueTuple<int, string> | 2 | 1 | CSharp7.cs:284:51:284:60 | Value |
 | (Int32,String) | (int, string) | ValueTuple<int, string> | 2 | 1 | CSharp7.cs:286:26:286:33 | b |
 | (String,(Int32,Int32)) | (string, (int, int)) | ValueTuple<string, (int, int)> | 2 | 0 | CSharp7.cs:109:10:109:15 | m1 |
-| (String,(Int32,Int32)) | (string, (int, int)) | ValueTuple<string, (int, int)> | 2 | 0 | CSharp7.cs:109:29:109:37 | Item1 |
 | (String,(Int32,Int32)) | (string, (int, int)) | ValueTuple<string, (int, int)> | 2 | 0 | CSharp7.cs:112:10:112:11 | m3 |
 | (String,(Int32,Int32)) | (string, (int, int)) | ValueTuple<string, (int, int)> | 2 | 0 | CSharp7.cs:114:10:114:15 | m7 |
 | (String,(Int32,Int32)) | (string, (int, int)) | ValueTuple<string, (int, int)> | 2 | 1 | CSharp7.cs:109:18:109:23 | m2 |
-| (String,(Int32,Int32)) | (string, (int, int)) | ValueTuple<string, (int, int)> | 2 | 1 | CSharp7.cs:109:40:109:45 | Item2 |
 | (String,(Int32,Int32)) | (string, (int, int)) | ValueTuple<string, (int, int)> | 2 | 1 | CSharp7.cs:112:14:112:21 | Item2 |
 | (String,(Int32,Int32)) | (string, (int, int)) | ValueTuple<string, (int, int)> | 2 | 1 | CSharp7.cs:114:18:114:33 | Item2 |
 | (String,Int32) | (string, int) | ValueTuple<string, int> | 2 | 0 | CSharp7.cs:79:14:79:14 | i |
 | (String,Int32) | (string, int) | ValueTuple<string, int> | 2 | 0 | CSharp7.cs:79:23:79:24 | Item1 |
 | (String,Int32) | (string, int) | ValueTuple<string, int> | 2 | 0 | CSharp7.cs:84:17:84:17 | a |
-| (String,Int32) | (string, int) | ValueTuple<string, int> | 2 | 0 | CSharp7.cs:98:23:98:38 | Item1 |
 | (String,Int32) | (string, int) | ValueTuple<string, int> | 2 | 1 | CSharp7.cs:79:17:79:17 | j |
 | (String,Int32) | (string, int) | ValueTuple<string, int> | 2 | 1 | CSharp7.cs:79:27:79:27 | x |
 | (String,Int32) | (string, int) | ValueTuple<string, int> | 2 | 1 | CSharp7.cs:84:23:84:23 | Item2 |
-| (String,Int32) | (string, int) | ValueTuple<string, int> | 2 | 1 | CSharp7.cs:98:41:98:41 | Item2 |
 | (String,String) | (string, string) | ValueTuple<string, string> | 2 | 0 | CSharp7.cs:89:19:89:27 | Item1 |
 | (String,String) | (string, string) | ValueTuple<string, string> | 2 | 0 | CSharp7.cs:90:10:90:15 | t2 |
 | (String,String) | (string, string) | ValueTuple<string, string> | 2 | 1 | CSharp7.cs:89:30:89:33 | Item2 |