Skip to content

Use symlinks when looking for module names for declaration emit #24874

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4091,7 +4091,14 @@ namespace ts {
}
else {
const contextFile = getSourceFileOfNode(getOriginalNode(context!.enclosingDeclaration))!;
return `"${file.moduleName || moduleSpecifiers.getModuleSpecifier(compilerOptions, contextFile, contextFile.path, file.path, context!.tracker.moduleResolverHost!)}"`;
return `"${file.moduleName || moduleSpecifiers.getModuleSpecifiers(
symbol,
compilerOptions,
contextFile,
context!.tracker.moduleResolverHost!,
context!.tracker.moduleResolverHost!.getSourceFiles!(),
{ importModuleSpecifierPreference: "non-relative" }
)[0]}"`;
}
}
const declaration = symbol.declarations[0];
Expand Down
59 changes: 52 additions & 7 deletions src/compiler/moduleSpecifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,20 @@ namespace ts.moduleSpecifiers {
// For each symlink/original for a module, returns a list of ways to import that file.
export function getModuleSpecifiers(
moduleSymbol: Symbol,
program: Program,
compilerOptions: CompilerOptions,
importingSourceFile: SourceFile,
host: ModuleSpecifierResolutionHost,
files: ReadonlyArray<SourceFile>,
preferences: ModuleSpecifierPreferences,
): ReadonlyArray<ReadonlyArray<string>> {
const ambient = tryGetModuleNameFromAmbientModule(moduleSymbol);
if (ambient) return [[ambient]];

const compilerOptions = program.getCompilerOptions();
const info = getInfo(compilerOptions, importingSourceFile, importingSourceFile.fileName, host);
const modulePaths = getAllModulePaths(program, getSourceFileOfNode(moduleSymbol.valueDeclaration));
const info = getInfo(compilerOptions, importingSourceFile, importingSourceFile.path, host);
if (!files) {
return Debug.fail("Files list must be present to resolve symlinks in specifier resolution");
}
const modulePaths = getAllModulePaths(files, getSourceFileOfNode(moduleSymbol.valueDeclaration), info.getCanonicalFileName, host);

const global = mapDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions));
return global.length ? global.map(g => [g]) : modulePaths.map(moduleFileName =>
Expand Down Expand Up @@ -130,15 +133,57 @@ namespace ts.moduleSpecifiers {
return firstDefined(imports, ({ text }) => pathIsRelative(text) ? fileExtensionIs(text, Extension.Js) : undefined) || false;
}

function discoverProbableSymlinks(files: ReadonlyArray<SourceFile>) {
const symlinks = mapDefined(files, sf =>
sf.resolvedModules && firstDefinedIterator(sf.resolvedModules.values(), res =>
res && res.originalPath && res.resolvedFileName !== res.originalPath ? [res.resolvedFileName, res.originalPath] : undefined));
const result = createMap<string>();
if (symlinks) {
for (const [resolvedPath, originalPath] of symlinks) {
const resolvedParts = getPathComponents(resolvedPath);
const originalParts = getPathComponents(originalPath);
while (resolvedParts[resolvedParts.length - 1] === originalParts[originalParts.length - 1]) {
resolvedParts.pop();
originalParts.pop();
}
result.set(getPathFromPathComponents(originalParts), getPathFromPathComponents(resolvedParts));
}
}
return result;
}

function getAllModulePathsUsingIndirectSymlinks(files: ReadonlyArray<SourceFile>, target: string, getCanonicalFileName: (file: string) => string, host: ModuleSpecifierResolutionHost) {
const links = discoverProbableSymlinks(files);
const paths = arrayFrom(links.keys());
let options: string[] | undefined;
for (const path of paths) {
const resolved = links.get(path)!;
if (startsWith(target, resolved + "/")) {
const relative = getRelativePathFromDirectory(resolved, target, getCanonicalFileName);
const option = resolvePath(path, relative);
if (!host.fileExists || host.fileExists(option)) {
if (!options) options = [];
options.push(option);
}
}
}
const resolvedtarget = host.getCurrentDirectory ? resolvePath(host.getCurrentDirectory(), target) : target;
if (options) {
options.push(resolvedtarget); // Since these are speculative, we also include the original resolved name as a possibility
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure i understand why we are adding the original name?

Copy link
Member Author

Choose a reason for hiding this comment

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

When we find a symlinked path that we know is correct, we make a choice to ignore the original path and not present is as an option. Since the file in this case wasn't imported directly via a symlink in the compilation already, in this case I think it's more appropriate to keep it around, that way you can still get, eg ./foo as an option even though module/foo is possible.

return options;
}
return [resolvedtarget];
}

/**
* 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<string> {
const symlinks = mapDefined(program.getSourceFiles(), sf =>
function getAllModulePaths(files: ReadonlyArray<SourceFile>, { fileName }: SourceFile, getCanonicalFileName: (file: string) => string, host: ModuleSpecifierResolutionHost): ReadonlyArray<string> {
const symlinks = mapDefined(files, sf =>
sf.resolvedModules && firstDefinedIterator(sf.resolvedModules.values(), res =>
res && res.resolvedFileName === fileName ? res.originalPath : undefined));
return symlinks.length === 0 ? [fileName] : symlinks;
return symlinks.length === 0 ? getAllModulePathsUsingIndirectSymlinks(files, fileName, getCanonicalFileName, host) : symlinks;
}

function getRelativePathNParents(relativePath: string): number {
Expand Down
22 changes: 18 additions & 4 deletions src/harness/harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1108,6 +1108,7 @@ namespace Harness {
{ name: "noImplicitReferences", type: "boolean" },
{ name: "currentDirectory", type: "string" },
{ name: "symlink", type: "string" },
{ name: "link", type: "string" },
// Emitted js baseline will print full paths for every output file
{ name: "fullEmitPaths", type: "boolean" }
];
Expand Down Expand Up @@ -1179,7 +1180,9 @@ namespace Harness {
harnessSettings: TestCaseParser.CompilerSettings | undefined,
compilerOptions: ts.CompilerOptions | undefined,
// Current directory is needed for rwcRunner to be able to use currentDirectory defined in json file
currentDirectory: string | undefined): compiler.CompilationResult {
currentDirectory: string | undefined,
symlinks?: vfs.FileSet
): compiler.CompilationResult {
const options: ts.CompilerOptions & HarnessOptions = compilerOptions ? ts.cloneCompilerOptions(compilerOptions) : { noResolve: false };
options.target = options.target || ts.ScriptTarget.ES3;
options.newLine = options.newLine || ts.NewLineKind.CarriageReturnLineFeed;
Expand Down Expand Up @@ -1216,6 +1219,9 @@ namespace Harness {

const docs = inputFiles.concat(otherFiles).map(documents.TextDocument.fromTestFile);
const fs = vfs.createFromFileSystem(IO, !useCaseSensitiveFileNames, { documents: docs, cwd: currentDirectory });
if (symlinks) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If symlinks were a vfs.FileSet, you could just call fs.apply(symlinks).

fs.apply(symlinks);
}
const host = new fakes.CompilerHost(fs, options);
return compiler.compileFiles(host, programFileNames, options);
}
Expand Down Expand Up @@ -1836,6 +1842,7 @@ namespace Harness {

// Regex for parsing options in the format "@Alpha: Value of any sort"
const optionRegex = /^[\/]{2}\s*@(\w+)\s*:\s*([^\r\n]*)/gm; // multiple matches on multiple lines
const linkRegex = /^[\/]{2}\s*@link\s*:\s*([^\r\n]*)\s*->\s*([^\r\n]*)/gm; // multiple matches on multiple lines

export function extractCompilerSettings(content: string): CompilerSettings {
const opts: CompilerSettings = {};
Expand All @@ -1855,6 +1862,7 @@ namespace Harness {
testUnitData: TestUnitData[];
tsConfig: ts.ParsedCommandLine | undefined;
tsConfigFileUnitData: TestUnitData | undefined;
symlinks?: vfs.FileSet;
}

/** Given a test file containing // @FileName directives, return an array of named units of code to be added to an existing compiler instance */
Expand All @@ -1869,10 +1877,16 @@ namespace Harness {
let currentFileOptions: any = {};
let currentFileName: any;
let refs: string[] = [];
let symlinks: vfs.FileSet | undefined;

for (const line of lines) {
const testMetaData = optionRegex.exec(line);
if (testMetaData) {
let testMetaData: RegExpExecArray | null;
const linkMetaData = linkRegex.exec(line);
if (linkMetaData) {
if (!symlinks) symlinks = {};
symlinks[linkMetaData[2].trim()] = new vfs.Symlink(linkMetaData[1].trim());
}
else if (testMetaData = optionRegex.exec(line)) {
// Comment line, check for global/file @options and record them
optionRegex.lastIndex = 0;
const metaDataName = testMetaData[1].toLowerCase();
Expand Down Expand Up @@ -1961,7 +1975,7 @@ namespace Harness {
break;
}
}
return { settings, testUnitData, tsConfig, tsConfigFileUnitData };
return { settings, testUnitData, tsConfig, tsConfigFileUnitData, symlinks };
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/harness/vfs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,7 @@ namespace vfs {
if (this.stringComparer(vpath.dirname(path), path) === 0) {
throw new TypeError("Roots cannot be symbolic links.");
}
this.symlinkSync(entry.symlink, path);
this.symlinkSync(vpath.resolve(dirname, entry.symlink), path);
this._applyFileExtendedOptions(path, entry);
}
else if (entry instanceof Link) {
Expand Down Expand Up @@ -1078,8 +1078,7 @@ namespace vfs {
if (symlink) {
for (const link of symlink.split(",").map(link => link.trim())) {
fs.mkdirpSync(vpath.dirname(link));
fs.symlinkSync(document.file, link);
fs.filemeta(link).set("document", document);
fs.symlinkSync(vpath.resolve(fs.cwd(), document.file), link);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/parser/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5255,6 +5255,7 @@ namespace ts {
useCaseSensitiveFileNames?(): boolean;
fileExists?(path: string): boolean;
readFile?(path: string): string | undefined;
getSourceFiles?(): ReadonlyArray<SourceFile>; // Used for cached resolutions to find symlinks without traversing the fs (again)
}

/** @deprecated See comment on SymbolWriter */
Expand Down
2 changes: 1 addition & 1 deletion src/services/codefixes/importFixes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ namespace ts.codefix {
preferences: UserPreferences,
): ReadonlyArray<NewImportInfo> {
const choicesForEachExportingModule = flatMap<SymbolExportInfo, NewImportInfo[]>(moduleSymbols, ({ moduleSymbol, importKind }) => {
const modulePathsGroups = moduleSpecifiers.getModuleSpecifiers(moduleSymbol, program, sourceFile, host, preferences);
const modulePathsGroups = moduleSpecifiers.getModuleSpecifiers(moduleSymbol, program.getCompilerOptions(), sourceFile, host, program.getSourceFiles(), preferences);
return modulePathsGroups.map(group => group.map(moduleSpecifier => ({ moduleSpecifier, importKind })));
});
// Sort to keep the shortest paths first, but keep [relativePath, importRelativeToBaseUrl] groups together
Expand Down
4 changes: 3 additions & 1 deletion src/testRunner/compilerRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,9 @@ class CompilerTest {
this.otherFiles,
this.harnessSettings,
/*options*/ tsConfigOptions,
/*currentDirectory*/ this.harnessSettings.currentDirectory);
/*currentDirectory*/ this.harnessSettings.currentDirectory,
testCaseContent.symlinks
);

this.options = this.result.options;
}
Expand Down
Loading