Skip to content
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

Cache accessibe symbol chains and serialized type parameter name generation #43973

Merged
merged 5 commits into from
May 12, 2021
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
113 changes: 98 additions & 15 deletions src/compiler/checker.ts

Large diffs are not rendered by default.

7 changes: 5 additions & 2 deletions src/compiler/transformers/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,10 @@ namespace ts {
symbolAccessibilityResult.errorSymbolName,
symbolAccessibilityResult.errorModuleName));
}
return true;
}
}
return false;
}

function trackExternalModuleSymbolOfImportTypeNode(symbol: Symbol) {
Expand All @@ -156,9 +158,10 @@ namespace ts {
}

function trackSymbol(symbol: Symbol, enclosingDeclaration?: Node, meaning?: SymbolFlags) {
if (symbol.flags & SymbolFlags.TypeParameter) return;
handleSymbolAccessibilityError(resolver.isSymbolAccessible(symbol, enclosingDeclaration, meaning, /*shouldComputeAliasesToMakeVisible*/ true));
if (symbol.flags & SymbolFlags.TypeParameter) return false;
const issuedDiagnostic = handleSymbolAccessibilityError(resolver.isSymbolAccessible(symbol, enclosingDeclaration, meaning, /*shouldComputeAliasesToMakeVisible*/ true));
recordTypeReferenceDirectivesIfNecessary(resolver.getTypeReferenceDirectivesForSymbol(symbol, meaning));
return issuedDiagnostic;
}

function reportPrivateInBaseOfClassExpression(propertyName: string) {
Expand Down
4 changes: 3 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4831,6 +4831,7 @@ namespace ts {
typeOnlyDeclaration?: TypeOnlyCompatibleAliasDeclaration | false; // First resolved alias declaration that makes the symbol only usable in type constructs
isConstructorDeclaredProperty?: boolean; // Property declared through 'this.x = ...' assignment in constructor
tupleLabelDeclaration?: NamedTupleMember | ParameterDeclaration; // Declaration associated with the tuple's label
accessibleChainCache?: ESMap<string, Symbol[] | undefined>;
}

/* @internal */
Expand Down Expand Up @@ -4986,6 +4987,7 @@ namespace ts {
isExhaustive?: boolean; // Is node an exhaustive switch statement
skipDirectInference?: true; // Flag set by the API `getContextualType` call on a node when `Completions` is passed to force the checker to skip making inferences to a node's type
declarationRequiresScopeChange?: boolean; // Set by `useOuterVariableScopeInParameter` in checker when downlevel emit would change the name resolution scope inside of a parameter.
serializedTypes?: ESMap<string, TypeNode & {truncating?: boolean, addedLength: number}>; // Collection of types serialized at this location
}

export const enum TypeFlags {
Expand Down Expand Up @@ -8118,7 +8120,7 @@ namespace ts {
// Called when the symbol writer encounters a symbol to write. Currently only used by the
// declaration emitter to help determine if it should patch up the final declaration file
// with import statements it previously saw (but chose not to emit).
trackSymbol?(symbol: Symbol, enclosingDeclaration: Node | undefined, meaning: SymbolFlags): void;
trackSymbol?(symbol: Symbol, enclosingDeclaration: Node | undefined, meaning: SymbolFlags): boolean;
reportInaccessibleThisError?(): void;
reportPrivateInBaseOfClassExpression?(propertyName: string): void;
reportInaccessibleUniqueSymbolError?(): void;
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ namespace ts {
increaseIndent: noop,
decreaseIndent: noop,
clear: () => str = "",
trackSymbol: noop,
trackSymbol: () => false,
reportInaccessibleThisError: noop,
reportInaccessibleUniqueSymbolError: noop,
reportPrivateInBaseOfClassExpression: noop,
Expand Down Expand Up @@ -4029,7 +4029,7 @@ namespace ts {
reportInaccessibleThisError: noop,
reportPrivateInBaseOfClassExpression: noop,
reportInaccessibleUniqueSymbolError: noop,
trackSymbol: noop,
trackSymbol: () => false,
writeKeyword: write,
writeOperator: write,
writeParameter: write,
Expand Down
6 changes: 5 additions & 1 deletion src/harness/compilerImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,11 @@ namespace compiler {
if (compilerOptions.skipDefaultLibCheck === undefined) compilerOptions.skipDefaultLibCheck = true;
if (compilerOptions.noErrorTruncation === undefined) compilerOptions.noErrorTruncation = true;

const preProgram = ts.length(rootFiles) < 100 ? ts.createProgram(rootFiles || [], { ...compilerOptions, configFile: compilerOptions.configFile, traceResolution: false }, host) : undefined;
// pre-emit/post-emit error comparison requires declaration emit twice, which can be slow. If it's unlikely to flag any error consistency issues
// and if the test is running `skipLibCheck` - an indicator that we want the tets to run quickly - skip the before/after error comparison, too
const skipErrorComparison = ts.length(rootFiles) >= 100 || (!!compilerOptions.skipLibCheck && !!compilerOptions.declaration);
Copy link
Member Author

Choose a reason for hiding this comment

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

Declaration emit for this test still takes 5s on my machine. getPreEmitDiagnostics invokes declaration emit to get declaration emit errors (for historical reasons they've been considered "preEmit" errors), meaning declaration emit had to run twice, so 10s. Single-threaded, this was fine, but in high resource contention scenarios (ie, runtests-parallel) this was long enough to stretch to 40s and timeout. So I added an opt-out case to the preEmit/preEmit-after-emit error consistency check to keep this test runtime low (so it could pass without a timeout when running in parallel, and probably on CI).


const preProgram = !skipErrorComparison ? ts.createProgram(rootFiles || [], { ...compilerOptions, configFile: compilerOptions.configFile, traceResolution: false }, host) : undefined;
const preErrors = preProgram && ts.getPreEmitDiagnostics(preProgram);

const program = ts.createProgram(rootFiles || [], compilerOptions, host);
Expand Down
2 changes: 1 addition & 1 deletion src/services/codefixes/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace ts.codefix {

export function getNoopSymbolTrackerWithResolver(context: TypeConstructionContext): SymbolTracker {
return {
trackSymbol: noop,
trackSymbol: () => false,
moduleResolverHost: getModuleSpecifierResolverHost(context.program, context.host),
};
}
Expand Down
3 changes: 2 additions & 1 deletion src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2093,7 +2093,7 @@ namespace ts {
increaseIndent: () => { indent++; },
decreaseIndent: () => { indent--; },
clear: resetWriter,
trackSymbol: noop,
trackSymbol: () => false,
reportInaccessibleThisError: noop,
reportInaccessibleUniqueSymbolError: noop,
reportPrivateInBaseOfClassExpression: noop,
Expand Down Expand Up @@ -2619,6 +2619,7 @@ namespace ts {
const res = checker.typeToTypeNode(type, enclosingScope, NodeBuilderFlags.NoTruncation, {
trackSymbol: (symbol, declaration, meaning) => {
typeIsAccessible = typeIsAccessible && checker.isSymbolAccessible(symbol, declaration, meaning, /*shouldComputeAliasToMarkVisible*/ false).accessibility === SymbolAccessibility.Accessible;
return !typeIsAccessible;
},
reportInaccessibleThisError: notAccessible,
reportPrivateInBaseOfClassExpression: notAccessible,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
tests/cases/compiler/Api.ts(6,5): error TS7056: The inferred type of this node exceeds the maximum length the compiler will serialize. An explicit type annotation is needed.
tests/cases/compiler/Api.ts(7,5): error TS7056: The inferred type of this node exceeds the maximum length the compiler will serialize. An explicit type annotation is needed.
tests/cases/compiler/Api.ts(8,5): error TS7056: The inferred type of this node exceeds the maximum length the compiler will serialize. An explicit type annotation is needed.


==== tests/cases/compiler/http-client.ts (0 errors) ====
type TPromise<ResolveType, RejectType = any> = Omit<Promise<ResolveType>, "then" | "catch"> & {
then<TResult1 = ResolveType, TResult2 = never>(
onfulfilled?: ((value: ResolveType) => TResult1 | PromiseLike<TResult1>) | undefined | null,
onrejected?: ((reason: RejectType) => TResult2 | PromiseLike<TResult2>) | undefined | null,
): TPromise<TResult1 | TResult2, RejectType>;
catch<TResult = never>(
onrejected?: ((reason: RejectType) => TResult | PromiseLike<TResult>) | undefined | null,
): TPromise<ResolveType | TResult, RejectType>;
};

export interface HttpResponse<D extends unknown, E extends unknown = unknown> extends Response {
data: D;
error: E;
}

export class HttpClient<SecurityDataType = unknown> {
public request = <T = any, E = any>(): TPromise<HttpResponse<T, E>> => {
return '' as any;
};
}
==== tests/cases/compiler/Api.ts (3 errors) ====
import { HttpClient } from "./http-client";

export class Api<SecurityDataType = unknown> {
constructor(private http: HttpClient<SecurityDataType>) { }

abc1 = () => this.http.request();
~~~~
!!! error TS7056: The inferred type of this node exceeds the maximum length the compiler will serialize. An explicit type annotation is needed.
abc2 = () => this.http.request();
~~~~
!!! error TS7056: The inferred type of this node exceeds the maximum length the compiler will serialize. An explicit type annotation is needed.
abc3 = () => this.http.request();
~~~~
!!! error TS7056: The inferred type of this node exceeds the maximum length the compiler will serialize. An explicit type annotation is needed.
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
//// [tests/cases/compiler/declarationEmitPrivatePromiseLikeInterface.ts] ////

//// [http-client.ts]
type TPromise<ResolveType, RejectType = any> = Omit<Promise<ResolveType>, "then" | "catch"> & {
then<TResult1 = ResolveType, TResult2 = never>(
onfulfilled?: ((value: ResolveType) => TResult1 | PromiseLike<TResult1>) | undefined | null,
onrejected?: ((reason: RejectType) => TResult2 | PromiseLike<TResult2>) | undefined | null,
): TPromise<TResult1 | TResult2, RejectType>;
catch<TResult = never>(
onrejected?: ((reason: RejectType) => TResult | PromiseLike<TResult>) | undefined | null,
): TPromise<ResolveType | TResult, RejectType>;
};

export interface HttpResponse<D extends unknown, E extends unknown = unknown> extends Response {
data: D;
error: E;
}

export class HttpClient<SecurityDataType = unknown> {
public request = <T = any, E = any>(): TPromise<HttpResponse<T, E>> => {
return '' as any;
};
}
//// [Api.ts]
import { HttpClient } from "./http-client";

export class Api<SecurityDataType = unknown> {
constructor(private http: HttpClient<SecurityDataType>) { }

abc1 = () => this.http.request();
abc2 = () => this.http.request();
abc3 = () => this.http.request();
}

//// [http-client.js]
"use strict";
exports.__esModule = true;
exports.HttpClient = void 0;
var HttpClient = /** @class */ (function () {
function HttpClient() {
this.request = function () {
return '';
};
}
return HttpClient;
}());
exports.HttpClient = HttpClient;
//// [Api.js]
"use strict";
exports.__esModule = true;
exports.Api = void 0;
var Api = /** @class */ (function () {
function Api(http) {
var _this = this;
this.http = http;
this.abc1 = function () { return _this.http.request(); };
this.abc2 = function () { return _this.http.request(); };
this.abc3 = function () { return _this.http.request(); };
}
return Api;
}());
exports.Api = Api;


//// [http-client.d.ts]
declare type TPromise<ResolveType, RejectType = any> = Omit<Promise<ResolveType>, "then" | "catch"> & {
then<TResult1 = ResolveType, TResult2 = never>(onfulfilled?: ((value: ResolveType) => TResult1 | PromiseLike<TResult1>) | undefined | null, onrejected?: ((reason: RejectType) => TResult2 | PromiseLike<TResult2>) | undefined | null): TPromise<TResult1 | TResult2, RejectType>;
catch<TResult = never>(onrejected?: ((reason: RejectType) => TResult | PromiseLike<TResult>) | undefined | null): TPromise<ResolveType | TResult, RejectType>;
};
export interface HttpResponse<D extends unknown, E extends unknown = unknown> extends Response {
data: D;
error: E;
}
export declare class HttpClient<SecurityDataType = unknown> {
request: <T = any, E = any>() => TPromise<HttpResponse<T, E>, any>;
}
export {};
Loading