C#: Address review comments and update test output.

Author: michaelnebel

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

@@ -137,20 +137,20 @@ private void ExtractArguments(TextWriter trapFile)
 
         public override bool NeedsPopulation => true;
 
-        private static void ExtractAttributes<T>(Context cx, T symbol, Func<T, IEnumerable<AttributeData>> getAttributes, IEntity entity, AttributeKind kind) where T : ISymbol
+        private static void ExtractAttributes(Context cx, IEnumerable<AttributeData> attributes, IEntity entity, AttributeKind kind)
         {
-            foreach (var attribute in getAttributes(symbol))
+            foreach (var attribute in attributes)
             {
                 Create(cx, attribute, entity, kind);
             }
         }
 
         public static void ExtractAttributes(Context cx, ISymbol symbol, IEntity entity)
         {
-            ExtractAttributes(cx, symbol, s => s.GetAttributes(), entity, AttributeKind.Default);
+            ExtractAttributes(cx, symbol.GetAttributes(), entity, AttributeKind.Default);
             if (symbol is IMethodSymbol method)
             {
-                ExtractAttributes(cx, method, s => s.GetReturnTypeAttributes(), entity, AttributeKind.Return);
+                ExtractAttributes(cx, method.GetReturnTypeAttributes(), entity, AttributeKind.Return);
             }
         }
 

csharp/extractor/Semmle.Extraction.CSharp/Populators/TypeContainerVisitor.cs

@@ -80,21 +80,18 @@ private void ExtractTypeDeclaration(BaseTypeDeclarationSyntax node)
             Entities.Type.Create(Cx, Cx.GetModel(node).GetDeclaredSymbol(node)).ExtractRecursive(TrapFile, Parent);
         }
 
-        private static Entities.AttributeKind ExtractGlobalTarget(AttributeListSyntax node) =>
-            node.Target?.Identifier.Kind() switch
-            {
-                SyntaxKind.AssemblyKeyword => Entities.AttributeKind.Assembly,
-                SyntaxKind.ModuleKeyword => Entities.AttributeKind.Module,
-                _ => throw new InternalError(node, "Unhandled global target")
-            };
-
         public override void VisitAttributeList(AttributeListSyntax node)
         {
             if (Cx.Extractor.Mode.HasFlag(ExtractorMode.Standalone))
                 return;
 
             var outputAssembly = Assembly.CreateOutputAssembly(Cx);
-            var kind = ExtractGlobalTarget(node);
+            var kind = node.Target?.Identifier.Kind() switch
+            {
+                SyntaxKind.AssemblyKeyword => Entities.AttributeKind.Assembly,
+                SyntaxKind.ModuleKeyword => Entities.AttributeKind.Module,
+                _ => throw new InternalError(node, "Unhandled global target")
+            };
             foreach (var attribute in node.Attributes)
             {
                 if (attributeLookup.Value(attribute) is AttributeData attributeData)

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

@@ -33,7 +33,7 @@ internal static void array_element_type(this TextWriter trapFile, ArrayType arra
             trapFile.WriteTuple("array_element_type", array, dimension, rank, elementType);
 
         internal static void attributes(this TextWriter trapFile, Attribute attribute, AttributeKind kind, Type attributeType, IEntity entity) =>
-            trapFile.WriteTuple("attributes", attribute, (int)kind, attributeType, entity);
+            trapFile.WriteTuple("attributes", attribute, kind, attributeType, entity);
 
         internal static void attribute_location(this TextWriter trapFile, Attribute attribute, Location location) =>
             trapFile.WriteTuple("attribute_location", attribute, location);

csharp/ql/lib/semmle/code/csharp/Attribute.qll

@@ -36,6 +36,12 @@ class Attributable extends @attributable {
   }
 }
 
+private string getAttributeName(Attribute a) {
+  exists(string type | type = a.getType().getName() |
+    if type.matches("%Attribute") then result = type.prefix(type.length() - 9) else result = type
+  )
+}
+
 /**
  * An attribute, for example `[...]` on line 1 in
  *
@@ -88,12 +94,7 @@ class Attribute extends TopLevelExprParent, @attribute {
 
   override Location getALocation() { attribute_location(this, result) }
 
-  override string toString() {
-    exists(string type, string name | type = this.getType().getName() |
-      (if type.matches("%Attribute") then name = type.prefix(type.length() - 9) else name = type) and
-      result = "[" + name + "(...)]"
-    )
-  }
+  override string toString() { result = "[" + getAttributeName(this) + "(...)]" }
 
   override string getAPrimaryQlClass() { result = "Attribute" }
 }
@@ -117,6 +118,8 @@ class DefaultAttribute extends Attribute, @attribute_default {
  * ```
  */
 class ReturnAttribute extends Attribute, @attribute_return {
+  override string toString() { result = "[return: " + getAttributeName(this) + "(...)]" }
+
   override string getAPrimaryQlClass() { result = "ReturnAttribute" }
 }
 
@@ -127,6 +130,8 @@ class ReturnAttribute extends Attribute, @attribute_return {
  * ```
  */
 class AssemblyAttribute extends Attribute, @attribute_assembly {
+  override string toString() { result = "[assembly: " + getAttributeName(this) + "(...)]" }
+
   override string getAPrimaryQlClass() { result = "AssemblyAttribute" }
 }
 
@@ -137,5 +142,7 @@ class AssemblyAttribute extends Attribute, @attribute_assembly {
  * ```
  */
 class ModuleAttribute extends Attribute, @attribute_module {
+  override string toString() { result = "[module: " + getAttributeName(this) + "(...)]" }
+
   override string getAPrimaryQlClass() { result = "ModuleAttribute" }
 }

csharp/ql/lib/upgrades/ba2201248071b2bf0bb52909b35014091d2e18a6/attribute_kind.ql

@@ -10,7 +10,6 @@ class TypeOrRef extends @type_or_ref {
   string toString() { none() }
 }
 
-query predicate add_default_kind(Attribute id, int kind, TypeOrRef type_id, Attributable target) {
-  kind = 0 and
-  attributes(id, type_id, target)
-}
+from Attribute id, TypeOrRef type_id, Attributable target
+where attributes(id, type_id, target)
+select id, 0, type_id, target