From ec36d73e7a5c00df8eb865c95aef620d089ce783 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 14 Sep 2020 13:12:51 -0700 Subject: [PATCH] Fix error on duplicate commonjs exports (#40545) * Fix error on duplicate commonjs exports Previously, the code missed setting the parent pointer for the lhs access expression. Also add declaration emit of element access expressions, missed in my previous PR. * Switch to excludes=None, add test case CommonJS exports have None excludes, but still have an error issued by the checker. This is the previous behaviour even though it would be nice to add some exclusions. --- src/compiler/binder.ts | 4 ++-- src/compiler/checker.ts | 1 + ...moduleExportAliasElementAccessExpression.js | 17 +++++++++++++++++ ...eExportAliasElementAccessExpression.symbols | 6 ++++++ ...uleExportAliasElementAccessExpression.types | 8 ++++++++ .../moduleExportDuplicateAlias.errors.txt | 16 ++++++++++++++++ .../reference/moduleExportDuplicateAlias.js | 14 ++++++++++++++ .../moduleExportDuplicateAlias.symbols | 16 ++++++++++++++++ .../reference/moduleExportDuplicateAlias.types | 18 ++++++++++++++++++ ...moduleExportAliasElementAccessExpression.ts | 5 ++++- .../salsa/moduleExportDuplicateAlias.ts | 7 +++++++ 11 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 tests/baselines/reference/moduleExportAliasElementAccessExpression.js create mode 100644 tests/baselines/reference/moduleExportDuplicateAlias.errors.txt create mode 100644 tests/baselines/reference/moduleExportDuplicateAlias.js create mode 100644 tests/baselines/reference/moduleExportDuplicateAlias.symbols create mode 100644 tests/baselines/reference/moduleExportDuplicateAlias.types create mode 100644 tests/cases/conformance/salsa/moduleExportDuplicateAlias.ts diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 1b53d3e2a05..aec962016a2 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -2810,8 +2810,8 @@ namespace ts { if (symbol) { const isAlias = isAliasableExpression(node.right) && (isExportsIdentifier(node.left.expression) || isModuleExportsAccessExpression(node.left.expression)); const flags = isAlias ? SymbolFlags.Alias : SymbolFlags.Property | SymbolFlags.ExportValue; - const excludeFlags = isAlias ? SymbolFlags.AliasExcludes : SymbolFlags.None; - declareSymbol(symbol.exports!, symbol, node.left, flags, excludeFlags); + setParent(node.left, node); + declareSymbol(symbol.exports!, symbol, node.left, flags, SymbolFlags.None); } } diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index f590695be32..bf489dae320 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -6831,6 +6831,7 @@ namespace ts { break; case SyntaxKind.BinaryExpression: case SyntaxKind.PropertyAccessExpression: + case SyntaxKind.ElementAccessExpression: // Could be best encoded as though an export specifier or as though an export assignment // If name is default or export=, do an export assignment // Otherwise do an export specifier diff --git a/tests/baselines/reference/moduleExportAliasElementAccessExpression.js b/tests/baselines/reference/moduleExportAliasElementAccessExpression.js new file mode 100644 index 00000000000..721acfe394c --- /dev/null +++ b/tests/baselines/reference/moduleExportAliasElementAccessExpression.js @@ -0,0 +1,17 @@ +//// [moduleExportAliasElementAccessExpression.js] +function D () { } +exports["D"] = D; + // (the only package I could find that uses spaces in identifiers is webidl-conversions) +exports["Does not work yet"] = D; + + +//// [moduleExportAliasElementAccessExpression.js] +function D() { } +exports["D"] = D; +// (the only package I could find that uses spaces in identifiers is webidl-conversions) +exports["Does not work yet"] = D; + + +//// [moduleExportAliasElementAccessExpression.d.ts] +export function D(): void; +export { D as _Does_not_work_yet }; diff --git a/tests/baselines/reference/moduleExportAliasElementAccessExpression.symbols b/tests/baselines/reference/moduleExportAliasElementAccessExpression.symbols index 501941eb682..022b5611744 100644 --- a/tests/baselines/reference/moduleExportAliasElementAccessExpression.symbols +++ b/tests/baselines/reference/moduleExportAliasElementAccessExpression.symbols @@ -7,3 +7,9 @@ exports["D"] = D; >"D" : Symbol("D", Decl(moduleExportAliasElementAccessExpression.js, 0, 17)) >D : Symbol(D, Decl(moduleExportAliasElementAccessExpression.js, 0, 0)) + // (the only package I could find that uses spaces in identifiers is webidl-conversions) +exports["Does not work yet"] = D; +>exports : Symbol("tests/cases/conformance/salsa/moduleExportAliasElementAccessExpression", Decl(moduleExportAliasElementAccessExpression.js, 0, 0)) +>"Does not work yet" : Symbol("Does not work yet", Decl(moduleExportAliasElementAccessExpression.js, 1, 17)) +>D : Symbol(D, Decl(moduleExportAliasElementAccessExpression.js, 0, 0)) + diff --git a/tests/baselines/reference/moduleExportAliasElementAccessExpression.types b/tests/baselines/reference/moduleExportAliasElementAccessExpression.types index 9b07a6672ce..f7f06ef485b 100644 --- a/tests/baselines/reference/moduleExportAliasElementAccessExpression.types +++ b/tests/baselines/reference/moduleExportAliasElementAccessExpression.types @@ -9,3 +9,11 @@ exports["D"] = D; >"D" : "D" >D : () => void + // (the only package I could find that uses spaces in identifiers is webidl-conversions) +exports["Does not work yet"] = D; +>exports["Does not work yet"] = D : () => void +>exports["Does not work yet"] : () => void +>exports : typeof import("tests/cases/conformance/salsa/moduleExportAliasElementAccessExpression") +>"Does not work yet" : "Does not work yet" +>D : () => void + diff --git a/tests/baselines/reference/moduleExportDuplicateAlias.errors.txt b/tests/baselines/reference/moduleExportDuplicateAlias.errors.txt new file mode 100644 index 00000000000..d216af6f764 --- /dev/null +++ b/tests/baselines/reference/moduleExportDuplicateAlias.errors.txt @@ -0,0 +1,16 @@ +tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias.js(1,1): error TS2323: Cannot redeclare exported variable 'apply'. +tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias.js(3,1): error TS2322: Type '() => void' is not assignable to type 'undefined'. +tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias.js(3,1): error TS2323: Cannot redeclare exported variable 'apply'. + + +==== tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias.js (3 errors) ==== + exports.apply = undefined; + ~~~~~~~~~~~~~ +!!! error TS2323: Cannot redeclare exported variable 'apply'. + function a() { } + exports.apply = a; + ~~~~~~~~~~~~~ +!!! error TS2322: Type '() => void' is not assignable to type 'undefined'. + ~~~~~~~~~~~~~ +!!! error TS2323: Cannot redeclare exported variable 'apply'. + \ No newline at end of file diff --git a/tests/baselines/reference/moduleExportDuplicateAlias.js b/tests/baselines/reference/moduleExportDuplicateAlias.js new file mode 100644 index 00000000000..305b19fd68b --- /dev/null +++ b/tests/baselines/reference/moduleExportDuplicateAlias.js @@ -0,0 +1,14 @@ +//// [moduleExportAliasDuplicateAlias.js] +exports.apply = undefined; +function a() { } +exports.apply = a; + + +//// [moduleExportAliasDuplicateAlias.js] +exports.apply = undefined; +function a() { } +exports.apply = a; + + +//// [moduleExportAliasDuplicateAlias.d.ts] +export { undefined as apply }; diff --git a/tests/baselines/reference/moduleExportDuplicateAlias.symbols b/tests/baselines/reference/moduleExportDuplicateAlias.symbols new file mode 100644 index 00000000000..5c9bfaae375 --- /dev/null +++ b/tests/baselines/reference/moduleExportDuplicateAlias.symbols @@ -0,0 +1,16 @@ +=== tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias.js === +exports.apply = undefined; +>exports.apply : Symbol(apply, Decl(moduleExportAliasDuplicateAlias.js, 0, 0), Decl(moduleExportAliasDuplicateAlias.js, 1, 16)) +>exports : Symbol(apply, Decl(moduleExportAliasDuplicateAlias.js, 0, 0), Decl(moduleExportAliasDuplicateAlias.js, 1, 16)) +>apply : Symbol(apply, Decl(moduleExportAliasDuplicateAlias.js, 0, 0), Decl(moduleExportAliasDuplicateAlias.js, 1, 16)) +>undefined : Symbol(apply) + +function a() { } +>a : Symbol(a, Decl(moduleExportAliasDuplicateAlias.js, 0, 26)) + +exports.apply = a; +>exports.apply : Symbol(apply, Decl(moduleExportAliasDuplicateAlias.js, 0, 0), Decl(moduleExportAliasDuplicateAlias.js, 1, 16)) +>exports : Symbol(apply, Decl(moduleExportAliasDuplicateAlias.js, 0, 0), Decl(moduleExportAliasDuplicateAlias.js, 1, 16)) +>apply : Symbol(apply, Decl(moduleExportAliasDuplicateAlias.js, 0, 0), Decl(moduleExportAliasDuplicateAlias.js, 1, 16)) +>a : Symbol(a, Decl(moduleExportAliasDuplicateAlias.js, 0, 26)) + diff --git a/tests/baselines/reference/moduleExportDuplicateAlias.types b/tests/baselines/reference/moduleExportDuplicateAlias.types new file mode 100644 index 00000000000..1c924ac41e6 --- /dev/null +++ b/tests/baselines/reference/moduleExportDuplicateAlias.types @@ -0,0 +1,18 @@ +=== tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias.js === +exports.apply = undefined; +>exports.apply = undefined : undefined +>exports.apply : undefined +>exports : typeof import("tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias") +>apply : undefined +>undefined : undefined + +function a() { } +>a : () => void + +exports.apply = a; +>exports.apply = a : () => void +>exports.apply : undefined +>exports : typeof import("tests/cases/conformance/salsa/moduleExportAliasDuplicateAlias") +>apply : undefined +>a : () => void + diff --git a/tests/cases/conformance/salsa/moduleExportAliasElementAccessExpression.ts b/tests/cases/conformance/salsa/moduleExportAliasElementAccessExpression.ts index f6826c68611..986ca9731d1 100644 --- a/tests/cases/conformance/salsa/moduleExportAliasElementAccessExpression.ts +++ b/tests/cases/conformance/salsa/moduleExportAliasElementAccessExpression.ts @@ -1,6 +1,9 @@ -// @noEmit: true +// @declaration: true // @checkJs: true // @filename: moduleExportAliasElementAccessExpression.js +// @outdir: out function D () { } exports["D"] = D; + // (the only package I could find that uses spaces in identifiers is webidl-conversions) +exports["Does not work yet"] = D; diff --git a/tests/cases/conformance/salsa/moduleExportDuplicateAlias.ts b/tests/cases/conformance/salsa/moduleExportDuplicateAlias.ts new file mode 100644 index 00000000000..32e1a8a1ac7 --- /dev/null +++ b/tests/cases/conformance/salsa/moduleExportDuplicateAlias.ts @@ -0,0 +1,7 @@ +// @checkJs: true +// @declaration: true +// @filename: moduleExportAliasDuplicateAlias.js +// @outdir: out +exports.apply = undefined; +function a() { } +exports.apply = a;