Merge pull request github#3341 from hvitved/csharp/generics-nested-types

Approved by calumgrant

Author: semmle-qlci

change-notes/1.25/analysis-csharp.md

@@ -0,0 +1,28 @@
+# Improvements to C# analysis
+
+The following changes in version 1.25 affect C# analysis in all applications.
+
+## New queries
+
+| **Query**                   | **Tags**  | **Purpose**                                                        |
+|-----------------------------|-----------|--------------------------------------------------------------------|
+
+
+## Changes to existing queries
+
+| **Query**                    | **Expected impact**    | **Change**                        |
+|------------------------------|------------------------|-----------------------------------|
+
+
+## Removal of old queries
+
+## Changes to code extraction
+
+## Changes to libraries
+
+* The class `UnboundGeneric` has been refined to only be those declarations that actually
+  have type parameters. This means that non-generic nested types inside construced types,
+  such as `A<int>.B`, no longer are considered unbound generics. (Such nested types do,
+  however, still have relevant `.getSourceDeclaration()`s, for example `A<>.B`.)
+
+## Changes to autobuilder

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

@@ -340,7 +340,6 @@ protected void PopulateGenerics(TextWriter trapFile)
 
                 if (isFullyConstructed)
                 {
-                    trapFile.is_constructed(this);
                     trapFile.constructed_generic(this, Method.Create(Context, ConstructedFromSymbol));
                     foreach (var tp in symbol.GetAnnotatedTypeArguments())
                     {
@@ -354,7 +353,6 @@ protected void PopulateGenerics(TextWriter trapFile)
                 }
                 else
                 {
-                    trapFile.is_generic(this);
                     foreach (var typeParam in symbol.TypeParameters.Select(tp => TypeParameter.Create(Context, tp)))
                     {
                         trapFile.type_parameters(typeParam, child, this);

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

@@ -40,8 +40,6 @@ public override void Populate(TextWriter trapFile)
                 }
                 else if (symbol.IsReallyUnbound())
                 {
-                    trapFile.is_generic(this);
-
                     for (int i = 0; i < symbol.TypeParameters.Length; ++i)
                     {
                         TypeParameter.Create(Context, symbol.TypeParameters[i]);
@@ -52,7 +50,6 @@ public override void Populate(TextWriter trapFile)
                 }
                 else
                 {
-                    trapFile.is_constructed(this);
                     trapFile.constructed_generic(this, Type.Create(Context, symbol.ConstructedFrom).TypeRef);
 
                     for (int i = 0; i < symbol.TypeArguments.Length; ++i)

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

@@ -306,16 +306,6 @@ internal static void indexers(this TextWriter trapFile, Indexer propKey, string
             trapFile.WriteTuple("indexers", propKey, name, declaringType, memberType, unboundProperty);
         }
 
-        internal static void is_constructed(this TextWriter trapFile, IEntity typeOrMethod)
-        {
-            trapFile.WriteTuple("is_constructed", typeOrMethod);
-        }
-
-        internal static void is_generic(this TextWriter trapFile, IEntity typeOrMethod)
-        {
-            trapFile.WriteTuple("is_generic", typeOrMethod);
-        }
-
         internal static void local_function_stmts(this TextWriter trapFile, Entities.Statements.LocalFunction fnStmt, LocalFunction fn)
         {
             trapFile.WriteTuple("local_function_stmts", fnStmt, fn);

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

@@ -23,19 +23,19 @@ private import dotnet
  */
 class Generic extends DotNet::Generic, Declaration, @generic {
   Generic() {
-    is_generic(this) or
-    is_constructed(this)
+    type_parameters(_, _, this, _) or
+    type_arguments(_, _, this)
   }
 }
 
 /**
- * A generic declaration that can have type parameters.
+ * A generic declaration with type parameters.
  *
  * Either an unbound generic type (`UnboundGenericType`) or an unbound generic method
  * (`UnboundGenericMethod`).
  */
 class UnboundGeneric extends DotNet::UnboundGeneric, Generic {
-  UnboundGeneric() { is_generic(this) }
+  UnboundGeneric() { type_parameters(_, _, this, _) }
 
   override TypeParameter getTypeParameter(int n) { type_parameters(result, n, this, _) }
 
@@ -47,24 +47,21 @@ class UnboundGeneric extends DotNet::UnboundGeneric, Generic {
 }
 
 /**
- * A declaration constructed from an `UnboundGeneric` by supplying type arguments.
+ * A constructed generic.
  *
  * Either a constructed generic type (`ConstructedType`) or a constructed
  * generic method (`ConstructedMethod`).
  */
 class ConstructedGeneric extends DotNet::ConstructedGeneric, Generic {
-  ConstructedGeneric() { is_constructed(this) }
+  ConstructedGeneric() { type_arguments(_, _, this) }
 
   override UnboundGeneric getUnboundGeneric() { constructed_generic(this, result) }
 
   override UnboundGeneric getSourceDeclaration() {
     result = getUnboundGeneric().getSourceDeclaration()
   }
 
-  override int getNumberOfTypeArguments() {
-    // getTypeArgument() could fail if the type does not exist in the database
-    result = count(int i | type_arguments(_, i, this))
-  }
+  override int getNumberOfTypeArguments() { result = count(int i | type_arguments(_, i, this)) }
 
   override Type getTypeArgument(int i) { none() }
 
@@ -84,11 +81,12 @@ class ConstructedGeneric extends DotNet::ConstructedGeneric, Generic {
  */
 class UnboundGenericType extends ValueOrRefType, UnboundGeneric {
   /**
-   * Gets a bound/constructed version of this unbound generic type. This includes not only closed constructed types such as `G<int>`,
-   * but also open constructed types such as the `G<T>` in `class Other<T> { G<T> g; }`. Note that such a type is distinct from the
-   * `G<T>` used in the class definition, since in `G<T> g;` the `T` will be the actual type parameter used for the `Other` that contains
-   * `g`, whereas in `class G<T> { ... }` the `T` is a formal type parameter of `G`. It is important not to get confused by the superficial
-   * syntactic similarity.
+   * Gets a bound/constructed version of this unbound generic type. This includes
+   * not only closed constructed types such as `G<int>`, but also open constructed
+   * types such as the `G<T>` in `class Other<T> { G<T> g; }`. Note that such a type
+   * is distinct from the `G<T>` used in the class definition, since in `G<T> g;`
+   * the `T` will be the actual type parameter used for the `Other` that contains
+   * `g`, whereas in `class G<T> { ... }` the `T` is a formal type parameter of `G`.
    */
   override ConstructedType getAConstructedGeneric() {
     result = UnboundGeneric.super.getAConstructedGeneric()
@@ -350,15 +348,16 @@ class UnboundGenericDelegateType extends DelegateType, UnboundGenericType {
 }
 
 /**
- * A constructed (bound) type. This is a generic type for which actual type arguments have been supplied,
- * for example `G<int>` or the `G<T>` in `class Other<T> { G<T> g; }`. Constructed types can be divided further into
- * those that are open (for example `G1<T>` or `G2<T,T,U,int>`), in the sense that one or more of their type arguments
- * is a type parameter, versus those that are closed (for example `G1<int>` or `G2<long,long,float,int>`). We do not
- * currently distinguish the two in this library.
- *
- * Either a constructed `struct` (`ConstructedStruct`), constructed `class` (`ConstructedClass`),
- * constructed `interface` (`ConstructedInterface`), or constructed method
- * (`ConstructedMethod`).
+ * A constructed (bound) type. This is a generic type for which actual type
+ * arguments have been supplied, for example `G<int>` or the `G<T>` in
+ * `class Other<T> { G<T> g; }`. Constructed types can be divided further into
+ * those that are open (for example `G1<T>` or `G2<T,T,U,int>`), in the sense
+ * that one or more of their type arguments is a type parameter, versus those
+ * that are closed (for example `G1<int>` or `G2<long,long,float,int>`).
+ *
+ * Either a constructed `struct` (`ConstructedStruct`), constructed `class`
+ * (`ConstructedClass`), constructed `interface` (`ConstructedInterface`),
+ * or constructed method (`ConstructedMethod`).
  */
 class ConstructedType extends ValueOrRefType, ConstructedGeneric {
   override UnboundGenericType getSourceDeclaration() {

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

@@ -13,7 +13,7 @@ abstract class UnboundGeneric extends Generic {
   /** Gets the `i`th type parameter, if any. */
   abstract TypeParameter getTypeParameter(int i);
 
-  /** Gets a type parameter, if any. */
+  /** Gets a type parameter. */
   TypeParameter getATypeParameter() { result = getTypeParameter(_) }
 
   /**
@@ -42,10 +42,13 @@ abstract class ConstructedGeneric extends Generic {
   /** Gets the `i`th type argument, if any. */
   abstract Type getTypeArgument(int i);
 
-  /** Gets a type argument, if any. */
+  /** Gets a type argument. */
   Type getATypeArgument() { result = getTypeArgument(_) }
 
-  /** Gets the unbound generic declaration from which this declaration was constructed. */
+  /**
+   * Gets the unbound generic declaration from which this declaration was
+   * constructed.
+   */
   UnboundGeneric getUnboundGeneric() { none() }
 
   /** Gets the total number of type arguments. */

csharp/ql/src/semmlecode.csharp.dbscheme

@@ -497,10 +497,6 @@ expr_flowstate(unique int id: @expr ref, int state: int ref);
 
 @generic = @type | @method | @local_function;
 
-is_generic(unique int id: @generic ref);
-
-is_constructed(unique int id: @generic ref);
-
 type_parameters(
   unique int id: @type_parameter ref,
   int index: int ref,

csharp/ql/src/semmlecode.csharp.dbscheme.stats

@@ -15479,28 +15479,6 @@
 </dependencies>
 </relation>
 <relation>
-<name>is_generic</name>
-<cardinality>77320</cardinality>
-<columnsizes>
-<e>
-<k>id</k>
-<v>77320</v>
-</e>
-</columnsizes>
-<dependencies/>
-</relation>
-<relation>
-<name>is_constructed</name>
-<cardinality>358124</cardinality>
-<columnsizes>
-<e>
-<k>id</k>
-<v>358124</v>
-</e>
-</columnsizes>
-<dependencies/>
-</relation>
-<relation>
 <name>type_parameters</name>
 <cardinality>84292</cardinality>
 <columnsizes>