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.

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

getTypeChecker returns the same checker manufactured for diagnostics … #28584

Closed
wants to merge 3 commits into from
Closed

getTypeChecker returns the same checker manufactured for diagnostics … #28584

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
bb975af
getTypeChecker returns the same checker manufactured for diagnostics …
weswigham Nov 16, 2018
c2cae1d
Drop (heh) vestigial API
weswigham Nov 17, 2018
3c8792b
Merge branch 'master' into use-diagnostics-producing-checker-if-manuf…
weswigham Feb 11, 2020
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
7 changes: 1 addition & 6 deletions src/compiler/program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,6 @@ namespace ts {
getResolvedTypeReferenceDirectives: () => resolvedTypeReferenceDirectives,
isSourceFileFromExternalLibrary,
isSourceFileDefaultLibrary,
dropDiagnosticsProducingTypeChecker,
getSourceFileFromReference,
getLibFileFromReference,
sourceFileToPackageName,
Expand Down Expand Up @@ -1558,12 +1557,8 @@ namespace ts {
return diagnosticsProducingTypeChecker || (diagnosticsProducingTypeChecker = createTypeChecker(program, /*produceDiagnostics:*/ true));
}

function dropDiagnosticsProducingTypeChecker() {
diagnosticsProducingTypeChecker = undefined!;
}

function getTypeChecker() {
return noDiagnosticsTypeChecker || (noDiagnosticsTypeChecker = createTypeChecker(program, /*produceDiagnostics:*/ false));
return noDiagnosticsTypeChecker || (noDiagnosticsTypeChecker = diagnosticsProducingTypeChecker || createTypeChecker(program, /*produceDiagnostics:*/ false));
Copy link
Member

Choose a reason for hiding this comment

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

no.. no.. no.. There is potential that the diagnostic producing checker was initialized and later was thrown away because there was some kind of cancellation. That makes it even harder to guarantee you have correct type checker or same type checker.

Copy link
Member Author

@weswigham weswigham Nov 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

runWithCancellationToken resets both anyway (for exactly this reason AFAIK).

Copy link
Member Author

Choose a reason for hiding this comment

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

dropDiagnosticsProducingTypeChecker is the only thing that could otherwise prematurely drop the checker; but despite it's presence in our internal API, it's never actually called anywhere (vestigial API?)

Copy link
Member Author

Choose a reason for hiding this comment

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

dropDiagnosticsProducingTypeChecker is now gone (it was unused) and runWithCancellationToken specifically resets both to avoid a situation like this, so I think this is fine now.

}

function emit(sourceFile?: SourceFile, writeFileCallback?: WriteFileCallback, cancellationToken?: CancellationToken, emitOnlyDtsFiles?: boolean, transformers?: CustomTransformers, forceDtsEmit?: boolean): EmitResult {
Expand Down
1 change: 0 additions & 1 deletion src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3222,7 +3222,6 @@ namespace ts {
// For testing purposes only. Should not be used by any other consumers (including the
// language service).
/* @internal */ getDiagnosticsProducingTypeChecker(): TypeChecker;
/* @internal */ dropDiagnosticsProducingTypeChecker(): void;

/* @internal */ getClassifiableNames(): UnderscoreEscapedMap<true>;

Expand Down
2 changes: 1 addition & 1 deletion src/harness/harnessIO.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ namespace Harness {
// These types are equivalent, but depend on what order the compiler observed
// certain parts of the program.

const fullWalker = new TypeWriterWalker(program, /*fullTypeCheck*/ true, !!hasErrorBaseline);
const fullWalker = new TypeWriterWalker(program, !!hasErrorBaseline);

// Produce baselines. The first gives the types for all expressions.
// The second gives symbols for all identifiers.
Expand Down
6 changes: 2 additions & 4 deletions src/harness/typeWriter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,10 @@ namespace Harness {

private checker: ts.TypeChecker;

constructor(private program: ts.Program, fullTypeCheck: boolean, private hadErrorBaseline: boolean) {
constructor(private program: ts.Program, private hadErrorBaseline: boolean) {
// Consider getting both the diagnostics checker and the non-diagnostics checker to verify
// they are consistent.
this.checker = fullTypeCheck
? program.getDiagnosticsProducingTypeChecker()
: program.getTypeChecker();
this.checker = program.getDiagnosticsProducingTypeChecker();
}

public *getSymbols(fileName: string): IterableIterator<TypeWriterSymbolResult> {
Expand Down