Merge pull request #3781 from asger-semmle/js/deprecate-type-member-lookup

Approved by erik-krogh

Author: semmle-qlci

change-notes/1.25/analysis-javascript.md

@@ -98,3 +98,4 @@ The following low-precision queries are no longer run by default on LGTM (their
   - `ParameterNode.asExpr()` and `.getAstNode()` now gets the parameter's AST node, whereas previously it had no result.
   - `Expr.flow()` now has a more meaningful result for destructuring patterns. Previously this node was disconnected from the data flow graph. Now it represents the values being destructured by the pattern.
 * The global data-flow and taint-tracking libraries now model indirect parameter accesses through the `arguments` object in some cases, which may lead to additional results from some of the security queries, particularly "Prototype pollution in utility function".
+* The predicates `Type.getProperty()` and variants of `Type.getMethod()` have been deprecated due to lack of use-cases. Looking up a named property of a static type is no longer supported, favoring faster extraction times instead.

javascript/extractor/lib/typescript/src/type_table.ts

@@ -875,21 +875,23 @@ export class TypeTable {
   }
 
   /**
-   * Returns the properties of the given type, or `null` if the properties of this
-   * type could not be computed.
+   * Returns the properties to extract for the given type or `null` if nothing should be extracted.
+   *
+   * For performance reasons we only extract properties needed to recognize promise types at the QL
+   * level.
    */
-  private tryGetProperties(type: ts.Type) {
-    // Workaround for https://github.com/Microsoft/TypeScript/issues/30845
-    // Should be safe to remove once that has been fixed.
-    try {
-      return type.getProperties();
-    } catch (e) {
-      return null;
+  private getPropertiesToExtract(type: ts.Type) {
+    if (this.getSelfType(type) === type) {
+      let thenSymbol = this.typeChecker.getPropertyOfType(type, "then");
+      if (thenSymbol != null) {
+        return [thenSymbol];
+      }
     }
+    return null;
   }
 
   private extractProperties(type: ts.Type, id: number) {
-    let props = this.tryGetProperties(type);
+    let props = this.getPropertiesToExtract(type);
     if (props == null) return;
     for (let symbol of props) {
       let propertyType = this.tryGetTypeOfSymbol(symbol);

javascript/ql/src/semmle/javascript/TypeScript.qll

@@ -1649,11 +1649,9 @@ class Type extends @type {
   Type getChild(int i) { type_child(result, this, i) }
 
   /**
-   * Gets the type of the given property of this type.
-   *
-   * Note that this does not account for properties implied by index signatures.
+   * DEPRECATED. Property lookup on types is no longer supported.
    */
-  Type getProperty(string name) { type_property(this, name, result) }
+  deprecated Type getProperty(string name) { none() }
 
   /**
    * Gets the type of the string index signature on this type,
@@ -1758,33 +1756,19 @@ class Type extends @type {
   int getNumConstructorSignature() { result = count(getAConstructorSignature()) }
 
   /**
-   * Gets the last signature of the method of the given name.
-   *
-   * For overloaded methods, this is the most general version of the its
-   * signature, which covers all cases, but with less precision than the
-   * overload signatures.
-   *
-   * Use `getAMethodOverload` to get any of its overload signatures.
+   * DEPRECATED. Method lookup on types is no longer supported.
    */
-  FunctionCallSignatureType getMethod(string name) {
-    result = getProperty(name).getLastFunctionSignature()
-  }
+  deprecated FunctionCallSignatureType getMethod(string name) { none() }
 
   /**
-   * Gets the `n`th overload signature of the given method.
+   * DEPRECATED. Method lookup on types is no longer supported.
    */
-  FunctionCallSignatureType getMethodOverload(string name, int n) {
-    result = getProperty(name).getFunctionSignature(n)
-  }
+  deprecated FunctionCallSignatureType getMethodOverload(string name, int n) { none() }
 
   /**
-   * Gets a signature of the method of the given name.
-   *
-   * Overloaded methods have multiple signatures.
+   * DEPRECATED. Method lookup on types is no longer supported.
    */
-  FunctionCallSignatureType getAMethodOverload(string name) {
-    result = getProperty(name).getAFunctionSignature()
-  }
+  deprecated FunctionCallSignatureType getAMethodOverload(string name) { none() }
 
   /**
    * Repeatedly unfolds union and intersection types and gets any of the underlying types,
@@ -2638,10 +2622,11 @@ private class PromiseTypeName extends TypeName {
       name.matches("%Deferred")
     ) and
     // The `then` method should take a callback, taking an argument of type `T`.
-    exists(TypeReference self | self = getType() |
+    exists(TypeReference self, Type thenMethod | self = getType() |
       self.getNumTypeArgument() = 1 and
-      self
-          .getAMethodOverload("then")
+      type_property(self, "then", thenMethod) and
+      thenMethod
+          .getAFunctionSignature()
           .getParameter(0)
           .unfold()
           .getAFunctionSignature()

javascript/ql/test/library-tests/TypeScript/ArrayTypes/ArrayTypes.expected

@@ -1,20 +1,5 @@
-| (T \| ConcatArray<T>)[] | `T \| ConcatArray<T>` |
-| (number \| ConcatArray<number>)[] | `number \| ConcatArray<number>` |
-| (number[] \| ConcatArray<number[]>)[] | `number[] \| ConcatArray<number[]>` |
-| (string \| number \| ConcatArray<string \| number>)[] | `string \| number \| ConcatArray<string \| number>` |
-| (string \| number)[] | `string \| number` |
-| ConcatArray<T>[] | `ConcatArray<T>` |
-| ConcatArray<number>[] | `ConcatArray<number>` |
-| ConcatArray<number[]>[] | `ConcatArray<number[]>` |
-| ConcatArray<string \| number>[] | `ConcatArray<string \| number>` |
-| S[] | `S` |
-| T[] | `T` |
-| U[] | `U` |
 | [number, string] | `string \| number` |
-| any[] | `any` |
 | number[] | `number` |
-| number[][] | `number[]` |
 | readonly T[] | `T` |
 | readonly number[] | `number` |
 | readonly number[][] | `number[]` |
-| string[] | `string` |

javascript/ql/test/library-tests/TypeScript/ArrayTypes/NumberIndexTypes.expected

@@ -1,22 +1,7 @@
-| (T \| ConcatArray<T>)[] | T \| ConcatArray<T> |
-| (number \| ConcatArray<number>)[] | number \| ConcatArray<number> |
-| (number[] \| ConcatArray<number[]>)[] | number[] \| ConcatArray<number[]> |
-| (string \| number \| ConcatArray<string \| number>)[] | string \| number \| ConcatArray<string \| number> |
-| (string \| number)[] | string \| number |
-| ConcatArray<T>[] | ConcatArray<T> |
-| ConcatArray<number>[] | ConcatArray<number> |
-| ConcatArray<number[]>[] | ConcatArray<number[]> |
-| ConcatArray<string \| number>[] | ConcatArray<string \| number> |
 | NumberIndexable | object |
-| S[] | S |
-| T[] | T |
-| U[] | U |
 | [number, string] | string \| number |
-| any[] | any |
 | number[] | number |
-| number[][] | number[] |
 | readonly T[] | T |
 | readonly number[] | number |
 | readonly number[][] | number[] |
 | string | string |
-| string[] | string |

javascript/ql/test/library-tests/TypeScript/ArrayTypes/TupleTypes.expected

@@ -1 +0,0 @@
-| [number, string] | (string \| number)[] |

javascript/ql/test/library-tests/TypeScript/BaseTypes/BaseTypes.expected

@@ -13,4 +13,3 @@
 | IMulti | IGenericBase |
 | IStringSub | IGenericBase |
 | ISub | IBase |
-| RegExpMatchArray | Array |

javascript/ql/test/library-tests/TypeScript/BaseTypes/SelfTypes.expected

@@ -7,7 +7,6 @@
 | CImplementsString | CImplementsString |
 | CStringSub | CStringSub |
 | CSub | CSub |
-| CollatorOptions | CollatorOptions |
 | IBase | IBase |
 | IEmpty | IEmpty |
 | IEmptySub | IEmptySub |
@@ -16,6 +15,3 @@
 | IMulti | IMulti<T> |
 | IStringSub | IStringSub |
 | ISub | ISub |
-| NumberFormatOptions | NumberFormatOptions |
-| RegExp | RegExp |
-| RegExpMatchArray | RegExpMatchArray |

javascript/ql/test/library-tests/TypeScript/CallSignatureTypes/test.expected

@@ -108,51 +108,24 @@ test_FunctionCallSig
 | tst.ts:63:3:63:23 | method2 ... ing[]); | (y: string[]): any |
 | tst.ts:64:3:64:21 | method3(y: string); | (y: string): any |
 test_getRestParameterType
-| (...items: (string \| ConcatArray<string>)[]): T[] | string \| ConcatArray<string> |
-| (...items: ConcatArray<string>[]): T[] | ConcatArray<string> |
-| (...items: string[]): number | string |
-| (...strings: string[]): string | string |
 | (...y: string[]): any | string |
-| (start: number, deleteCount: number, ...items: string[]): T[] | string |
-| (substring: string, ...args: any[]): string | any |
 | (x: number, ...y: string[]): any | string |
 | new (...y: string[]): any | string |
 | new (x: number, ...y: string[]): any | string |
 test_getRestParameterArray
-| (...items: (string \| ConcatArray<string>)[]): T[] | (string \| ConcatArray<string>)[] |
-| (...items: ConcatArray<string>[]): T[] | ConcatArray<string>[] |
-| (...items: string[]): number | string[] |
-| (...strings: string[]): string | string[] |
 | (...y: string[]): any | string[] |
-| (start: number, deleteCount: number, ...items: string[]): T[] | string[] |
-| (substring: string, ...args: any[]): string | any[] |
 | (x: number, ...y: string[]): any | string[] |
 | new (...y: string[]): any | string[] |
 | new (x: number, ...y: string[]): any | string[] |
 test_RestSig_getParameter
-| (...items: (string \| ConcatArray<string>)[]): T[] | 0 | items | string \| ConcatArray<string> |
-| (...items: ConcatArray<string>[]): T[] | 0 | items | ConcatArray<string> |
-| (...items: string[]): number | 0 | items | string |
-| (...strings: string[]): string | 0 | strings | string |
 | (...y: string[]): any | 0 | y | string |
-| (start: number, deleteCount: number, ...items: string[]): T[] | 0 | start | number |
-| (start: number, deleteCount: number, ...items: string[]): T[] | 1 | deleteCount | number |
-| (start: number, deleteCount: number, ...items: string[]): T[] | 2 | items | string |
-| (substring: string, ...args: any[]): string | 0 | substring | string |
-| (substring: string, ...args: any[]): string | 1 | args | any |
 | (x: number, ...y: string[]): any | 0 | x | number |
 | (x: number, ...y: string[]): any | 1 | y | string |
 | new (...y: string[]): any | 0 | y | string |
 | new (x: number, ...y: string[]): any | 0 | x | number |
 | new (x: number, ...y: string[]): any | 1 | y | string |
 test_RestSig_numRequiredParams
-| (...items: (string \| ConcatArray<string>)[]): T[] | 0 |
-| (...items: ConcatArray<string>[]): T[] | 0 |
-| (...items: string[]): number | 0 |
-| (...strings: string[]): string | 0 |
 | (...y: string[]): any | 0 |
-| (start: number, deleteCount: number, ...items: string[]): T[] | 2 |
-| (substring: string, ...args: any[]): string | 1 |
 | (x: number, ...y: string[]): any | 1 |
 | new (...y: string[]): any | 0 |
 | new (x: number, ...y: string[]): any | 1 |

javascript/ql/test/library-tests/TypeScript/ExpansiveTypes/ExpansiveTypes.expected

@@ -0,0 +1,77 @@
+| After |
+| AfterX |
+| Before |
+| BeforeX |
+| Box<Expand<T[]>> |
+| Box<S> |
+| Box<S> |
+| Box<T[]> |
+| Box<number> |
+| C<T> |
+| C<T[]> |
+| Expand<T> |
+| Expand<T[]> |
+| ExpandUsingObjectLiteral<T> |
+| ExpandUsingObjectLiteral<T[]> |
+| Expansive<T> |
+| Expansive<T> |
+| Expansive<T[]> |
+| Expansive<T[]> |
+| Expansive<number> |
+| Expansive<string> |
+| ExpansiveA<S> |
+| ExpansiveA<S> |
+| ExpansiveA<T> |
+| ExpansiveA<T> |
+| ExpansiveB<S> |
+| ExpansiveB<S> |
+| ExpansiveB<T> |
+| ExpansiveB<T[]> |
+| ExpansiveB<T[]> |
+| ExpansiveB<number> |
+| ExpansiveByInference<T> |
+| ExpansiveByInference<T[]> |
+| ExpansiveC<T> |
+| ExpansiveC<T> |
+| ExpansiveC<T> |
+| ExpansiveC<T[]> |
+| ExpansiveC<T[]> |
+| ExpansiveC<number> |
+| ExpansiveConstructSignature<T> |
+| ExpansiveConstructSignature<T[]> |
+| ExpansiveD<T> |
+| ExpansiveD<T> |
+| ExpansiveD<T> |
+| ExpansiveD<T> |
+| ExpansiveFunctionType<T> |
+| ExpansiveFunctionType<T[]> |
+| ExpansiveMethod<T> |
+| ExpansiveMethod<T[]> |
+| ExpansiveParameter<T> |
+| ExpansiveParameter<T[]> |
+| ExpansiveSignature<T> |
+| ExpansiveSignature<T[]> |
+| ExpansiveSignatureTypeBound<T> |
+| ExpansiveSignatureTypeBound<T[]> |
+| ExpansiveX<T> |
+| ExpansiveX<T[]> |
+| NonExpansive<Box<number>> |
+| NonExpansive<T> |
+| T[] |
+| T[] |
+| T[] |
+| T[] |
+| T[] |
+| T[] |
+| T[] |
+| T[] |
+| T[] |
+| T[] |
+| T[] |
+| T[] |
+| T[] |
+| T[] |
+| T[] |
+| T[] |
+| T[] |
+| T[] |

javascript/ql/test/library-tests/TypeScript/ExpansiveTypes/ExpansiveTypes.ql

@@ -0,0 +1,4 @@
+import javascript
+
+from TypeReference type
+select type

javascript/ql/test/library-tests/TypeScript/ExpansiveTypes/NonExpansiveTypes.expected

@@ -1,8 +1,4 @@
-| Intl.CollatorOptions | CollatorOptions |
-| Intl.NumberFormatOptions | NumberFormatOptions |
 | LegacyGlobals.LegacySubclass | LegacySubclass |
 | Modern.ModernClass | ModernClass |
 | ModernGlobals.ModernSubclass | ModernSubclass |
-| RegExp | RegExp |
-| RegExpMatchArray | RegExpMatchArray |
 | __Legacy.LegacyClass | LegacyClass |