Skip to content

JS: deprecate member lookup on static types #3781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions change-notes/1.25/analysis-javascript.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,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.
22 changes: 12 additions & 10 deletions javascript/extractor/lib/typescript/src/type_table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
39 changes: 12 additions & 27 deletions javascript/ql/src/semmle/javascript/TypeScript.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
@@ -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` |
Original file line number Diff line number Diff line change
@@ -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 |
Original file line number Diff line number Diff line change
@@ -1 +0,0 @@
| [number, string] | (string \| number)[] |
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,3 @@
| IMulti | IGenericBase |
| IStringSub | IGenericBase |
| ISub | IBase |
| RegExpMatchArray | Array |
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
| CImplementsString | CImplementsString |
| CStringSub | CStringSub |
| CSub | CSub |
| CollatorOptions | CollatorOptions |
| IBase | IBase |
| IEmpty | IEmpty |
| IEmptySub | IEmptySub |
Expand All @@ -16,6 +15,3 @@
| IMulti | IMulti<T> |
| IStringSub | IStringSub |
| ISub | ISub |
| NumberFormatOptions | NumberFormatOptions |
| RegExp | RegExp |
| RegExpMatchArray | RegExpMatchArray |
Original file line number Diff line number Diff line change
Expand Up @@ -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 |

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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[] |
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import javascript

from TypeReference type
select type
Original file line number Diff line number Diff line change
@@ -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 |
Loading