From 46e7ccae7c9bc89d9004466f7c0c0d828b04d016 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 13 Jun 2018 14:14:38 -0700 Subject: [PATCH 1/6] Test for amd resolution setting the recursive directory watcher in the parents of root folder --- .../unittests/tsserverProjectSystem.ts | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index 04720fe42af..262ccdce645 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -8506,6 +8506,66 @@ new C();` } }); }); + + it("when watching directories for failed lookup locations in amd resolution", () => { + const projectRoot = "/user/username/projects/project"; + const nodeFile: File = { + path: `${projectRoot}/src/typings/node.d.ts`, + content: ` +declare module "fs" { + export interface something { + } +}` + }; + const electronFile: File = { + path: `${projectRoot}/src/typings/electron.d.ts`, + content: ` +declare module 'original-fs' { + import * as fs from 'fs'; + export = fs; +}` + }; + const srcFile: File = { + path: `${projectRoot}/src/somefolder/srcfile.ts`, + content: ` +import { x } from "somefolder/module1"; +import { x } from "somefolder/module2"; +const y = x;` + }; + const moduleFile: File = { + path: `${projectRoot}/src/somefolder/module1.ts`, + content: ` +export const x = 10;` + }; + const configFile: File = { + path: `${projectRoot}/src/tsconfig.json`, + content: JSON.stringify({ + compilerOptions: { + module: "amd", + moduleResolution: "classic", + target: "es5", + outDir: "../out", + baseUrl: "./", + typeRoots: ["typings"] + + } + }) + }; + const files = [nodeFile, electronFile, srcFile, moduleFile, configFile, libFile]; + const host = createServerHost(files); + const service = createProjectService(host); + service.openClientFile(srcFile.path, srcFile.content, ScriptKind.TS, projectRoot); + checkProjectActualFiles(service.configuredProjects.get(configFile.path)!, files.map(f => f.path)); + checkWatchedFilesDetailed(host, mapDefined(files, f => f === srcFile ? undefined : f.path), 1); + checkWatchedDirectories(host, emptyArray, /*recursive*/ false); + const expectedWatchedDirectories = createMap(); + expectedWatchedDirectories.set(`${projectRoot}/src`, 2); // Wild card and failed lookup + expectedWatchedDirectories.set(`${projectRoot}/somefolder`, 1); // failed lookup for somefolder/module2 + expectedWatchedDirectories.set(`${projectRoot}`, 1); // failed lookup for fs + expectedWatchedDirectories.set(`${projectRoot}/node_modules`, 1); // failed lookup for with node_modules/@types/fs + expectedWatchedDirectories.set(`${projectRoot}/src/typings`, 1); // typeroot directory + checkWatchedDirectoriesDetailed(host, expectedWatchedDirectories, /*recursive*/ true); + }); }); describe("tsserverProjectSystem watchDirectories implementation", () => { From cd97adbf3bd6e3fc10202b68ae5f3d38930c1b35 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 13 Jun 2018 14:19:45 -0700 Subject: [PATCH 2/6] Do not watch the parent folders recursively if not needed. This avoids watching folders like parent folder of the project root just to watch files created in the folder Fixes Microsoft/vscode#51139 --- src/compiler/resolutionCache.ts | 25 ++++++++++++------- .../unittests/tsserverProjectSystem.ts | 16 ++++++------ 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/compiler/resolutionCache.ts b/src/compiler/resolutionCache.ts index de84b9bb665..d5f574762a1 100644 --- a/src/compiler/resolutionCache.ts +++ b/src/compiler/resolutionCache.ts @@ -60,11 +60,14 @@ namespace ts { watcher: FileWatcher; /** ref count keeping this directory watch alive */ refCount: number; + /** is the directory watched being non recursive */ + nonRecursive?: boolean; } interface DirectoryOfFailedLookupWatch { dir: string; dirPath: Path; + nonRecursive?: boolean; ignore?: true; } @@ -251,7 +254,6 @@ namespace ts { perDirectoryResolution = createMap(); perDirectoryCache.set(dirPath, perDirectoryResolution); } - const resolvedModules: R[] = []; const compilerOptions = resolutionHost.getCompilationSettings(); const hasInvalidatedNonRelativeUnresolvedImport = logChanges && isFileWithInvalidatedNonRelativeUnresolvedImports(path); @@ -393,6 +395,7 @@ namespace ts { function getDirectoryToWatchFailedLookupLocation(failedLookupLocation: string, failedLookupLocationPath: Path): DirectoryOfFailedLookupWatch { if (isInDirectoryPath(rootPath, failedLookupLocationPath)) { + // Always watch root directory recursively return { dir: rootDir!, dirPath: rootPath }; // TODO: GH#18217 } @@ -409,11 +412,12 @@ namespace ts { dirPath = getDirectoryPath(dirPath); } - // If the directory is node_modules use it to watch + // If the directory is node_modules use it to watch, always watch it recursively if (isNodeModulesDirectory(dirPath)) { return filterFSRootDirectoriesToWatch({ dir, dirPath }, getDirectoryPath(dirPath)); } + let nonRecursive = true; // Use some ancestor of the root directory let subDirectoryPath: Path | undefined, subDirectory: string | undefined; if (rootPath !== undefined) { @@ -422,6 +426,7 @@ namespace ts { if (parentPath === dirPath) { break; } + nonRecursive = false; subDirectoryPath = dirPath; subDirectory = dir; dirPath = parentPath; @@ -429,7 +434,7 @@ namespace ts { } } - return filterFSRootDirectoriesToWatch({ dir: subDirectory || dir, dirPath: subDirectoryPath || dirPath }, dirPath); + return filterFSRootDirectoriesToWatch({ dir: subDirectory || dir, dirPath: subDirectoryPath || dirPath, nonRecursive }, dirPath); } function isPathWithDefaultFailedLookupExtension(path: Path) { @@ -452,7 +457,7 @@ namespace ts { let setAtRoot = false; for (const failedLookupLocation of failedLookupLocations) { const failedLookupLocationPath = resolutionHost.toPath(failedLookupLocation); - const { dir, dirPath, ignore } = getDirectoryToWatchFailedLookupLocation(failedLookupLocation, failedLookupLocationPath); + const { dir, dirPath, nonRecursive, ignore } = getDirectoryToWatchFailedLookupLocation(failedLookupLocation, failedLookupLocationPath); if (!ignore) { // If the failed lookup location path is not one of the supported extensions, // store it in the custom path @@ -464,23 +469,25 @@ namespace ts { setAtRoot = true; } else { - setDirectoryWatcher(dir, dirPath); + setDirectoryWatcher(dir, dirPath, nonRecursive); } } } if (setAtRoot) { + // This is always recursive setDirectoryWatcher(rootDir!, rootPath); // TODO: GH#18217 } } - function setDirectoryWatcher(dir: string, dirPath: Path) { + function setDirectoryWatcher(dir: string, dirPath: Path, nonRecursive?: boolean) { const dirWatcher = directoryWatchesOfFailedLookups.get(dirPath); if (dirWatcher) { + Debug.assert(!!nonRecursive === !!dirWatcher.nonRecursive); dirWatcher.refCount++; } else { - directoryWatchesOfFailedLookups.set(dirPath, { watcher: createDirectoryWatcher(dir, dirPath), refCount: 1 }); + directoryWatchesOfFailedLookups.set(dirPath, { watcher: createDirectoryWatcher(dir, dirPath, nonRecursive), refCount: 1, nonRecursive }); } } @@ -530,7 +537,7 @@ namespace ts { dirWatcher.refCount--; } - function createDirectoryWatcher(directory: string, dirPath: Path) { + function createDirectoryWatcher(directory: string, dirPath: Path, nonRecursive: boolean | undefined) { return resolutionHost.watchDirectoryOfFailedLookupLocation(directory, fileOrDirectory => { const fileOrDirectoryPath = resolutionHost.toPath(fileOrDirectory); if (cachedDirectoryStructureHost) { @@ -541,7 +548,7 @@ namespace ts { if (!allFilesHaveInvalidatedResolution && invalidateResolutionOfFailedLookupLocation(fileOrDirectoryPath, dirPath === fileOrDirectoryPath)) { resolutionHost.onInvalidatedResolution(); } - }, WatchDirectoryFlags.Recursive); + }, nonRecursive ? WatchDirectoryFlags.None : WatchDirectoryFlags.Recursive); } function removeResolutionsOfFileFromCache(cache: Map>, filePath: Path) { diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index 262ccdce645..b9966f101f4 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -8011,10 +8011,10 @@ new C();` checkCompleteEvent(session, 2, expectedSequenceId); } - function verifyWatchedFilesAndDirectories(host: TestServerHost, files: string[], directories: string[]) { + function verifyWatchedFilesAndDirectories(host: TestServerHost, files: string[], recursiveDirectories: string[], nonRecursiveDirectories: string[]) { checkWatchedFilesDetailed(host, files.filter(f => f !== recognizersDateTimeSrcFile.path), 1); - checkWatchedDirectories(host, emptyArray, /*recursive*/ false); - checkWatchedDirectoriesDetailed(host, directories, 1, /*recursive*/ true); + checkWatchedDirectoriesDetailed(host, nonRecursiveDirectories, 1, /*recursive*/ false); + checkWatchedDirectoriesDetailed(host, recursiveDirectories, 1, /*recursive*/ true); } function createSessionAndOpenFile(host: TestServerHost) { @@ -8035,14 +8035,15 @@ new C();` const filesWithNodeModulesSetup = [...filesWithSources, nodeModulesRecorgnizersText]; const filesAfterCompilation = [...filesWithNodeModulesSetup, recongnizerTextDistTypingFile]; - const watchedDirectoriesWithResolvedModule = [`${recognizersDateTime}/src`, withPathMapping ? packages : recognizersDateTime, ...getTypeRootsFromLocation(recognizersDateTime)]; + const watchedDirectoriesWithResolvedModule = [`${recognizersDateTime}/src`, ...(withPathMapping ? emptyArray : [recognizersDateTime]), ...getTypeRootsFromLocation(recognizersDateTime)]; const watchedDirectoriesWithUnresolvedModule = [recognizersDateTime, ...(withPathMapping ? [recognizersText] : emptyArray), ...watchedDirectoriesWithResolvedModule, ...getNodeModuleDirectories(packages)]; + const nonRecursiveWatchedDirectories = withPathMapping ? [packages] : emptyArray; function verifyProjectWithResolvedModule(session: TestSession) { const projectService = session.getProjectService(); const project = projectService.configuredProjects.get(recognizerDateTimeTsconfigPath)!; checkProjectActualFiles(project, filesInProjectWithResolvedModule); - verifyWatchedFilesAndDirectories(session.host, filesInProjectWithResolvedModule, watchedDirectoriesWithResolvedModule); + verifyWatchedFilesAndDirectories(session.host, filesInProjectWithResolvedModule, watchedDirectoriesWithResolvedModule, nonRecursiveWatchedDirectories); verifyErrors(session, []); } @@ -8050,7 +8051,7 @@ new C();` const projectService = session.getProjectService(); const project = projectService.configuredProjects.get(recognizerDateTimeTsconfigPath)!; checkProjectActualFiles(project, filesInProjectWithUnresolvedModule); - verifyWatchedFilesAndDirectories(session.host, filesInProjectWithUnresolvedModule, watchedDirectoriesWithUnresolvedModule); + verifyWatchedFilesAndDirectories(session.host, filesInProjectWithUnresolvedModule, watchedDirectoriesWithUnresolvedModule, nonRecursiveWatchedDirectories); const startOffset = recognizersDateTimeSrcFile.content.indexOf('"') + 1; verifyErrors(session, [ createDiagnostic({ line: 1, offset: startOffset }, { line: 1, offset: startOffset + moduleNameInFile.length }, Diagnostics.Cannot_find_module_0, [moduleName]) @@ -8557,11 +8558,10 @@ export const x = 10;` service.openClientFile(srcFile.path, srcFile.content, ScriptKind.TS, projectRoot); checkProjectActualFiles(service.configuredProjects.get(configFile.path)!, files.map(f => f.path)); checkWatchedFilesDetailed(host, mapDefined(files, f => f === srcFile ? undefined : f.path), 1); - checkWatchedDirectories(host, emptyArray, /*recursive*/ false); + checkWatchedDirectoriesDetailed(host, [`${projectRoot}`], 1, /*recursive*/ false); // failed lookup for fs const expectedWatchedDirectories = createMap(); expectedWatchedDirectories.set(`${projectRoot}/src`, 2); // Wild card and failed lookup expectedWatchedDirectories.set(`${projectRoot}/somefolder`, 1); // failed lookup for somefolder/module2 - expectedWatchedDirectories.set(`${projectRoot}`, 1); // failed lookup for fs expectedWatchedDirectories.set(`${projectRoot}/node_modules`, 1); // failed lookup for with node_modules/@types/fs expectedWatchedDirectories.set(`${projectRoot}/src/typings`, 1); // typeroot directory checkWatchedDirectoriesDetailed(host, expectedWatchedDirectories, /*recursive*/ true); From 8e16bfffc24e9a895dfda63e8978fd7f23f29c94 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 14 Jun 2018 09:46:50 -0700 Subject: [PATCH 3/6] Handle the json extension when ResolvedModule result is json file Fixes #24932 --- src/parser/utilities.ts | 2 +- src/testRunner/unittests/tscWatchMode.ts | 39 ++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/parser/utilities.ts b/src/parser/utilities.ts index 548705834ce..5c0b00132c2 100644 --- a/src/parser/utilities.ts +++ b/src/parser/utilities.ts @@ -7995,7 +7995,7 @@ namespace ts { } export function tryGetExtensionFromPath(path: string): Extension | undefined { - return find(supportedTypescriptExtensionsForExtractExtension, e => fileExtensionIs(path, e)) || find(supportedJavascriptExtensions, e => fileExtensionIs(path, e)); + return find(extensionsToRemove, e => fileExtensionIs(path, e)); } function getAnyExtensionFromPathWorker(path: string, extensions: string | ReadonlyArray, stringEqualityComparer: (a: string, b: string) => boolean) { diff --git a/src/testRunner/unittests/tscWatchMode.ts b/src/testRunner/unittests/tscWatchMode.ts index 4d6797f08da..2aaef43f2d9 100644 --- a/src/testRunner/unittests/tscWatchMode.ts +++ b/src/testRunner/unittests/tscWatchMode.ts @@ -2532,4 +2532,43 @@ declare module "fs" { checkWatchedDirectoriesDetailed(host, [mainPackageRoot, linkedPackageRoot, `${mainPackageRoot}/node_modules/@types`, `${projectRoot}/node_modules/@types`], 1, /*recursive*/ true); }); }); + + describe("tsc-watch with custom module resolution", () => { + const projectRoot = "/user/username/projects/project"; + const configFileJson: any = { + compilerOptions: { module: "commonjs", resolveJsonModule: true }, + files: ["index.ts"] + }; + const mainFile: File = { + path: `${projectRoot}/index.ts`, + content: "import settings from './settings.json';" + }; + const config: File = { + path: `${projectRoot}/tsconfig.json`, + content: JSON.stringify(configFileJson) + }; + const settingsJson: File = { + path: `${projectRoot}/settings.json`, + content: JSON.stringify({ content: "Print this" }) + }; + + it("verify that module resolution with json extension works when returned without extension", () => { + const files = [libFile, mainFile, config, settingsJson]; + const host = createWatchedSystem(files, { currentDirectory: projectRoot }); + const compilerHost = createWatchCompilerHostOfConfigFile(config.path, {}, host); + const parsedCommandResult = parseJsonConfigFileContent(configFileJson, host, config.path); + compilerHost.resolveModuleNames = (moduleNames, containingFile) => moduleNames.map(m => { + const result = resolveModuleName(m, containingFile, parsedCommandResult.options, compilerHost); + const resolvedModule = result.resolvedModule!; + return { + resolvedFileName: resolvedModule.resolvedFileName, + isExternalLibraryImport: resolvedModule.isExternalLibraryImport, + originalFileName: resolvedModule.originalPath, + }; + }); + const watch = createWatchProgram(compilerHost); + const program = watch.getCurrentProgram().getProgram(); + checkProgramActualFiles(program, [mainFile.path, libFile.path, settingsJson.path]); + }); + }); } From fe260588fa9f3054fb20c85c731c81c7a2d3023b Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 14 Jun 2018 11:51:18 -0700 Subject: [PATCH 4/6] Handle package.json, jsconfig.json, tsconfig.json in the getDiagnostics of fourslash tests --- src/harness/fourslash.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 8045f3a948b..0ca37715b9f 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -507,8 +507,17 @@ namespace FourSlash { } private getAllDiagnostics(): ts.Diagnostic[] { - return ts.flatMap(this.languageServiceAdapterHost.getFilenames(), fileName => - ts.isAnySupportedFileExtension(fileName) ? this.getDiagnostics(fileName) : []); + return ts.flatMap(this.languageServiceAdapterHost.getFilenames(), fileName => { + if (!ts.isAnySupportedFileExtension(fileName)) { + return []; + } + + const baseName = ts.getBaseFileName(fileName); + if (baseName === "package.json" || baseName === "tsconfig.json" || baseName === "jsconfig.json") { + return []; + } + return this.getDiagnostics(fileName); + }); } public verifyErrorExistsAfterMarker(markerName: string, shouldExist: boolean, after: boolean) { From 345012e29d27a13fa3488892fc67938b2e3a22e6 Mon Sep 17 00:00:00 2001 From: Andy Date: Thu, 14 Jun 2018 12:55:12 -0700 Subject: [PATCH 5/6] Don't add diagnostic on unused import starting with underscore (#24958) * Don't add diagnostic on unused import starting with underscore * Fix lint --- src/compiler/checker.ts | 10 ++++--- ...sedLocalsStartingWithUnderscore.errors.txt | 13 +++++++--- .../unusedLocalsStartingWithUnderscore.js | 26 +++++++++++-------- ...unusedLocalsStartingWithUnderscore.symbols | 21 ++++++++------- .../unusedLocalsStartingWithUnderscore.types | 11 +++++--- .../unusedLocalsStartingWithUnderscore.ts | 8 +++--- 6 files changed, 54 insertions(+), 35 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 4f1b797e58a..c9d0a41f076 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -22895,7 +22895,11 @@ namespace ts { } for (const declaration of local.declarations) { - if (isAmbientModule(declaration)) continue; + if (isAmbientModule(declaration) || + (isVariableDeclaration(declaration) && isForInOrOfStatement(declaration.parent.parent) || isImportedDeclaration(declaration)) && isIdentifierThatStartsWithUnderScore(declaration.name!)) { + continue; + } + if (isImportedDeclaration(declaration)) { addToGroup(unusedImports, importClauseFromImported(declaration), declaration, getNodeId); } @@ -22907,9 +22911,7 @@ namespace ts { } } else if (isVariableDeclaration(declaration)) { - if (!isIdentifierThatStartsWithUnderScore(declaration.name) || !isForInOrOfStatement(declaration.parent.parent)) { - addToGroup(unusedVariables, declaration.parent, declaration, getNodeId); - } + addToGroup(unusedVariables, declaration.parent, declaration, getNodeId); } else { const parameter = local.valueDeclaration && tryGetRootParameterDeclaration(local.valueDeclaration); diff --git a/tests/baselines/reference/unusedLocalsStartingWithUnderscore.errors.txt b/tests/baselines/reference/unusedLocalsStartingWithUnderscore.errors.txt index b8a0a50fea6..4a708b6aa87 100644 --- a/tests/baselines/reference/unusedLocalsStartingWithUnderscore.errors.txt +++ b/tests/baselines/reference/unusedLocalsStartingWithUnderscore.errors.txt @@ -1,12 +1,17 @@ -tests/cases/compiler/unusedLocalsStartingWithUnderscore.ts(6,9): error TS6133: '_' is declared but its value is never read. +/a.ts(7,11): error TS6133: '_ns' is declared but its value is never read. +/a.ts(8,9): error TS6133: '_' is declared but its value is never read. -==== tests/cases/compiler/unusedLocalsStartingWithUnderscore.ts (1 errors) ==== +==== /a.ts (2 errors) ==== + import * as _ from "./a"; + for (const _ of []) { } for (const _ in []) { } - namespace M { + namespace _ns { + ~~~ +!!! error TS6133: '_ns' is declared but its value is never read. let _; ~ !!! error TS6133: '_' is declared but its value is never read. @@ -14,4 +19,4 @@ tests/cases/compiler/unusedLocalsStartingWithUnderscore.ts(6,9): error TS6133: ' for (const _ in []) { } } - \ No newline at end of file + \ No newline at end of file diff --git a/tests/baselines/reference/unusedLocalsStartingWithUnderscore.js b/tests/baselines/reference/unusedLocalsStartingWithUnderscore.js index 90bbb7a4c02..54b029445e3 100644 --- a/tests/baselines/reference/unusedLocalsStartingWithUnderscore.js +++ b/tests/baselines/reference/unusedLocalsStartingWithUnderscore.js @@ -1,26 +1,30 @@ -//// [unusedLocalsStartingWithUnderscore.ts] +//// [a.ts] +import * as _ from "./a"; + for (const _ of []) { } for (const _ in []) { } -namespace M { +namespace _ns { let _; for (const _ of []) { } for (const _ in []) { } } - -//// [unusedLocalsStartingWithUnderscore.js] + +//// [a.js] +"use strict"; +exports.__esModule = true; for (var _i = 0, _a = []; _i < _a.length; _i++) { - var _ = _a[_i]; + var _1 = _a[_i]; } -for (var _ in []) { } -var M; -(function (M) { +for (var _2 in []) { } +var _ns; +(function (_ns) { var _; for (var _i = 0, _a = []; _i < _a.length; _i++) { - var _1 = _a[_i]; + var _3 = _a[_i]; } - for (var _2 in []) { } -})(M || (M = {})); + for (var _4 in []) { } +})(_ns || (_ns = {})); diff --git a/tests/baselines/reference/unusedLocalsStartingWithUnderscore.symbols b/tests/baselines/reference/unusedLocalsStartingWithUnderscore.symbols index df78d17e2af..4c13df877cd 100644 --- a/tests/baselines/reference/unusedLocalsStartingWithUnderscore.symbols +++ b/tests/baselines/reference/unusedLocalsStartingWithUnderscore.symbols @@ -1,20 +1,23 @@ -=== tests/cases/compiler/unusedLocalsStartingWithUnderscore.ts === +=== /a.ts === +import * as _ from "./a"; +>_ : Symbol(_, Decl(a.ts, 0, 6)) + for (const _ of []) { } ->_ : Symbol(_, Decl(unusedLocalsStartingWithUnderscore.ts, 0, 10)) +>_ : Symbol(_, Decl(a.ts, 2, 10)) for (const _ in []) { } ->_ : Symbol(_, Decl(unusedLocalsStartingWithUnderscore.ts, 2, 10)) +>_ : Symbol(_, Decl(a.ts, 4, 10)) -namespace M { ->M : Symbol(M, Decl(unusedLocalsStartingWithUnderscore.ts, 2, 23)) +namespace _ns { +>_ns : Symbol(_ns, Decl(a.ts, 4, 23)) let _; ->_ : Symbol(_, Decl(unusedLocalsStartingWithUnderscore.ts, 5, 7)) +>_ : Symbol(_, Decl(a.ts, 7, 7)) for (const _ of []) { } ->_ : Symbol(_, Decl(unusedLocalsStartingWithUnderscore.ts, 6, 14)) +>_ : Symbol(_, Decl(a.ts, 8, 14)) for (const _ in []) { } ->_ : Symbol(_, Decl(unusedLocalsStartingWithUnderscore.ts, 8, 14)) +>_ : Symbol(_, Decl(a.ts, 10, 14)) } - + diff --git a/tests/baselines/reference/unusedLocalsStartingWithUnderscore.types b/tests/baselines/reference/unusedLocalsStartingWithUnderscore.types index 9f6a58e8d1e..88aa923a3e4 100644 --- a/tests/baselines/reference/unusedLocalsStartingWithUnderscore.types +++ b/tests/baselines/reference/unusedLocalsStartingWithUnderscore.types @@ -1,4 +1,7 @@ -=== tests/cases/compiler/unusedLocalsStartingWithUnderscore.ts === +=== /a.ts === +import * as _ from "./a"; +>_ : typeof _ + for (const _ of []) { } >_ : any >[] : undefined[] @@ -7,8 +10,8 @@ for (const _ in []) { } >_ : string >[] : undefined[] -namespace M { ->M : typeof M +namespace _ns { +>_ns : typeof _ns let _; >_ : any @@ -21,4 +24,4 @@ namespace M { >_ : string >[] : undefined[] } - + diff --git a/tests/cases/compiler/unusedLocalsStartingWithUnderscore.ts b/tests/cases/compiler/unusedLocalsStartingWithUnderscore.ts index 4e6930a6282..431d4fd15d8 100644 --- a/tests/cases/compiler/unusedLocalsStartingWithUnderscore.ts +++ b/tests/cases/compiler/unusedLocalsStartingWithUnderscore.ts @@ -1,13 +1,15 @@ -//@noUnusedLocals:true +// @noUnusedLocals:true + +// @Filename: /a.ts +import * as _ from "./a"; for (const _ of []) { } for (const _ in []) { } -namespace M { +namespace _ns { let _; for (const _ of []) { } for (const _ in []) { } } - \ No newline at end of file From 57e652dd023d3e304f0ac6e693f6980e5412e039 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 14 Jun 2018 13:11:52 -0700 Subject: [PATCH 6/6] Js/check type tags (#24967) * Check the type expression of `@type` tags * Update existing tests and baselines --- src/compiler/checker.ts | 6 ++++++ .../reference/checkJsdocTypeTag4.errors.txt | 21 +++++++++++++++++++ .../reference/checkJsdocTypeTag4.symbols | 20 ++++++++++++++++++ .../reference/checkJsdocTypeTag4.types | 20 ++++++++++++++++++ ...docParameterParsingInfiniteLoop.errors.txt | 5 ++++- .../typedefMultipleTypeParameters.errors.txt | 10 ++++++--- .../typedefMultipleTypeParameters.symbols | 5 ++--- .../typedefMultipleTypeParameters.types | 1 - .../conformance/jsdoc/checkJsdocTypeTag4.ts | 16 ++++++++++++++ .../jsdoc/typedefMultipleTypeParameters.ts | 1 - 10 files changed, 96 insertions(+), 9 deletions(-) create mode 100644 tests/baselines/reference/checkJsdocTypeTag4.errors.txt create mode 100644 tests/baselines/reference/checkJsdocTypeTag4.symbols create mode 100644 tests/baselines/reference/checkJsdocTypeTag4.types create mode 100644 tests/cases/conformance/jsdoc/checkJsdocTypeTag4.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index c9d0a41f076..48083056ff8 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -22601,6 +22601,10 @@ namespace ts { checkSourceElement(node.typeExpression); } + function checkJSDocTypeTag(node: JSDocTypeTag) { + checkSourceElement(node.typeExpression); + } + function checkJSDocParameterTag(node: JSDocParameterTag) { checkSourceElement(node.typeExpression); if (!getParameterSymbolFromJSDoc(node)) { @@ -25562,6 +25566,8 @@ namespace ts { case SyntaxKind.JSDocTypedefTag: case SyntaxKind.JSDocCallbackTag: return checkJSDocTypeAliasTag(node as JSDocTypedefTag); + case SyntaxKind.JSDocTypeTag: + return checkJSDocTypeTag(node as JSDocTypeTag); case SyntaxKind.JSDocParameterTag: return checkJSDocParameterTag(node as JSDocParameterTag); case SyntaxKind.JSDocFunctionType: diff --git a/tests/baselines/reference/checkJsdocTypeTag4.errors.txt b/tests/baselines/reference/checkJsdocTypeTag4.errors.txt new file mode 100644 index 00000000000..4683f256093 --- /dev/null +++ b/tests/baselines/reference/checkJsdocTypeTag4.errors.txt @@ -0,0 +1,21 @@ +tests/cases/conformance/jsdoc/test.js(5,14): error TS2344: Type 'number' does not satisfy the constraint 'string'. +tests/cases/conformance/jsdoc/test.js(7,14): error TS2344: Type 'number' does not satisfy the constraint 'string'. + + +==== tests/cases/conformance/jsdoc/t.d.ts (0 errors) ==== + type A = { a: T } + +==== tests/cases/conformance/jsdoc/test.js (2 errors) ==== + /** Also should error for jsdoc typedefs + * @template {string} U + * @typedef {{ b: U }} B + */ + /** @type {A} */ + ~~~~~~ +!!! error TS2344: Type 'number' does not satisfy the constraint 'string'. + var a; + /** @type {B} */ + ~~~~~~ +!!! error TS2344: Type 'number' does not satisfy the constraint 'string'. + var b; + \ No newline at end of file diff --git a/tests/baselines/reference/checkJsdocTypeTag4.symbols b/tests/baselines/reference/checkJsdocTypeTag4.symbols new file mode 100644 index 00000000000..50e70ad3944 --- /dev/null +++ b/tests/baselines/reference/checkJsdocTypeTag4.symbols @@ -0,0 +1,20 @@ +=== tests/cases/conformance/jsdoc/t.d.ts === +type A = { a: T } +>A : Symbol(A, Decl(t.d.ts, 0, 0)) +>T : Symbol(T, Decl(t.d.ts, 0, 7)) +>a : Symbol(a, Decl(t.d.ts, 0, 28)) +>T : Symbol(T, Decl(t.d.ts, 0, 7)) + +=== tests/cases/conformance/jsdoc/test.js === +/** Also should error for jsdoc typedefs + * @template {string} U + * @typedef {{ b: U }} B + */ +/** @type {A} */ +var a; +>a : Symbol(a, Decl(test.js, 5, 3)) + +/** @type {B} */ +var b; +>b : Symbol(b, Decl(test.js, 7, 3)) + diff --git a/tests/baselines/reference/checkJsdocTypeTag4.types b/tests/baselines/reference/checkJsdocTypeTag4.types new file mode 100644 index 00000000000..04e3707a400 --- /dev/null +++ b/tests/baselines/reference/checkJsdocTypeTag4.types @@ -0,0 +1,20 @@ +=== tests/cases/conformance/jsdoc/t.d.ts === +type A = { a: T } +>A : A +>T : T +>a : T +>T : T + +=== tests/cases/conformance/jsdoc/test.js === +/** Also should error for jsdoc typedefs + * @template {string} U + * @typedef {{ b: U }} B + */ +/** @type {A} */ +var a; +>a : A + +/** @type {B} */ +var b; +>b : { b: number; } + diff --git a/tests/baselines/reference/jsdocParameterParsingInfiniteLoop.errors.txt b/tests/baselines/reference/jsdocParameterParsingInfiniteLoop.errors.txt index a43c72cf0c7..71286e23bdb 100644 --- a/tests/baselines/reference/jsdocParameterParsingInfiniteLoop.errors.txt +++ b/tests/baselines/reference/jsdocParameterParsingInfiniteLoop.errors.txt @@ -1,11 +1,14 @@ tests/cases/compiler/example.js(3,20): error TS1110: Type expected. +tests/cases/compiler/example.js(3,21): error TS2304: Cannot find name 'foo'. -==== tests/cases/compiler/example.js (1 errors) ==== +==== tests/cases/compiler/example.js (2 errors) ==== // @ts-check /** * @type {function(@foo)} ~ !!! error TS1110: Type expected. + ~~~ +!!! error TS2304: Cannot find name 'foo'. */ let x; \ No newline at end of file diff --git a/tests/baselines/reference/typedefMultipleTypeParameters.errors.txt b/tests/baselines/reference/typedefMultipleTypeParameters.errors.txt index e8d0e52ba0f..afd2e274317 100644 --- a/tests/baselines/reference/typedefMultipleTypeParameters.errors.txt +++ b/tests/baselines/reference/typedefMultipleTypeParameters.errors.txt @@ -1,9 +1,11 @@ -tests/cases/conformance/jsdoc/a.js(16,12): error TS2314: Generic type 'Everything' requires 5 type argument(s). +tests/cases/conformance/jsdoc/a.js(12,23): error TS2344: Type '{ a: number; }' does not satisfy the constraint '{ a: number; b: string; }'. + Property 'b' is missing in type '{ a: number; }'. +tests/cases/conformance/jsdoc/a.js(15,12): error TS2314: Generic type 'Everything' requires 5 type argument(s). tests/cases/conformance/jsdoc/test.ts(1,34): error TS2344: Type '{ a: number; }' does not satisfy the constraint '{ a: number; b: string; }'. Property 'b' is missing in type '{ a: number; }'. -==== tests/cases/conformance/jsdoc/a.js (1 errors) ==== +==== tests/cases/conformance/jsdoc/a.js (2 errors) ==== /** * @template {{ a: number, b: string }} T,U A Comment * @template {{ c: boolean }} V uh ... are comments even supported?? @@ -15,8 +17,10 @@ tests/cases/conformance/jsdoc/test.ts(1,34): error TS2344: Type '{ a: number; }' /** @type {Everything<{ a: number, b: 'hi', c: never }, undefined, { c: true, d: 1 }, number, string>} */ var tuvwx; - // TODO: will error when #24592 is fixed /** @type {Everything<{ a: number }, undefined, { c: 1, d: 1 }, number, string>} */ + ~~~~~~~~~~~~~~ +!!! error TS2344: Type '{ a: number; }' does not satisfy the constraint '{ a: number; b: string; }'. +!!! error TS2344: Property 'b' is missing in type '{ a: number; }'. var wrong; /** @type {Everything<{ a: number }>} */ diff --git a/tests/baselines/reference/typedefMultipleTypeParameters.symbols b/tests/baselines/reference/typedefMultipleTypeParameters.symbols index 9acd30fea1b..f961ea79698 100644 --- a/tests/baselines/reference/typedefMultipleTypeParameters.symbols +++ b/tests/baselines/reference/typedefMultipleTypeParameters.symbols @@ -11,14 +11,13 @@ var tuvwx; >tuvwx : Symbol(tuvwx, Decl(a.js, 9, 3)) -// TODO: will error when #24592 is fixed /** @type {Everything<{ a: number }, undefined, { c: 1, d: 1 }, number, string>} */ var wrong; ->wrong : Symbol(wrong, Decl(a.js, 13, 3)) +>wrong : Symbol(wrong, Decl(a.js, 12, 3)) /** @type {Everything<{ a: number }>} */ var insufficient; ->insufficient : Symbol(insufficient, Decl(a.js, 16, 3)) +>insufficient : Symbol(insufficient, Decl(a.js, 15, 3)) === tests/cases/conformance/jsdoc/test.ts === declare var actually: Everything<{ a: number }, undefined, { c: 1, d: 1 }, number, string>; diff --git a/tests/baselines/reference/typedefMultipleTypeParameters.types b/tests/baselines/reference/typedefMultipleTypeParameters.types index 5bc38bb835b..ba3456b7faa 100644 --- a/tests/baselines/reference/typedefMultipleTypeParameters.types +++ b/tests/baselines/reference/typedefMultipleTypeParameters.types @@ -11,7 +11,6 @@ var tuvwx; >tuvwx : { t: { a: number; b: "hi"; c: never; }; u: undefined; v: { c: true; d: 1; }; w: number; x: string; } -// TODO: will error when #24592 is fixed /** @type {Everything<{ a: number }, undefined, { c: 1, d: 1 }, number, string>} */ var wrong; >wrong : { t: { a: number; }; u: undefined; v: { c: 1; d: 1; }; w: number; x: string; } diff --git a/tests/cases/conformance/jsdoc/checkJsdocTypeTag4.ts b/tests/cases/conformance/jsdoc/checkJsdocTypeTag4.ts new file mode 100644 index 00000000000..63eae378b9e --- /dev/null +++ b/tests/cases/conformance/jsdoc/checkJsdocTypeTag4.ts @@ -0,0 +1,16 @@ +// @checkJs: true +// @allowJs: true +// @noEmit: true + +// @Filename: t.d.ts +type A = { a: T } + +// @Filename: test.js +/** Also should error for jsdoc typedefs + * @template {string} U + * @typedef {{ b: U }} B + */ +/** @type {A} */ +var a; +/** @type {B} */ +var b; diff --git a/tests/cases/conformance/jsdoc/typedefMultipleTypeParameters.ts b/tests/cases/conformance/jsdoc/typedefMultipleTypeParameters.ts index 9fc7c0d0d41..d70f1d6ef4d 100644 --- a/tests/cases/conformance/jsdoc/typedefMultipleTypeParameters.ts +++ b/tests/cases/conformance/jsdoc/typedefMultipleTypeParameters.ts @@ -13,7 +13,6 @@ /** @type {Everything<{ a: number, b: 'hi', c: never }, undefined, { c: true, d: 1 }, number, string>} */ var tuvwx; -// TODO: will error when #24592 is fixed /** @type {Everything<{ a: number }, undefined, { c: 1, d: 1 }, number, string>} */ var wrong;