From 18a7c3fb533a99898ecf6a1ebf2bc7e17e574fa0 Mon Sep 17 00:00:00 2001 From: Andy Date: Wed, 6 Dec 2017 11:27:38 -0800 Subject: [PATCH] For import fix, prefer symlink over a real path (#20395) * For import fix, prefer symlink over a real path * fixes * Use best result from all symlinks * Make originalPath optional more * Only include real path if a symlink isn't available --- src/compiler/builder.ts | 6 +- src/compiler/core.ts | 92 ++++++++---- src/compiler/moduleNameResolver.ts | 27 ++-- src/compiler/types.ts | 4 +- src/compiler/utilities.ts | 5 + src/harness/fourslash.ts | 90 ++++++------ src/harness/harnessLanguageService.ts | 29 +++- src/harness/unittests/languageService.ts | 3 - src/harness/unittests/moduleResolution.ts | 8 +- src/harness/unittests/programMissingFiles.ts | 2 +- src/harness/virtualFileSystemWithWatch.ts | 4 +- src/services/codefixes/importFixes.ts | 135 ++++++++++-------- src/services/completions.ts | 9 +- src/services/refactorProvider.ts | 4 +- src/services/services.ts | 3 +- src/services/shims.ts | 2 +- src/services/types.ts | 1 + .../reference/api/tsserverlibrary.d.ts | 1 + tests/baselines/reference/api/typescript.d.ts | 1 + .../fourslash/importNameCodeFix_symlink.ts | 22 +++ 20 files changed, 284 insertions(+), 164 deletions(-) create mode 100644 tests/cases/fourslash/importNameCodeFix_symlink.ts diff --git a/src/compiler/builder.ts b/src/compiler/builder.ts index 34ca1bdf0e9..7e44a9608f0 100644 --- a/src/compiler/builder.ts +++ b/src/compiler/builder.ts @@ -468,9 +468,9 @@ namespace ts { } function getReferencedByPaths(referencedFilePath: Path) { - return mapDefinedIter(references.entries(), ([filePath, referencesInFile]) => + return arrayFrom(mapDefinedIterator(references.entries(), ([filePath, referencesInFile]) => referencesInFile.has(referencedFilePath) ? filePath as Path : undefined - ); + )); } function getFilesAffectedByUpdatedShape(program: Program, sourceFile: SourceFile): ReadonlyArray { @@ -504,7 +504,7 @@ namespace ts { } // Return array of values that needs emit - return flatMapIter(seenFileNamesMap.values(), value => value); + return arrayFrom(mapDefinedIterator(seenFileNamesMap.values(), value => value)); } } } diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 48c1b84dbaa..ec2eff50c3d 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -191,6 +191,19 @@ namespace ts { return undefined; } + export function firstDefinedIterator(iter: Iterator, callback: (element: T) => U | undefined): U | undefined { + while (true) { + const { value, done } = iter.next(); + if (done) { + return undefined; + } + const result = callback(value); + if (result !== undefined) { + return result; + } + } + } + /** * Iterates through the parent chain of a node and performs the callback on each parent until the callback * returns a truthy value, then returns that value. @@ -474,22 +487,32 @@ namespace ts { return result; } - export function flatMapIter(iter: Iterator, mapfn: (x: T) => U | U[] | undefined): U[] { - const result: U[] = []; - while (true) { - const { value, done } = iter.next(); - if (done) break; - const res = mapfn(value); - if (res) { - if (isArray(res)) { - result.push(...res); - } - else { - result.push(res); - } - } + export function flatMapIterator(iter: Iterator, mapfn: (x: T) => U[] | Iterator | undefined): Iterator { + const first = iter.next(); + if (first.done) { + return emptyIterator; + } + let currentIter = getIterator(first.value); + return { + next() { + while (true) { + const currentRes = currentIter.next(); + if (!currentRes.done) { + return currentRes; + } + const iterRes = iter.next(); + if (iterRes.done) { + return iterRes; + } + currentIter = getIterator(iterRes.value); + } + }, + }; + + function getIterator(x: T): Iterator { + const res = mapfn(x); + return res === undefined ? emptyIterator : isArray(res) ? arrayIterator(res) : res; } - return result; } /** @@ -537,17 +560,34 @@ namespace ts { return result; } - export function mapDefinedIter(iter: Iterator, mapFn: (x: T) => U | undefined): U[] { - const result: U[] = []; - while (true) { - const { value, done } = iter.next(); - if (done) break; - const res = mapFn(value); - if (res !== undefined) { - result.push(res); + export function mapDefinedIterator(iter: Iterator, mapFn: (x: T) => U | undefined): Iterator { + return { + next() { + while (true) { + const res = iter.next(); + if (res.done) { + return res; + } + const value = mapFn(res.value); + if (value !== undefined) { + return { value, done: false }; + } + } } - } - return result; + }; + } + + export const emptyIterator: Iterator = { next: () => ({ value: undefined as never, done: true }) }; + + export function singleIterator(value: T): Iterator { + let done = false; + return { + next() { + const wasDone = done; + done = true; + return wasDone ? { value: undefined as never, done: true } : { value, done: false }; + } + }; } /** @@ -1360,7 +1400,7 @@ namespace ts { /** * Tests whether a value is an array. */ - export function isArray(value: any): value is ReadonlyArray { + export function isArray(value: any): value is ReadonlyArray<{}> { return Array.isArray ? Array.isArray(value) : value instanceof Array; } diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index b0633ef4d5a..1cc11acd32d 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -64,9 +64,9 @@ namespace ts { return { fileName: resolved.path, packageId: resolved.packageId }; } - function createResolvedModuleWithFailedLookupLocations(resolved: Resolved | undefined, isExternalLibraryImport: boolean, failedLookupLocations: string[]): ResolvedModuleWithFailedLookupLocations { + function createResolvedModuleWithFailedLookupLocations(resolved: Resolved | undefined, originalPath: string | undefined, isExternalLibraryImport: boolean, failedLookupLocations: string[]): ResolvedModuleWithFailedLookupLocations { return { - resolvedModule: resolved && { resolvedFileName: resolved.path, extension: resolved.extension, isExternalLibraryImport, packageId: resolved.packageId }, + resolvedModule: resolved && { resolvedFileName: resolved.path, originalPath, extension: resolved.extension, isExternalLibraryImport, packageId: resolved.packageId }, failedLookupLocations }; } @@ -732,12 +732,12 @@ namespace ts { const result = jsOnly ? tryResolve(Extensions.JavaScript) : (tryResolve(Extensions.TypeScript) || tryResolve(Extensions.JavaScript)); if (result && result.value) { - const { resolved, isExternalLibraryImport } = result.value; - return createResolvedModuleWithFailedLookupLocations(resolved, isExternalLibraryImport, failedLookupLocations); + const { resolved, originalPath, isExternalLibraryImport } = result.value; + return createResolvedModuleWithFailedLookupLocations(resolved, originalPath, isExternalLibraryImport, failedLookupLocations); } return { resolvedModule: undefined, failedLookupLocations }; - function tryResolve(extensions: Extensions): SearchResult<{ resolved: Resolved, isExternalLibraryImport: boolean }> { + function tryResolve(extensions: Extensions): SearchResult<{ resolved: Resolved, originalPath?: string, isExternalLibraryImport: boolean }> { const loader: ResolutionKindSpecificLoader = (extensions, candidate, failedLookupLocations, onlyRecordFailures, state) => nodeLoadModuleByRelativeName(extensions, candidate, failedLookupLocations, onlyRecordFailures, state, /*considerPackageJson*/ true); const resolved = tryLoadModuleUsingOptionalResolutionSettings(extensions, moduleName, containingDirectory, loader, failedLookupLocations, state); if (resolved) { @@ -752,11 +752,17 @@ namespace ts { if (!resolved) return undefined; let resolvedValue = resolved.value; - if (!compilerOptions.preserveSymlinks) { - resolvedValue = resolvedValue && { ...resolved.value, path: realPath(resolved.value.path, host, traceEnabled), extension: resolved.value.extension }; + let originalPath: string | undefined; + if (!compilerOptions.preserveSymlinks && resolvedValue) { + originalPath = resolvedValue.path; + const path = realPath(resolved.value.path, host, traceEnabled); + if (path === originalPath) { + originalPath = undefined; + } + resolvedValue = { ...resolvedValue, path }; } // For node_modules lookups, get the real path so that multiple accesses to an `npm link`-ed module do not create duplicate files. - return { value: resolvedValue && { resolved: resolvedValue, isExternalLibraryImport: true } }; + return { value: resolvedValue && { resolved: resolvedValue, originalPath, isExternalLibraryImport: true } }; } else { const { path: candidate, parts } = normalizePathAndParts(combinePaths(containingDirectory, moduleName)); @@ -1115,7 +1121,8 @@ namespace ts { const containingDirectory = getDirectoryPath(containingFile); const resolved = tryResolve(Extensions.TypeScript) || tryResolve(Extensions.JavaScript); - return createResolvedModuleWithFailedLookupLocations(resolved && resolved.value, /*isExternalLibraryImport*/ false, failedLookupLocations); + // No originalPath because classic resolution doesn't resolve realPath + return createResolvedModuleWithFailedLookupLocations(resolved && resolved.value, /*originalPath*/ undefined, /*isExternalLibraryImport*/ false, failedLookupLocations); function tryResolve(extensions: Extensions): SearchResult { const resolvedUsingSettings = tryLoadModuleUsingOptionalResolutionSettings(extensions, moduleName, containingDirectory, loadModuleFromFileNoPackageId, failedLookupLocations, state); @@ -1162,7 +1169,7 @@ namespace ts { const state: ModuleResolutionState = { compilerOptions, host, traceEnabled }; const failedLookupLocations: string[] = []; const resolved = loadModuleFromNodeModulesOneLevel(Extensions.DtsOnly, moduleName, globalCache, failedLookupLocations, state); - return createResolvedModuleWithFailedLookupLocations(resolved, /*isExternalLibraryImport*/ true, failedLookupLocations); + return createResolvedModuleWithFailedLookupLocations(resolved, /*originalPath*/ undefined, /*isExternalLibraryImport*/ true, failedLookupLocations); } /** diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 5e7fa298255..0c9747f4837 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2478,7 +2478,7 @@ namespace ts { // Stores a mapping 'external module reference text' -> 'resolved file name' | undefined // It is used to resolve module names in the checker. // Content of this field should never be used directly - use getResolvedModuleFileName/setResolvedModuleFileName functions instead - /* @internal */ resolvedModules: Map; + /* @internal */ resolvedModules: Map; /* @internal */ resolvedTypeReferenceDirectiveNames: Map; /* @internal */ imports: ReadonlyArray; // Identifier only if `declare global` @@ -4243,6 +4243,8 @@ namespace ts { * If changing this, remember to change `moduleResolutionIsEqualTo`. */ export interface ResolvedModuleFull extends ResolvedModule { + /* @internal */ + readonly originalPath?: string; /** * Extension of resolvedFileName. This must match what's at the end of resolvedFileName. * This is optional for backwards-compatibility, but will be added if not provided. diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 0e62c2d8cea..eac101c028a 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -101,6 +101,7 @@ namespace ts { return oldResolution.isExternalLibraryImport === newResolution.isExternalLibraryImport && oldResolution.extension === newResolution.extension && oldResolution.resolvedFileName === newResolution.resolvedFileName && + oldResolution.originalPath === newResolution.originalPath && packageIdIsEqual(oldResolution.packageId, newResolution.packageId); } @@ -3696,6 +3697,10 @@ namespace ts { export function typeHasCallOrConstructSignatures(type: Type, checker: TypeChecker) { return checker.getSignaturesOfType(type, SignatureKind.Call).length !== 0 || checker.getSignaturesOfType(type, SignatureKind.Construct).length !== 0; } + + export function forSomeAncestorDirectory(directory: string, callback: (directory: string) => boolean): boolean { + return !!forEachAncestorDirectory(directory, d => callback(d) ? true : undefined); + } } namespace ts { diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 8f270b8cf4b..f5cd0c1967e 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -27,6 +27,7 @@ namespace FourSlash { // The contents of the file (with markers, etc stripped out) content: string; fileName: string; + symlinks?: string[]; version: number; // File-specific options (name/value pairs) fileOptions: Harness.TestCaseParser.CompilerSettings; @@ -106,15 +107,16 @@ namespace FourSlash { // Name of testcase metadata including ts.CompilerOptions properties that will be used by globalOptions // To add additional option, add property into the testOptMetadataNames, refer the property in either globalMetadataNames or fileMetadataNames // Add cases into convertGlobalOptionsToCompilationsSettings function for the compiler to acknowledge such option from meta data - const metadataOptionNames = { - baselineFile: "BaselineFile", - emitThisFile: "emitThisFile", // This flag is used for testing getEmitOutput feature. It allows test-cases to indicate what file to be output in multiple files project - fileName: "Filename", - resolveReference: "ResolveReference", // This flag is used to specify entry file for resolve file references. The flag is only allow once per test file - }; + const enum MetadataOptionNames { + baselineFile = "BaselineFile", + emitThisFile = "emitThisFile", // This flag is used for testing getEmitOutput feature. It allows test-cases to indicate what file to be output in multiple files project + fileName = "Filename", + resolveReference = "ResolveReference", // This flag is used to specify entry file for resolve file references. The flag is only allow once per test file + symlink = "Symlink", + } // List of allowed metadata names - const fileMetadataNames = [metadataOptionNames.fileName, metadataOptionNames.emitThisFile, metadataOptionNames.resolveReference]; + const fileMetadataNames = [MetadataOptionNames.fileName, MetadataOptionNames.emitThisFile, MetadataOptionNames.resolveReference, MetadataOptionNames.symlink]; function convertGlobalOptionsToCompilerOptions(globalOptions: Harness.TestCaseParser.CompilerSettings): ts.CompilerOptions { const settings: ts.CompilerOptions = { target: ts.ScriptTarget.ES5 }; @@ -281,7 +283,7 @@ namespace FourSlash { configFileName = file.fileName; } - if (!startResolveFileRef && file.fileOptions[metadataOptionNames.resolveReference] === "true") { + if (!startResolveFileRef && file.fileOptions[MetadataOptionNames.resolveReference] === "true") { startResolveFileRef = file; } else if (startResolveFileRef) { @@ -354,6 +356,10 @@ namespace FourSlash { Harness.Compiler.getDefaultLibrarySourceFile().text, /*isRootFile*/ false); } + for (const file of testData.files) { + ts.forEach(file.symlinks, link => this.languageServiceAdapterHost.addSymlink(link, file.fileName)); + } + this.formatCodeSettings = { baseIndentSize: 0, indentSize: 4, @@ -653,7 +659,7 @@ namespace FourSlash { this.verifyGoToXPlain(arg0, endMarkerNames, getDefs); } else if (ts.isArray(arg0)) { - const pairs: ReadonlyArray<[string | string[], string | string[]]> = arg0; + const pairs = arg0 as ReadonlyArray<[string | string[], string | string[]]>; for (const [start, end] of pairs) { this.verifyGoToXPlain(start, end, getDefs); } @@ -1562,7 +1568,7 @@ Actual: ${stringify(fullActual)}`); } public baselineCurrentFileBreakpointLocations() { - let baselineFile = this.testData.globalOptions[metadataOptionNames.baselineFile]; + let baselineFile = this.testData.globalOptions[MetadataOptionNames.baselineFile]; if (!baselineFile) { baselineFile = this.activeFile.fileName.replace(this.basePath + "/breakpointValidation", "bpSpan"); baselineFile = baselineFile.replace(ts.Extension.Ts, ".baseline"); @@ -1581,7 +1587,7 @@ Actual: ${stringify(fullActual)}`); const allFourSlashFiles = this.testData.files; for (const file of allFourSlashFiles) { - if (file.fileOptions[metadataOptionNames.emitThisFile] === "true") { + if (file.fileOptions[MetadataOptionNames.emitThisFile] === "true") { // Find a file with the flag emitThisFile turned on emitFiles.push(file); } @@ -1593,7 +1599,7 @@ Actual: ${stringify(fullActual)}`); } Harness.Baseline.runBaseline( - this.testData.globalOptions[metadataOptionNames.baselineFile], + this.testData.globalOptions[MetadataOptionNames.baselineFile], () => { let resultString = ""; // Loop through all the emittedFiles and emit them one by one @@ -1633,7 +1639,7 @@ Actual: ${stringify(fullActual)}`); } public baselineQuickInfo() { - let baselineFile = this.testData.globalOptions[metadataOptionNames.baselineFile]; + let baselineFile = this.testData.globalOptions[MetadataOptionNames.baselineFile]; if (!baselineFile) { baselineFile = ts.getBaseFileName(this.activeFile.fileName).replace(ts.Extension.Ts, ".baseline"); } @@ -2243,7 +2249,7 @@ Actual: ${stringify(fullActual)}`); public baselineCurrentFileNameOrDottedNameSpans() { Harness.Baseline.runBaseline( - this.testData.globalOptions[metadataOptionNames.baselineFile], + this.testData.globalOptions[MetadataOptionNames.baselineFile], () => { return this.baselineCurrentFileLocations(pos => this.getNameOrDottedNameSpan(pos)); @@ -3269,12 +3275,21 @@ ${code} // Stuff related to the subfile we're parsing let currentFileContent: string = undefined; let currentFileName = fileName; + let currentFileSymlinks: string[] | undefined; let currentFileOptions: { [s: string]: string } = {}; - function resetLocalData() { + function nextFile() { + const file = parseFileContent(currentFileContent, currentFileName, markerPositions, markers, ranges); + file.fileOptions = currentFileOptions; + file.symlinks = currentFileSymlinks; + + // Store result file + files.push(file); + currentFileContent = undefined; currentFileOptions = {}; currentFileName = fileName; + currentFileSymlinks = undefined; } for (let line of lines) { @@ -3303,8 +3318,7 @@ ${code} const match = optionRegex.exec(line.substr(2)); if (match) { const [key, value] = match.slice(1); - const fileMetadataNamesIndex = fileMetadataNames.indexOf(key); - if (fileMetadataNamesIndex === -1) { + if (!ts.contains(fileMetadataNames, key)) { // Check if the match is already existed in the global options if (globalOptions[key] !== undefined) { throw new Error(`Global option '${key}' already exists`); @@ -3312,24 +3326,22 @@ ${code} globalOptions[key] = value; } else { - if (fileMetadataNamesIndex === fileMetadataNames.indexOf(metadataOptionNames.fileName)) { - // Found an @FileName directive, if this is not the first then create a new subfile - if (currentFileContent) { - const file = parseFileContent(currentFileContent, currentFileName, markerPositions, markers, ranges); - file.fileOptions = currentFileOptions; + switch (key) { + case MetadataOptionNames.fileName: + // Found an @FileName directive, if this is not the first then create a new subfile + if (currentFileContent) { + nextFile(); + } - // Store result file - files.push(file); - - resetLocalData(); - } - - currentFileName = ts.isRootedDiskPath(value) ? value : basePath + "/" + value; - currentFileOptions[key] = value; - } - else { - // Add other fileMetadata flag - currentFileOptions[key] = value; + currentFileName = ts.isRootedDiskPath(value) ? value : basePath + "/" + value; + currentFileOptions[key] = value; + break; + case MetadataOptionNames.symlink: + currentFileSymlinks = ts.append(currentFileSymlinks, value); + break; + default: + // Add other fileMetadata flag + currentFileOptions[key] = value; } } } @@ -3341,13 +3353,7 @@ ${code} else { // Empty line or code line, terminate current subfile if there is one if (currentFileContent) { - const file = parseFileContent(currentFileContent, currentFileName, markerPositions, markers, ranges); - file.fileOptions = currentFileOptions; - - // Store result file - files.push(file); - - resetLocalData(); + nextFile(); } } } @@ -3382,7 +3388,7 @@ ${code} function getNonFileNameOptionInObject(optionObject: { [s: string]: string }): string { for (const option in optionObject) { - if (option !== metadataOptionNames.fileName) { + if (option !== MetadataOptionNames.fileName) { return option; } } diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index c81875139e5..59c3a0f8293 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -122,7 +122,7 @@ namespace Harness.LanguageService { getPreProcessedFileInfo(fileName: string, fileContents: string): ts.PreProcessedFileInfo; } - export class LanguageServiceAdapterHost { + export abstract class LanguageServiceAdapterHost { public typesRegistry: ts.Map | undefined; protected virtualFileSystem: Utils.VirtualFileSystem = new Utils.VirtualFileSystem(virtualFileSystemRoot, /*useCaseSensitiveFilenames*/false); @@ -166,6 +166,8 @@ namespace Harness.LanguageService { throw new Error("No script with name '" + fileName + "'"); } + public abstract addSymlink(from: string, target: string): void; + public openFile(_fileName: string, _content?: string, _scriptKindName?: string): void { /*overridden*/ } /** @@ -180,7 +182,9 @@ namespace Harness.LanguageService { } /// Native adapter - class NativeLanguageServiceHost extends LanguageServiceAdapterHost implements ts.LanguageServiceHost { + class NativeLanguageServiceHost extends LanguageServiceAdapterHost implements ts.LanguageServiceHost, LanguageServiceAdapterHost { + symlinks = ts.createMap(); + isKnownTypesPackageName(name: string): boolean { return this.typesRegistry && this.typesRegistry.has(name); } @@ -211,13 +215,16 @@ namespace Harness.LanguageService { } directoryExists(dirName: string): boolean { + if (ts.forEachEntry(this.symlinks, (_, key) => ts.forSomeAncestorDirectory(key, ancestor => ancestor === dirName))) { + return true; + } + const fileEntry = this.virtualFileSystem.traversePath(dirName); return fileEntry && fileEntry.isDirectory(); } fileExists(fileName: string): boolean { - const script = this.getScriptSnapshot(fileName); - return script !== undefined; + return this.symlinks.has(fileName) || this.getScriptSnapshot(fileName) !== undefined; } readDirectory(path: string, extensions?: ReadonlyArray, exclude?: ReadonlyArray, include?: ReadonlyArray, depth?: number): string[] { return ts.matchFiles(path, extensions, exclude, include, @@ -227,9 +234,19 @@ namespace Harness.LanguageService { (p) => this.virtualFileSystem.getAccessibleFileSystemEntries(p)); } readFile(path: string): string | undefined { + const target = this.symlinks.get(path); + if (target !== undefined) { + return this.readFile(target); + } + const snapshot = this.getScriptSnapshot(path); return snapshot.getText(0, snapshot.getLength()); } + addSymlink(from: string, target: string) { this.symlinks.set(from, target); } + realpath(path: string): string { + const target = this.symlinks.get(path); + return target === undefined ? path : target; + } getTypeRootsVersion() { return 0; } @@ -245,7 +262,7 @@ namespace Harness.LanguageService { constructor(cancellationToken?: ts.HostCancellationToken, options?: ts.CompilerOptions) { this.host = new NativeLanguageServiceHost(cancellationToken, options); } - getHost() { return this.host; } + getHost(): LanguageServiceAdapterHost { return this.host; } getLanguageService(): ts.LanguageService { return ts.createLanguageService(this.host); } getClassifier(): ts.Classifier { return ts.createClassifier(); } getPreProcessedFileInfo(fileName: string, fileContents: string): ts.PreProcessedFileInfo { return ts.preProcessFile(fileContents, /* readImportFiles */ true, ts.hasJavaScriptFileExtension(fileName)); } @@ -258,6 +275,8 @@ namespace Harness.LanguageService { public getModuleResolutionsForFile: (fileName: string) => string; public getTypeReferenceDirectiveResolutionsForFile: (fileName: string) => string; + addSymlink() { return ts.notImplemented(); } + constructor(preprocessToResolve: boolean, cancellationToken?: ts.HostCancellationToken, options?: ts.CompilerOptions) { super(cancellationToken, options); this.nativeHost = new NativeLanguageServiceHost(cancellationToken, options); diff --git a/src/harness/unittests/languageService.ts b/src/harness/unittests/languageService.ts index a24a124e27c..2760df63c8c 100644 --- a/src/harness/unittests/languageService.ts +++ b/src/harness/unittests/languageService.ts @@ -40,9 +40,6 @@ export function Component(x: Config): any;` getDefaultLibFileName(options) { return ts.getDefaultLibFilePath(options); }, - fileExists: noop as any, - readFile: noop as any, - readDirectory: noop as any, }); const definitions = languageService.getDefinitionAtPosition("foo.ts", 160); // 160 is the latter `vueTemplateHtml` position assert.isDefined(definitions); diff --git a/src/harness/unittests/moduleResolution.ts b/src/harness/unittests/moduleResolution.ts index f77030aa183..1c18ac273ae 100644 --- a/src/harness/unittests/moduleResolution.ts +++ b/src/harness/unittests/moduleResolution.ts @@ -313,7 +313,7 @@ namespace ts { const host = createModuleResolutionHost(/*hasDirectoryExists*/ true, { name: realFileName, symlinks: [symlinkFileName] }); const resolution = nodeModuleNameResolver("linked", "/app/app.ts", { preserveSymlinks }, host); const resolvedFileName = preserveSymlinks ? symlinkFileName : realFileName; - checkResolvedModule(resolution.resolvedModule, { resolvedFileName, isExternalLibraryImport: true, extension: Extension.Dts }); + checkResolvedModule(resolution.resolvedModule, createResolvedModule(resolvedFileName, /*isExternalLibraryImport*/ true)); }); } }); @@ -338,7 +338,7 @@ namespace ts { const path = normalizePath(combinePaths(currentDirectory, fileName)); return files.has(path); }, - readFile: notImplemented + readFile: notImplemented, }; const program = createProgram(rootFiles, options, host); @@ -426,7 +426,7 @@ export = C; const path = getCanonicalFileName(normalizePath(combinePaths(currentDirectory, fileName))); return files.has(path); }, - readFile: notImplemented + readFile: notImplemented, }; const program = createProgram(rootFiles, options, host); const diagnostics = sortAndDeduplicateDiagnostics([...program.getSemanticDiagnostics(), ...program.getOptionsDiagnostics()]); @@ -1067,7 +1067,7 @@ import b = require("./moduleB"); readFile: fileName => { const file = sourceFiles.get(fileName); return file && file.text; - } + }, }; const program1 = createProgram(names, {}, compilerHost); const diagnostics1 = program1.getFileProcessingDiagnostics().getDiagnostics(); diff --git a/src/harness/unittests/programMissingFiles.ts b/src/harness/unittests/programMissingFiles.ts index 8d4e8a246b9..e6e2b9af714 100644 --- a/src/harness/unittests/programMissingFiles.ts +++ b/src/harness/unittests/programMissingFiles.ts @@ -9,7 +9,7 @@ namespace ts { assert(value, `${missing} to be ${value === undefined ? "not present" : "present only once"}, in actual: ${missingPaths} expected: ${expected}`); map.set(missing, false); } - const notFound = mapDefinedIter(map.keys(), k => map.get(k) === true ? k : undefined); + const notFound = arrayFrom(mapDefinedIterator(map.keys(), k => map.get(k) === true ? k : undefined)); assert.equal(notFound.length, 0, `Not found ${notFound} in actual: ${missingPaths} expected: ${expected}`); } diff --git a/src/harness/virtualFileSystemWithWatch.ts b/src/harness/virtualFileSystemWithWatch.ts index d410fec9206..3cf480ead83 100644 --- a/src/harness/virtualFileSystemWithWatch.ts +++ b/src/harness/virtualFileSystemWithWatch.ts @@ -168,7 +168,7 @@ interface Array {}` mapSeen.set(f, true); } } - assert.equal(mapExpected.size, 0, `Output has missing ${JSON.stringify(flatMapIter(mapExpected.keys(), key => key))} in ${JSON.stringify(host.getOutput())}`); + assert.equal(mapExpected.size, 0, `Output has missing ${JSON.stringify(arrayFrom(mapExpected.keys()))} in ${JSON.stringify(host.getOutput())}`); } export function checkOutputDoesNotContain(host: TestServerHost, expectedToBeAbsent: string[] | ReadonlyArray) { @@ -241,7 +241,7 @@ interface Array {}` ignoreWatchInvokedWithTriggerAsFileCreate: boolean; } - export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost { + export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost, ModuleResolutionHost { args: string[] = []; private readonly output: string[] = []; diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 4f422b10036..e786a77c6b6 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -32,6 +32,7 @@ namespace ts.codefix { interface ImportCodeFixContext extends SymbolAndTokenContext { host: LanguageServiceHost; + program: Program; checker: TypeChecker; compilerOptions: CompilerOptions; getCanonicalFileName: GetCanonicalFileName; @@ -161,15 +162,17 @@ namespace ts.codefix { function convertToImportCodeFixContext(context: CodeFixContext): ImportCodeFixContext { const useCaseSensitiveFileNames = context.host.useCaseSensitiveFileNames ? context.host.useCaseSensitiveFileNames() : false; - const checker = context.program.getTypeChecker(); + const { program } = context; + const checker = program.getTypeChecker(); const symbolToken = getTokenAtPosition(context.sourceFile, context.span.start, /*includeJsDocComment*/ false); return { host: context.host, newLineCharacter: context.newLineCharacter, formatContext: context.formatContext, sourceFile: context.sourceFile, + program, checker, - compilerOptions: context.program.getCompilerOptions(), + compilerOptions: program.getCompilerOptions(), cachedImportDeclarations: [], getCanonicalFileName: createGetCanonicalFileName(useCaseSensitiveFileNames), symbolName: symbolToken.getText(), @@ -309,6 +312,7 @@ namespace ts.codefix { } export function getModuleSpecifiersForNewImport( + program: Program, sourceFile: SourceFile, moduleSymbols: ReadonlyArray, options: CompilerOptions, @@ -316,68 +320,79 @@ namespace ts.codefix { host: LanguageServiceHost, ): string[] { const { baseUrl, paths, rootDirs } = options; - const choicesForEachExportingModule = mapIterator(arrayIterator(moduleSymbols), moduleSymbol => { - const moduleFileName = moduleSymbol.valueDeclaration.getSourceFile().fileName; - const sourceDirectory = getDirectoryPath(sourceFile.fileName); - const global = tryGetModuleNameFromAmbientModule(moduleSymbol) - || tryGetModuleNameFromTypeRoots(options, host, getCanonicalFileName, moduleFileName) - || tryGetModuleNameAsNodeModule(options, moduleFileName, host, getCanonicalFileName, sourceDirectory) - || rootDirs && tryGetModuleNameFromRootDirs(rootDirs, moduleFileName, sourceDirectory, getCanonicalFileName); - if (global) { - return [global]; - } - - const relativePath = removeExtensionAndIndexPostFix(getRelativePath(moduleFileName, sourceDirectory, getCanonicalFileName), options); - if (!baseUrl) { - return [relativePath]; - } - - const relativeToBaseUrl = getRelativePathIfInDirectory(moduleFileName, baseUrl, getCanonicalFileName); - if (!relativeToBaseUrl) { - return [relativePath]; - } - - const importRelativeToBaseUrl = removeExtensionAndIndexPostFix(relativeToBaseUrl, options); - if (paths) { - const fromPaths = tryGetModuleNameFromPaths(removeFileExtension(relativeToBaseUrl), importRelativeToBaseUrl, paths); - if (fromPaths) { - return [fromPaths]; + const choicesForEachExportingModule = flatMap(moduleSymbols, moduleSymbol => + getAllModulePaths(program, moduleSymbol.valueDeclaration.getSourceFile()).map(moduleFileName => { + const sourceDirectory = getDirectoryPath(sourceFile.fileName); + const global = tryGetModuleNameFromAmbientModule(moduleSymbol) + || tryGetModuleNameFromTypeRoots(options, host, getCanonicalFileName, moduleFileName) + || tryGetModuleNameAsNodeModule(options, moduleFileName, host, getCanonicalFileName, sourceDirectory) + || rootDirs && tryGetModuleNameFromRootDirs(rootDirs, moduleFileName, sourceDirectory, getCanonicalFileName); + if (global) { + return [global]; } - } - /* - Prefer a relative import over a baseUrl import if it doesn't traverse up to baseUrl. + const relativePath = removeExtensionAndIndexPostFix(getRelativePath(moduleFileName, sourceDirectory, getCanonicalFileName), options); + if (!baseUrl) { + return [relativePath]; + } - Suppose we have: - baseUrl = /base - sourceDirectory = /base/a/b - moduleFileName = /base/foo/bar - Then: - relativePath = ../../foo/bar - getRelativePathNParents(relativePath) = 2 - pathFromSourceToBaseUrl = ../../ - getRelativePathNParents(pathFromSourceToBaseUrl) = 2 - 2 < 2 = false - In this case we should prefer using the baseUrl path "/a/b" instead of the relative path "../../foo/bar". + const relativeToBaseUrl = getRelativePathIfInDirectory(moduleFileName, baseUrl, getCanonicalFileName); + if (!relativeToBaseUrl) { + return [relativePath]; + } - Suppose we have: - baseUrl = /base - sourceDirectory = /base/foo/a - moduleFileName = /base/foo/bar - Then: - relativePath = ../a - getRelativePathNParents(relativePath) = 1 - pathFromSourceToBaseUrl = ../../ - getRelativePathNParents(pathFromSourceToBaseUrl) = 2 - 1 < 2 = true - In this case we should prefer using the relative path "../a" instead of the baseUrl path "foo/a". - */ - const pathFromSourceToBaseUrl = getRelativePath(baseUrl, sourceDirectory, getCanonicalFileName); - const relativeFirst = getRelativePathNParents(pathFromSourceToBaseUrl) < getRelativePathNParents(relativePath); - return relativeFirst ? [relativePath, importRelativeToBaseUrl] : [importRelativeToBaseUrl, relativePath]; - }); + const importRelativeToBaseUrl = removeExtensionAndIndexPostFix(relativeToBaseUrl, options); + if (paths) { + const fromPaths = tryGetModuleNameFromPaths(removeFileExtension(relativeToBaseUrl), importRelativeToBaseUrl, paths); + if (fromPaths) { + return [fromPaths]; + } + } + + /* + Prefer a relative import over a baseUrl import if it doesn't traverse up to baseUrl. + + Suppose we have: + baseUrl = /base + sourceDirectory = /base/a/b + moduleFileName = /base/foo/bar + Then: + relativePath = ../../foo/bar + getRelativePathNParents(relativePath) = 2 + pathFromSourceToBaseUrl = ../../ + getRelativePathNParents(pathFromSourceToBaseUrl) = 2 + 2 < 2 = false + In this case we should prefer using the baseUrl path "/a/b" instead of the relative path "../../foo/bar". + + Suppose we have: + baseUrl = /base + sourceDirectory = /base/foo/a + moduleFileName = /base/foo/bar + Then: + relativePath = ../a + getRelativePathNParents(relativePath) = 1 + pathFromSourceToBaseUrl = ../../ + getRelativePathNParents(pathFromSourceToBaseUrl) = 2 + 1 < 2 = true + In this case we should prefer using the relative path "../a" instead of the baseUrl path "foo/a". + */ + const pathFromSourceToBaseUrl = getRelativePath(baseUrl, sourceDirectory, getCanonicalFileName); + const relativeFirst = getRelativePathNParents(pathFromSourceToBaseUrl) < getRelativePathNParents(relativePath); + return relativeFirst ? [relativePath, importRelativeToBaseUrl] : [importRelativeToBaseUrl, relativePath]; + })); // Only return results for the re-export with the shortest possible path (and also give the other path even if that's long.) - return best(choicesForEachExportingModule, (a, b) => a[0].length < b[0].length); + return best(arrayIterator(choicesForEachExportingModule), (a, b) => a[0].length < b[0].length); + } + + /** + * Looks for a existing imports that use symlinks to this module. + * Only if no symlink is available, the real path will be used. + */ + function getAllModulePaths(program: Program, { fileName }: SourceFile): ReadonlyArray { + const symlinks = mapDefined(program.getSourceFiles(), sf => + sf.resolvedModules && firstDefinedIterator(sf.resolvedModules.values(), res => + res && res.resolvedFileName === fileName ? res.originalPath : undefined)); + return symlinks.length === 0 ? [fileName] : symlinks; } function getRelativePathNParents(relativePath: string): number { @@ -613,7 +628,7 @@ namespace ts.codefix { } const existingDeclaration = firstDefined(declarations, moduleSpecifierFromAnyImport); - const moduleSpecifiers = existingDeclaration ? [existingDeclaration] : getModuleSpecifiersForNewImport(ctx.sourceFile, moduleSymbols, ctx.compilerOptions, ctx.getCanonicalFileName, ctx.host); + const moduleSpecifiers = existingDeclaration ? [existingDeclaration] : getModuleSpecifiersForNewImport(ctx.program, ctx.sourceFile, moduleSymbols, ctx.compilerOptions, ctx.getCanonicalFileName, ctx.host); return moduleSpecifiers.map(spec => getCodeActionForNewImport(ctx, spec)); } diff --git a/src/services/completions.ts b/src/services/completions.ts index 8de32b95225..d893f4852e4 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -466,7 +466,7 @@ namespace ts.Completions { } export function getCompletionEntryDetails( - typeChecker: TypeChecker, + program: Program, log: (message: string) => void, compilerOptions: CompilerOptions, sourceFile: SourceFile, @@ -477,6 +477,7 @@ namespace ts.Completions { formatContext: formatting.FormatContext, getCanonicalFileName: GetCanonicalFileName, ): CompletionEntryDetails { + const typeChecker = program.getTypeChecker(); const { name } = entryId; // Compute all the completion symbols again. const symbolCompletion = getSymbolCompletionFromEntryId(typeChecker, log, compilerOptions, sourceFile, position, entryId, allSourceFiles); @@ -496,7 +497,7 @@ namespace ts.Completions { } case "symbol": { const { symbol, location, symbolToOriginInfoMap } = symbolCompletion; - const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, typeChecker, host, compilerOptions, sourceFile, formatContext, getCanonicalFileName, allSourceFiles); + const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, program, typeChecker, host, compilerOptions, sourceFile, formatContext, getCanonicalFileName, allSourceFiles); const kindModifiers = SymbolDisplay.getSymbolModifiers(symbol); const { displayParts, documentation, symbolKind, tags } = SymbolDisplay.getSymbolDisplayPartsDocumentationAndSymbolKind(typeChecker, symbol, sourceFile, location, location, SemanticMeaning.All); return { name, kindModifiers, kind: symbolKind, displayParts, documentation, tags, codeActions, source: sourceDisplay }; @@ -523,6 +524,7 @@ namespace ts.Completions { function getCompletionEntryCodeActionsAndSourceDisplay( symbolToOriginInfoMap: SymbolOriginInfoMap, symbol: Symbol, + program: Program, checker: TypeChecker, host: LanguageServiceHost, compilerOptions: CompilerOptions, @@ -541,9 +543,10 @@ namespace ts.Completions { const moduleSymbols = getAllReExportingModules(exportedSymbol, checker, allSourceFiles); Debug.assert(contains(moduleSymbols, moduleSymbol)); - const sourceDisplay = [textPart(first(codefix.getModuleSpecifiersForNewImport(sourceFile, moduleSymbols, compilerOptions, getCanonicalFileName, host)))]; + const sourceDisplay = [textPart(first(codefix.getModuleSpecifiersForNewImport(program, sourceFile, moduleSymbols, compilerOptions, getCanonicalFileName, host)))]; const codeActions = codefix.getCodeActionForImport(moduleSymbols, { host, + program, checker, newLineCharacter: host.getNewLine(), compilerOptions, diff --git a/src/services/refactorProvider.ts b/src/services/refactorProvider.ts index e0a19247939..85ef9113bda 100644 --- a/src/services/refactorProvider.ts +++ b/src/services/refactorProvider.ts @@ -33,8 +33,8 @@ namespace ts { } export function getApplicableRefactors(context: RefactorContext): ApplicableRefactorInfo[] { - return flatMapIter(refactors.values(), refactor => - context.cancellationToken && context.cancellationToken.isCancellationRequested() ? undefined : refactor.getAvailableActions(context)); + return arrayFrom(flatMapIterator(refactors.values(), refactor => + context.cancellationToken && context.cancellationToken.isCancellationRequested() ? undefined : refactor.getAvailableActions(context))); } export function getEditsForRefactor(context: RefactorContext, refactorName: string, actionName: string): RefactorEditInfo | undefined { diff --git a/src/services/services.ts b/src/services/services.ts index 3c39affcacb..770d42dd3dd 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1269,6 +1269,7 @@ namespace ts { } return host.readFile && host.readFile(fileName); }, + realpath: host.realpath && (path => host.realpath(path)), directoryExists: directoryName => { return directoryProbablyExists(directoryName, host); }, @@ -1447,7 +1448,7 @@ namespace ts { function getCompletionEntryDetails(fileName: string, position: number, name: string, formattingOptions?: FormatCodeSettings, source?: string): CompletionEntryDetails { synchronizeHostData(); return Completions.getCompletionEntryDetails( - program.getTypeChecker(), + program, log, program.getCompilerOptions(), getValidSourceFile(fileName), diff --git a/src/services/shims.ts b/src/services/shims.ts index 7c932361c10..8000a046831 100644 --- a/src/services/shims.ts +++ b/src/services/shims.ts @@ -330,7 +330,7 @@ namespace ts { // if shimHost is a COM object then property check will become method call with no arguments. // 'in' does not have this effect. if ("getModuleResolutionsForFile" in this.shimHost) { - this.resolveModuleNames = (moduleNames: string[], containingFile: string) => { + this.resolveModuleNames = (moduleNames: string[], containingFile: string): ResolvedModuleFull[] => { const resolutionsInFile = >JSON.parse(this.shimHost.getModuleResolutionsForFile(containingFile)); return map(moduleNames, name => { const result = getProperty(resolutionsInFile, name); diff --git a/src/services/types.ts b/src/services/types.ts index 7224fdbdda8..272ae19f007 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -181,6 +181,7 @@ namespace ts { */ readDirectory?(path: string, extensions?: ReadonlyArray, exclude?: ReadonlyArray, include?: ReadonlyArray, depth?: number): string[]; readFile?(path: string, encoding?: string): string | undefined; + realpath?(path: string): string; fileExists?(path: string): boolean; /* diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index abd03c26b0b..29009985080 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -3925,6 +3925,7 @@ declare namespace ts { useCaseSensitiveFileNames?(): boolean; readDirectory?(path: string, extensions?: ReadonlyArray, exclude?: ReadonlyArray, include?: ReadonlyArray, depth?: number): string[]; readFile?(path: string, encoding?: string): string | undefined; + realpath?(path: string): string; fileExists?(path: string): boolean; getTypeRootsVersion?(): number; resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames?: string[]): ResolvedModule[]; diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 62a3880827c..84b7d9a19be 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -3925,6 +3925,7 @@ declare namespace ts { useCaseSensitiveFileNames?(): boolean; readDirectory?(path: string, extensions?: ReadonlyArray, exclude?: ReadonlyArray, include?: ReadonlyArray, depth?: number): string[]; readFile?(path: string, encoding?: string): string | undefined; + realpath?(path: string): string; fileExists?(path: string): boolean; getTypeRootsVersion?(): number; resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames?: string[]): ResolvedModule[]; diff --git a/tests/cases/fourslash/importNameCodeFix_symlink.ts b/tests/cases/fourslash/importNameCodeFix_symlink.ts new file mode 100644 index 00000000000..8da44de7797 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFix_symlink.ts @@ -0,0 +1,22 @@ +/// + +// @moduleResolution: node +// @noLib: true + +// @Filename: /node_modules/real/index.d.ts +// @Symlink: /node_modules/link/index.d.ts +////export const foo: number; + +// @Filename: /a.ts +////import { foo } from "link"; + +// @Filename: /b.ts +////[|foo/**/;|] + +// Uses "link" instead of "real" because `a` did. +goTo.file("/b.ts"); +verify.importFixAtPosition([ +`import { foo } from "link"; + +foo;`, +]);