Skip to content

TS: fix a case of infinite type expansion #1059

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.

Sign up for GitHub

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 3 commits into from
Mar 11, 2019
Merged

TS: fix a case of infinite type expansion #1059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc6c33d
TS: Fix infinite type expansion issue
asger-semmle Mar 7, 2019
d7f0099
TS: bump extractor version
asger-semmle Mar 7, 2019
16a2177
JS: update tests
asger-semmle Mar 11, 2019
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
24 changes: 23 additions & 1 deletion javascript/extractor/lib/typescript/src/type_table.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,28 @@ function isTypeVariable(type: ts.Type): type is ts.TypeVariable {
return (type.flags & ts.TypeFlags.TypeVariable) !== 0;
}

/**
* Returns `true` if the properties of the given type can safely be extracted
* without restricting expansion depth.
*
* This predicate is very approximate, and considers all unions, intersections,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very approximate indeed.
Maybe we should look into special casing on some common cases later.

Copy link
Contributor Author

@asger-semmle asger-semmle Mar 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't be that bad. The structure of a type is still extracted, this just determines whether its properties are. So for a type like Buffer | string, we can still tell it's a Buffer | string, but we won't necessarily compute the members of the union itself (i.e. the intersection of members on Buffer and string).

Also, members are always computed for types directly referenced from the AST, for example, if some expression is inferred to have the type A & B, that type is fully extracted, members and all, but A and B themselves might be shallow.

(edited for a better example)

* named types, and mapped types as potentially unsafe.
*/
function isTypeAlwaysSafeToExpand(type: ts.Type): boolean {
let flags = type.flags;
if (flags & ts.TypeFlags.UnionOrIntersection) {
return false;
}
if (flags & ts.TypeFlags.Object) {
let objectType = type as ts.ObjectType;
let objectFlags = objectType.objectFlags;
if (objectFlags & (ts.ObjectFlags.Reference | ts.ObjectFlags.Mapped)) {
return false;
}
}
return true;
}

/**
* If `type` is a `this` type, returns the enclosing type.
* Otherwise returns `null`.
Expand Down Expand Up @@ -784,7 +806,7 @@ export class TypeTable {
while (worklist.length > 0) {
let [type, id] = worklist.pop();
let isShallowContext = typeExtractionState[id] === TypeExtractionState.PendingShallow;
if (isShallowContext && isTypeReference(type)) {
if (isShallowContext && !isTypeAlwaysSafeToExpand(type)) {
typeExtractionState[id] = TypeExtractionState.DoneShallow;
} else {
typeExtractionState[id] = TypeExtractionState.DoneFull;
Expand Down
2 changes: 1 addition & 1 deletion javascript/extractor/src/com/semmle/js/extractor/Main.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public class Main {
* A version identifier that should be updated every time the extractor changes in such a way that
* it may produce different tuples for the same file under the same {@link ExtractorConfig}.
*/
public static final String EXTRACTOR_VERSION = "2019-02-27";
public static final String EXTRACTOR_VERSION = "2019-03-07";

public static final Pattern NEWLINE = Pattern.compile("\n");

Expand Down
2 changes: 0 additions & 2 deletions javascript/ql/test/library-tests/TypeScript/ArrayTypes/ArrayTypes.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
| (T \| ConcatArray<T>)[] | `T \| ConcatArray<T>` |
| (number \| ConcatArray<number>)[] | `number \| ConcatArray<number>` |
| (string \| ConcatArray<string>)[] | `string \| ConcatArray<string>` |
| (string \| number \| ConcatArray<string \| number>)[] | `string \| number \| ConcatArray<string \| number>` |
| (string \| number)[] | `string \| number` |
| ConcatArray<T>[] | `ConcatArray<T>` |
| ConcatArray<number>[] | `ConcatArray<number>` |
| ConcatArray<string \| number>[] | `ConcatArray<string \| number>` |
| ConcatArray<string>[] | `ConcatArray<string>` |
| ReadonlyArray<T> | `T` |
| ReadonlyArray<number> | `number` |
| S[] | `S` |
Expand Down
4 changes: 0 additions & 4 deletions javascript/ql/test/library-tests/TypeScript/ArrayTypes/NumberIndexTypes.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
| (T \| ConcatArray<T>)[] | T \| ConcatArray<T> |
| (number \| ConcatArray<number>)[] | number \| ConcatArray<number> |
| (string \| ConcatArray<string>)[] | string \| ConcatArray<string> |
| (string \| number \| ConcatArray<string \| number>)[] | string \| number \| ConcatArray<string \| number> |
| (string \| number)[] | string \| number |
| ConcatArray<T>[] | ConcatArray<T> |
| ConcatArray<number>[] | ConcatArray<number> |
| ConcatArray<string \| number>[] | ConcatArray<string \| number> |
| ConcatArray<string>[] | ConcatArray<string> |
| NumberIndexable | object |
| ReadonlyArray<T> | T |
| ReadonlyArray<number> | number |
Expand All @@ -17,6 +15,4 @@
| any[] | any |
| number[] | number |
| string | string |
| string \| ConcatArray<string> | string |
| string \| string[] | string |
| string[] | string |
2 changes: 0 additions & 2 deletions javascript/ql/test/library-tests/TypeScript/BaseTypes/SelfTypes.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
| CStringSub | CStringSub |
| CSub | CSub |
| CollatorOptions | CollatorOptions |
| Function | Function |
| IBase | IBase |
| IEmpty | IEmpty |
| IEmptySub | IEmptySub |
Expand All @@ -18,6 +17,5 @@
| IStringSub | IStringSub |
| ISub | ISub |
| NumberFormatOptions | NumberFormatOptions |
| Object | Object |
| RegExp | RegExp |
| RegExpMatchArray | RegExpMatchArray |
2 changes: 0 additions & 2 deletions javascript/ql/test/library-tests/TypeScript/ExpansiveTypes/NonExpansiveTypes.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@
| ExpansiveSignature in expansive_signature.ts | has properties |
| ExpansiveSignatureTypeBound in expansive_signature.ts | has properties |
| ExpansiveX in used_from_expansion.ts | has properties |
| Function in global scope | has properties |
| Intl.CollatorOptions in global scope | has properties |
| Intl.NumberFormatOptions in global scope | has properties |
| NonExpansive in shared_non_expansive.ts | has properties |
| Object in global scope | has properties |
2 changes: 0 additions & 2 deletions javascript/ql/test/library-tests/TypeScript/ExternalTypes/GlobalQualifiedNames.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
| Function | Function |
| Intl.CollatorOptions | CollatorOptions |
| Intl.NumberFormatOptions | NumberFormatOptions |
| LegacyGlobals.LegacySubclass | LegacySubclass |
| Modern.ModernClass | ModernClass |
| ModernGlobals.ModernSubclass | ModernSubclass |
| Object | Object |
| RegExp | RegExp |
| RegExpMatchArray | RegExpMatchArray |
| __Legacy.LegacyClass | LegacyClass |
2 changes: 0 additions & 2 deletions javascript/ql/test/library-tests/TypeScript/ExternalTypes/Types.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@
| CollatorOptions | has no definition |
| ExternalType1 | has no definition |
| ExternalType2 | has no definition |
| Function | has no definition |
| InternalType | defined in client_esmodule.ts |
| LegacyClass | has no definition |
| LegacySubclass | has no definition |
| ModernClass | has no definition |
| ModernSubclass | has no definition |
| NumberFormatOptions | has no definition |
| Object | has no definition |
| OtherClass | has no definition |
| RegExp | has no definition |
| RegExpMatchArray | has no definition |
Expand Down
8 changes: 8 additions & 0 deletions javascript/ql/test/library-tests/TypeScript/InfiniteTypes/recursiveMappedType.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
interface X {
a: RecursiveMappedType<this> & X;
b: boolean;
}

type RecursiveMappedType<V> = {
[P in keyof V]?: X & RecursiveMappedType<V[P]>
}
2 changes: 2 additions & 0 deletions javascript/ql/test/library-tests/TypeScript/InfiniteTypes/test.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
| recursiveMappedType.ts:2:5:2:5 | a | RecursiveMappedType<this> & X |
| recursiveMappedType.ts:3:5:3:5 | b | boolean |
4 changes: 4 additions & 0 deletions javascript/ql/test/library-tests/TypeScript/InfiniteTypes/test.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import javascript

from Expr e
select e, e.getType()
3 changes: 3 additions & 0 deletions javascript/ql/test/library-tests/TypeScript/InfiniteTypes/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"include": ["."]
}