Skip to content

Minor cleanup to symbolWalker #18549

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
2 commits merged into from
Sep 22, 2017
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
16 changes: 11 additions & 5 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -994,11 +994,6 @@ namespace ts {

/**
* Gets the owned, enumerable property keys of a map-like.
*
* NOTE: This is intended for use with MapLike<T> objects. For Map<T> objects, use
Copy link
Author

Choose a reason for hiding this comment

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

Comment outdated; Map<T> is now a real ES6 map, so we would use the .keys() method.

* Object.keys instead as it offers better performance.
*
* @param map A map-like.
*/
export function getOwnKeys<T>(map: MapLike<T>): string[] {
const keys: string[] = [];
Expand All @@ -1011,6 +1006,17 @@ namespace ts {
return keys;
}

export function getOwnValues<T>(sparseArray: T[]): T[] {
const values: T[] = [];
for (const key in sparseArray) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm sure I'm missing something, but how is this different from a for-of loop?

Copy link
Author

Choose a reason for hiding this comment

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

A for-in loop iterates over the keys of an object, while a for-of loop only works on an object implementing the iterator protocol. See MDN

Copy link
Member

@weswigham weswigham Sep 18, 2017

Choose a reason for hiding this comment

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

It's also supposedly identical to Object.values, which, much like hasOwnProperty, which we should probably check for the existence of and use if available.

if (hasOwnProperty.call(sparseArray, key)) {
values.push(sparseArray[key]);
}
}

return values;
}

/** Shims `Array.from`. */
export function arrayFrom<T, U>(iterator: Iterator<T>, map: (t: T) => U): U[];
export function arrayFrom<T>(iterator: Iterator<T>): T[];
Expand Down
58 changes: 28 additions & 30 deletions src/compiler/symbolWalker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,29 @@ namespace ts {
return getSymbolWalker;

function getSymbolWalker(accept: (symbol: Symbol) => boolean = () => true): SymbolWalker {
const visitedTypes = createMap<Type>(); // Key is id as string
const visitedSymbols = createMap<Symbol>(); // Key is id as string
const visitedTypes: Type[] = []; // Sparse array from id to type
const visitedSymbols: Symbol[] = []; // Sparse array from id to symbol

return {
walkType: type => {
visitedTypes.clear();
visitedSymbols.clear();
visitType(type);
return { visitedTypes: arrayFrom(visitedTypes.values()), visitedSymbols: arrayFrom(visitedSymbols.values()) };
try {
visitType(type);
return { visitedTypes: getOwnValues(visitedTypes), visitedSymbols: getOwnValues(visitedSymbols) };
}
finally {
clear(visitedTypes);
clear(visitedSymbols);
}
},
walkSymbol: symbol => {
visitedTypes.clear();
visitedSymbols.clear();
visitSymbol(symbol);
return { visitedTypes: arrayFrom(visitedTypes.values()), visitedSymbols: arrayFrom(visitedSymbols.values()) };
try {
visitSymbol(symbol);
return { visitedTypes: getOwnValues(visitedTypes), visitedSymbols: getOwnValues(visitedSymbols) };
}
finally {
clear(visitedTypes);
clear(visitedSymbols);
}
},
};

Expand All @@ -37,11 +45,10 @@ namespace ts {
return;
}

const typeIdString = type.id.toString();
if (visitedTypes.has(typeIdString)) {
if (visitedTypes[type.id]) {
return;
}
visitedTypes.set(typeIdString, type);
visitedTypes[type.id] = type;

// Reuse visitSymbol to visit the type's symbol,
// but be sure to bail on recuring into the type if accept declines the symbol.
Expand Down Expand Up @@ -79,26 +86,17 @@ namespace ts {
}
}

function visitTypeList(types: Type[]): void {
if (!types) {
return;
}
for (let i = 0; i < types.length; i++) {
visitType(types[i]);
}
}

function visitTypeReference(type: TypeReference): void {
visitType(type.target);
visitTypeList(type.typeArguments);
forEach(type.typeArguments, visitType);
}

function visitTypeParameter(type: TypeParameter): void {
visitType(getConstraintFromTypeParameter(type));
}

function visitUnionOrIntersectionType(type: UnionOrIntersectionType): void {
visitTypeList(type.types);
forEach(type.types, visitType);
}

function visitIndexType(type: IndexType): void {
Expand All @@ -122,7 +120,7 @@ namespace ts {
if (signature.typePredicate) {
visitType(signature.typePredicate.type);
}
visitTypeList(signature.typeParameters);
forEach(signature.typeParameters, visitType);

for (const parameter of signature.parameters){
visitSymbol(parameter);
Expand All @@ -133,8 +131,8 @@ namespace ts {

function visitInterfaceType(interfaceT: InterfaceType): void {
visitObjectType(interfaceT);
visitTypeList(interfaceT.typeParameters);
visitTypeList(getBaseTypes(interfaceT));
forEach(interfaceT.typeParameters, visitType);
forEach(getBaseTypes(interfaceT), visitType);
visitType(interfaceT.thisType);
}

Expand All @@ -161,11 +159,11 @@ namespace ts {
if (!symbol) {
return;
}
const symbolIdString = getSymbolId(symbol).toString();
if (visitedSymbols.has(symbolIdString)) {
const symbolId = getSymbolId(symbol);
if (visitedSymbols[symbolId]) {
return;
}
visitedSymbols.set(symbolIdString, symbol);
visitedSymbols[symbolId] = symbol;
if (!accept(symbol)) {
return true;
}
Expand Down