diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 1792a393234ea..d11fc414f6df7 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -953,7 +953,7 @@ namespace ts { * @param map A map-like. * @param key A property key. */ - export function hasProperty(map: MapLike, key: string): boolean { + export function hasProperty(map: MapLike, key: string): boolean { return hasOwnProperty.call(map, key); } @@ -2478,21 +2478,24 @@ namespace ts { } Debug.fail(`File ${path} has unknown extension.`); } + + export function isAnySupportedFileExtension(path: string): boolean { + return tryGetExtensionFromPath(path) !== undefined; + } + + const allExtensions = [Extension.Dts, Extension.Ts, Extension.Tsx, Extension.Js, Extension.Jsx]; + export function tryGetExtensionFromPath(path: string): Extension | undefined { - if (fileExtensionIs(path, ".d.ts")) { - return Extension.Dts; - } - if (fileExtensionIs(path, ".ts")) { - return Extension.Ts; - } - if (fileExtensionIs(path, ".tsx")) { - return Extension.Tsx; - } - if (fileExtensionIs(path, ".js")) { - return Extension.Js; - } - if (fileExtensionIs(path, ".jsx")) { - return Extension.Jsx; + return ts.find(allExtensions, ext => fileExtensionIs(path, extensionText(ext))); + } + + export function extensionText(ext: Extension): string { + switch (ext) { + case Extension.Dts: return ".d.ts"; + case Extension.Ts: return ".ts"; + case Extension.Tsx: return ".tsx"; + case Extension.Js: return ".js"; + case Extension.Jsx: return ".jsx"; } } diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 4c6616c36d46a..c38f99f059b97 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -19,13 +19,26 @@ namespace ts { push(value: T): void; } - /** - * Result of trying to resolve a module. - * At least one of `ts` and `js` should be defined, or the whole thing should be `undefined`. - */ + function withPackageId(packageId: PackageId | undefined, r: PathAndExtension | undefined): Resolved { + return r && { path: r.path, extension: r.ext, packageId }; + } + + function noPackageId(r: PathAndExtension | undefined): Resolved { + return withPackageId(/*packageId*/ undefined, r); + } + + /** Result of trying to resolve a module. */ interface Resolved { path: string; extension: Extension; + packageId: PackageId | undefined; + } + + /** Result of trying to resolve a module at a file. Needs to have 'packageId' added later. */ + interface PathAndExtension { + path: string; + // (Use a different name than `extension` to make sure Resolved isn't assignable to PathAndExtension.) + ext: Extension; } /** @@ -49,7 +62,7 @@ namespace ts { function createResolvedModuleWithFailedLookupLocations(resolved: Resolved | undefined, isExternalLibraryImport: boolean, failedLookupLocations: string[]): ResolvedModuleWithFailedLookupLocations { return { - resolvedModule: resolved && { resolvedFileName: resolved.path, extension: resolved.extension, isExternalLibraryImport }, + resolvedModule: resolved && { resolvedFileName: resolved.path, extension: resolved.extension, isExternalLibraryImport, packageId: resolved.packageId }, failedLookupLocations }; } @@ -65,8 +78,7 @@ namespace ts { } /** Reads from "main" or "types"/"typings" depending on `extensions`. */ - function tryReadPackageJsonFields(readTypes: boolean, packageJsonPath: string, baseDirectory: string, state: ModuleResolutionState): string | undefined { - const jsonContent = readJson(packageJsonPath, state.host); + function tryReadPackageJsonFields(readTypes: boolean, jsonContent: PackageJson, baseDirectory: string, state: ModuleResolutionState): string | undefined { return readTypes ? tryReadFromField("typings") || tryReadFromField("types") : tryReadFromField("main"); function tryReadFromField(fieldName: "typings" | "types" | "main"): string | undefined { @@ -93,7 +105,8 @@ namespace ts { } } - function readJson(path: string, host: ModuleResolutionHost): { typings?: string, types?: string, main?: string } { + interface PackageJson { name?: string; version?: string; typings?: string; types?: string; main?: string; } + function readJson(path: string, host: ModuleResolutionHost): PackageJson { try { const jsonText = host.readFile(path); return jsonText ? JSON.parse(jsonText) : {}; @@ -658,7 +671,7 @@ namespace ts { if (extension !== undefined) { const path = tryFile(candidate, failedLookupLocations, /*onlyRecordFailures*/ false, state); if (path !== undefined) { - return { path, extension }; + return { path, extension, packageId: undefined }; } } @@ -721,7 +734,7 @@ namespace ts { } const resolved = loadModuleFromNodeModules(extensions, moduleName, containingDirectory, failedLookupLocations, state, cache); // For node_modules lookups, get the real path so that multiple accesses to an `npm link`-ed module do not create duplicate files. - return resolved && { value: resolved.value && { resolved: { path: realpath(resolved.value.path, host, traceEnabled), extension: resolved.value.extension }, isExternalLibraryImport: true } }; + return resolved && { value: resolved.value && { resolved: { ...resolved.value, path: realpath(resolved.value.path, host, traceEnabled) }, isExternalLibraryImport: true } }; } else { const candidate = normalizePath(combinePaths(containingDirectory, moduleName)); @@ -759,7 +772,7 @@ namespace ts { } const resolvedFromFile = loadModuleFromFile(extensions, candidate, failedLookupLocations, onlyRecordFailures, state); if (resolvedFromFile) { - return resolvedFromFile; + return noPackageId(resolvedFromFile); } } if (!onlyRecordFailures) { @@ -780,11 +793,15 @@ namespace ts { return !host.directoryExists || host.directoryExists(directoryName); } + function loadModuleFromFileNoPackageId(extensions: Extensions, candidate: string, failedLookupLocations: Push, onlyRecordFailures: boolean, state: ModuleResolutionState): Resolved { + return noPackageId(loadModuleFromFile(extensions, candidate, failedLookupLocations, onlyRecordFailures, state)); + } + /** * @param {boolean} onlyRecordFailures - if true then function won't try to actually load files but instead record all attempts as failures. This flag is necessary * in cases when we know upfront that all load attempts will fail (because containing folder does not exists) however we still need to record all failed lookup locations. */ - function loadModuleFromFile(extensions: Extensions, candidate: string, failedLookupLocations: Push, onlyRecordFailures: boolean, state: ModuleResolutionState): Resolved | undefined { + function loadModuleFromFile(extensions: Extensions, candidate: string, failedLookupLocations: Push, onlyRecordFailures: boolean, state: ModuleResolutionState): PathAndExtension | undefined { // First, try adding an extension. An import of "foo" could be matched by a file "foo.ts", or "foo.js" by "foo.js.ts" const resolvedByAddingExtension = tryAddingExtensions(candidate, extensions, failedLookupLocations, onlyRecordFailures, state); if (resolvedByAddingExtension) { @@ -804,7 +821,7 @@ namespace ts { } /** Try to return an existing file that adds one of the `extensions` to `candidate`. */ - function tryAddingExtensions(candidate: string, extensions: Extensions, failedLookupLocations: Push, onlyRecordFailures: boolean, state: ModuleResolutionState): Resolved | undefined { + function tryAddingExtensions(candidate: string, extensions: Extensions, failedLookupLocations: Push, onlyRecordFailures: boolean, state: ModuleResolutionState): PathAndExtension | undefined { if (!onlyRecordFailures) { // check if containing folder exists - if it doesn't then just record failures for all supported extensions without disk probing const directory = getDirectoryPath(candidate); @@ -815,16 +832,16 @@ namespace ts { switch (extensions) { case Extensions.DtsOnly: - return tryExtension(".d.ts", Extension.Dts); + return tryExtension(Extension.Dts); case Extensions.TypeScript: - return tryExtension(".ts", Extension.Ts) || tryExtension(".tsx", Extension.Tsx) || tryExtension(".d.ts", Extension.Dts); + return tryExtension(Extension.Ts) || tryExtension(Extension.Tsx) || tryExtension(Extension.Dts); case Extensions.JavaScript: - return tryExtension(".js", Extension.Js) || tryExtension(".jsx", Extension.Jsx); + return tryExtension(Extension.Js) || tryExtension(Extension.Jsx); } - function tryExtension(ext: string, extension: Extension): Resolved | undefined { - const path = tryFile(candidate + ext, failedLookupLocations, onlyRecordFailures, state); - return path && { path, extension }; + function tryExtension(ext: Extension): PathAndExtension | undefined { + const path = tryFile(candidate + extensionText(ext), failedLookupLocations, onlyRecordFailures, state); + return path && { path, ext }; } } @@ -850,12 +867,23 @@ namespace ts { function loadNodeModuleFromDirectory(extensions: Extensions, candidate: string, failedLookupLocations: Push, onlyRecordFailures: boolean, state: ModuleResolutionState, considerPackageJson = true): Resolved | undefined { const directoryExists = !onlyRecordFailures && directoryProbablyExists(candidate, state.host); + let packageId: PackageId | undefined; + if (considerPackageJson) { const packageJsonPath = pathToPackageJson(candidate); if (directoryExists && state.host.fileExists(packageJsonPath)) { - const fromPackageJson = loadModuleFromPackageJson(packageJsonPath, extensions, candidate, failedLookupLocations, state); + if (state.traceEnabled) { + trace(state.host, Diagnostics.Found_package_json_at_0, packageJsonPath); + } + const jsonContent = readJson(packageJsonPath, state.host); + + if (typeof jsonContent.name === "string" && typeof jsonContent.version === "string") { + packageId = { name: jsonContent.name, version: jsonContent.version }; + } + + const fromPackageJson = loadModuleFromPackageJson(jsonContent, extensions, candidate, failedLookupLocations, state); if (fromPackageJson) { - return fromPackageJson; + return withPackageId(packageId, fromPackageJson); } } else { @@ -867,15 +895,11 @@ namespace ts { } } - return loadModuleFromFile(extensions, combinePaths(candidate, "index"), failedLookupLocations, !directoryExists, state); + return withPackageId(packageId, loadModuleFromFile(extensions, combinePaths(candidate, "index"), failedLookupLocations, !directoryExists, state)); } - function loadModuleFromPackageJson(packageJsonPath: string, extensions: Extensions, candidate: string, failedLookupLocations: Push, state: ModuleResolutionState): Resolved | undefined { - if (state.traceEnabled) { - trace(state.host, Diagnostics.Found_package_json_at_0, packageJsonPath); - } - - const file = tryReadPackageJsonFields(extensions !== Extensions.JavaScript, packageJsonPath, candidate, state); + function loadModuleFromPackageJson(jsonContent: PackageJson, extensions: Extensions, candidate: string, failedLookupLocations: Push, state: ModuleResolutionState): PathAndExtension | undefined { + const file = tryReadPackageJsonFields(extensions !== Extensions.JavaScript, jsonContent, candidate, state); if (!file) { return undefined; } @@ -895,13 +919,18 @@ namespace ts { // Even if extensions is DtsOnly, we can still look up a .ts file as a result of package.json "types" const nextExtensions = extensions === Extensions.DtsOnly ? Extensions.TypeScript : extensions; // Don't do package.json lookup recursively, because Node.js' package lookup doesn't. - return nodeLoadModuleByRelativeName(nextExtensions, file, failedLookupLocations, onlyRecordFailures, state, /*considerPackageJson*/ false); + const result = nodeLoadModuleByRelativeName(nextExtensions, file, failedLookupLocations, onlyRecordFailures, state, /*considerPackageJson*/ false); + if (result) { + // It won't have a `packageId` set, because we disabled `considerPackageJson`. + Debug.assert(result.packageId === undefined); + return { path: result.path, ext: result.extension }; + } } /** Resolve from an arbitrarily specified file. Return `undefined` if it has an unsupported extension. */ - function resolvedIfExtensionMatches(extensions: Extensions, path: string): Resolved | undefined { - const extension = tryGetExtensionFromPath(path); - return extension !== undefined && extensionIsOk(extensions, extension) ? { path, extension } : undefined; + function resolvedIfExtensionMatches(extensions: Extensions, path: string): PathAndExtension | undefined { + const ext = tryGetExtensionFromPath(path); + return ext !== undefined && extensionIsOk(extensions, ext) ? { path, ext } : undefined; } /** True if `extension` is one of the supported `extensions`. */ @@ -923,7 +952,7 @@ namespace ts { function loadModuleFromNodeModulesFolder(extensions: Extensions, moduleName: string, nodeModulesFolder: string, nodeModulesFolderExists: boolean, failedLookupLocations: Push, state: ModuleResolutionState): Resolved | undefined { const candidate = normalizePath(combinePaths(nodeModulesFolder, moduleName)); - return loadModuleFromFile(extensions, candidate, failedLookupLocations, !nodeModulesFolderExists, state) || + return loadModuleFromFileNoPackageId(extensions, candidate, failedLookupLocations, !nodeModulesFolderExists, state) || loadNodeModuleFromDirectory(extensions, candidate, failedLookupLocations, !nodeModulesFolderExists, state); } @@ -1008,7 +1037,7 @@ namespace ts { if (traceEnabled) { trace(host, Diagnostics.Resolution_for_module_0_was_found_in_cache, moduleName); } - return { value: result.resolvedModule && { path: result.resolvedModule.resolvedFileName, extension: result.resolvedModule.extension } }; + return { value: result.resolvedModule && { path: result.resolvedModule.resolvedFileName, extension: result.resolvedModule.extension, packageId: result.resolvedModule.packageId } }; } } @@ -1022,7 +1051,7 @@ namespace ts { return createResolvedModuleWithFailedLookupLocations(resolved && resolved.value, /*isExternalLibraryImport*/ false, failedLookupLocations); function tryResolve(extensions: Extensions): SearchResult { - const resolvedUsingSettings = tryLoadModuleUsingOptionalResolutionSettings(extensions, moduleName, containingDirectory, loadModuleFromFile, failedLookupLocations, state); + const resolvedUsingSettings = tryLoadModuleUsingOptionalResolutionSettings(extensions, moduleName, containingDirectory, loadModuleFromFileNoPackageId, failedLookupLocations, state); if (resolvedUsingSettings) { return { value: resolvedUsingSettings }; } @@ -1036,7 +1065,7 @@ namespace ts { return resolutionFromCache; } const searchName = normalizePath(combinePaths(directory, moduleName)); - return toSearchResult(loadModuleFromFile(extensions, searchName, failedLookupLocations, /*onlyRecordFailures*/ false, state)); + return toSearchResult(loadModuleFromFileNoPackageId(extensions, searchName, failedLookupLocations, /*onlyRecordFailures*/ false, state)); }); if (resolved) { return resolved; @@ -1048,7 +1077,7 @@ namespace ts { } else { const candidate = normalizePath(combinePaths(containingDirectory, moduleName)); - return toSearchResult(loadModuleFromFile(extensions, candidate, failedLookupLocations, /*onlyRecordFailures*/ false, state)); + return toSearchResult(loadModuleFromFileNoPackageId(extensions, candidate, failedLookupLocations, /*onlyRecordFailures*/ false, state)); } } } diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 39d82b8606d7e..e3e4712b63d91 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -471,6 +471,14 @@ namespace ts { resolveTypeReferenceDirectiveNamesWorker = (typeReferenceDirectiveNames, containingFile) => loadWithLocalCache(typeReferenceDirectiveNames, containingFile, loader); } + // Map from a stringified PackageId to the source file with that id. + // Only one source file may have a given packageId. Others become redirects (see createRedirectSourceFile). + const packageIdToSourceFile = createMap(); + // Maps from a SourceFile's `.path` to the packageId it was imported with. + const sourceFileToPackageId = createMap(); + // See `sourceFileIsRedirectedTo`. + const sourceFileIsRedirectedToSet = createMap(); + const filesByName = createFileMap(); // stores 'filename -> file association' ignoring case // used to track cases when two file names differ only in casing @@ -544,6 +552,8 @@ namespace ts { isSourceFileFromExternalLibrary, dropDiagnosticsProducingTypeChecker, getSourceFileFromReference, + getPackageIdOfSourceFile: ({ path }) => sourceFileToPackageId.get(path), + sourceFileIsRedirectedTo: ({ path }) => !!sourceFileIsRedirectedToSet.get(path), }; verifyCompilerOptions(); @@ -765,11 +775,42 @@ namespace ts { const modifiedSourceFiles: { oldFile: SourceFile, newFile: SourceFile }[] = []; oldProgram.structureIsReused = StructureIsReused.Completely; - for (const oldSourceFile of oldProgram.getSourceFiles()) { + const oldSourceFiles = oldProgram.getSourceFiles(); + for (const oldSourceFile of oldSourceFiles) { const newSourceFile = host.getSourceFileByPath ? host.getSourceFileByPath(oldSourceFile.fileName, oldSourceFile.path, options.target) : host.getSourceFile(oldSourceFile.fileName, options.target); + const packageId = oldProgram.getPackageIdOfSourceFile(oldSourceFile); + if (packageId) sourceFileToPackageId.set(newSourceFile.path, packageId); + + if (oldSourceFile.redirect) { + // We got `newSourceFile` by path, so it is actually for the underlying file. + // This lets us know if the underlying file has changed. If it has we should break the redirect. + const { underlying } = oldSourceFile.redirect; + if (newSourceFile !== underlying) { + // Underlying file has changed. Might not redirect anymore. Must rebuild program. + return oldProgram.structureIsReused = StructureIsReused.Not; + } + else { + filePaths.push(oldSourceFile.path); + newSourceFiles.push(oldSourceFile); + continue; + } + } + else if (oldProgram.sourceFileIsRedirectedTo(oldSourceFile)) { + // This is similar to the above case. If a redirected-to source file changes, the redirect may be broken. + if (newSourceFile !== oldSourceFile) { + return oldProgram.structureIsReused = StructureIsReused.Not; + } + else { + sourceFileIsRedirectedToSet.set(oldSourceFile.path, true); + filePaths.push(oldSourceFile.path); + newSourceFiles.push(oldSourceFile); + continue; + } + } + if (!newSourceFile) { return oldProgram.structureIsReused = StructureIsReused.Not; } @@ -778,6 +819,22 @@ namespace ts { filePaths.push(newSourceFile.path); if (oldSourceFile !== newSourceFile) { + if (packageId) { + // If this has a package id, and some other source file has the same package id, they are candidates to become redirects. + // In that case we must rebuild the program. + const hasDuplicate = oldSourceFiles.some(o => { + if (o !== oldSourceFile) return false; + const id = oldProgram.getPackageIdOfSourceFile(o); + return id && id.name === packageId.name; + }); + if (hasDuplicate) { + // There exist 2 different source files with the same package id. + // (As in, /a/node_modules/x/index.d.ts and /b/node_modules/x/index.d.ts, where the package.json versions are identical.) + // One of them changed. These might now become redirects. So we must rebuild the program. + return oldProgram.structureIsReused = StructureIsReused.Not; + } + } + // The `newSourceFile` object was created for the new program. if (oldSourceFile.hasNoDefaultLib !== newSourceFile.hasNoDefaultLib) { @@ -1498,7 +1555,7 @@ namespace ts { /** This has side effects through `findSourceFile`. */ function processSourceFile(fileName: string, isDefaultLib: boolean, refFile?: SourceFile, refPos?: number, refEnd?: number): void { getSourceFileFromReferenceWorker(fileName, - fileName => findSourceFile(fileName, toPath(fileName, currentDirectory, getCanonicalFileName), isDefaultLib, refFile, refPos, refEnd), + fileName => findSourceFile(fileName, toPath(fileName, currentDirectory, getCanonicalFileName), isDefaultLib, refFile, refPos, refEnd, /*packageId*/ undefined), (diagnostic, ...args) => { fileProcessingDiagnostics.add(refFile !== undefined && refEnd !== undefined && refPos !== undefined ? createFileDiagnostic(refFile, refPos, refEnd - refPos, diagnostic, ...args) @@ -1517,8 +1574,28 @@ namespace ts { } } + function createRedirectSourceFile(redirectTo: SourceFile, underlying: SourceFile, fileName: string, path: Path): SourceFile { + sourceFileIsRedirectedToSet.set(redirectTo.path, true); + + const redirect: SourceFile = Object.create(redirectTo); + redirect.fileName = fileName; + redirect.path = path; + redirect.redirect = { redirectTo, underlying }; + Object.defineProperties(redirect, { + id: { + get(this: SourceFile) { return this.redirect.redirectTo.id; }, + set(this: SourceFile, value: SourceFile["id"]) { this.redirect.redirectTo.id = value; }, + }, + symbol: { + get(this: SourceFile) { return this.redirect.redirectTo.symbol; }, + set(this: SourceFile, value: SourceFile["symbol"]) { this.redirect.redirectTo.symbol = value; }, + }, + }); + return redirect; + } + // Get source file from normalized fileName - function findSourceFile(fileName: string, path: Path, isDefaultLib: boolean, refFile?: SourceFile, refPos?: number, refEnd?: number): SourceFile { + function findSourceFile(fileName: string, path: Path, isDefaultLib: boolean, refFile: SourceFile, refPos: number, refEnd: number, packageId: PackageId | undefined): SourceFile | undefined { if (filesByName.contains(path)) { const file = filesByName.get(path); // try to check if we've already seen this file but with a different casing in path @@ -1561,6 +1638,25 @@ namespace ts { } }); + if (packageId) { + const packageIdKey = `${packageId.name}@${packageId.version}`; + const fileFromPackageId = packageIdToSourceFile.get(packageIdKey); + if (fileFromPackageId) { + // Some other SourceFile already exists with this package name and version. + // Instead of creating a duplicate, just redirect to the existing one. + const dupFile = createRedirectSourceFile(fileFromPackageId, file, fileName, path); + filesByName.set(path, dupFile); + sourceFileToPackageId.set(path, packageId); + files.push(dupFile); + return dupFile; + } + else if (file) { + // This is the first source file to have this packageId. + packageIdToSourceFile.set(packageIdKey, file); + sourceFileToPackageId.set(path, packageId); + } + } + filesByName.set(path, file); if (file) { sourceFilesFoundSearchingNodeModules.set(path, currentNodeModulesDepth > 0); @@ -1722,7 +1818,7 @@ namespace ts { else if (shouldAddFile) { const path = toPath(resolvedFileName, currentDirectory, getCanonicalFileName); const pos = skipTrivia(file.text, file.imports[i].pos); - findSourceFile(resolvedFileName, path, /*isDefaultLib*/ false, file, pos, file.imports[i].end); + findSourceFile(resolvedFileName, path, /*isDefaultLib*/ false, file, pos, file.imports[i].end, resolution.packageId); } if (isFromNodeModulesSearch) { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 83aa50fb803e8..f7d07ba42ebca 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2282,6 +2282,21 @@ namespace ts { /* @internal */ path: Path; text: string; + /** + * If two source files are for the same version of the same package, one will redirect to the other. + * (See `createRedirectSourceFile` in program.ts.) + * The redirect will have this set. The other will not have anything set, but see Program#sourceFileIsRedirectedTo. + */ + /* @internal */ redirect?: { + /** Source file this redirects to. */ + readonly redirectTo: SourceFile, + /** + * Source file for the duplicate package. This will not be used by the Program, + * but we need to keep this around so we can watch for changes in underlying. + */ + readonly underlying: SourceFile, + } | undefined; + amdDependencies: AmdDependency[]; moduleName: string; referencedFiles: FileReference[]; @@ -2440,6 +2455,11 @@ namespace ts { /* @internal */ structureIsReused?: StructureIsReused; /* @internal */ getSourceFileFromReference(referencingFile: SourceFile, ref: FileReference): SourceFile | undefined; + + /** Given a source file, get the PackageId it was imported from. */ + /* @internal */ getPackageIdOfSourceFile(sourceFile: SourceFile): PackageId | undefined; + /** True if some other source file redirects to this one. */ + /* @internal */ sourceFileIsRedirectedTo(sourceFile: SourceFile): boolean; } /* @internal */ @@ -3849,6 +3869,7 @@ namespace ts { /** * ResolvedModule with an explicitly provided `extension` property. * Prefer this over `ResolvedModule`. + * If changing this, remember to change `moduleResolutionIsEqualTo`. */ export interface ResolvedModuleFull extends ResolvedModule { /** @@ -3856,6 +3877,22 @@ namespace ts { * This is optional for backwards-compatibility, but will be added if not provided. */ extension: Extension; + packageId?: PackageId; + } + + /** + * Unique identifier with a package name and version. + * If changing this, remember to change `packageIdIsEqual`. + */ + export interface PackageId { + /** + * Name of the package. + * Should not include `@types`. + * If accessing a non-index file, this should include its name e.g. "foo/bar". + */ + name: string; + /** Version of the package, e.g. "1.2.3" */ + version: string; } export enum Extension { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index faec63d5a28ad..9b456ab3f5e85 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -112,7 +112,12 @@ namespace ts { export function moduleResolutionIsEqualTo(oldResolution: ResolvedModuleFull, newResolution: ResolvedModuleFull): boolean { return oldResolution.isExternalLibraryImport === newResolution.isExternalLibraryImport && oldResolution.extension === newResolution.extension && - oldResolution.resolvedFileName === newResolution.resolvedFileName; + oldResolution.resolvedFileName === newResolution.resolvedFileName && + (oldResolution.packageId === newResolution.packageId || oldResolution.packageId && newResolution.packageId && packageIdIsEqual(oldResolution.packageId, newResolution.packageId)); + } + + function packageIdIsEqual(a: PackageId, b: PackageId): boolean { + return a.name === b.name && a.version === b.version; } /* @internal */ diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 4b5af197a3690..ae2035f8085a6 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -451,7 +451,7 @@ namespace FourSlash { this.languageServiceAdapterHost.openFile(fileToOpen.fileName, content, scriptKindName); } - public verifyErrorExistsBetweenMarkers(startMarkerName: string, endMarkerName: string, negative: boolean) { + public verifyErrorExistsBetweenMarkers(startMarkerName: string, endMarkerName: string, shouldExist: boolean) { const startMarker = this.getMarkerByName(startMarkerName); const endMarker = this.getMarkerByName(endMarkerName); const predicate = (errorMinChar: number, errorLimChar: number, startPos: number, endPos: number) => @@ -459,9 +459,9 @@ namespace FourSlash { const exists = this.anyErrorInRange(predicate, startMarker, endMarker); - if (exists !== negative) { - this.printErrorLog(negative, this.getAllDiagnostics()); - throw new Error(`Failure between markers: '${startMarkerName}', '${endMarkerName}'`); + if (exists !== shouldExist) { + this.printErrorLog(shouldExist, this.getAllDiagnostics()); + throw new Error(`${shouldExist ? "Expected" : "Did not expect"} failure between markers: '${startMarkerName}', '${endMarkerName}'`); } } @@ -483,10 +483,11 @@ namespace FourSlash { } private getAllDiagnostics(): ts.Diagnostic[] { - return ts.flatMap(this.languageServiceAdapterHost.getFilenames(), fileName => this.getDiagnostics(fileName)); + return ts.flatMap(this.languageServiceAdapterHost.getFilenames(), fileName => + ts.isAnySupportedFileExtension(fileName) ? this.getDiagnostics(fileName) : []); } - public verifyErrorExistsAfterMarker(markerName: string, negative: boolean, after: boolean) { + public verifyErrorExistsAfterMarker(markerName: string, shouldExist: boolean, after: boolean) { const marker: Marker = this.getMarkerByName(markerName); let predicate: (errorMinChar: number, errorLimChar: number, startPos: number, endPos: number) => boolean; @@ -502,30 +503,15 @@ namespace FourSlash { const exists = this.anyErrorInRange(predicate, marker); const diagnostics = this.getAllDiagnostics(); - if (exists !== negative) { - this.printErrorLog(negative, diagnostics); - throw new Error("Failure at marker: " + markerName); + if (exists !== shouldExist) { + this.printErrorLog(shouldExist, diagnostics); + throw new Error(`${shouldExist ? "Expected" : "Did not expect"} failure at marker '${markerName}'`); } } - private anyErrorInRange(predicate: (errorMinChar: number, errorLimChar: number, startPos: number, endPos: number) => boolean, startMarker: Marker, endMarker?: Marker) { - - const errors = this.getDiagnostics(startMarker.fileName); - let exists = false; - - const startPos = startMarker.position; - let endPos: number = undefined; - if (endMarker !== undefined) { - endPos = endMarker.position; - } - - errors.forEach(function (error: ts.Diagnostic) { - if (predicate(error.start, error.start + error.length, startPos, endPos)) { - exists = true; - } - }); - - return exists; + private anyErrorInRange(predicate: (errorMinChar: number, errorLimChar: number, startPos: number, endPos: number) => boolean, startMarker: Marker, endMarker?: Marker): boolean { + return this.getDiagnostics(startMarker.fileName).some(({ start, length }) => + predicate(start, start + length, startMarker.position, endMarker === undefined ? undefined : endMarker.position)); } private printErrorLog(expectErrors: boolean, errors: ts.Diagnostic[]) { @@ -545,6 +531,7 @@ namespace FourSlash { public verifyNoErrors() { ts.forEachKey(this.inputFiles, fileName => { + if (!ts.isAnySupportedFileExtension(fileName)) return; const errors = this.getDiagnostics(fileName); if (errors.length) { this.printErrorLog(/*expectErrors*/ false, errors); diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index 7aefb0f3a1fc3..09bc861fd3ca9 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -193,7 +193,10 @@ namespace Harness.LanguageService { } getCurrentDirectory(): string { return virtualFileSystemRoot; } getDefaultLibFileName(): string { return Harness.Compiler.defaultLibFileName; } - getScriptFileNames(): string[] { return this.getFilenames(); } + getScriptFileNames(): string[] { + return this.getFilenames().filter(f => + ts.isAnySupportedFileExtension(f) !== undefined); + } getScriptSnapshot(fileName: string): ts.IScriptSnapshot { const script = this.getScriptInfo(fileName); return script ? new ScriptSnapshot(script) : undefined; diff --git a/tests/baselines/reference/duplicatePackage.errors.txt b/tests/baselines/reference/duplicatePackage.errors.txt new file mode 100644 index 0000000000000..917ed711c640d --- /dev/null +++ b/tests/baselines/reference/duplicatePackage.errors.txt @@ -0,0 +1,48 @@ +/src/a.ts(5,3): error TS2345: Argument of type 'X' is not assignable to parameter of type 'X'. + Types have separate declarations of a private property 'x'. + + +==== /src/a.ts (1 errors) ==== + import { a } from "a"; + import { b } from "b"; + import { c } from "c"; + a(b); // Works + a(c); // Error, these are from different versions of the library. + ~ +!!! error TS2345: Argument of type 'X' is not assignable to parameter of type 'X'. +!!! error TS2345: Types have separate declarations of a private property 'x'. + +==== /node_modules/a/index.d.ts (0 errors) ==== + import X from "x"; + export function a(x: X): void; + +==== /node_modules/a/node_modules/x/index.d.ts (0 errors) ==== + export default class X { + private x: number; + } + +==== /node_modules/a/node_modules/x/package.json (0 errors) ==== + { "name": "x", "version": "1.2.3" } + +==== /node_modules/b/index.d.ts (0 errors) ==== + import X from "x"; + export const b: X; + +==== /node_modules/b/node_modules/x/index.d.ts (0 errors) ==== + content not parsed + +==== /node_modules/b/node_modules/x/package.json (0 errors) ==== + { "name": "x", "version": "1.2.3" } + +==== /node_modules/c/index.d.ts (0 errors) ==== + import X from "x"; + export const c: X; + +==== /node_modules/c/node_modules/x/index.d.ts (0 errors) ==== + export default class X { + private x: number; + } + +==== /node_modules/c/node_modules/x/package.json (0 errors) ==== + { "name": "x", "version": "1.2.4" } + \ No newline at end of file diff --git a/tests/baselines/reference/duplicatePackage.js b/tests/baselines/reference/duplicatePackage.js new file mode 100644 index 0000000000000..ada5c900b9371 --- /dev/null +++ b/tests/baselines/reference/duplicatePackage.js @@ -0,0 +1,52 @@ +//// [tests/cases/compiler/duplicatePackage.ts] //// + +//// [index.d.ts] +import X from "x"; +export function a(x: X): void; + +//// [index.d.ts] +export default class X { + private x: number; +} + +//// [package.json] +{ "name": "x", "version": "1.2.3" } + +//// [index.d.ts] +import X from "x"; +export const b: X; + +//// [index.d.ts] +content not parsed + +//// [package.json] +{ "name": "x", "version": "1.2.3" } + +//// [index.d.ts] +import X from "x"; +export const c: X; + +//// [index.d.ts] +export default class X { + private x: number; +} + +//// [package.json] +{ "name": "x", "version": "1.2.4" } + +//// [a.ts] +import { a } from "a"; +import { b } from "b"; +import { c } from "c"; +a(b); // Works +a(c); // Error, these are from different versions of the library. + + +//// [a.js] +"use strict"; +exports.__esModule = true; +var a_1 = require("a"); +var b_1 = require("b"); +var c_1 = require("c"); +a_1.a(b_1.b); // Works +a_1.a(c_1.c); // Error, these are from different versions of the library. diff --git a/tests/baselines/reference/duplicatePackage_withErrors.errors.txt b/tests/baselines/reference/duplicatePackage_withErrors.errors.txt new file mode 100644 index 0000000000000..ad24637e98331 --- /dev/null +++ b/tests/baselines/reference/duplicatePackage_withErrors.errors.txt @@ -0,0 +1,27 @@ +/node_modules/a/node_modules/x/index.d.ts(1,18): error TS1254: A 'const' initializer in an ambient context must be a string or numeric literal. + + +==== /src/a.ts (0 errors) ==== + import { x as xa } from "a"; + import { x as xb } from "b"; + +==== /node_modules/a/index.d.ts (0 errors) ==== + export { x } from "x"; + +==== /node_modules/a/node_modules/x/index.d.ts (1 errors) ==== + export const x = 1 + 1; + ~~~~~ +!!! error TS1254: A 'const' initializer in an ambient context must be a string or numeric literal. + +==== /node_modules/a/node_modules/x/package.json (0 errors) ==== + { "name": "x", "version": "1.2.3" } + +==== /node_modules/b/index.d.ts (0 errors) ==== + export { x } from "x"; + +==== /node_modules/b/node_modules/x/index.d.ts (0 errors) ==== + content not parsed + +==== /node_modules/b/node_modules/x/package.json (0 errors) ==== + { "name": "x", "version": "1.2.3" } + \ No newline at end of file diff --git a/tests/baselines/reference/duplicatePackage_withErrors.js b/tests/baselines/reference/duplicatePackage_withErrors.js new file mode 100644 index 0000000000000..a7fdafd522f5e --- /dev/null +++ b/tests/baselines/reference/duplicatePackage_withErrors.js @@ -0,0 +1,28 @@ +//// [tests/cases/compiler/duplicatePackage_withErrors.ts] //// + +//// [index.d.ts] +export { x } from "x"; + +//// [index.d.ts] +export const x = 1 + 1; + +//// [package.json] +{ "name": "x", "version": "1.2.3" } + +//// [index.d.ts] +export { x } from "x"; + +//// [index.d.ts] +content not parsed + +//// [package.json] +{ "name": "x", "version": "1.2.3" } + +//// [a.ts] +import { x as xa } from "a"; +import { x as xb } from "b"; + + +//// [a.js] +"use strict"; +exports.__esModule = true; diff --git a/tests/cases/compiler/duplicatePackage.ts b/tests/cases/compiler/duplicatePackage.ts new file mode 100644 index 0000000000000..31df2d2450883 --- /dev/null +++ b/tests/cases/compiler/duplicatePackage.ts @@ -0,0 +1,42 @@ +// @noImplicitReferences: true + +// @Filename: /node_modules/a/index.d.ts +import X from "x"; +export function a(x: X): void; + +// @Filename: /node_modules/a/node_modules/x/index.d.ts +export default class X { + private x: number; +} + +// @Filename: /node_modules/a/node_modules/x/package.json +{ "name": "x", "version": "1.2.3" } + +// @Filename: /node_modules/b/index.d.ts +import X from "x"; +export const b: X; + +// @Filename: /node_modules/b/node_modules/x/index.d.ts +content not parsed + +// @Filename: /node_modules/b/node_modules/x/package.json +{ "name": "x", "version": "1.2.3" } + +// @Filename: /node_modules/c/index.d.ts +import X from "x"; +export const c: X; + +// @Filename: /node_modules/c/node_modules/x/index.d.ts +export default class X { + private x: number; +} + +// @Filename: /node_modules/c/node_modules/x/package.json +{ "name": "x", "version": "1.2.4" } + +// @Filename: /src/a.ts +import { a } from "a"; +import { b } from "b"; +import { c } from "c"; +a(b); // Works +a(c); // Error, these are from different versions of the library. diff --git a/tests/cases/compiler/duplicatePackage_withErrors.ts b/tests/cases/compiler/duplicatePackage_withErrors.ts new file mode 100644 index 0000000000000..266c7239971d7 --- /dev/null +++ b/tests/cases/compiler/duplicatePackage_withErrors.ts @@ -0,0 +1,23 @@ +// @noImplicitReferences: true + +// @Filename: /node_modules/a/index.d.ts +export { x } from "x"; + +// @Filename: /node_modules/a/node_modules/x/index.d.ts +export const x = 1 + 1; + +// @Filename: /node_modules/a/node_modules/x/package.json +{ "name": "x", "version": "1.2.3" } + +// @Filename: /node_modules/b/index.d.ts +export { x } from "x"; + +// @Filename: /node_modules/b/node_modules/x/index.d.ts +content not parsed + +// @Filename: /node_modules/b/node_modules/x/package.json +{ "name": "x", "version": "1.2.3" } + +// @Filename: /src/a.ts +import { x as xa } from "a"; +import { x as xb } from "b"; diff --git a/tests/cases/fourslash/duplicatePackageServices.ts b/tests/cases/fourslash/duplicatePackageServices.ts new file mode 100644 index 0000000000000..360611ad1403b --- /dev/null +++ b/tests/cases/fourslash/duplicatePackageServices.ts @@ -0,0 +1,46 @@ +/// +// @noImplicitReferences: true + +// @Filename: /node_modules/a/index.d.ts +////import /*useAX*/[|{| "isWriteAccess": true, "isDefinition": true |}X|] from "x"; +////export function a(x: [|X|]): void; + +// @Filename: /node_modules/a/node_modules/x/index.d.ts +////export default class /*defAX*/[|{| "isWriteAccess": true, "isDefinition": true |}X|] { +//// private x: number; +////} + +// @Filename: /node_modules/a/node_modules/x/package.json +////{ "name": "x", "version": "1.2.3" } + +// @Filename: /node_modules/b/index.d.ts +////import /*useBX*/[|{| "isWriteAccess": true, "isDefinition": true |}X|] from "x"; +////export const b: [|X|]; + +// @Filename: /node_modules/b/node_modules/x/index.d.ts +////export default class /*defBX*/[|{| "isWriteAccess": true, "isDefinition": true |}X|] { +//// private x: number; +////} + +// @Filename: /node_modules/b/node_modules/x/package.json +////{ "name": "x", "version": "1.2./*bVersionPatch*/3" } + +// @Filename: /src/a.ts +////import { a } from "a"; +////import { b } from "b"; +////a(/*error*/b); + +goTo.file("/src/a.ts"); +verify.numberOfErrorsInCurrentFile(0); +verify.goToDefinition("useAX", "defAX"); +verify.goToDefinition("useBX", "defAX"); + +const [r0, r1, r2, r3, r4, r5] = test.ranges(); +const aImport = { definition: "import X", ranges: [r0, r1] }; +const def = { definition: "class X", ranges: [r2] }; +const bImport = { definition: "import X", ranges: [r3, r4] }; +verify.referenceGroups([r0, r1], [aImport, def, bImport]); +verify.referenceGroups([r2], [def, aImport, bImport]); +verify.referenceGroups([r3, r4], [bImport, def, aImport]); + +verify.referenceGroups(r5, [def, aImport, bImport]); diff --git a/tests/cases/fourslash/duplicatePackageServices_fileChanges.ts b/tests/cases/fourslash/duplicatePackageServices_fileChanges.ts new file mode 100644 index 0000000000000..203277d7ad467 --- /dev/null +++ b/tests/cases/fourslash/duplicatePackageServices_fileChanges.ts @@ -0,0 +1,57 @@ +/// +// @noImplicitReferences: true + +// @Filename: /node_modules/a/index.d.ts +////import X from "x"; +////export function a(x: X): void; + +// @Filename: /node_modules/a/node_modules/x/index.d.ts +////export default class /*defAX*/X { +//// private x: number; +////} + +// @Filename: /node_modules/a/node_modules/x/package.json +////{ "name": "x", "version": "1.2./*aVersionPatch*/3" } + +// @Filename: /node_modules/b/index.d.ts +////import X from "x"; +////export const b: X; + +// @Filename: /node_modules/b/node_modules/x/index.d.ts +////export default class /*defBX*/X { +//// private x: number; +////} + +// @Filename: /node_modules/b/node_modules/x/package.json +////{ "name": "x", "version": "1.2./*bVersionPatch*/3" } + +// @Filename: /src/a.ts +////import { a } from "a"; +////import { b } from "b"; +////a(/*error*/b); + +goTo.file("/src/a.ts"); +verify.numberOfErrorsInCurrentFile(0); + +testChangeAndChangeBack("aVersionPatch", "defAX"); +testChangeAndChangeBack("bVersionPatch", "defBX"); + +function testChangeAndChangeBack(versionPatch: string, def: string) { + goTo.marker(versionPatch); + edit.insert("4"); + goTo.marker(def); + edit.insert(" "); + + // No longer have identical packageId, so we get errors. + verify.errorExistsAfterMarker("error"); + + // Undo the change. + goTo.marker(versionPatch); + edit.deleteAtCaret(); + goTo.marker(def); + edit.deleteAtCaret(); + + // Back to being identical. + goTo.file("/src/a.ts"); + verify.numberOfErrorsInCurrentFile(0); +}