From acf557dd3c9b138e4e124fcfaf64ba1feff0015f Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Thu, 4 Sep 2014 13:16:22 -0700 Subject: [PATCH 1/3] Do not emit references in generated declaration files if the --noresolve flag was set. This fixes a crash in the compiler when generating declarations with /// reference and noResolve --- scripts/importDefinitllyTypedTests.ts | 124 ++++++++++++++++++ src/compiler/emitter.ts | 52 ++++---- .../declarationEmit_invalidReference.js | 11 ++ .../declarationEmit_invalidReference.types | 5 + .../declarationEmit_invalidReference.ts | 4 + 5 files changed, 171 insertions(+), 25 deletions(-) create mode 100644 scripts/importDefinitllyTypedTests.ts create mode 100644 tests/baselines/reference/declarationEmit_invalidReference.js create mode 100644 tests/baselines/reference/declarationEmit_invalidReference.types create mode 100644 tests/cases/compiler/declarationEmit_invalidReference.ts diff --git a/scripts/importDefinitllyTypedTests.ts b/scripts/importDefinitllyTypedTests.ts new file mode 100644 index 0000000000..caa6be0063 --- /dev/null +++ b/scripts/importDefinitllyTypedTests.ts @@ -0,0 +1,124 @@ +declare var require: any, process: any; +declare var __dirname: any; + +var fs = require("fs"); +var path = require("path"); +var child_process = require('child_process'); + +var tscRoot = path.join(__dirname, "..\\"); +var tscPath = path.join(tscRoot, "built", "instrumented", "tsc.js"); +var rwcTestPath = path.join(tscRoot, "tests", "cases", "rwc", "dt"); +var definitelyTypedRoot = process.argv[2]; + +function fileExtensionIs(path: string, extension: string): boolean { + var pathLen = path.length; + var extLen = extension.length; + return pathLen > extLen && path.substr(pathLen - extLen, extLen).toLocaleLowerCase() === extension.toLocaleLowerCase(); +} + +function copyFileSync(source, destination) { + var text = fs.readFileSync(source); + fs.writeFileSync(destination, text); +} + +function importDefinitelyTypedTest(testCaseName: string, testFiles: string[], responseFile: string ) { + var cmd = "node " + tscPath + " --module commonjs " + testFiles.join(" "); + if (responseFile) cmd += " @" + responseFile; + + var testDirectoryName = testCaseName + "_" + Math.floor((Math.random() * 10000) + 1); + var testDirectoryPath = path.join(process.env["temp"], testDirectoryName); + if (fs.existsSync(testDirectoryPath)) { + throw new Error("Could not create test directory"); + } + fs.mkdirSync(testDirectoryPath); + + child_process.exec(cmd, { + maxBuffer: 1 * 1024 * 1024, + cwd: testDirectoryPath + }, (error, stdout, stderr) => { + //console.log("importing " + testCaseName + " ..."); + //console.log(cmd); + + if (error) { + console.log("importing " + testCaseName + " ..."); + console.log(cmd); + console.log("==> error " + JSON.stringify(error)); + console.log("==> stdout " + String(stdout)); + console.log("==> stderr " + String(stderr)); + console.log("\r\n"); + return; + } + + // copy generated file to output location + var outputFilePath = path.join(testDirectoryPath, "iocapture0.json"); + var testCasePath = path.join(rwcTestPath, "DefinitelyTyped_" + testCaseName + ".json"); + copyFileSync(outputFilePath, testCasePath); + + //console.log("output generated at: " + outputFilePath); + + if (!fs.existsSync(testCasePath)) { + throw new Error("could not find test case at: " + testCasePath); + } + else { + fs.unlinkSync(outputFilePath); + fs.rmdirSync(testDirectoryPath); + //console.log("testcase generated at: " + testCasePath); + //console.log("Done."); + } + //console.log("\r\n"); + + }) + .on('error', function (error) { + console.log("==> error " + JSON.stringify(error)); + console.log("\r\n"); + }); +} + +function importDefinitelyTypedTests(definitelyTypedRoot: string): void { + fs.readdir(definitelyTypedRoot, (err, subDirectorys) => { + if (err) throw err; + + subDirectorys + .filter(d => ["_infrastructure", "node_modules", ".git"].indexOf(d) >= 0) + .filter(i => fs.statSync(path.join(definitelyTypedRoot, i)).isDirectory()) + .forEach(d => { + var directoryPath = path.join(definitelyTypedRoot, d); + fs.readdir(directoryPath, function (err, files) { + if (err) throw err; + + var tsFiles = []; + var testFiles = []; + var paramFile; + + files + .map(f => path.join(directoryPath, f)) + .forEach(f => { + if (fileExtensionIs(f, ".ts")) tsFiles.push(f); + else if (fileExtensionIs(f, ".tscparams")) paramFile = f; + + if (fileExtensionIs(f, "-tests.ts")) testFiles.push(f); + }); + + if (testFiles.length === 0) { + // no test files but multiple d.ts's, e.g. winjs + var regexp = new RegExp(d + "(([-][0-9])|([\.]d[\.]ts))"); + if (tsFiles.length > 1 && tsFiles.every(t => fileExtensionIs(t, ".d.ts") && regexp.test(t))) { + tsFiles.forEach(filename => { + importDefinitelyTypedTest(path.basename(filename, ".d.ts"), [filename], paramFile); + }); + } + else { + importDefinitelyTypedTest(d, tsFiles, paramFile); + } + } + else { + testFiles.forEach(filename => { + importDefinitelyTypedTest(path.basename(filename, "-tests.ts"), [filename], paramFile); + }); + } + }); + }) + }); +} + +importDefinitelyTypedTests(definitelyTypedRoot); \ No newline at end of file diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 7634e6a778..57805859bf 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -3079,10 +3079,8 @@ module ts { } } - function resolveScriptReference(sourceFile: SourceFile, reference: FileReference) { - var referenceFileName = compilerOptions.noResolve - ? reference.filename - : normalizePath(combinePaths(getDirectoryPath(sourceFile.filename), reference.filename)); + function tryResolveScriptReference(sourceFile: SourceFile, reference: FileReference) { + var referenceFileName = normalizePath(combinePaths(getDirectoryPath(sourceFile.filename), reference.filename)); return program.getSourceFile(referenceFileName); } @@ -3108,21 +3106,23 @@ module ts { if (root) { // Emiting single file so emit references in this file only - var addedGlobalFileReference = false; - forEach(root.referencedFiles, fileReference => { - var referencedFile = resolveScriptReference(root, fileReference); + if (!compilerOptions.noResolve) { + var addedGlobalFileReference = false; + forEach(root.referencedFiles, fileReference => { + var referencedFile = tryResolveScriptReference(root, fileReference); - // All the references that are not going to be part of same file - if ((referencedFile.flags & NodeFlags.DeclarationFile) || // This is a declare file reference - shouldEmitToOwnFile(referencedFile) || // This is referenced file is emitting its own js file - !addedGlobalFileReference) { // Or the global out file corresponding to this reference was not added + // All the references that are not going to be part of same file + if (referencedFile && (referencedFile.flags & NodeFlags.DeclarationFile) || // This is a declare file reference + shouldEmitToOwnFile(referencedFile) || // This is referenced file is emitting its own js file + !addedGlobalFileReference) { // Or the global out file corresponding to this reference was not added - writeReferencePath(referencedFile); - if (!isExternalModuleOrDeclarationFile(referencedFile)) { - addedGlobalFileReference = true; + writeReferencePath(referencedFile); + if (!isExternalModuleOrDeclarationFile(referencedFile)) { + addedGlobalFileReference = true; + } } - } - }); + }); + } emitNode(root); } @@ -3132,17 +3132,19 @@ module ts { forEach(program.getSourceFiles(), sourceFile => { if (!isExternalModuleOrDeclarationFile(sourceFile)) { // Check what references need to be added - forEach(sourceFile.referencedFiles, fileReference => { - var referencedFile = resolveScriptReference(sourceFile, fileReference); + if (!compilerOptions.noResolve) { + forEach(sourceFile.referencedFiles, fileReference => { + var referencedFile = tryResolveScriptReference(sourceFile, fileReference); - // If the reference file is declaration file or external module emit that reference - if (isExternalModuleOrDeclarationFile(referencedFile) && - !contains(emittedReferencedFiles, referencedFile)) { // If the file refernece was not already emitted + // If the reference file is declaration file or external module emit that reference + if (referencedFile && isExternalModuleOrDeclarationFile(referencedFile) && + !contains(emittedReferencedFiles, referencedFile)) { // If the file refernece was not already emitted - writeReferencePath(referencedFile); - emittedReferencedFiles.push(referencedFile); - } - }); + writeReferencePath(referencedFile); + emittedReferencedFiles.push(referencedFile); + } + }); + } emitNode(sourceFile); } diff --git a/tests/baselines/reference/declarationEmit_invalidReference.js b/tests/baselines/reference/declarationEmit_invalidReference.js new file mode 100644 index 0000000000..d6ef9f48e5 --- /dev/null +++ b/tests/baselines/reference/declarationEmit_invalidReference.js @@ -0,0 +1,11 @@ +//// [declarationEmit_invalidReference.ts] +/// +var x = 0; + +//// [declarationEmit_invalidReference.js] +/// +var x = 0; + + +//// [declarationEmit_invalidReference.d.ts] +declare var x: number; diff --git a/tests/baselines/reference/declarationEmit_invalidReference.types b/tests/baselines/reference/declarationEmit_invalidReference.types new file mode 100644 index 0000000000..1fe465887e --- /dev/null +++ b/tests/baselines/reference/declarationEmit_invalidReference.types @@ -0,0 +1,5 @@ +=== tests/cases/compiler/declarationEmit_invalidReference.ts === +/// +var x = 0; +>x : number + diff --git a/tests/cases/compiler/declarationEmit_invalidReference.ts b/tests/cases/compiler/declarationEmit_invalidReference.ts new file mode 100644 index 0000000000..9635f69596 --- /dev/null +++ b/tests/cases/compiler/declarationEmit_invalidReference.ts @@ -0,0 +1,4 @@ +// @declaration: true +// @noresolve: true +/// +var x = 0; \ No newline at end of file From 05ebe9c214246ae5cf23f039498cb07482a20470 Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Thu, 4 Sep 2014 14:41:11 -0700 Subject: [PATCH 2/3] add negative test for invalid references and declaration emit --- .../reference/declarationEmit_invalidReference2.errors.txt | 5 +++++ tests/cases/compiler/declarationEmit_invalidReference2.ts | 3 +++ 2 files changed, 8 insertions(+) create mode 100644 tests/baselines/reference/declarationEmit_invalidReference2.errors.txt create mode 100644 tests/cases/compiler/declarationEmit_invalidReference2.ts diff --git a/tests/baselines/reference/declarationEmit_invalidReference2.errors.txt b/tests/baselines/reference/declarationEmit_invalidReference2.errors.txt new file mode 100644 index 0000000000..91368320ba --- /dev/null +++ b/tests/baselines/reference/declarationEmit_invalidReference2.errors.txt @@ -0,0 +1,5 @@ +==== tests/cases/compiler/declarationEmit_invalidReference2.ts (1 errors) ==== + /// + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +!!! File 'invalid.ts' not found. + var x = 0; \ No newline at end of file diff --git a/tests/cases/compiler/declarationEmit_invalidReference2.ts b/tests/cases/compiler/declarationEmit_invalidReference2.ts new file mode 100644 index 0000000000..829c674c48 --- /dev/null +++ b/tests/cases/compiler/declarationEmit_invalidReference2.ts @@ -0,0 +1,3 @@ +// @declaration: true +/// +var x = 0; \ No newline at end of file From cc3cae34e96624fda99ec84cb7898b575b1529c6 Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Fri, 5 Sep 2014 13:06:10 -0700 Subject: [PATCH 3/3] respond to code review remarks and reverted tryResolveScriptReference to resolveScriptReference --- src/compiler/emitter.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 57805859bf..b12bf478e1 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -3079,7 +3079,7 @@ module ts { } } - function tryResolveScriptReference(sourceFile: SourceFile, reference: FileReference) { + function resolveScriptReference(sourceFile: SourceFile, reference: FileReference) { var referenceFileName = normalizePath(combinePaths(getDirectoryPath(sourceFile.filename), reference.filename)); return program.getSourceFile(referenceFileName); } @@ -3109,10 +3109,10 @@ module ts { if (!compilerOptions.noResolve) { var addedGlobalFileReference = false; forEach(root.referencedFiles, fileReference => { - var referencedFile = tryResolveScriptReference(root, fileReference); + var referencedFile = resolveScriptReference(root, fileReference); // All the references that are not going to be part of same file - if (referencedFile && (referencedFile.flags & NodeFlags.DeclarationFile) || // This is a declare file reference + if ((referencedFile.flags & NodeFlags.DeclarationFile) || // This is a declare file reference shouldEmitToOwnFile(referencedFile) || // This is referenced file is emitting its own js file !addedGlobalFileReference) { // Or the global out file corresponding to this reference was not added @@ -3134,10 +3134,10 @@ module ts { // Check what references need to be added if (!compilerOptions.noResolve) { forEach(sourceFile.referencedFiles, fileReference => { - var referencedFile = tryResolveScriptReference(sourceFile, fileReference); + var referencedFile = resolveScriptReference(sourceFile, fileReference); // If the reference file is declaration file or external module emit that reference - if (referencedFile && isExternalModuleOrDeclarationFile(referencedFile) && + if (isExternalModuleOrDeclarationFile(referencedFile) && !contains(emittedReferencedFiles, referencedFile)) { // If the file refernece was not already emitted writeReferencePath(referencedFile);