Skip to content

Commit

Permalink
Merge pull request #8678 from RyanCavanaugh/fix8255
Browse files Browse the repository at this point in the history
Tweak UMD / global semantics
  • Loading branch information
RyanCavanaugh committed May 23, 2016
2 parents 92d465d + 91b8f20 commit 27292e4
Show file tree
Hide file tree
Showing 11 changed files with 36 additions and 29 deletions.
6 changes: 3 additions & 3 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1764,8 +1764,8 @@ namespace ts {
case SyntaxKind.ImportSpecifier:
case SyntaxKind.ExportSpecifier:
return declareSymbolAndAddToSymbolTable(<Declaration>node, SymbolFlags.Alias, SymbolFlags.AliasExcludes);
case SyntaxKind.GlobalModuleExportDeclaration:
return bindGlobalModuleExportDeclaration(<GlobalModuleExportDeclaration>node);
case SyntaxKind.NamespaceExportDeclaration:
return bindNamespaceExportDeclaration(<NamespaceExportDeclaration>node);
case SyntaxKind.ImportClause:
return bindImportClause(<ImportClause>node);
case SyntaxKind.ExportDeclaration:
Expand Down Expand Up @@ -1815,7 +1815,7 @@ namespace ts {
}
}

function bindGlobalModuleExportDeclaration(node: GlobalModuleExportDeclaration) {
function bindNamespaceExportDeclaration(node: NamespaceExportDeclaration) {
if (node.modifiers && node.modifiers.length) {
file.bindDiagnostics.push(createDiagnosticForNode(node, Diagnostics.Modifiers_cannot_appear_here));
}
Expand Down
22 changes: 16 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,7 @@ namespace ts {
let propertyWithInvalidInitializer: Node;
const errorLocation = location;
let grandparent: Node;
let isInExternalModule = false;

loop: while (location) {
// Locals of a source file are not in scope (because they get merged into the global symbol table)
Expand Down Expand Up @@ -686,6 +687,7 @@ namespace ts {
switch (location.kind) {
case SyntaxKind.SourceFile:
if (!isExternalOrCommonJsModule(<SourceFile>location)) break;
isInExternalModule = true;
case SyntaxKind.ModuleDeclaration:
const moduleExports = getSymbolOfNode(location).exports;
if (location.kind === SyntaxKind.SourceFile || isAmbientModule(location)) {
Expand Down Expand Up @@ -879,6 +881,14 @@ namespace ts {
checkResolvedBlockScopedVariable(exportOrLocalSymbol, errorLocation);
}
}

// If we're in an external module, we can't reference symbols created from UMD export declarations
if (result && isInExternalModule) {
const decls = result.declarations;
if (decls && decls.length === 1 && decls[0].kind === SyntaxKind.NamespaceExportDeclaration) {
error(errorLocation, Diagnostics.Identifier_0_must_be_imported_from_a_module, name);
}
}
}
return result;
}
Expand Down Expand Up @@ -1076,7 +1086,7 @@ namespace ts {
return getExternalModuleMember(<ImportDeclaration>node.parent.parent.parent, node);
}

function getTargetOfGlobalModuleExportDeclaration(node: GlobalModuleExportDeclaration): Symbol {
function getTargetOfGlobalModuleExportDeclaration(node: NamespaceExportDeclaration): Symbol {
return resolveExternalModuleSymbol(node.parent.symbol);
}

Expand Down Expand Up @@ -1104,8 +1114,8 @@ namespace ts {
return getTargetOfExportSpecifier(<ExportSpecifier>node);
case SyntaxKind.ExportAssignment:
return getTargetOfExportAssignment(<ExportAssignment>node);
case SyntaxKind.GlobalModuleExportDeclaration:
return getTargetOfGlobalModuleExportDeclaration(<GlobalModuleExportDeclaration>node);
case SyntaxKind.NamespaceExportDeclaration:
return getTargetOfGlobalModuleExportDeclaration(<NamespaceExportDeclaration>node);
}
}

Expand Down Expand Up @@ -6374,7 +6384,7 @@ namespace ts {
if (kind === SignatureKind.Construct && sourceSignatures.length && targetSignatures.length) {
if (isAbstractConstructorType(source) && !isAbstractConstructorType(target)) {
// An abstract constructor type is not assignable to a non-abstract constructor type
// as it would otherwise be possible to new an abstract class. Note that the assignability
// as it would otherwise be possible to new an abstract class. Note that the assignability
// check we perform for an extends clause excludes construct signatures from the target,
// so this check never proceeds.
if (reportErrors) {
Expand Down Expand Up @@ -12878,7 +12888,7 @@ namespace ts {
if (checkIfTypePredicateVariableIsDeclaredInBindingPattern(
<BindingPattern>name,
predicateVariableNode,
predicateVariableName)) {
predicateVariableName)) {
return true;
}
}
Expand Down Expand Up @@ -17497,7 +17507,7 @@ namespace ts {
if (file.moduleAugmentations.length) {
(augmentations || (augmentations = [])).push(file.moduleAugmentations);
}
if (file.wasReferenced && file.symbol && file.symbol.globalExports) {
if (file.symbol && file.symbol.globalExports) {
mergeSymbolTable(globals, file.symbol.globalExports);
}
});
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1923,6 +1923,10 @@
"category": "Error",
"code": 2685
},
"Identifier '{0}' must be imported from a module": {
"category": "Error",
"code": 2686
},
"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
"code": 4000
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,8 @@ namespace ts {
case SyntaxKind.ImportClause:
return visitNode(cbNode, (<ImportClause>node).name) ||
visitNode(cbNode, (<ImportClause>node).namedBindings);
case SyntaxKind.GlobalModuleExportDeclaration:
return visitNode(cbNode, (<GlobalModuleExportDeclaration>node).name);
case SyntaxKind.NamespaceExportDeclaration:
return visitNode(cbNode, (<NamespaceExportDeclaration>node).name);

case SyntaxKind.NamespaceImport:
return visitNode(cbNode, (<NamespaceImport>node).name);
Expand Down Expand Up @@ -5344,8 +5344,8 @@ namespace ts {
return nextToken() === SyntaxKind.SlashToken;
}

function parseGlobalModuleExportDeclaration(fullStart: number, decorators: NodeArray<Decorator>, modifiers: ModifiersArray): GlobalModuleExportDeclaration {
const exportDeclaration = <GlobalModuleExportDeclaration>createNode(SyntaxKind.GlobalModuleExportDeclaration, fullStart);
function parseGlobalModuleExportDeclaration(fullStart: number, decorators: NodeArray<Decorator>, modifiers: ModifiersArray): NamespaceExportDeclaration {
const exportDeclaration = <NamespaceExportDeclaration>createNode(SyntaxKind.NamespaceExportDeclaration, fullStart);
exportDeclaration.decorators = decorators;
exportDeclaration.modifiers = modifiers;
parseExpected(SyntaxKind.AsKeyword);
Expand Down
5 changes: 0 additions & 5 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1762,10 +1762,6 @@ namespace ts {
reportFileNamesDifferOnlyInCasingError(fileName, file.fileName, refFile, refPos, refEnd);
}

if (file) {
file.wasReferenced = file.wasReferenced || isReference;
}

return file;
}

Expand All @@ -1782,7 +1778,6 @@ namespace ts {

filesByName.set(path, file);
if (file) {
file.wasReferenced = file.wasReferenced || isReference;
file.path = path;

if (host.useCaseSensitiveFileNames()) {
Expand Down
8 changes: 3 additions & 5 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ namespace ts {
ModuleDeclaration,
ModuleBlock,
CaseBlock,
GlobalModuleExportDeclaration,
NamespaceExportDeclaration,
ImportEqualsDeclaration,
ImportDeclaration,
ImportClause,
Expand Down Expand Up @@ -1341,8 +1341,8 @@ namespace ts {
name: Identifier;
}

// @kind(SyntaxKind.GlobalModuleImport)
export interface GlobalModuleExportDeclaration extends DeclarationStatement {
// @kind(SyntaxKind.NamespaceExportDeclaration)
export interface NamespaceExportDeclaration extends DeclarationStatement {
name: Identifier;
moduleReference: LiteralLikeNode;
}
Expand Down Expand Up @@ -1599,8 +1599,6 @@ namespace ts {
/* @internal */ externalModuleIndicator: Node;
// The first node that causes this file to be a CommonJS module
/* @internal */ commonJsModuleIndicator: Node;
// True if the file was a root file in a compilation or a /// reference targets
/* @internal */ wasReferenced?: boolean;

/* @internal */ identifiers: Map<string>;
/* @internal */ nodeCount: number;
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1643,7 +1643,7 @@ namespace ts {
// export default ...
export function isAliasSymbolDeclaration(node: Node): boolean {
return node.kind === SyntaxKind.ImportEqualsDeclaration ||
node.kind === SyntaxKind.GlobalModuleExportDeclaration ||
node.kind === SyntaxKind.NamespaceExportDeclaration ||
node.kind === SyntaxKind.ImportClause && !!(<ImportClause>node).name ||
node.kind === SyntaxKind.NamespaceImport ||
node.kind === SyntaxKind.ImportSpecifier ||
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/umd-augmentation-3.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export = M2D;
>M2D : Symbol(M2D, Decl(index.d.ts, 3, 13))

declare namespace M2D {
>M2D : Symbol(, Decl(index.d.ts, 3, 13), Decl(math2d-augment.d.ts, 0, 33))
>M2D : Symbol(Math2d, Decl(index.d.ts, 3, 13), Decl(math2d-augment.d.ts, 0, 33))

interface Point {
>Point : Symbol(Point, Decl(index.d.ts, 5, 23))
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/umd-augmentation-3.types
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export = M2D;
>M2D : typeof M2D

declare namespace M2D {
>M2D : typeof
>M2D : typeof Math2d

interface Point {
>Point : Point
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/umd5.errors.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
tests/cases/conformance/externalModules/a.ts(6,9): error TS2304: Cannot find name 'Foo'.
tests/cases/conformance/externalModules/a.ts(6,9): error TS2686: Identifier 'Foo' must be imported from a module


==== tests/cases/conformance/externalModules/a.ts (1 errors) ====
Expand All @@ -9,7 +9,7 @@ tests/cases/conformance/externalModules/a.ts(6,9): error TS2304: Cannot find nam
// should error
let z = Foo;
~~~
!!! error TS2304: Cannot find name 'Foo'.
!!! error TS2686: Identifier 'Foo' must be imported from a module

==== tests/cases/conformance/externalModules/foo.d.ts (0 errors) ====

Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/umd5.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ Bar.fn();
var x;
var y = x.n;
// should error
var z = Foo;
var z = exports.Foo;

0 comments on commit 27292e4

Please sign in to comment.