Skip to content

Commit

Permalink
Merge pull request #4738 from Microsoft/properExternalModules
Browse files Browse the repository at this point in the history 
Check if imported file is a proper external module for imported node typings
  • Loading branch information
@vladima
vladima committed Sep 11, 2015
2 parents dcb9e1c + 87e1569 commit 026632b
Show file tree
Hide file tree
Showing 28 changed files with 347 additions and 145 deletions.
4 changes: 2 additions & 2 deletions src/compiler/checker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -976,8 +976,8 @@ namespace ts {
}
}

let fileName = getResolvedModuleFileName(getSourceFile(location), moduleReferenceLiteral.text);
let sourceFile = fileName && host.getSourceFile(fileName);
let resolvedModule = getResolvedModule(getSourceFile(location), moduleReferenceLiteral.text);
let sourceFile = resolvedModule && host.getSourceFile(resolvedModule.resolvedFileName);
if (sourceFile) {
if (sourceFile.symbol) {
return sourceFile.symbol;
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/diagnosticInformationMap.generated.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,8 @@ namespace ts {
A_member_initializer_in_a_enum_declaration_cannot_reference_members_declared_after_it_including_members_defined_in_other_enums: { code: 2651, category: DiagnosticCategory.Error, key: "A member initializer in a enum declaration cannot reference members declared after it, including members defined in other enums." },
Merged_declaration_0_cannot_include_a_default_export_declaration_Consider_adding_a_separate_export_default_0_declaration_instead: { code: 2652, category: DiagnosticCategory.Error, key: "Merged declaration '{0}' cannot include a default export declaration. Consider adding a separate 'export default {0}' declaration instead." },
Non_abstract_class_expression_does_not_implement_inherited_abstract_member_0_from_class_1: { code: 2653, category: DiagnosticCategory.Error, key: "Non-abstract class expression does not implement inherited abstract member '{0}' from class '{1}'." },
Exported_external_package_typings_file_cannot_contain_tripleslash_references_Please_contact_the_package_author_to_update_the_package_definition: { code: 2654, category: DiagnosticCategory.Error, key: "Exported external package typings file cannot contain tripleslash references. Please contact the package author to update the package definition." },
Exported_external_package_typings_can_only_be_in_d_ts_files_Please_contact_the_package_author_to_update_the_package_definition: { code: 2655, category: DiagnosticCategory.Error, key: "Exported external package typings can only be in '.d.ts' files. Please contact the package author to update the package definition." },
Import_declaration_0_is_using_private_name_1: { code: 4000, category: DiagnosticCategory.Error, key: "Import declaration '{0}' is using private name '{1}'." },
Type_parameter_0_of_exported_class_has_or_is_using_private_name_1: { code: 4002, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported class has or is using private name '{1}'." },
Type_parameter_0_of_exported_interface_has_or_is_using_private_name_1: { code: 4004, category: DiagnosticCategory.Error, key: "Type parameter '{0}' of exported interface has or is using private name '{1}'." },
Expand Down
9 changes: 8 additions & 1 deletion src/compiler/diagnosticMessages.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1705,7 +1705,14 @@
"category": "Error",
"code": 2653
},

"Exported external package typings file cannot contain tripleslash references. Please contact the package author to update the package definition.": {
"category": "Error",
"code": 2654
},
"Exported external package typings can only be in '.d.ts' files. Please contact the package author to update the package definition.": {
"category": "Error",
"code": 2655
},
"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
"code": 4000
Expand Down
91 changes: 44 additions & 47 deletions src/compiler/program.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ namespace ts {
return normalizePath(referencedFileName);
}

export function resolveModuleName(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost): ResolvedModule {
export function resolveModuleName(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost): ResolvedModuleWithFailedLookupLocations {
let moduleResolution = compilerOptions.moduleResolution !== undefined
? compilerOptions.moduleResolution
: compilerOptions.module === ModuleKind.CommonJS ? ModuleResolutionKind.NodeJs : ModuleResolutionKind.Classic;
Expand All @@ -47,7 +47,7 @@ namespace ts {
}
}

export function nodeModuleNameResolver(moduleName: string, containingFile: string, host: ModuleResolutionHost): ResolvedModule {
export function nodeModuleNameResolver(moduleName: string, containingFile: string, host: ModuleResolutionHost): ResolvedModuleWithFailedLookupLocations {
let containingDirectory = getDirectoryPath(containingFile);

if (getRootLength(moduleName) !== 0 || nameStartsWithDotSlashOrDotDotSlash(moduleName)) {
Expand All @@ -56,11 +56,13 @@ namespace ts {
let resolvedFileName = loadNodeModuleFromFile(candidate, /* loadOnlyDts */ false, failedLookupLocations, host);

if (resolvedFileName) {
return { resolvedFileName, failedLookupLocations };
return { resolvedModule: { resolvedFileName }, failedLookupLocations };
}

resolvedFileName = loadNodeModuleFromDirectory(candidate, /* loadOnlyDts */ false, failedLookupLocations, host);
return { resolvedFileName, failedLookupLocations };
return resolvedFileName
? { resolvedModule: { resolvedFileName }, failedLookupLocations }
: { resolvedModule: undefined, failedLookupLocations };
}
else {
return loadModuleFromNodeModules(moduleName, containingDirectory, host);
Expand Down Expand Up @@ -117,7 +119,7 @@ namespace ts {
return loadNodeModuleFromFile(combinePaths(candidate, "index"), loadOnlyDts, failedLookupLocation, host);
}

function loadModuleFromNodeModules(moduleName: string, directory: string, host: ModuleResolutionHost): ResolvedModule {
function loadModuleFromNodeModules(moduleName: string, directory: string, host: ModuleResolutionHost): ResolvedModuleWithFailedLookupLocations {
let failedLookupLocations: string[] = [];
directory = normalizeSlashes(directory);
while (true) {
Expand All @@ -127,12 +129,12 @@ namespace ts {
let candidate = normalizePath(combinePaths(nodeModulesFolder, moduleName));
let result = loadNodeModuleFromFile(candidate, /* loadOnlyDts */ true, failedLookupLocations, host);
if (result) {
return { resolvedFileName: result, failedLookupLocations };
return { resolvedModule: { resolvedFileName: result, isExternalLibraryImport: true }, failedLookupLocations };
}

result = loadNodeModuleFromDirectory(candidate, /* loadOnlyDts */ true, failedLookupLocations, host);
if (result) {
return { resolvedFileName: result, failedLookupLocations };
return { resolvedModule: { resolvedFileName: result, isExternalLibraryImport: true }, failedLookupLocations };
}
}

Expand All @@ -144,47 +146,19 @@ namespace ts {
directory = parentPath;
}

return { resolvedFileName: undefined, failedLookupLocations };
}

export function baseUrlModuleNameResolver(moduleName: string, containingFile: string, baseUrl: string, host: ModuleResolutionHost): ResolvedModule {
Debug.assert(baseUrl !== undefined);

let normalizedModuleName = normalizeSlashes(moduleName);
let basePart = useBaseUrl(moduleName) ? baseUrl : getDirectoryPath(containingFile);
let candidate = normalizePath(combinePaths(basePart, moduleName));

let failedLookupLocations: string[] = [];

return forEach(supportedExtensions, ext => tryLoadFile(candidate + ext)) || { resolvedFileName: undefined, failedLookupLocations };

function tryLoadFile(location: string): ResolvedModule {
if (host.fileExists(location)) {
return { resolvedFileName: location, failedLookupLocations };
}
else {
failedLookupLocations.push(location);
return undefined;
}
}
return { resolvedModule: undefined, failedLookupLocations };
}

function nameStartsWithDotSlashOrDotDotSlash(name: string) {
let i = name.lastIndexOf("./", 1);
return i === 0 || (i === 1 && name.charCodeAt(0) === CharacterCodes.dot);
}

function useBaseUrl(moduleName: string): boolean {
// path is not rooted
// module name does not start with './' or '../'
return getRootLength(moduleName) === 0 && !nameStartsWithDotSlashOrDotDotSlash(moduleName);
}

export function classicNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost): ResolvedModule {
export function classicNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost): ResolvedModuleWithFailedLookupLocations {

// module names that contain '!' are used to reference resources and are not resolved to actual files on disk
if (moduleName.indexOf('!') != -1) {
return { resolvedFileName: undefined, failedLookupLocations: [] };
return { resolvedModule: undefined, failedLookupLocations: [] };
}

let searchPath = getDirectoryPath(containingFile);
Expand Down Expand Up @@ -222,7 +196,9 @@ namespace ts {
searchPath = parentPath;
}

return { resolvedFileName: referencedSourceFile, failedLookupLocations };
return referencedSourceFile
? { resolvedModule: { resolvedFileName: referencedSourceFile }, failedLookupLocations }
: { resolvedModule: undefined, failedLookupLocations };
}

/* @internal */
Expand Down Expand Up @@ -372,9 +348,9 @@ namespace ts {

host = host || createCompilerHost(options);

const resolveModuleNamesWorker =
host.resolveModuleNames ||
((moduleNames, containingFile) => map(moduleNames, moduleName => resolveModuleName(moduleName, containingFile, options, host).resolvedFileName));
const resolveModuleNamesWorker = host.resolveModuleNames
? ((moduleNames: string[], containingFile: string) => host.resolveModuleNames(moduleNames, containingFile))
: ((moduleNames: string[], containingFile: string) => map(moduleNames, moduleName => resolveModuleName(moduleName, containingFile, options, host).resolvedModule));

let filesByName = createFileMap<SourceFile>(fileName => host.getCanonicalFileName(fileName));

Expand Down Expand Up @@ -494,10 +470,17 @@ namespace ts {
let resolutions = resolveModuleNamesWorker(moduleNames, newSourceFile.fileName);
// ensure that module resolution results are still correct
for (let i = 0; i < moduleNames.length; ++i) {
let oldResolution = getResolvedModuleFileName(oldSourceFile, moduleNames[i]);
if (oldResolution !== resolutions[i]) {
let newResolution = resolutions[i];
let oldResolution = getResolvedModule(oldSourceFile, moduleNames[i]);
let resolutionChanged = oldResolution
? !newResolution ||
oldResolution.resolvedFileName !== newResolution.resolvedFileName ||
!!oldResolution.isExternalLibraryImport !== !!newResolution.isExternalLibraryImport
: newResolution;

if (resolutionChanged) {
return false;
}
}
}
}
// pass the cache of module resolutions from the old source file
Expand Down Expand Up @@ -874,9 +857,23 @@ namespace ts {
let resolutions = resolveModuleNamesWorker(moduleNames, file.fileName);
for (let i = 0; i < file.imports.length; ++i) {
let resolution = resolutions[i];
setResolvedModuleName(file, moduleNames[i], resolution);
setResolvedModule(file, moduleNames[i], resolution);
if (resolution && !options.noResolve) {
findModuleSourceFile(resolution, file.imports[i]);
const importedFile = findModuleSourceFile(resolution.resolvedFileName, file.imports[i]);
if (importedFile && resolution.isExternalLibraryImport) {
if (!isExternalModule(importedFile)) {
let start = getTokenPosOfNode(file.imports[i], file)
fileProcessingDiagnostics.add(createFileDiagnostic(file, start, file.imports[i].end - start, Diagnostics.File_0_is_not_a_module, importedFile.fileName));
}
else if (!fileExtensionIs(importedFile.fileName, ".d.ts")) {
let start = getTokenPosOfNode(file.imports[i], file)
fileProcessingDiagnostics.add(createFileDiagnostic(file, start, file.imports[i].end - start, Diagnostics.Exported_external_package_typings_can_only_be_in_d_ts_files_Please_contact_the_package_author_to_update_the_package_definition));
}
else if (importedFile.referencedFiles.length) {
let firstRef = importedFile.referencedFiles[0];
fileProcessingDiagnostics.add(createFileDiagnostic(importedFile, firstRef.pos, firstRef.end - firstRef.pos, Diagnostics.Exported_external_package_typings_file_cannot_contain_tripleslash_references_Please_contact_the_package_author_to_update_the_package_definition));
}
}
}
}
}
Expand Down
17 changes: 13 additions & 4 deletions src/compiler/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1284,7 +1284,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 fiels should never be used directly - use getResolvedModuleFileName/setResolvedModuleFileName functions instead
/* @internal */ resolvedModules: Map<string>;
/* @internal */ resolvedModules: Map<ResolvedModule>;
/* @internal */ imports: LiteralExpression[];
}

Expand Down Expand Up @@ -2273,11 +2273,20 @@ namespace ts {

export interface ResolvedModule {
resolvedFileName: string;
/*
* Denotes if 'resolvedFileName' is isExternalLibraryImport and thus should be proper external module:
* - be a .d.ts file
* - use top level imports\exports
* - don't use tripleslash references
*/
isExternalLibraryImport?: boolean;
}

export interface ResolvedModuleWithFailedLookupLocations {
resolvedModule: ResolvedModule;
failedLookupLocations: string[];
}

export type ModuleNameResolver = (moduleName: string, containingFile: string, options: CompilerOptions, host: ModuleResolutionHost) => ResolvedModule;

export interface CompilerHost extends ModuleResolutionHost {
getSourceFile(fileName: string, languageVersion: ScriptTarget, onError?: (message: string) => void): SourceFile;
getCancellationToken?(): CancellationToken;
Expand All @@ -2295,7 +2304,7 @@ namespace ts {
* If resolveModuleNames is implemented then implementation for members from ModuleResolutionHost can be just
* 'throw new Error("NotImplemented")'
*/
resolveModuleNames?(moduleNames: string[], containingFile: string): string[];
resolveModuleNames?(moduleNames: string[], containingFile: string): ResolvedModule[];
}

export interface TextSpan {
Expand Down
10 changes: 5 additions & 5 deletions src/compiler/utilities.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,20 +99,20 @@ namespace ts {
return true;
}

export function hasResolvedModuleName(sourceFile: SourceFile, moduleNameText: string): boolean {
export function hasResolvedModule(sourceFile: SourceFile, moduleNameText: string): boolean {
return sourceFile.resolvedModules && hasProperty(sourceFile.resolvedModules, moduleNameText);
}

export function getResolvedModuleFileName(sourceFile: SourceFile, moduleNameText: string): string {
return hasResolvedModuleName(sourceFile, moduleNameText) ? sourceFile.resolvedModules[moduleNameText] : undefined;
export function getResolvedModule(sourceFile: SourceFile, moduleNameText: string): ResolvedModule {
return hasResolvedModule(sourceFile, moduleNameText) ? sourceFile.resolvedModules[moduleNameText] : undefined;
}

export function setResolvedModuleName(sourceFile: SourceFile, moduleNameText: string, resolvedFileName: string): void {
export function setResolvedModule(sourceFile: SourceFile, moduleNameText: string, resolvedModule: ResolvedModule): void {
if (!sourceFile.resolvedModules) {
sourceFile.resolvedModules = {};
}

sourceFile.resolvedModules[moduleNameText] = resolvedFileName;
sourceFile.resolvedModules[moduleNameText] = resolvedModule;
}

// Returns true if this node contains a parse error anywhere underneath it.
Expand Down
4 changes: 2 additions & 2 deletions src/harness/harnessLanguageService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,8 @@ module Harness.LanguageService {
let imports: ts.Map<string> = {};
for (let module of preprocessInfo.importedFiles) {
let resolutionInfo = ts.resolveModuleName(module.fileName, fileName, compilerOptions, moduleResolutionHost);
if (resolutionInfo.resolvedFileName) {
imports[module.fileName] = resolutionInfo.resolvedFileName;
if (resolutionInfo.resolvedModule) {
imports[module.fileName] = resolutionInfo.resolvedModule.resolvedFileName;
}
}
return JSON.stringify(imports);
Expand Down
Loading

0 comments on commit 026632b

Please sign in to comment.