Fix code review findings

Author: tamasvajk

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

@@ -482,13 +482,6 @@ private static void BuildNamedTypeDisplayName(this INamedTypeSymbol namedType, C
             {
                 trapFile.Write(TrapExtensions.EncodeString(namedType.Name));
             }
-
-            if (namedType.IsGenericType && namedType.TypeKind != TypeKind.Error && namedType.TypeArguments.Any())
-            {
-                var args = string.Join(',', namedType.TypeArguments.Select(ta => ta.MetadataName));
-
-                cx.Extractor.Logger.Log(Util.Logging.Severity.Debug, $"Found generic type '{namedType.MetadataName}' with type arguments '{args}', skipping type arguments in type name.");
-            }
         }
 
         public static bool IsReallyUnbound(this INamedTypeSymbol type) =>

csharp/ql/src/definitions.qll

@@ -158,11 +158,10 @@ private class TypeMentionUse extends Use, TypeMention {
     Use.super.hasLocationInfo(filepath, startline, startcolumn, endline, _) and
     endcolumn =
       startcolumn +
-          this.getType().(ConstructedType).getUnboundGeneric().getNameWithoutBrackets().length() - 1
+          this.getType().(ConstructedType).getUnboundGeneric().getUndecoratedName().length() - 1
     or
     Use.super.hasLocationInfo(filepath, startline, startcolumn, endline, _) and
-    endcolumn =
-      startcolumn + this.getType().(UnboundGenericType).getNameWithoutBrackets().length() - 1
+    endcolumn = startcolumn + this.getType().(UnboundGenericType).getUndecoratedName().length() - 1
     or
     not this.getType() instanceof ConstructedType and
     not this.getType() instanceof UnboundGenericType and

csharp/ql/src/semmle/code/csharp/AnnotatedType.qll

@@ -407,7 +407,7 @@ class AnnotatedConstructedType extends AnnotatedType {
 
   override string toString() {
     result =
-      annotations.getTypePrefix() + type.getUnboundGeneric().getNameWithoutBrackets() + "<" +
+      annotations.getTypePrefix() + type.getUnboundGeneric().getUndecoratedName() + "<" +
         this.getTypeArgumentsString() + ">" + annotations.getTypeSuffix()
   }
 

csharp/ql/src/semmle/code/csharp/Generics.qll

@@ -116,23 +116,23 @@ class UnboundGenericType extends ValueOrRefType, UnboundGeneric {
   }
 
   override string toStringWithTypes() {
-    result = this.getNameWithoutBrackets() + "<" + this.typeParametersToString() + ">"
+    result = this.getUndecoratedName() + "<" + this.typeParametersToString() + ">"
   }
 
   final override string getName() {
-    result = this.getNameWithoutBrackets() + "<" + this.getTypeParameterCommas() + ">"
+    result = this.getUndecoratedName() + "<" + this.getTypeParameterCommas() + ">"
   }
 
   final override predicate hasQualifiedName(string qualifier, string name) {
     exists(string name0 | name = name0 + "<" + this.getTypeParameterCommas() + ">" |
       exists(string enclosing |
         this.getDeclaringType().hasQualifiedName(qualifier, enclosing) and
-        name0 = enclosing + "+" + this.getNameWithoutBrackets()
+        name0 = enclosing + "+" + this.getUndecoratedName()
       )
       or
       not exists(this.getDeclaringType()) and
       qualifier = this.getNamespace().getQualifiedName() and
-      name0 = this.getNameWithoutBrackets()
+      name0 = this.getUndecoratedName()
     )
   }
 }
@@ -348,8 +348,8 @@ class UnboundGenericDelegateType extends DelegateType, UnboundGenericType {
 
   override string toStringWithTypes() {
     result =
-      getNameWithoutBrackets() + "<" + this.typeParametersToString() + ">(" +
-        parameterTypesToString() + ")"
+      getUndecoratedName() + "<" + this.typeParametersToString() + ">(" + parameterTypesToString() +
+        ")"
   }
 }
 
@@ -400,29 +400,23 @@ class ConstructedType extends ValueOrRefType, ConstructedGeneric {
   }
 
   final override string toStringWithTypes() {
-    exists(string undecorated |
-      types(this, _, undecorated) and
-      result = undecorated + "<" + this.getTypeArgumentsString() + ">"
-    )
+    result = this.getUndecoratedName() + "<" + this.getTypeArgumentsString() + ">"
   }
 
   final override string getName() {
-    exists(string undecorated |
-      types(this, _, undecorated) and
-      result = undecorated + "<" + this.getTypeArgumentsNames() + ">"
-    )
+    result = this.getUndecoratedName() + "<" + this.getTypeArgumentsNames() + ">"
   }
 
   final override predicate hasQualifiedName(string qualifier, string name) {
     exists(string name0 | name = name0 + "<" + this.getTypeArgumentsQualifiedNames() + ">" |
       exists(string enclosing |
         this.getDeclaringType().hasQualifiedName(qualifier, enclosing) and
-        name0 = enclosing + "+" + this.getNameWithoutBrackets()
+        name0 = enclosing + "+" + this.getUndecoratedName()
       )
       or
       not exists(this.getDeclaringType()) and
       qualifier = this.getNamespace().getQualifiedName() and
-      name0 = this.getNameWithoutBrackets()
+      name0 = this.getUndecoratedName()
     )
   }
 }

csharp/ql/src/semmle/code/csharp/Type.qll

@@ -55,8 +55,12 @@ private predicate isObjectClass(Class c) { c instanceof ObjectType }
  * Either a value type (`ValueType`) or a reference type (`RefType`).
  */
 class ValueOrRefType extends DotNet::ValueOrRefType, Type, Attributable, @value_or_ref_type {
-  /** Gets the name of this type without `<...>` brackets, in case it is a generic type. */
-  string getNameWithoutBrackets() { types(this, _, result) }
+  /**
+   * DEPRECATED: use `getUndecoratedName()` instead
+   *
+   * Gets the name of this type without `<...>` brackets, in case it is a generic type.
+   */
+  deprecated string getNameWithoutBrackets() { types(this, _, result) }
 
   /**
    * Holds if this type has the qualified name `qualifier`.`name`.
@@ -67,12 +71,12 @@ class ValueOrRefType extends DotNet::ValueOrRefType, Type, Attributable, @value_
   override predicate hasQualifiedName(string qualifier, string name) {
     exists(string enclosing |
       this.getDeclaringType().hasQualifiedName(qualifier, enclosing) and
-      name = enclosing + "+" + this.getNameWithoutBrackets()
+      name = enclosing + "+" + this.getUndecoratedName()
     )
     or
     not exists(this.getDeclaringType()) and
     qualifier = this.getNamespace().getQualifiedName() and
-    name = this.getNameWithoutBrackets()
+    name = this.getUndecoratedName()
   }
 
   /** Gets the namespace containing this type. */
@@ -86,16 +90,7 @@ class ValueOrRefType extends DotNet::ValueOrRefType, Type, Attributable, @value_
 
   override ValueOrRefType getDeclaringType() { none() }
 
-  override string getUndecoratedName() {
-    if this.getName().indexOf("<") > 0
-    then
-      exists(string name, int p |
-        name = this.getName() and p = min(int p2 | p2 = name.indexOf("<") and p2 > 0)
-      |
-        result = name.substring(0, p)
-      )
-    else result = this.getName()
-  }
+  override string getUndecoratedName() { types(this, _, result) }
 
   /** Gets a nested child type, if any. */
   NestedType getAChildType() { nested_types(result, this, _) }

csharp/ql/src/semmle/code/csharp/frameworks/system/data/Entity.qll

@@ -39,7 +39,7 @@ module SystemDataEntity {
   /** The `System.Data.Entity.DbSet` class. */
   class DbSet extends Class {
     DbSet() {
-      this.getUnboundDeclaration().(csharp::UnboundGenericClass).getNameWithoutBrackets() = "DbSet"
+      this.getUnboundDeclaration().(csharp::UnboundGenericClass).getUndecoratedName() = "DbSet"
     }
 
     /** Gets the `SqlQuery` method. */
@@ -100,7 +100,7 @@ module SystemDataEntityInfrastructure {
       this.getABaseType*()
           .getUnboundDeclaration()
           .(csharp::UnboundGenericClass)
-          .getNameWithoutBrackets() = "DbRawSqlQuery"
+          .getUndecoratedName() = "DbRawSqlQuery"
     }
   }
 }