From 6c9b2d9de91bdae20186b609a9b293ca22afd6e4 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Tue, 23 Jan 2018 15:48:48 -0800 Subject: [PATCH 1/6] Check syntax kind in isDeclarationNameOrImportPropertyName Otherwise, keywords are accepted. Assert was ``` fail (e:\ts_gh\src\compiler\core.ts:2867) assert (e:\ts_gh\src\compiler\core.ts:2837) isExportSpecifierAlias (e:\ts_gh\src\services\findAllReferences.ts:931) getLocalSymbolForExportSpecifier (e:\ts_gh\src\services\findAllReferences.ts:926) skipPastExportOrImportSpecifierOrUnion (e:\ts_gh\src\services\findAllReferences.ts:411) getReferencedSymbolsForSymbol (e:\ts_gh\src\services\findAllReferences.ts:359) getReferencedSymbolsForNode (e:\ts_gh\src\services\findAllReferences.ts:271) getReferenceEntriesForNode (e:\ts_gh\src\services\findAllReferences.ts:91) getSemanticDocumentHighlights (e:\ts_gh\src\services\documentHighlights.ts:25) getDocumentHighlights (e:\ts_gh\src\services\documentHighlights.ts:13) getDocumentHighlights (e:\ts_gh\src\services\services.ts:1584) getOccurrencesAtPositionCore (e:\ts_gh\src\services\services.ts:1588) getOccurrencesAtPosition (e:\ts_gh\src\services\services.ts:1567) Session.getOccurrences (e:\ts_gh\src\server\session.ts:718) Session.handlers.ts.createMapFromTemplate._a.(anonymous function) (e:\ts_gh\src\server\session.ts:1999) (anonymous function) (e:\ts_gh\src\server\session.ts:2079) Session.executeWithRequestId (e:\ts_gh\src\server\session.ts:2069) Session.executeCommand (e:\ts_gh\src\server\session.ts:2079) Session.onMessage (e:\ts_gh\src\server\session.ts:2101) (anonymous function) (e:\ts_gh\src\server\server.ts:593) ``` --- src/compiler/checker.ts | 2 +- src/harness/fourslash.ts | 13 +++++++++++++ tests/cases/fourslash/documentHighlightInExport1.ts | 11 +++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/documentHighlightInExport1.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 01b3a047d1d..c58b69252fc 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -26580,7 +26580,7 @@ namespace ts { switch (name.parent.kind) { case SyntaxKind.ImportSpecifier: case SyntaxKind.ExportSpecifier: - return true; + return isIdentifier(name); default: return isDeclarationName(name); } diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index bf5f46de248..79ddf0ace40 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2876,6 +2876,15 @@ Actual: ${stringify(fullActual)}`); } } + public verifyNoDocumentHighlights(startRange: Range) { + this.goToRangeStart(startRange); + const documentHighlights = this.getDocumentHighlightsAtCurrentPosition([this.activeFile.fileName]); + const numHighlights = ts.length(documentHighlights); + if (numHighlights > 0) { + this.raiseError(`verifyNoDocumentHighlights failed - unexpectedly got ${numHighlights} highlights`); + } + } + private verifyDocumentHighlights(expectedRanges: Range[], fileNames: string[] = [this.activeFile.fileName]) { const documentHighlights = this.getDocumentHighlightsAtCurrentPosition(fileNames) || []; @@ -4284,6 +4293,10 @@ namespace FourSlashInterface { this.state.verifyDocumentHighlightsOf(startRange, ranges); } + public noDocumentHighlights(startRange: FourSlash.Range) { + this.state.verifyNoDocumentHighlights(startRange); + } + public completionEntryDetailIs(entryName: string, text: string, documentation?: string, kind?: string, tags?: ts.JSDocTagInfo[]) { this.state.verifyCompletionEntryDetails(entryName, text, documentation, kind, tags); } diff --git a/tests/cases/fourslash/documentHighlightInExport1.ts b/tests/cases/fourslash/documentHighlightInExport1.ts new file mode 100644 index 00000000000..16b5261c5e0 --- /dev/null +++ b/tests/cases/fourslash/documentHighlightInExport1.ts @@ -0,0 +1,11 @@ +/// + +// @Filename: file1.ts +//// class [|C|] {} +//// [|export|] { [|C|] [|as|] [|D|] }; + +const [classRange, exportKeywordRange, propertyNameRange, asKeywordRange, nameRange] = test.ranges(); +verify.noDocumentHighlights(exportKeywordRange); +verify.documentHighlightsOf(propertyNameRange, [classRange, propertyNameRange, nameRange]); +verify.noDocumentHighlights(asKeywordRange); +verify.documentHighlightsOf(nameRange, [nameRange]); From cd0000110d0c13ed3a38f022204421289f97dbc7 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Wed, 31 Jan 2018 11:12:09 -0800 Subject: [PATCH 2/6] Add FAR test --- tests/cases/fourslash/documentHighlightInExport1.ts | 1 - tests/cases/fourslash/findAllRefsInExport1.ts | 10 ++++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/findAllRefsInExport1.ts diff --git a/tests/cases/fourslash/documentHighlightInExport1.ts b/tests/cases/fourslash/documentHighlightInExport1.ts index 16b5261c5e0..bf5bc7cfa94 100644 --- a/tests/cases/fourslash/documentHighlightInExport1.ts +++ b/tests/cases/fourslash/documentHighlightInExport1.ts @@ -1,6 +1,5 @@ /// -// @Filename: file1.ts //// class [|C|] {} //// [|export|] { [|C|] [|as|] [|D|] }; diff --git a/tests/cases/fourslash/findAllRefsInExport1.ts b/tests/cases/fourslash/findAllRefsInExport1.ts new file mode 100644 index 00000000000..02ec901177f --- /dev/null +++ b/tests/cases/fourslash/findAllRefsInExport1.ts @@ -0,0 +1,10 @@ +/// + +//// class C {} +//// /*1*/export { C /*2*/as D }; + +goTo.marker("1"); +verify.noReferences(); + +goTo.marker("2"); +verify.noReferences(); \ No newline at end of file From cf540198e69551bfccdf823f1bec1669a3cc0019 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 31 Jan 2018 16:22:27 -0800 Subject: [PATCH 3/6] Always get diagnostics when emitting irrespective of whether its declaration only emit The diagnostics reporting and expression resolution caching is quite intermingled at present. Hence when we tried to get the declaration output without getting diagnostics, the resolution for functions return expression is cached but errors arent reported Symbols arent marked as referenced. So at later time when trying to get the diagnostics since the expression resolution is cached, it doesnt even go through all checks For now get diagnostics irrespective of declaration only output to avoid this issue. Fixes #21518 --- src/compiler/checker.ts | 6 ++---- src/compiler/program.ts | 2 +- src/compiler/types.ts | 2 +- src/harness/unittests/tscWatchMode.ts | 27 +++++++++++++++++++++++++++ 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 16b2091733a..a2b0d4b823e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -749,12 +749,10 @@ namespace ts { return _jsxNamespace; } - function getEmitResolver(sourceFile: SourceFile, cancellationToken: CancellationToken, ignoreDiagnostics?: boolean) { + function getEmitResolver(sourceFile: SourceFile, cancellationToken: CancellationToken) { // Ensure we have all the type information in place for this file so that all the // emitter questions of this resolver will return the right information. - if (!ignoreDiagnostics) { - getDiagnostics(sourceFile, cancellationToken); - } + getDiagnostics(sourceFile, cancellationToken); return emitResolver; } diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 6ea50b30f6f..e334c1ce80f 100755 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -1179,7 +1179,7 @@ namespace ts { // This is because in the -out scenario all files need to be emitted, and therefore all // files need to be type checked. And the way to specify that all files need to be type // checked is to not pass the file to getEmitResolver. - const emitResolver = getDiagnosticsProducingTypeChecker().getEmitResolver((options.outFile || options.out) ? undefined : sourceFile, cancellationToken, emitOnlyDtsFiles); + const emitResolver = getDiagnosticsProducingTypeChecker().getEmitResolver((options.outFile || options.out) ? undefined : sourceFile, cancellationToken); performance.mark("beforeEmit"); diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 481b91c9276..ce2c10a8dda 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2892,7 +2892,7 @@ namespace ts { // Should not be called directly. Should only be accessed through the Program instance. /* @internal */ getDiagnostics(sourceFile?: SourceFile, cancellationToken?: CancellationToken): Diagnostic[]; /* @internal */ getGlobalDiagnostics(): Diagnostic[]; - /* @internal */ getEmitResolver(sourceFile?: SourceFile, cancellationToken?: CancellationToken, ignoreDiagnostics?: boolean): EmitResolver; + /* @internal */ getEmitResolver(sourceFile?: SourceFile, cancellationToken?: CancellationToken): EmitResolver; /* @internal */ getNodeCount(): number; /* @internal */ getIdentifierCount(): number; diff --git a/src/harness/unittests/tscWatchMode.ts b/src/harness/unittests/tscWatchMode.ts index a2b79b00dc7..6915a6ad37d 100644 --- a/src/harness/unittests/tscWatchMode.ts +++ b/src/harness/unittests/tscWatchMode.ts @@ -1086,6 +1086,33 @@ namespace ts.tscWatch { // This should be 0 host.checkTimeoutQueueLengthAndRun(0); }); + + it("shouldnt report error about unused function incorrectly when file changes from global to module", () => { + const getFileContent = (asModule: boolean) => ` + function one() {} + ${asModule ? "export " : ""}function two() { + return function three() { + one(); + } + }`; + const file: FileOrFolder = { + path: "/a/b/file.ts", + content: getFileContent(/*asModule*/ false) + }; + const files = [file, libFile]; + const host = createWatchedSystem(files); + const watch = createWatchOfFilesAndCompilerOptions([file.path], host, { + noUnusedLocals: true + }); + checkProgramActualFiles(watch(), files.map(file => file.path)); + checkOutputErrors(host, [], ExpectedOutputErrorsPosition.AfterCompilationStarting); + + file.content = getFileContent(/*asModule*/ true); + host.reloadFS(files); + host.runQueuedTimeoutCallbacks(); + checkProgramActualFiles(watch(), files.map(file => file.path)); + checkOutputErrors(host, [], ExpectedOutputErrorsPosition.AfterFileChangeDetected); + }); }); describe("tsc-watch emit with outFile or out setting", () => { From 11214b9dcd572b674204371ddc4b321102774ea7 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 31 Jan 2018 17:15:54 -0800 Subject: [PATCH 4/6] Removing the test added for cancellation during affected list since thats not possible anymore as the affected files would anyways be semantically checked This is just part missed during revert of 0b79f4a --- src/harness/unittests/builder.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/harness/unittests/builder.ts b/src/harness/unittests/builder.ts index 8808a151c04..3a275d39e86 100644 --- a/src/harness/unittests/builder.ts +++ b/src/harness/unittests/builder.ts @@ -70,13 +70,6 @@ namespace ts { // Change e.ts and verify previously b.js as well as a.js get emitted again since previous change was consumed completely but not d.ts program = updateProgramFile(program, "/e.ts", "export function bar3() { }"); assertChanges(["/b.js", "/a.js", "/e.js"]); - - // Cancel in the middle of affected files list after b.js emit - program = updateProgramFile(program, "/b.ts", "export class b { foo2() { c + 1; } }"); - assertChanges(["/b.js", "/a.js"], 1); - // Change e.ts and verify previously b.js as well as a.js get emitted again since previous change was consumed completely but not d.ts - program = updateProgramFile(program, "/e.ts", "export function bar5() { }"); - assertChanges(["/b.js", "/a.js", "/e.js"]); }); }); From df15d5b97732a863591f34ab407db48693c6b147 Mon Sep 17 00:00:00 2001 From: Andy Date: Thu, 1 Feb 2018 09:24:57 -0800 Subject: [PATCH 5/6] Don't treat class name contextToken as a completion list blocker if it is not the previousToken (#21534) --- src/services/completions.ts | 3 ++- tests/cases/fourslash/completionsKeywordsExtends.ts | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index bce1eadaf26..60dfa8aaaa2 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1921,7 +1921,8 @@ namespace ts.Completions { return isDeclarationName(contextToken) && !isJsxAttribute(contextToken.parent) // Don't block completions if we're in `class C /**/`, because we're *past* the end of the identifier and might want to complete `extends`. - && !(isClassLike(contextToken.parent) && position > previousToken.end); + // If `contextToken !== previousToken`, this is `class C ex/**/`. + && !(isClassLike(contextToken.parent) && (contextToken !== previousToken || position > previousToken.end)); } function isFunctionLikeButNotConstructor(kind: SyntaxKind) { diff --git a/tests/cases/fourslash/completionsKeywordsExtends.ts b/tests/cases/fourslash/completionsKeywordsExtends.ts index 065d30d1286..50c9b741cda 100644 --- a/tests/cases/fourslash/completionsKeywordsExtends.ts +++ b/tests/cases/fourslash/completionsKeywordsExtends.ts @@ -1,6 +1,7 @@ /// ////class C/*a*/ /*b*/ { } +////class C e/*c*/ {} // Tests that `isCompletionListBlocker` is true *at* the class name, but false *after* it. @@ -9,3 +10,6 @@ verify.completionListIsEmpty(); goTo.marker("b"); verify.completionListContains("extends"); + +goTo.marker("c"); +verify.completionListContains("extends"); From d4c84368e58bf2b3edd49e0d9ca6d08e0d2e6f6c Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 1 Feb 2018 10:07:28 -0800 Subject: [PATCH 6/6] Do not clear console in watch mode if --diagnostics or --extendedDiagnostics is specified --- src/compiler/watch.ts | 37 +++++++------ src/harness/unittests/tscWatchMode.ts | 52 +++++++++++-------- tests/baselines/reference/api/typescript.d.ts | 4 +- 3 files changed, 53 insertions(+), 40 deletions(-) diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index 790295c2254..471a9323a3d 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -31,8 +31,11 @@ namespace ts { }; } - function clearScreenIfNotWatchingForFileChanges(system: System, diagnostic: Diagnostic) { - if (system.clearScreen && diagnostic.code !== Diagnostics.Compilation_complete_Watching_for_file_changes.code) { + function clearScreenIfNotWatchingForFileChanges(system: System, diagnostic: Diagnostic, options: CompilerOptions) { + if (system.clearScreen && + diagnostic.code !== Diagnostics.Compilation_complete_Watching_for_file_changes.code && + !options.extendedDiagnostics && + !options.diagnostics) { system.clearScreen(); } } @@ -42,18 +45,18 @@ namespace ts { */ export function createWatchStatusReporter(system: System, pretty?: boolean): WatchStatusReporter { return pretty ? - (diagnostic: Diagnostic, newLine: string) => { - clearScreenIfNotWatchingForFileChanges(system, diagnostic); - let output = `[${ formatColorAndReset(new Date().toLocaleTimeString(), ForegroundColorEscapeSequences.Grey) }] `; - output += `${flattenDiagnosticMessageText(diagnostic.messageText, system.newLine)}${newLine + newLine + newLine}`; - system.write(output); - } : - (diagnostic: Diagnostic, newLine: string) => { - clearScreenIfNotWatchingForFileChanges(system, diagnostic); - let output = new Date().toLocaleTimeString() + " - "; - output += `${flattenDiagnosticMessageText(diagnostic.messageText, system.newLine)}${newLine + newLine + newLine}`; - system.write(output); - }; + (diagnostic, newLine, options) => { + clearScreenIfNotWatchingForFileChanges(system, diagnostic, options); + let output = `[${formatColorAndReset(new Date().toLocaleTimeString(), ForegroundColorEscapeSequences.Grey)}] `; + output += `${flattenDiagnosticMessageText(diagnostic.messageText, system.newLine)}${newLine + newLine + newLine}`; + system.write(output); + } : + (diagnostic, newLine, options) => { + clearScreenIfNotWatchingForFileChanges(system, diagnostic, options); + let output = new Date().toLocaleTimeString() + " - "; + output += `${flattenDiagnosticMessageText(diagnostic.messageText, system.newLine)}${newLine + newLine + newLine}`; + system.write(output); + }; } /** @@ -254,7 +257,7 @@ namespace ts { namespace ts { export type DiagnosticReporter = (diagnostic: Diagnostic) => void; - export type WatchStatusReporter = (diagnostic: Diagnostic, newLine: string) => void; + export type WatchStatusReporter = (diagnostic: Diagnostic, newLine: string, options: CompilerOptions) => void; export type CreateProgram = (rootNames: ReadonlyArray, options: CompilerOptions, host?: CompilerHost, oldProgram?: T) => T; export interface WatchCompilerHost { /** @@ -264,7 +267,7 @@ namespace ts { /** If provided, callback to invoke after every new program creation */ afterProgramCreate?(program: T): void; /** If provided, called with Diagnostic message that informs about change in watch status */ - onWatchStatusChange?(diagnostic: Diagnostic, newLine: string): void; + onWatchStatusChange?(diagnostic: Diagnostic, newLine: string, options: CompilerOptions): void; // Only for testing /*@internal*/ @@ -725,7 +728,7 @@ namespace ts { function reportWatchDiagnostic(message: DiagnosticMessage) { if (host.onWatchStatusChange) { - host.onWatchStatusChange(createCompilerDiagnostic(message), newLine); + host.onWatchStatusChange(createCompilerDiagnostic(message), newLine, compilerOptions); } } diff --git a/src/harness/unittests/tscWatchMode.ts b/src/harness/unittests/tscWatchMode.ts index 6915a6ad37d..b6d3099dc77 100644 --- a/src/harness/unittests/tscWatchMode.ts +++ b/src/harness/unittests/tscWatchMode.ts @@ -2102,35 +2102,45 @@ declare module "fs" { }); describe("tsc-watch console clearing", () => { - it("clears the console when it starts", () => { + function checkConsoleClearing(diagnostics: boolean, extendedDiagnostics: boolean) { const file = { path: "f.ts", content: "" }; - const host = createWatchedSystem([file]); + const files = [file]; + const host = createWatchedSystem(files); + let clearCount: number | undefined; + checkConsoleClears(); - createWatchOfFilesAndCompilerOptions([file.path], host); + createWatchOfFilesAndCompilerOptions([file.path], host, { diagnostics, extendedDiagnostics }); + checkConsoleClears(); + + file.content = "//"; + host.reloadFS(files); host.runQueuedTimeoutCallbacks(); - host.checkScreenClears(1); + checkConsoleClears(); + + function checkConsoleClears() { + if (clearCount === undefined) { + clearCount = 0; + } + else if (!diagnostics && !extendedDiagnostics) { + clearCount++; + } + host.checkScreenClears(clearCount); + return clearCount; + } + } + + it("without --diagnostics or --extendedDiagnostics", () => { + checkConsoleClearing(/*diagnostics*/ false, /*extendedDiagnostics*/ false); }); - - it("clears the console on recompile", () => { - const file = { - path: "f.ts", - content: "" - }; - const host = createWatchedSystem([file]); - createWatchOfFilesAndCompilerOptions([file.path], host); - - const modifiedFile = { - ...file, - content: "//" - }; - host.reloadFS([modifiedFile]); - host.runQueuedTimeoutCallbacks(); - - host.checkScreenClears(2); + it("with --diagnostics", () => { + checkConsoleClearing(/*diagnostics*/ true, /*extendedDiagnostics*/ false); + }); + it("with --extendedDiagnostics", () => { + checkConsoleClearing(/*diagnostics*/ false, /*extendedDiagnostics*/ true); }); }); } diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index bdcc29af76e..6a0a701ca1a 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -3978,7 +3978,7 @@ declare namespace ts { } declare namespace ts { type DiagnosticReporter = (diagnostic: Diagnostic) => void; - type WatchStatusReporter = (diagnostic: Diagnostic, newLine: string) => void; + type WatchStatusReporter = (diagnostic: Diagnostic, newLine: string, options: CompilerOptions) => void; type CreateProgram = (rootNames: ReadonlyArray, options: CompilerOptions, host?: CompilerHost, oldProgram?: T) => T; interface WatchCompilerHost { /** @@ -3988,7 +3988,7 @@ declare namespace ts { /** If provided, callback to invoke after every new program creation */ afterProgramCreate?(program: T): void; /** If provided, called with Diagnostic message that informs about change in watch status */ - onWatchStatusChange?(diagnostic: Diagnostic, newLine: string): void; + onWatchStatusChange?(diagnostic: Diagnostic, newLine: string, options: CompilerOptions): void; useCaseSensitiveFileNames(): boolean; getNewLine(): string; getCurrentDirectory(): string;