From 9974526101a4e28c2707ba0390b35ea086549d11 Mon Sep 17 00:00:00 2001 From: togru Date: Thu, 5 Feb 2015 12:07:00 +0100 Subject: [PATCH 1/5] updated code style, added tests, fixed regex bug, merged to latest branch --- .gitignore | 1 + src/compiler/emitter.ts | 8 +++++++- src/compiler/parser.ts | 12 +++++++++--- src/compiler/types.ts | 2 +- src/services/services.ts | 2 +- .../reference/amdDependencyCommentName1.errors.txt | 10 ++++++++++ .../baselines/reference/amdDependencyCommentName1.js | 10 ++++++++++ .../reference/amdDependencyCommentName2.errors.txt | 10 ++++++++++ .../baselines/reference/amdDependencyCommentName2.js | 11 +++++++++++ tests/cases/compiler/amdDependencyCommentName1.ts | 5 +++++ tests/cases/compiler/amdDependencyCommentName2.ts | 5 +++++ 11 files changed, 70 insertions(+), 6 deletions(-) create mode 100644 tests/baselines/reference/amdDependencyCommentName1.errors.txt create mode 100644 tests/baselines/reference/amdDependencyCommentName1.js create mode 100644 tests/baselines/reference/amdDependencyCommentName2.errors.txt create mode 100644 tests/baselines/reference/amdDependencyCommentName2.js create mode 100644 tests/cases/compiler/amdDependencyCommentName1.ts create mode 100644 tests/cases/compiler/amdDependencyCommentName2.ts diff --git a/.gitignore b/.gitignore index 57e8804d7c..c54c8018ea 100644 --- a/.gitignore +++ b/.gitignore @@ -44,3 +44,4 @@ scripts/ior.js scripts/*.js.map coverage/ internal/ +**/.DS_Store diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index a452197780..ebd9f3b25c 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -3916,7 +3916,7 @@ module ts { emitLiteral(getExternalModuleImportDeclarationExpression(imp)); }); forEach(node.amdDependencies, amdDependency => { - var text = "\"" + amdDependency + "\""; + var text = "\"" + amdDependency.path + "\""; write(", "); write(text); }); @@ -3925,6 +3925,12 @@ module ts { write(", "); emit(imp.name); }); + forEach(node.amdDependencies, amdDependency => { + if (amdDependency.name) { + write(", "); + write(amdDependency.name); + } + }); write(") {"); increaseIndent(); emitCaptureThisForNodeIfNecessary(node); diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 866d66d515..373c378241 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -4670,7 +4670,7 @@ module ts { function processReferenceComments(sourceFile: SourceFile): void { var triviaScanner = createScanner(sourceFile.languageVersion, /*skipTrivia*/false, sourceText); var referencedFiles: FileReference[] = []; - var amdDependencies: string[] = []; + var amdDependencies: {path: string; name: string}[] = []; var amdModuleName: string; // Keep scanning all the leading trivia in the file until we get to something that @@ -4710,10 +4710,16 @@ module ts { amdModuleName = amdModuleNameMatchResult[2]; } - var amdDependencyRegEx = /^\/\/\/\s*; public endOfFileToken: Node; - public amdDependencies: string[]; + public amdDependencies: {name: string; path: string}[]; public amdModuleName: string; public referencedFiles: FileReference[]; diff --git a/tests/baselines/reference/amdDependencyCommentName1.errors.txt b/tests/baselines/reference/amdDependencyCommentName1.errors.txt new file mode 100644 index 0000000000..081c86b25e --- /dev/null +++ b/tests/baselines/reference/amdDependencyCommentName1.errors.txt @@ -0,0 +1,10 @@ +tests/cases/compiler/amdDependencyCommentName1.ts(3,21): error TS2307: Cannot find external module 'm2'. + + +==== tests/cases/compiler/amdDependencyCommentName1.ts (1 errors) ==== + /// + + import m1 = require("m2") + ~~~~ +!!! error TS2307: Cannot find external module 'm2'. + m1.f(); \ No newline at end of file diff --git a/tests/baselines/reference/amdDependencyCommentName1.js b/tests/baselines/reference/amdDependencyCommentName1.js new file mode 100644 index 0000000000..0333653637 --- /dev/null +++ b/tests/baselines/reference/amdDependencyCommentName1.js @@ -0,0 +1,10 @@ +//// [amdDependencyCommentName1.ts] +/// + +import m1 = require("m2") +m1.f(); + +//// [amdDependencyCommentName1.js] +/// +var m1 = require("m2"); +m1.f(); diff --git a/tests/baselines/reference/amdDependencyCommentName2.errors.txt b/tests/baselines/reference/amdDependencyCommentName2.errors.txt new file mode 100644 index 0000000000..137f9335ff --- /dev/null +++ b/tests/baselines/reference/amdDependencyCommentName2.errors.txt @@ -0,0 +1,10 @@ +tests/cases/compiler/amdDependencyCommentName2.ts(3,21): error TS2307: Cannot find external module 'm2'. + + +==== tests/cases/compiler/amdDependencyCommentName2.ts (1 errors) ==== + /// + + import m1 = require("m2") + ~~~~ +!!! error TS2307: Cannot find external module 'm2'. + m1.f(); \ No newline at end of file diff --git a/tests/baselines/reference/amdDependencyCommentName2.js b/tests/baselines/reference/amdDependencyCommentName2.js new file mode 100644 index 0000000000..d923df8019 --- /dev/null +++ b/tests/baselines/reference/amdDependencyCommentName2.js @@ -0,0 +1,11 @@ +//// [amdDependencyCommentName2.ts] +/// + +import m1 = require("m2") +m1.f(); + +//// [amdDependencyCommentName2.js] +/// +define(["require", "exports", "m2", "bar"], function (require, exports, m1, b) { + m1.f(); +}); diff --git a/tests/cases/compiler/amdDependencyCommentName1.ts b/tests/cases/compiler/amdDependencyCommentName1.ts new file mode 100644 index 0000000000..958d7ff825 --- /dev/null +++ b/tests/cases/compiler/amdDependencyCommentName1.ts @@ -0,0 +1,5 @@ +//@module: commonjs +/// + +import m1 = require("m2") +m1.f(); \ No newline at end of file diff --git a/tests/cases/compiler/amdDependencyCommentName2.ts b/tests/cases/compiler/amdDependencyCommentName2.ts new file mode 100644 index 0000000000..6cd7d5531f --- /dev/null +++ b/tests/cases/compiler/amdDependencyCommentName2.ts @@ -0,0 +1,5 @@ +//@module: amd +/// + +import m1 = require("m2") +m1.f(); \ No newline at end of file From 36990570c4e38594cda2b45af51a0239b906d07a Mon Sep 17 00:00:00 2001 From: togru Date: Mon, 9 Feb 2015 08:44:34 +0100 Subject: [PATCH 2/5] Added AMD dependency reordering, so import order matches with provided names --- src/compiler/parser.ts | 8 +++++++- .../amdDependencyCommentName3.errors.txt | 12 ++++++++++++ .../reference/amdDependencyCommentName3.js | 15 +++++++++++++++ tests/cases/compiler/amdDependencyCommentName3.ts | 7 +++++++ 4 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 tests/baselines/reference/amdDependencyCommentName3.errors.txt create mode 100644 tests/baselines/reference/amdDependencyCommentName3.js create mode 100644 tests/cases/compiler/amdDependencyCommentName3.ts diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 373c378241..c863f7e896 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -4718,7 +4718,13 @@ module ts { var pathMatchResult = pathRegex.exec(comment); var nameMatchResult = nameRegex.exec(comment); if (pathMatchResult) { - amdDependencies.push({path: pathMatchResult[2], name: nameMatchResult ? nameMatchResult[2] : undefined }); + var amdDependency = {path: pathMatchResult[2], name: nameMatchResult ? nameMatchResult[2] : undefined }; + // AMD dependencies with names have to go first in define header + if (nameMatchResult) { + amdDependencies.push(amdDependency); + } else { + amdDependencies.unshift(amdDependency); + } } } } diff --git a/tests/baselines/reference/amdDependencyCommentName3.errors.txt b/tests/baselines/reference/amdDependencyCommentName3.errors.txt new file mode 100644 index 0000000000..1fa997b9c9 --- /dev/null +++ b/tests/baselines/reference/amdDependencyCommentName3.errors.txt @@ -0,0 +1,12 @@ +tests/cases/compiler/amdDependencyCommentName3.ts(5,21): error TS2307: Cannot find external module 'm2'. + + +==== tests/cases/compiler/amdDependencyCommentName3.ts (1 errors) ==== + /// + /// + /// + + import m1 = require("m2") + ~~~~ +!!! error TS2307: Cannot find external module 'm2'. + m1.f(); \ No newline at end of file diff --git a/tests/baselines/reference/amdDependencyCommentName3.js b/tests/baselines/reference/amdDependencyCommentName3.js new file mode 100644 index 0000000000..53ac77537f --- /dev/null +++ b/tests/baselines/reference/amdDependencyCommentName3.js @@ -0,0 +1,15 @@ +//// [amdDependencyCommentName3.ts] +/// +/// +/// + +import m1 = require("m2") +m1.f(); + +//// [amdDependencyCommentName3.js] +/// +/// +/// +define(["require", "exports", "m2", "foo", "bar", "goo"], function (require, exports, m1, b, c) { + m1.f(); +}); diff --git a/tests/cases/compiler/amdDependencyCommentName3.ts b/tests/cases/compiler/amdDependencyCommentName3.ts new file mode 100644 index 0000000000..4800aed963 --- /dev/null +++ b/tests/cases/compiler/amdDependencyCommentName3.ts @@ -0,0 +1,7 @@ +//@module: amd +/// +/// +/// + +import m1 = require("m2") +m1.f(); \ No newline at end of file From a27a893eeba28b6bb2e787daf6b443c5754cebfa Mon Sep 17 00:00:00 2001 From: togru Date: Mon, 9 Feb 2015 09:00:42 +0100 Subject: [PATCH 3/5] previous AMD ordering was not correct --- src/compiler/parser.ts | 4 ++-- tests/baselines/reference/amdDependencyCommentName3.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index c863f7e896..258f7df0b4 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -4721,9 +4721,9 @@ module ts { var amdDependency = {path: pathMatchResult[2], name: nameMatchResult ? nameMatchResult[2] : undefined }; // AMD dependencies with names have to go first in define header if (nameMatchResult) { - amdDependencies.push(amdDependency); - } else { amdDependencies.unshift(amdDependency); + } else { + amdDependencies.push(amdDependency); } } } diff --git a/tests/baselines/reference/amdDependencyCommentName3.js b/tests/baselines/reference/amdDependencyCommentName3.js index 53ac77537f..2230a357bd 100644 --- a/tests/baselines/reference/amdDependencyCommentName3.js +++ b/tests/baselines/reference/amdDependencyCommentName3.js @@ -10,6 +10,6 @@ m1.f(); /// /// /// -define(["require", "exports", "m2", "foo", "bar", "goo"], function (require, exports, m1, b, c) { +define(["require", "exports", "m2", "goo", "bar", "foo"], function (require, exports, m1, c, b) { m1.f(); }); From 8492dfdffd89c00a4d96ce25152b30256e650311 Mon Sep 17 00:00:00 2001 From: togru Date: Tue, 10 Feb 2015 10:28:09 +0100 Subject: [PATCH 4/5] moved AMD module sorting to emitter, updated test case --- src/compiler/emitter.ts | 14 ++++++++++++++ src/compiler/parser.ts | 7 +------ .../reference/amdDependencyCommentName3.js | 2 +- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index ebd9f3b25c..08c29ab8dc 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -3902,11 +3902,25 @@ module ts { } }); } + + function sortAMDModules(amdModules: {name: string; path: string}[]) { + // AMD modules with declared variable names goes first + return amdModules.sort((moduleA, moduleB) => { + if (moduleA.name == moduleB.name) { + return 0; + } else if (moduleA.name == undefined) { + return 1; + } else { + return -1; + } + }); + } function emitAMDModule(node: SourceFile, startIndex: number) { var imports = getExternalImportDeclarations(node); writeLine(); write("define("); + sortAMDModules(node.amdDependencies); if (node.amdModuleName) { write("\"" + node.amdModuleName + "\", "); } diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 258f7df0b4..5288d6761d 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -4719,12 +4719,7 @@ module ts { var nameMatchResult = nameRegex.exec(comment); if (pathMatchResult) { var amdDependency = {path: pathMatchResult[2], name: nameMatchResult ? nameMatchResult[2] : undefined }; - // AMD dependencies with names have to go first in define header - if (nameMatchResult) { - amdDependencies.unshift(amdDependency); - } else { - amdDependencies.push(amdDependency); - } + amdDependencies.push(amdDependency); } } } diff --git a/tests/baselines/reference/amdDependencyCommentName3.js b/tests/baselines/reference/amdDependencyCommentName3.js index 2230a357bd..76e00e1e0c 100644 --- a/tests/baselines/reference/amdDependencyCommentName3.js +++ b/tests/baselines/reference/amdDependencyCommentName3.js @@ -10,6 +10,6 @@ m1.f(); /// /// /// -define(["require", "exports", "m2", "goo", "bar", "foo"], function (require, exports, m1, c, b) { +define(["require", "exports", "m2", "bar", "goo", "foo"], function (require, exports, m1, b, c) { m1.f(); }); From 091f38b3e0b2df1b0075f9e3d358e688898ff27c Mon Sep 17 00:00:00 2001 From: togru Date: Wed, 11 Feb 2015 10:10:11 +0100 Subject: [PATCH 5/5] improved equality checks in AMD module sorting function --- src/compiler/emitter.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 08c29ab8dc..95f3b970a3 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -3904,11 +3904,11 @@ module ts { } function sortAMDModules(amdModules: {name: string; path: string}[]) { - // AMD modules with declared variable names goes first + // AMD modules with declared variable names go first return amdModules.sort((moduleA, moduleB) => { - if (moduleA.name == moduleB.name) { + if (moduleA.name === moduleB.name) { return 0; - } else if (moduleA.name == undefined) { + } else if (!moduleA.name) { return 1; } else { return -1;