Skip to content
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.

Sign up for GitHub

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

Package scopes #4913

Closed
wants to merge 25 commits into from
Closed

Package scopes #4913

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
ecf7c1d
Some types - I'll get back to this
weswigham Sep 18, 2015
c232afc
trashy initial prototype - ironing out issues
weswigham Sep 19, 2015
b2cd788
Remove debug stuff, working package scopes
weswigham Sep 21, 2015
02166e2
merge with master
weswigham Sep 21, 2015
cb6e430
Merge branch 'master' into package-scopes
weswigham Oct 5, 2015
3bfd92a
Fix issues remaining form merge, lint
weswigham Oct 5, 2015
314cfc1
update tests
weswigham Oct 5, 2015
c89ef0b
another test, update unit tests to use equals, lint
weswigham Oct 6, 2015
ffb38da
another tests and lint
weswigham Oct 6, 2015
ab269f6
Merge branch 'master' into package-scopes
weswigham Oct 9, 2015
1e32030
Merge branch 'master' into package-scopes
weswigham Nov 10, 2015
ae158ad
cleaning up with changes form pr
weswigham Nov 10, 2015
3e58162
finish cleaning up pr lints
weswigham Nov 10, 2015
f86c28d
pull out export
weswigham Nov 10, 2015
4224586
Update with work items from PR
weswigham Nov 11, 2015
149a1d9
Merge branch 'master' into package-scopes
weswigham Nov 12, 2015
22c6377
Merge branch 'master' into package-scopes
weswigham Nov 13, 2015
312d97c
use relative paths for package names
weswigham Nov 13, 2015
b1ea98c
use ts.
weswigham Nov 13, 2015
d32cedb
move convertToRelativePath into core so the generateDiagnostics scrip…
weswigham Nov 14, 2015
340702e
Merge branch 'master' into package-scopes
weswigham Nov 17, 2015
41c21b5
fix lint
weswigham Nov 17, 2015
fd33df3
Merge branch 'master' into package-scopes
weswigham Nov 17, 2015
733db05
Merge branch 'master' into package-scopes
weswigham Nov 30, 2015
1163d38
accept baselines
weswigham Nov 30, 2015
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
55 changes: 46 additions & 9 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ namespace ts {
let anySignature = createSignature(undefined, undefined, emptyArray, anyType, undefined, 0, false, false);
let unknownSignature = createSignature(undefined, undefined, emptyArray, unknownType, undefined, 0, false, false);

let globals: SymbolTable = {};
let globalScope: Scope = {
Copy link
Member

Choose a reason for hiding this comment

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

I get why it's nice to have this, but you don't actually end up using globalScope on its own - we always just grab symbols anyhow.

symbols: {}
};

let globalESSymbolConstructorSymbol: Symbol;

Expand Down Expand Up @@ -426,6 +428,12 @@ namespace ts {
}
switch (location.kind) {
case SyntaxKind.SourceFile:
if ((<SourceFile>location).package) {
if (hasProperty((<SourceFile>location).package.symbols, name)) {
result = (<SourceFile>location).package.symbols[name];
break loop;
}
}
if (!isExternalModule(<SourceFile>location)) break;
case SyntaxKind.ModuleDeclaration:
let moduleExports = getSymbolOfNode(location).exports;
Expand Down Expand Up @@ -578,7 +586,7 @@ namespace ts {
}

if (!result) {
result = getSymbol(globals, name, meaning);
result = getSymbol(globalScope.symbols, name, meaning);
}

if (!result) {
Expand Down Expand Up @@ -967,7 +975,8 @@ namespace ts {
}
let isRelative = isExternalModuleNameRelative(moduleName);
if (!isRelative) {
let symbol = getSymbol(globals, "\"" + moduleName + "\"", SymbolFlags.ValueModule);
let file = getSourceFileOfNode(location);
let symbol = getSymbol(file.package ? file.package.symbols : globalScope.symbols, "\"" + moduleName + "\"", SymbolFlags.ValueModule);
Copy link
Member

Choose a reason for hiding this comment

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

const

if (symbol) {
return symbol;
}
Expand Down Expand Up @@ -1175,6 +1184,11 @@ namespace ts {
}
switch (location.kind) {
case SyntaxKind.SourceFile:
if ((<SourceFile>location).package) {
if (result = callback((<SourceFile>location).package.symbols)) {
return result;
}
}
if (!isExternalModule(<SourceFile>location)) {
break;
}
Expand All @@ -1192,7 +1206,7 @@ namespace ts {
}
}

return callback(globals);
return callback(globalScope.symbols);
}

function getQualifiedLeftMeaning(rightMeaning: SymbolFlags) {
Expand Down Expand Up @@ -14094,6 +14108,9 @@ namespace ts {

switch (location.kind) {
case SyntaxKind.SourceFile:
if ((<SourceFile>location).package) {
copySymbols((<SourceFile>location).package.symbols, meaning);
}
if (!isExternalModule(<SourceFile>location)) {
break;
}
Expand Down Expand Up @@ -14136,7 +14153,7 @@ namespace ts {
location = location.parent;
}

copySymbols(globals, meaning);
copySymbols(globalScope.symbols, meaning);
}

/**
Expand Down Expand Up @@ -14762,7 +14779,11 @@ namespace ts {
}

function hasGlobalName(name: string): boolean {
return hasProperty(globals, name);
return hasProperty(globalScope.symbols, name);
}

function hasPackageInternalName(file: SourceFile, name: string): boolean {
return file.package && hasProperty(file.package.symbols, name);
}

function getReferencedValueSymbol(reference: Identifier): Symbol {
Expand Down Expand Up @@ -14799,6 +14820,7 @@ namespace ts {
isNestedRedeclaration,
isValueAliasDeclaration,
hasGlobalName,
hasPackageInternalName,
isReferencedAliasDeclaration,
getNodeCheckFlags,
isTopLevelValueImportEqualsWithEntityName,
Expand All @@ -14823,18 +14845,29 @@ namespace ts {
bindSourceFile(file);
});

// Initialize global symbol table
let packages: Map<PackageDescriptor> = {};
Copy link
Member

Choose a reason for hiding this comment

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

const


// Initialize package/global symbol table(s)
forEach(host.getSourceFiles(), file => {
if (!isExternalModule(file)) {
mergeSymbolTable(globals, file.locals);
if (file.package) {
if (!packages[file.package.packageFile]) {
packages[file.package.packageFile] = file.package;
}
file.package = packages[file.package.packageFile]; // Dedupe packages
Copy link
Member

Choose a reason for hiding this comment

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

This should be in an else, and move the comment to above the line.

mergeSymbolTable(file.package.symbols, file.locals);
}
else {
mergeSymbolTable(globalScope.symbols, file.locals);
}
}
});

// Initialize special symbols
getSymbolLinks(undefinedSymbol).type = undefinedType;
getSymbolLinks(argumentsSymbol).type = getGlobalType("IArguments");
getSymbolLinks(unknownSymbol).type = unknownType;
globals[undefinedSymbol.name] = undefinedSymbol;
globalScope.symbols[undefinedSymbol.name] = undefinedSymbol;
// Initialize special types
globalArrayType = <GenericType>getGlobalType("Array", /*arity*/ 1);
globalObjectType = getGlobalType("Object");
Expand Down Expand Up @@ -14881,6 +14914,10 @@ namespace ts {
}

anyArrayType = createArrayType(anyType);

forEachValue(packages, package => { // Once all packages are merged and global scope is setup, merge global scope into each package
Copy link
Member

Choose a reason for hiding this comment

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

Move comment above

mergeSymbolTable(package.symbols, globalScope.symbols);
});
}

function createInstantiatedPromiseLikeType(): ObjectType {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1708,7 +1708,7 @@
"category": "Error",
"code": 2653
},
"Exported external package typings file cannot contain tripleslash references. Please contact the package author to update the package definition.": {
"Exported external package typings file cannot contain script file tripleslash references. Please contact the package author to update the package definition.": {
"category": "Error",
"code": 2654
},
Expand Down
1 change: 1 addition & 0 deletions src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi

function isUniqueName(name: string): boolean {
return !resolver.hasGlobalName(name) &&
!resolver.hasPackageInternalName(currentSourceFile, name) &&
!hasProperty(currentSourceFile.identifiers, name) &&
!hasProperty(generatedNameSet, name);
}
Expand Down
50 changes: 25 additions & 25 deletions src/compiler/program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,8 @@ namespace ts {
return { resolvedModule: { resolvedFileName }, failedLookupLocations };
}

resolvedFileName = loadNodeModuleFromDirectory(candidate, /* loadOnlyDts */ false, failedLookupLocations, host);
return resolvedFileName
? { resolvedModule: { resolvedFileName }, failedLookupLocations }
: { resolvedModule: undefined, failedLookupLocations };
let res = loadNodeModuleFromDirectory(candidate, /* loadOnlyDts */ false, failedLookupLocations, host, /*mustBePackage*/false);
return { resolvedModule: res, failedLookupLocations };
}
else {
return loadModuleFromNodeModules(moduleName, containingDirectory, host);
Expand All @@ -89,7 +87,7 @@ namespace ts {
}
}

function loadNodeModuleFromDirectory(candidate: string, loadOnlyDts: boolean, failedLookupLocation: string[], host: ModuleResolutionHost): string {
function loadNodeModuleFromDirectory(candidate: string, loadOnlyDts: boolean, failedLookupLocation: string[], host: ModuleResolutionHost, mustBePackage: boolean): ResolvedModule {
let packageJsonPath = combinePaths(candidate, "package.json");
if (host.fileExists(packageJsonPath)) {

Expand All @@ -107,7 +105,7 @@ namespace ts {
if (jsonContent.typings) {
let result = loadNodeModuleFromFile(normalizePath(combinePaths(candidate, jsonContent.typings)), loadOnlyDts, failedLookupLocation, host);
if (result) {
return result;
return { resolvedFileName: result, packageRoot: packageJsonPath };
}
}
}
Expand All @@ -116,7 +114,10 @@ namespace ts {
failedLookupLocation.push(packageJsonPath);
}

return loadNodeModuleFromFile(combinePaths(candidate, "index"), loadOnlyDts, failedLookupLocation, host);
let result = loadNodeModuleFromFile(combinePaths(candidate, "index"), loadOnlyDts, failedLookupLocation, host);
Copy link
Member

Choose a reason for hiding this comment

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

const

if (result) {
return { resolvedFileName: result, packageRoot: mustBePackage ? result : undefined };
}
}

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

result = loadNodeModuleFromDirectory(candidate, /* loadOnlyDts */ true, failedLookupLocations, host);
if (result) {
return { resolvedModule: { resolvedFileName: result, isExternalLibraryImport: true }, failedLookupLocations };
let res = loadNodeModuleFromDirectory(candidate, /* loadOnlyDts */ true, failedLookupLocations, host, /*mustBePackage*/ true);
Copy link
Member

Choose a reason for hiding this comment

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

const

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

Expand Down Expand Up @@ -474,7 +475,7 @@ namespace ts {
let resolutionChanged = oldResolution
? !newResolution ||
oldResolution.resolvedFileName !== newResolution.resolvedFileName ||
!!oldResolution.isExternalLibraryImport !== !!newResolution.isExternalLibraryImport
!!oldResolution.packageRoot !== !!newResolution.packageRoot
: newResolution;

if (resolutionChanged) {
Expand Down Expand Up @@ -740,7 +741,7 @@ namespace ts {
diagnostic = Diagnostics.File_0_has_unsupported_extension_The_only_supported_extensions_are_1;
diagnosticArgument = [fileName, "'" + supportedExtensions.join("', '") + "'"];
}
else if (!findSourceFile(fileName, isDefaultLib, refFile, refPos, refEnd)) {
else if (!findSourceFile(fileName, isDefaultLib, refFile, refPos, refEnd, refFile && refFile.package)) {
diagnostic = Diagnostics.File_0_not_found;
diagnosticArgument = [fileName];
}
Expand All @@ -750,13 +751,13 @@ namespace ts {
}
}
else {
let nonTsFile: SourceFile = options.allowNonTsExtensions && findSourceFile(fileName, isDefaultLib, refFile, refPos, refEnd);
let nonTsFile: SourceFile = options.allowNonTsExtensions && findSourceFile(fileName, isDefaultLib, refFile, refPos, refEnd, refFile && refFile.package);
if (!nonTsFile) {
if (options.allowNonTsExtensions) {
diagnostic = Diagnostics.File_0_not_found;
diagnosticArgument = [fileName];
}
else if (!forEach(supportedExtensions, extension => findSourceFile(fileName + extension, isDefaultLib, refFile, refPos, refEnd))) {
else if (!forEach(supportedExtensions, extension => findSourceFile(fileName + extension, isDefaultLib, refFile, refPos, refEnd, refFile && refFile.package))) {
diagnostic = Diagnostics.File_0_not_found;
fileName += ".ts";
diagnosticArgument = [fileName];
Expand All @@ -775,7 +776,7 @@ namespace ts {
}

// Get source file from normalized fileName
function findSourceFile(fileName: string, isDefaultLib: boolean, refFile?: SourceFile, refPos?: number, refEnd?: number): SourceFile {
function findSourceFile(fileName: string, isDefaultLib: boolean, refFile?: SourceFile, refPos?: number, refEnd?: number, package?: PackageDescriptor): SourceFile {
if (filesByName.contains(fileName)) {
// We've already looked for this file, use cached result
return getSourceFileFromCache(fileName, /*useAbsolutePath*/ false);
Expand All @@ -800,9 +801,9 @@ namespace ts {
fileProcessingDiagnostics.add(createCompilerDiagnostic(Diagnostics.Cannot_read_file_0_Colon_1, fileName, hostErrorMessage));
}
});

filesByName.set(fileName, file);
if (file) {
file.package = package;
skipDefaultLib = skipDefaultLib || file.hasNoDefaultLib;

// Set the source file for normalized absolute path
Expand Down Expand Up @@ -848,6 +849,9 @@ namespace ts {
function processReferencedFiles(file: SourceFile, basePath: string) {
forEach(file.referencedFiles, ref => {
let referencedFileName = resolveTripleslashReference(ref.fileName, file.fileName);
if (file.package && !fileExtensionIs(ref.fileName, ".d.ts")) {
fileProcessingDiagnostics.add(createFileDiagnostic(file, ref.pos, ref.end - ref.pos, Diagnostics.Exported_external_package_typings_file_cannot_contain_script_file_tripleslash_references_Please_contact_the_package_author_to_update_the_package_definition));
}
processSourceFile(referencedFileName, /* isDefaultLib */ false, file, ref.pos, ref.end);
});
}
Expand All @@ -862,8 +866,8 @@ namespace ts {
let resolution = resolutions[i];
setResolvedModule(file, moduleNames[i], resolution);
if (resolution && !options.noResolve) {
const importedFile = findModuleSourceFile(resolution.resolvedFileName, file.imports[i]);
if (importedFile && resolution.isExternalLibraryImport) {
const importedFile = findModuleSourceFile(resolution.resolvedFileName, file.imports[i], resolution.packageRoot ? {packageFile: resolution.packageRoot, symbols: {}} : file.package);
if (importedFile && resolution.packageRoot) {
if (!isExternalModule(importedFile)) {
let start = getTokenPosOfNode(file.imports[i], file);
fileProcessingDiagnostics.add(createFileDiagnostic(file, start, file.imports[i].end - start, Diagnostics.Exported_external_package_typings_file_0_is_not_a_module_Please_contact_the_package_author_to_update_the_package_definition, importedFile.fileName));
Expand All @@ -872,10 +876,6 @@ namespace 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 All @@ -886,8 +886,8 @@ namespace ts {
}
return;

function findModuleSourceFile(fileName: string, nameLiteral: Expression) {
return findSourceFile(fileName, /* isDefaultLib */ false, file, skipTrivia(file.text, nameLiteral.pos), nameLiteral.end);
function findModuleSourceFile(fileName: string, nameLiteral: Expression, package?: PackageDescriptor) {
return findSourceFile(fileName, /* isDefaultLib */ false, file, skipTrivia(file.text, nameLiteral.pos), nameLiteral.end, package);
}
}

Expand Down
15 changes: 13 additions & 2 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1233,6 +1233,10 @@ namespace ts {
isBracketed: boolean;
}

export interface PackageDescriptor extends Scope {
packageFile: string;
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment? Specifically mention that this can be a package.json or index.d.ts.

}

// Source files are declarations when they are external modules.
export interface SourceFile extends Declaration {
statements: NodeArray<Statement>;
Expand All @@ -1246,6 +1250,9 @@ namespace ts {
referencedFiles: FileReference[];
languageVariant: LanguageVariant;

/* @internal */
package?: PackageDescriptor;

// this map is used by transpiler to supply alternative names for dependencies (i.e. in case of bundling)
/* @internal */
renamedDependencies?: Map<string>;
Expand Down Expand Up @@ -1289,6 +1296,10 @@ namespace ts {
/* @internal */ imports: LiteralExpression[];
}

export interface Scope {
symbols: SymbolTable; // Locals associated with node (initialized by binding)
}

export interface ScriptReferenceHost {
getCompilerOptions(): CompilerOptions;
getSourceFile(fileName: string): SourceFile;
Expand Down Expand Up @@ -1580,6 +1591,7 @@ namespace ts {
/* @internal */
export interface EmitResolver {
hasGlobalName(name: string): boolean;
hasPackageInternalName(node: SourceFile, name: string): boolean;
getReferencedExportContainer(node: Identifier): SourceFile | ModuleDeclaration | EnumDeclaration;
getReferencedImportDeclaration(node: Identifier): Declaration;
getReferencedNestedRedeclaration(node: Identifier): Declaration;
Expand Down Expand Up @@ -2308,9 +2320,8 @@ namespace ts {
* 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;
packageRoot?: string;
}

export interface ResolvedModuleWithFailedLookupLocations {
Expand Down
Loading