From d165708141b6e09302b6c5e0e4670ad8b84f7dec Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Tue, 22 Jul 2014 12:09:25 -0700 Subject: [PATCH] check groups of local symbols --- src/compiler/binder.ts | 9 +- src/compiler/checker.ts | 88 ++++++++++++++++++- .../diagnosticInformationMap.generated.ts | 1 + src/compiler/diagnosticMessages.json | 4 + src/compiler/types.ts | 2 + .../reference/anonymousModules.errors.txt | 2 +- .../duplicateSymbolsExportMatching.errors.txt | 20 +++-- .../functionOverloadErrors.errors.txt | 18 ++-- tests/baselines/reference/multivar.errors.txt | 2 +- .../overloadModifiersMustAgree.errors.txt | 2 +- 10 files changed, 113 insertions(+), 35 deletions(-) diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index ec67b496db..faf8d96e2b 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -136,30 +136,27 @@ module ts { // but return the export symbol (by calling getExportSymbolOfValueSymbolIfExported). That way // when the emitter comes back to it, it knows not to qualify the name if it was found in a containing scope. var exportKind = 0; - var exportExcludes = 0; if (symbolKind & SymbolFlags.Value) { exportKind |= SymbolFlags.ExportValue; - exportExcludes |= SymbolFlags.Value; } if (symbolKind & SymbolFlags.Type) { exportKind |= SymbolFlags.ExportType; - exportExcludes |= SymbolFlags.Type; } if (symbolKind & SymbolFlags.Namespace) { exportKind |= SymbolFlags.ExportNamespace; - exportExcludes |= SymbolFlags.Namespace; } if (node.flags & NodeFlags.Export || (node.kind !== SyntaxKind.ImportDeclaration && isAmbientContext(container))) { if (exportKind) { - var local = declareSymbol(container.locals, undefined, node, exportKind, exportExcludes); + var local = declareSymbol(container.locals, undefined, node, exportKind, symbolExcludes); local.exportSymbol = declareSymbol(container.symbol.exports, container.symbol, node, symbolKind, symbolExcludes); + node.localSymbol = local; } else { declareSymbol(container.symbol.exports, container.symbol, node, symbolKind, symbolExcludes); } } else { - declareSymbol(container.locals, undefined, node, symbolKind, symbolExcludes | exportKind); + declareSymbol(container.locals, undefined, node, symbolKind, symbolExcludes); } } diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 7faabfae6a..9872b7ee42 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4841,15 +4841,89 @@ module ts { // TODO: Check at least one return statement in non-void/any function (except single throw) } + function checkExportsOnMergedDeclarations(node: Node) { + var symbol: Symbol; + + // if node.localSymbol !== undefined - this node has both export and local symbol. + // local symbol includes all declarations (that can be both exported and non exported) + var symbol = node.localSymbol; + if (!symbol) { + // current declaration is not exported. + // pick a symbol for it and check if it contains any exported declaration + symbol = getSymbolOfNode(node); + if (!(symbol.flags & SymbolFlags.Export)) { + // this is a pure local symbol - no need to check anything + return; + } + } + + if (getDeclarationOfKind(symbol, node.kind) !== node) { + return; + } + + // we use SymbolFlags.ExportValue, SymbolFlags.ExportType and SymbolFlags.ExportNamespace + // to denote disjoint declarationSpaces (without making new enum type). + var declarationSpaces: SymbolFlags = 0; + var hasExport: boolean; + + var declarations = symbol.declarations; + for (var i = 0, len = declarations.length; i < len; ++i) { + var currentDeclarationSpaces = getDeclarationSpaces(declarations[i]); + var currentDeclarationHasExport = (declarations[i].flags & NodeFlags.Export) !== 0; + if (declarationSpaces & currentDeclarationSpaces) { + if (hasExport !== undefined) { + if (hasExport !== currentDeclarationHasExport) { + error(declarations[i].name, Diagnostics.All_declarations_of_merged_declaration_0_must_be_exported_or_not_exported, symbolToString(symbol)); + } + } + } + hasExport = hasExport || currentDeclarationHasExport; + declarationSpaces |= currentDeclarationSpaces; + } + + function getDeclarationSpaces(d: Declaration): SymbolFlags { + switch (d.kind) { + case SyntaxKind.InterfaceDeclaration: + return SymbolFlags.ExportType; + case SyntaxKind.ModuleDeclaration: + return (d).name.kind === SyntaxKind.StringLiteral || isInstantiated(d) + ? SymbolFlags.ExportNamespace | SymbolFlags.ExportValue + : SymbolFlags.ExportNamespace; + case SyntaxKind.ClassDeclaration: + case SyntaxKind.EnumDeclaration: + return SymbolFlags.ExportType | SymbolFlags.ExportValue; + case SyntaxKind.ImportDeclaration: + var target = resolveImport(getSymbolOfNode(d)); + return target.flags & SymbolFlags.Export; + default: + return SymbolFlags.ExportValue; + } + } + } + function checkFunctionDeclaration(node: FunctionDeclaration) { checkSignatureDeclaration(node); - var symbol = getSymbolOfNode(node); - var firstDeclaration = getDeclarationOfKind(symbol, node.kind); + var symbol = getSymbolOfNode(node) + // first we want to check the local symbol that contain this declaration + // - if node.localSymbol !== undefined - this is current declaration is exported and localSymbol points to the local symbol + // - if node.localSymbol === undefined - this node is non-exported so we can just pick the result of getSymbolOfNode + var localSymbol = node.localSymbol || symbol; + + var firstDeclaration = getDeclarationOfKind(localSymbol, node.kind); // Only type check the symbol once if (node === firstDeclaration) { - checkFunctionOrConstructorSymbol(symbol); + checkFunctionOrConstructorSymbol(localSymbol); } + + if (symbol !== localSymbol) { + // here we'll check exported side of the symbol + Debug.assert(symbol.parent); + if (getDeclarationOfKind(symbol, node.kind) === node) { + checkFunctionOrConstructorSymbol(symbol); + } + } + checkSourceElement(node.body); // If there is no body and no explicit return type, then report an error. @@ -5041,8 +5115,10 @@ module ts { function checkVariableDeclaration(node: VariableDeclaration) { checkSourceElement(node.type); - var symbol = getSymbolOfNode(node); + checkExportsOnMergedDeclarations(node); + var symbol = getSymbolOfNode(node); + var typeOfValueDeclaration = getTypeOfVariableOrParameterOrProperty(symbol); var type: Type; var useTypeFromValueDeclaration = node === symbol.valueDeclaration; @@ -5330,6 +5406,7 @@ module ts { checkTypeNameIsReserved(node.name, Diagnostics.Class_name_cannot_be_0); checkTypeParameters(node.typeParameters); checkCollisionWithCapturedThisVariable(node, node.name); + checkExportsOnMergedDeclarations(node); var symbol = getSymbolOfNode(node); var type = getDeclaredTypeOfSymbol(symbol); var staticType = getTypeOfSymbol(symbol); @@ -5484,6 +5561,7 @@ module ts { function checkInterfaceDeclaration(node: InterfaceDeclaration) { checkTypeNameIsReserved(node.name, Diagnostics.Interface_name_cannot_be_0); checkTypeParameters(node.typeParameters); + checkExportsOnMergedDeclarations(node); var symbol = getSymbolOfNode(node); var firstInterfaceDecl = getDeclarationOfKind(symbol, SyntaxKind.InterfaceDeclaration); if (symbol.declarations.length > 1) { @@ -5529,6 +5607,7 @@ module ts { function checkEnumDeclaration(node: EnumDeclaration) { checkTypeNameIsReserved(node.name, Diagnostics.Enum_name_cannot_be_0); checkCollisionWithCapturedThisVariable(node, node.name); + checkExportsOnMergedDeclarations(node); var enumSymbol = getSymbolOfNode(node); var enumType = getDeclaredTypeOfSymbol(enumSymbol); var autoValue = 0; @@ -5600,6 +5679,7 @@ module ts { function checkModuleDeclaration(node: ModuleDeclaration) { checkCollisionWithCapturedThisVariable(node, node.name); + checkExportsOnMergedDeclarations(node); var symbol = getSymbolOfNode(node); if (symbol.flags & SymbolFlags.ValueModule && symbol.declarations.length > 1 && !isInAmbientContext(node)) { var classOrFunc = getFirstNonAmbientClassOrFunctionDeclaration(symbol); diff --git a/src/compiler/diagnosticInformationMap.generated.ts b/src/compiler/diagnosticInformationMap.generated.ts index 2c967b40b6..59cef3c088 100644 --- a/src/compiler/diagnosticInformationMap.generated.ts +++ b/src/compiler/diagnosticInformationMap.generated.ts @@ -131,6 +131,7 @@ module ts { A_signature_with_an_implementation_cannot_use_a_string_literal_type: { code: 2163, category: DiagnosticCategory.Error, key: "A signature with an implementation cannot use a string literal type." }, Interface_0_cannot_simultaneously_extend_types_1_and_2_Colon: { code: 2189, category: DiagnosticCategory.Error, key: "Interface '{0}' cannot simultaneously extend types '{1}' and '{2}':" }, Initializer_of_parameter_0_cannot_reference_identifier_1_declared_after_it: { code: 2190, category: DiagnosticCategory.Error, key: "Initializer of parameter '{0}' cannot reference identifier '{1}' declared after it." }, + All_declarations_of_merged_declaration_0_must_be_exported_or_not_exported: { code: 2192, category: DiagnosticCategory.Error, key: "All declarations of merged declaration '{0}' must be exported or not exported." }, super_cannot_be_referenced_in_constructor_arguments: { code: 2193, category: DiagnosticCategory.Error, key: "'super' cannot be referenced in constructor arguments." }, Return_type_of_constructor_signature_must_be_assignable_to_the_instance_type_of_the_class: { code: 2194, category: DiagnosticCategory.Error, key: "Return type of constructor signature must be assignable to the instance type of the class" }, Ambient_external_module_declaration_cannot_specify_relative_module_name: { code: 2196, category: DiagnosticCategory.Error, key: "Ambient external module declaration cannot specify relative module name." }, diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 55d0ffa226..ad7c1bdc4d 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -518,6 +518,10 @@ "category": "Error", "code": 2190 }, + "All declarations of merged declaration '{0}' must be exported or not exported.": { + "category": "Error", + "code": 2192 + }, "'super' cannot be referenced in constructor arguments.":{ "category": "Error", "code": 2193 diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 5decadcfe1..36cf334d06 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -241,6 +241,7 @@ module ts { symbol?: Symbol; // Symbol declared by node (initialized by binding) locals?: SymbolTable; // Locals associated with node (initialized by binding) nextContainer?: Node; // Next container in declaration order (initialized by binding) + localSymbol?: Symbol; // Local symbol declared by node (initialized by binding only for exported nodes) } export interface NodeArray extends Array, TextRange { } @@ -697,6 +698,7 @@ module ts { IsContainer = HasLocals | HasExports | HasMembers, PropertyOrAccessor = Property | Accessor, + Export = ExportNamespace | ExportType | ExportValue, } export interface Symbol { diff --git a/tests/baselines/reference/anonymousModules.errors.txt b/tests/baselines/reference/anonymousModules.errors.txt index bebc718c00..2663477cc3 100644 --- a/tests/baselines/reference/anonymousModules.errors.txt +++ b/tests/baselines/reference/anonymousModules.errors.txt @@ -24,7 +24,7 @@ var bar = 2; ~~~ -!!! Duplicate identifier 'bar'. +!!! All declarations of merged declaration 'bar' must be exported or not exported. module { ~ diff --git a/tests/baselines/reference/duplicateSymbolsExportMatching.errors.txt b/tests/baselines/reference/duplicateSymbolsExportMatching.errors.txt index 3bc10adf7e..1d712f9c5c 100644 --- a/tests/baselines/reference/duplicateSymbolsExportMatching.errors.txt +++ b/tests/baselines/reference/duplicateSymbolsExportMatching.errors.txt @@ -1,4 +1,4 @@ -==== tests/cases/compiler/duplicateSymbolsExportMatching.ts (8 errors) ==== +==== tests/cases/compiler/duplicateSymbolsExportMatching.ts (9 errors) ==== module M { export interface E { } interface I { } @@ -25,11 +25,11 @@ interface I { } export interface I { } // error ~ -!!! Duplicate identifier 'I'. +!!! All declarations of merged declaration 'I' must be exported or not exported. export interface E { } interface E { } // error ~ -!!! Duplicate identifier 'E'. +!!! All declarations of merged declaration 'E' must be exported or not exported. } // Should report error only once for instantiated module @@ -39,7 +39,7 @@ } export module inst { // one error ~~~~ -!!! Duplicate identifier 'inst'. +!!! All declarations of merged declaration 'inst' must be exported or not exported. var t; } } @@ -49,20 +49,22 @@ var v: string; export var v: string; // one error (visibility) ~ -!!! Duplicate identifier 'v'. +!!! All declarations of merged declaration 'v' must be exported or not exported. var w: number; export var w: string; // two errors (visibility and type mismatch) ~ -!!! Duplicate identifier 'w'. +!!! All declarations of merged declaration 'w' must be exported or not exported. } module M { module F { + ~ +!!! A module declaration cannot be located prior to a class or function with which it is merged var t; } export function F() { } // Only one error for duplicate identifier (don't consider visibility) ~ -!!! Duplicate identifier 'F'. +!!! All declarations of merged declaration 'F' must be exported or not exported. } module M { @@ -70,7 +72,7 @@ module C { } export module C { // Two visibility errors (one for the clodule symbol, and one for the merged container symbol) ~ -!!! Duplicate identifier 'C'. +!!! All declarations of merged declaration 'C' must be exported or not exported. var t; } } @@ -79,4 +81,4 @@ interface D { } export interface D { } ~ -!!! Duplicate identifier 'D'. \ No newline at end of file +!!! All declarations of merged declaration 'D' must be exported or not exported. \ No newline at end of file diff --git a/tests/baselines/reference/functionOverloadErrors.errors.txt b/tests/baselines/reference/functionOverloadErrors.errors.txt index a839c286f8..394607010e 100644 --- a/tests/baselines/reference/functionOverloadErrors.errors.txt +++ b/tests/baselines/reference/functionOverloadErrors.errors.txt @@ -1,4 +1,4 @@ -==== tests/cases/conformance/functions/functionOverloadErrors.ts (19 errors) ==== +==== tests/cases/conformance/functions/functionOverloadErrors.ts (15 errors) ==== //Function overload signature with initializer function fn1(x = 3); ~~~~~ @@ -88,24 +88,16 @@ export function fn1(); ~~~~~~~~~~~~~~~~~~~~~~ !!! Function implementation expected. + ~~~ +!!! Overload signatures must all be exported or not exported. function fn1(n: string); - ~~~~~~~~~~~~~~~~~~~~~~~~ -!!! Function implementation expected. - ~~~ -!!! Duplicate identifier 'fn1'. function fn1() { } - ~~~ -!!! Duplicate identifier 'fn1'. function fn2(n: string); - ~~~~~~~~~~~~~~~~~~~~~~~~ -!!! Function implementation expected. + ~~~ +!!! Overload signatures must all be exported or not exported. export function fn2(); - ~~~ -!!! Duplicate identifier 'fn2'. export function fn2() { } - ~~~ -!!! Duplicate identifier 'fn2'. } //Function overloads with differing ambience diff --git a/tests/baselines/reference/multivar.errors.txt b/tests/baselines/reference/multivar.errors.txt index bc46109903..66db194234 100644 --- a/tests/baselines/reference/multivar.errors.txt +++ b/tests/baselines/reference/multivar.errors.txt @@ -22,7 +22,7 @@ declare var d1, d2; var b2; ~~ -!!! Duplicate identifier 'b2'. +!!! All declarations of merged declaration 'b2' must be exported or not exported. declare var v1; } diff --git a/tests/baselines/reference/overloadModifiersMustAgree.errors.txt b/tests/baselines/reference/overloadModifiersMustAgree.errors.txt index cad91215f0..adb535ce38 100644 --- a/tests/baselines/reference/overloadModifiersMustAgree.errors.txt +++ b/tests/baselines/reference/overloadModifiersMustAgree.errors.txt @@ -13,7 +13,7 @@ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ !!! Function implementation expected. ~~~ -!!! Duplicate identifier 'bar'. +!!! Overload signatures must all be exported or not exported. function bar(s?: string) { } interface I {