Merge pull request github#6268 from tamasvajk/feature/generic-type-name

C#: Remove type args/params from generic type names in extractor

Author: tamasvajk

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

@@ -482,22 +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())
-            {
-                trapFile.Write('<');
-                trapFile.BuildList(
-                    ",",
-                    namedType.TypeArguments,
-                    p =>
-                    {
-                        if (IsReallyBound(namedType))
-                        {
-                            p.BuildDisplayName(cx, trapFile);
-                        }
-                    });
-                trapFile.Write('>');
-            }
         }
 
         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/Callable.qll

@@ -313,6 +313,7 @@ class ExtensionMethod extends Method {
   override predicate isStatic() { any() }
 
   /** Gets the type being extended by this method. */
+  pragma[noinline]
   Type getExtendedType() { result = getParameter(0).getType() }
 
   override string getAPrimaryQlClass() { result = "ExtensionMethod" }

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

@@ -105,20 +105,36 @@ class UnboundGenericType extends ValueOrRefType, UnboundGeneric {
 
   override Location getALocation() { type_location(this, result) }
 
-  /** Gets the name of this generic type without the `<...>` brackets. */
-  string getNameWithoutBrackets() {
-    result = getName().prefix(getName().length() - getNumberOfTypeParameters() - 1)
+  override UnboundGenericType getUnboundDeclaration() {
+    result = ValueOrRefType.super.getUnboundDeclaration()
+  }
+
+  final override Type getChild(int n) { result = getTypeParameter(n) }
+
+  private string getTypeParameterCommas() {
+    result = strictconcat(int i | exists(this.getTypeParameter(i)) | "", ",")
   }
 
   override string toStringWithTypes() {
-    result = getNameWithoutBrackets() + "<" + this.typeParametersToString() + ">"
+    result = this.getUndecoratedName() + "<" + this.typeParametersToString() + ">"
   }
 
-  override UnboundGenericType getUnboundDeclaration() {
-    result = ValueOrRefType.super.getUnboundDeclaration()
+  final override string getName() {
+    result = this.getUndecoratedName() + "<" + this.getTypeParameterCommas() + ">"
   }
 
-  final override Type getChild(int n) { result = getTypeParameter(n) }
+  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.getUndecoratedName()
+      )
+      or
+      not exists(this.getDeclaringType()) and
+      qualifier = this.getNamespace().getQualifiedName() and
+      name0 = this.getUndecoratedName()
+    )
+  }
 }
 
 /**
@@ -332,8 +348,8 @@ class UnboundGenericDelegateType extends DelegateType, UnboundGenericType {
 
   override string toStringWithTypes() {
     result =
-      getNameWithoutBrackets() + "<" + this.typeParametersToString() + ">(" +
-        parameterTypesToString() + ")"
+      getUndecoratedName() + "<" + this.typeParametersToString() + ">(" + parameterTypesToString() +
+        ")"
   }
 }
 
@@ -360,22 +376,49 @@ class ConstructedType extends ValueOrRefType, ConstructedGeneric {
 
   override UnboundGenericType getUnboundGeneric() { constructed_generic(this, getTypeRef(result)) }
 
-  override string toStringWithTypes() {
+  final override Type getChild(int n) { result = getTypeArgument(n) }
+
+  language[monotonicAggregates]
+  private string getTypeArgumentsString() {
     result =
-      getUnboundGeneric().getNameWithoutBrackets() + "<" + this.getTypeArgumentsString() + ">"
+      strictconcat(Type t, int i | t = this.getTypeArgument(i) | t.toString(), ", " order by i)
   }
 
-  final override Type getChild(int n) { result = getTypeArgument(n) }
+  language[monotonicAggregates]
+  private string getTypeArgumentsNames() {
+    result = strictconcat(Type t, int i | t = this.getTypeArgument(i) | t.getName(), "," order by i)
+  }
 
   language[monotonicAggregates]
-  private string getTypeArgumentsString() {
+  private string getTypeArgumentsQualifiedNames() {
     result =
-      concat(int i |
-        exists(this.getTypeArgument(i))
+      strictconcat(Type t, int i |
+        t = this.getTypeArgument(i)
       |
-        this.getTypeArgument(i).toString(), ", " order by i
+        t.getQualifiedName(), "," order by i
       )
   }
+
+  final override string toStringWithTypes() {
+    result = this.getUndecoratedName() + "<" + this.getTypeArgumentsString() + ">"
+  }
+
+  final override string getName() {
+    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.getUndecoratedName()
+      )
+      or
+      not exists(this.getDeclaringType()) and
+      qualifier = this.getNamespace().getQualifiedName() and
+      name0 = this.getUndecoratedName()
+    )
+  }
 }
 
 /**

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

@@ -55,27 +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 constructed type. */
-  private string getNameWithoutBrackets() {
-    exists(UnboundGenericType unbound, string name |
-      unbound = this.(ConstructedType).getUnboundDeclaration() and
-      name = unbound.getName() and
-      result = name.prefix(name.length() - unbound.getNumberOfTypeParameters() - 1)
-    )
-    or
-    not this instanceof ConstructedType and
-    result = this.getName()
-  }
-
-  language[monotonicAggregates]
-  private string getQualifiedTypeArguments() {
-    result =
-      strictconcat(Type t, int i |
-        t = this.(ConstructedType).getTypeArgument(i)
-      |
-        t.getQualifiedName(), "," order by i
-      )
-  }
+  /**
+   * 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`.
@@ -84,21 +69,14 @@ class ValueOrRefType extends DotNet::ValueOrRefType, Type, Attributable, @value_
    * `qualifier`=`System.IO` and `name`=`IOException`.
    */
   override predicate hasQualifiedName(string qualifier, string name) {
-    exists(string name0 |
-      not this instanceof ConstructedType and
-      name = name0
-      or
-      name = name0 + "<" + this.getQualifiedTypeArguments() + ">"
-    |
-      exists(string enclosing |
-        this.getDeclaringType().hasQualifiedName(qualifier, enclosing) and
-        name0 = enclosing + "+" + this.getNameWithoutBrackets()
-      )
-      or
-      not exists(this.getDeclaringType()) and
-      qualifier = this.getNamespace().getQualifiedName() and
-      name0 = this.getNameWithoutBrackets()
+    exists(string enclosing |
+      this.getDeclaringType().hasQualifiedName(qualifier, enclosing) and
+      name = enclosing + "+" + this.getUndecoratedName()
     )
+    or
+    not exists(this.getDeclaringType()) and
+    qualifier = this.getNamespace().getQualifiedName() and
+    name = this.getUndecoratedName()
   }
 
   /** Gets the namespace containing this type. */
@@ -112,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, _) }
@@ -201,6 +170,7 @@ class ValueOrRefType extends DotNet::ValueOrRefType, Type, Attributable, @value_
    * }
    * ```
    */
+  pragma[inline]
   predicate hasMethod(Method m) { this.hasMember(m) }
 
   /**
@@ -227,6 +197,7 @@ class ValueOrRefType extends DotNet::ValueOrRefType, Type, Attributable, @value_
    * }
    * ```
    */
+  pragma[inline]
   predicate hasCallable(Callable c) {
     hasMethod(c)
     or
@@ -256,25 +227,15 @@ class ValueOrRefType extends DotNet::ValueOrRefType, Type, Attributable, @value_
    * }
    * ```
    */
+  pragma[inline]
   predicate hasMember(Member m) {
-    // For performance reasons, split up into "cheap" computation
-    // (non-overridden members) and "expensive" computation
-    // (overridden members). The latter is cached, and generally
-    // much smaller than the full relation.
-    hasNonOverriddenMember(m)
+    m = this.getAMember()
+    or
+    hasNonOverriddenMember(this.getBaseClass+(), m)
     or
     hasOverriddenMember(m)
   }
 
-  private predicate hasNonOverriddenMember(Member m) {
-    isNonOverridden(m) and
-    (
-      m = getAMember()
-      or
-      getBaseClass+().getAMember() = m and not m.isPrivate()
-    )
-  }
-
   cached
   private predicate hasOverriddenMember(Virtualizable v) {
     v.isOverridden() and
@@ -753,8 +714,12 @@ class RefType extends ValueOrRefType, @ref_type {
   override predicate isRefType() { any() }
 }
 
-// Helper predicate to avoid slow "negation_body"
-private predicate isNonOverridden(Member m) { not m.(Virtualizable).isOverridden() }
+pragma[noinline]
+private predicate hasNonOverriddenMember(Class c, Member m) {
+  m = c.getAMember() and
+  not m.(Virtualizable).isOverridden() and
+  not m.isPrivate()
+}
 
 /**
  * A `class`, for example
@@ -963,6 +928,15 @@ class NullableType extends ValueType, DotNet::ConstructedGeneric, @nullable_type
   override Type getTypeArgument(int p) { p = 0 and result = getUnderlyingType() }
 
   override string getAPrimaryQlClass() { result = "NullableType" }
+
+  final override string getName() {
+    result = "Nullable<" + this.getUnderlyingType().getName() + ">"
+  }
+
+  final override predicate hasQualifiedName(string qualifier, string name) {
+    qualifier = "System" and
+    name = "Nullable<" + this.getUnderlyingType().getQualifiedName() + ">"
+  }
 }
 
 /**
@@ -1046,7 +1020,7 @@ class PointerType extends DotNet::PointerType, Type, @pointer_type {
 
   override Type getChild(int n) { result = getReferentType() and n = 0 }
 
-  override string getName() { result = DotNet::PointerType.super.getName() }
+  override string getName() { types(this, _, result) }
 
   override Location getALocation() { result = getReferentType().getALocation() }
 
@@ -1120,11 +1094,14 @@ class TupleType extends ValueType, @tuple_type {
   override string toStringWithTypes() {
     result =
       "(" +
-        concat(int i |
-          exists(getElement(i))
-        |
-          getElement(i).getType().toStringWithTypes(), ", " order by i
-        ) + ")"
+        concat(Type t, int i | t = getElement(i).getType() | t.toStringWithTypes(), ", " order by i)
+        + ")"
+  }
+
+  language[monotonicAggregates]
+  override string getName() {
+    result =
+      "(" + concat(Type t, int i | t = getElement(i).getType() | t.getName(), "," order by i) + ")"
   }
 
   override string getLabel() { result = getUnderlyingType().getLabel() }

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"
     }
   }
 }

csharp/ql/src/semmle/code/dotnet/Element.qll

@@ -57,6 +57,7 @@ class Element extends @dotnet_element {
 /** An element that has a name. */
 class NamedElement extends Element, @dotnet_named_element {
   /** Gets the name of this element. */
+  cached
   string getName() { none() }
 
   /** Holds if this element has name 'name'. */

csharp/ql/src/semmlecode.csharp.dbscheme

@@ -1,3 +1,13 @@
+/* This is a dummy line to alter the dbscheme, so we can make a database upgrade
+ * without actually changing any of the dbscheme predicates. It contains a date
+ * to allow for such updates in the future as well.
+ *
+ * 2021-07-14
+ *
+ * DO NOT remove this comment carelessly, since it can revert the dbscheme back to a
+ * previously seen state (matching a previously seen SHA), which would make the upgrade
+ * mechanism not work properly.
+ */
 
 /**
  * An invocation of the compiler. Note that more than one file may be