Split options.name into options.name and options.prefix, and prefix n…

…on-relative imports with options.prefix (#94)

* Prefix absolute imports with options.name

* Validate main input instead of changing the main generation logic

* Improve validation logic and comments

* Improve testing

* comments and cleanup

* Refactor to match spec we've agreed on in discussion on #94 - split name and prefix, no autoprefixing for resolve* function results; no prefixing for external modules (.d.ts inputs).

* Fix tabs vs. spaces.  Make error message clearer about dts-generator vs. typescript versions.

* fix more tabs v spaces

* absolute -> non-relative.  Also kill a stale TODO

index.ts

@@ -37,6 +37,8 @@ export interface Options {
 	name?: string;
 	out: string;
 	outDir?: string;
+	prefix?: string;
+	rootDir?: string;
 	target?: ts.ScriptTarget;
 	sendMessage?: (message: any, ...optionalParams: any[]) => void;
 	resolveModuleId?: (params: ResolveModuleIdParams) => string;
@@ -185,6 +187,24 @@ function isNodeKindModuleDeclaration(value: ts.Node): value is ts.ModuleDeclarat
 
 export default function generate(options: Options): Promise<void> {
 
+	if (Boolean(options.main) !== Boolean(options.name)) {
+		if (Boolean(options.name)) {
+			// since options.name used to do double duty as the prefix, let's be
+			// considerate and point out that name should be replaced with prefix.
+			// TODO update this error message when we finalize which version this change
+			// will be released in.
+			throw new Error(
+				`name and main must be used together.  Perhaps you want prefix instead of
+				name? In dts-generator version 2.1, name did double duty as the option to
+				use to prefix module names with, but in >=2.2 the name option was split
+				into two; prefix is what is now used to prefix imports and module names
+				in the output.`
+			);
+		} else {
+			throw new Error('name and main must be used together.');
+		}
+	}
+
 	const noop = function (message?: any, ...optionalParams: any[]): void {};
 	const sendMessage = options.sendMessage || noop;
 	const verboseMessage = options.verbose ? sendMessage : noop;
@@ -302,6 +322,7 @@ export default function generate(options: Options): Promise<void> {
 		sendMessage('processing:');
 		let mainExportDeclaration = false;
 		let mainExportAssignment = false;
+		let foundMain = false;
 
 		program.getSourceFiles().forEach(function (sourceFile) {
 			processTree(sourceFile, function (node) {
@@ -335,7 +356,8 @@ export default function generate(options: Options): Promise<void> {
 			}
 
 			// We can optionally output the main module if there's something to export.
-			if (options.main && options.main === (options.name + filenameToMid(sourceFile.fileName.slice(baseDir.length, -3)))) {
+			if (options.main && options.main === (options.prefix + filenameToMid(sourceFile.fileName.slice(baseDir.length, -3)))) {
+				foundMain = true;
 				ts.forEachChild(sourceFile, function (node: ts.Node) {
 					mainExportDeclaration = mainExportDeclaration || isNodeKindExportDeclaration(node);
 					mainExportAssignment = mainExportAssignment || isNodeKindExportAssignment(node);
@@ -355,7 +377,11 @@ export default function generate(options: Options): Promise<void> {
 			}
 		});
 
-		if (options.main && options.name) {
+		if (options.main && !foundMain) {
+			throw new Error(`main module ${options.main} was not found`);
+		}
+
+		if (options.main) {
 			output.write(`declare module '${options.name}' {` + eol + indent);
 			if (compilerOptions.target >= ts.ScriptTarget.ES2015) {
 				if (mainExportAssignment) {
@@ -387,27 +413,44 @@ export default function generate(options: Options): Promise<void> {
 		// alongside the source, so use baseDir in that case too.
 		const outputDir = (isOutput && Boolean(outDir)) ? pathUtil.resolve(outDir) : baseDir;
 
-		const sourceModuleId = options.name ? options.name + filenameToMid(filename.slice(outputDir.length, -DTSLEN)) : filenameToMid(filename.slice(outputDir.length + 1, -DTSLEN));
+		const sourceModuleId = filenameToMid(filename.slice(outputDir.length + 1, -DTSLEN));
 
 		const currentModuleId = filenameToMid(filename.slice(outputDir.length + 1, -DTSLEN));
 		function resolveModuleImport(moduleId: string): string {
 			const isDeclaredExternalModule: boolean = declaredExternalModules.indexOf(moduleId) !== -1;
+			let resolved: string;
 
 			if (options.resolveModuleImport) {
-				const resolved: string = options.resolveModuleImport({
+				resolved = options.resolveModuleImport({
 					importedModuleId: moduleId,
 					currentModuleId: currentModuleId,
 					isDeclaredExternalModule: isDeclaredExternalModule
 				});
-				if (resolved) {
-					return resolved;
-				}
 			}
 
-			if (moduleId.charAt(0) === '.') {
-				return filenameToMid(pathUtil.join(pathUtil.dirname(sourceModuleId), moduleId));
+			if (!resolved) {
+				// resolve relative imports relative to the current module id.
+				if (moduleId.charAt(0) === '.') {
+					resolved = filenameToMid(pathUtil.join(pathUtil.dirname(sourceModuleId), moduleId));
+				} else {
+					resolved = moduleId;
+				}
+
+				// prefix the import with options.prefix, so that both non-relative imports
+				// and relative imports end up prefixed with options.prefix.  We only
+				// do this when no resolveModuleImport function is given so that that
+				// function has complete control of the imports that get outputed.
+				// NOTE: we may want to revisit the isDeclaredExternalModule behavior.
+				// discussion is on https://github.com/SitePen/dts-generator/pull/94
+				// but currently there's no strong argument against this behavior.
+				if (Boolean(options.prefix) && !isDeclaredExternalModule) {
+					resolved = `${options.prefix}/${resolved}`;
+				}
 			}
+
+			return resolved;
 		}
+
 		/* For some reason, SourceFile.externalModuleIndicator is missing from 1.6+, so having
 		 * to use a sledgehammer on the nut */
 		if ((<any> declarationFile).externalModuleIndicator) {
@@ -418,19 +461,26 @@ export default function generate(options: Options): Promise<void> {
 				});
 				if (resolveModuleIdResult) {
 					resolvedModuleId = resolveModuleIdResult;
+				} else if (options.prefix) {
+					resolvedModuleId = `${options.prefix}/${resolvedModuleId}`;
 				}
+			} else if (options.prefix) {
+				resolvedModuleId = `${options.prefix}/${resolvedModuleId}`;
 			}
 
 			output.write('declare module \'' + resolvedModuleId + '\' {' + eol + indent);
 
 			const content = processTree(declarationFile, function (node) {
 				if (isNodeKindExternalModuleReference(node)) {
+					// TODO figure out if this branch is possible, and if so, write a test
+					// that covers it.
+
 					const expression = node.expression as ts.LiteralExpression;
 
+					// convert both relative and non-relative module names in import = require(...)
+					// statements.
 					const resolved: string = resolveModuleImport(expression.text);
-					if (resolved) {
-						return ' require(\'' + resolved + '\')';
-					}
+					return ` require('${resolved}')`;
 				}
 				else if (node.kind === ts.SyntaxKind.DeclareKeyword) {
 					return '';
@@ -439,8 +489,8 @@ export default function generate(options: Options): Promise<void> {
 					isNodeKindStringLiteral(node) && node.parent &&
 					(isNodeKindExportDeclaration(node.parent) || isNodeKindImportDeclaration(node.parent))
 				) {
+					// This block of code is modifying the names of imported modules
 					const text = node.text;
-
 					const resolved: string = resolveModuleImport(text);
 					if (resolved) {
 						return ` '${resolved}'`;

tests/support/foo-prefix/sub/Bar.ts

@@ -0,0 +1,8 @@
+export default class Bar {
+	private bar: string;
+	constructor () {
+		this.bar = 'bar';
+	}
+	foo: number;
+	qat: { foo?: string; } = {};
+}

tests/support/foo-prefix/sub/baz.ts

@@ -0,0 +1,21 @@
+/**
+ * Some JSDoc for you!
+ * @param baz Something
+ */
+export function baz() {
+	console.log('baz');
+	return 'baz';
+}
+
+/**
+ * JSDOC Type Annotation
+ */
+export type Foo = { bar: string } & { qat: number };
+
+/**
+ * Some more JSDOC
+ * @param something Blah blah blah
+ */
+export function qat(something: Foo) {
+	return something;
+}

tests/support/foo-prefix/sub/index.ts

@@ -0,0 +1,20 @@
+/// <reference path="./somejs.d.ts" />
+
+import Bar from 'sub/Bar';
+
+import { baz as fooBaz, qat as fooQat, Foo as BazFoo } from 'sub/baz';
+import somejs = require('sub/somejs');
+
+export default class Foo {
+	private foo: BazFoo;
+	bar: Bar;
+	baz = fooBaz;
+	qat = fooQat;
+	constructor (foo: BazFoo) {
+		this.bar = new Bar();
+		this.foo = foo;
+	}
+}
+
+export const foo = new Foo({ bar: '', qat: 1 });
+export const js = somejs.js;

tests/support/foo-prefix/sub/somejs.d.ts

@@ -0,0 +1,13 @@
+
+interface RubeGoldbergMachine {
+	pieces: string[];
+	stages: any[];
+	start: Function;
+}
+
+declare module 'sub/somejs' {
+	let exports: {
+		js: RubeGoldbergMachine
+	};
+	export = exports;
+}

tests/support/foo-prefix/tsconfig.json

@@ -0,0 +1,16 @@
+{
+	"version": "1.6.2",
+	"compilerOptions": {
+		"target": "es5",
+		"module": "umd",
+		"jsx": "react",
+		"rootDir": ".",
+		"declaration": false,
+		"noImplicitAny": true
+	},
+	"files": [
+		"./sub/index.ts",
+		"./sub/baz.ts",
+		"./sub/Bar.ts"
+	]
+}

tests/unit/bin/dts-generator.ts

@@ -10,7 +10,7 @@ registerSuite({
 	},
 	basic: function () {
 		return dtsGenerator([
-			'-name',
+			'-prefix',
 			'foo',
 			'-project',
 			'tests/support/foo',

tests/unit/index.ts

@@ -10,7 +10,7 @@ registerSuite({
 	},
 	'generate': function () {
 		return generate({
-			name: 'foo',
+			prefix: 'foo',
 			baseDir: 'tests/support/foo',
 			files: [ 'index.ts' ],
 			out: 'tmp/foo.d.ts'
@@ -23,7 +23,7 @@ registerSuite({
 	},
 	'no files': function () {
 		return generate({
-			name: 'foo',
+			prefix: 'foo',
 			baseDir: 'tests/support/foo',
 			out: 'tmp/foo.nofiles.d.ts'
 		}).then(function () {
@@ -35,7 +35,7 @@ registerSuite({
 	},
 	'project that explicitly lists all files': function () {
 		return generate({
-			name: 'foo',
+			prefix: 'foo',
 			project: 'tests/support/foo',
 			out: 'tmp/foo.config.d.ts'
 		}).then(function () {
@@ -48,7 +48,7 @@ registerSuite({
 	},
 	'project json file': function () {
 		return generate({
-			name: 'foo',
+			prefix: 'foo',
 			project: 'tests/support/foo/tsconfig-alt.json',
 			out: 'tmp/foo-alt.config.d.ts'
 		}).then(function () {
@@ -67,10 +67,10 @@ registerSuite({
 		// stresses our path-handling logic - if we mix up the directories, it'll
 		// show in the output module names.
 		//
-		// This project uses absolute paths, for extra fun.
+		// This project uses non-relative paths, for extra fun.
 		return generate({
 			project: 'tests/support/foo-directories',
-			out: 'tmp/foo.config.d.ts',
+			out: 'tmp/foo.config.d.ts'
 		}).then(function () {
 			const contents = fs.readFileSync('tmp/foo.config.d.ts', { encoding: 'utf8' });
 			assert(contents, 'foo.config.d.ts should exist and have contents');
@@ -83,12 +83,41 @@ registerSuite({
 			assert.include(contents, `from 'sub/baz';`);
 		});
 	},
+	'test prefixing of non-relative paths with options.prefix': function () {
+		return generate({
+			project: 'tests/support/foo-prefix',
+			out: 'tmp/foo.config.d.ts',
+			main: '__abs_prefix/sub/index',
+			name: 'prefix_test',
+			prefix: '__abs_prefix'
+		}).then(function () {
+			const contents = fs.readFileSync('tmp/foo.config.d.ts', { encoding: 'utf8' });
+			assert(contents, 'foo.config.d.ts should exist and have contents');
+			assert.include(contents, `module '__abs_prefix/sub/index'`);
+			assert.include(contents, `module '__abs_prefix/sub/Bar'`);
+			assert.include(contents, `module '__abs_prefix/sub/baz'`);
+
+			// also check imports look right
+			assert.include(contents, `import Bar from '__abs_prefix/sub/Bar'`);
+			assert.include(contents, `from '__abs_prefix/sub/baz';`);
+
+			// for some reason import = require imports seem to be dropped.  I suppose
+			// the intention may be that these modules require /// <reference> directives
+			// to work, and those show up properly.
+			assert.notInclude(contents, `import somejs = require('__abs_prefix/sub/somejs');`);
+
+			// and look at the generated main code
+			// make sure name is used as expected, and the main module looks right
+			assert.include(contents, `module 'prefix_test'`);
+			assert.include(contents, `import main = require('__abs_prefix/sub/index')`);
+		});
+	},
 	'project that lets typescript resolve tsx imports for a jsx:react project': function () {
 		// This essentially tests that we properly handle the jsx option, if any.
 		// tsx alone, or module resolution with just ts files (no tsx), does need the
 		// jsx option to be handled correctly to work.
 		return generate({
-			name: 'foo2', // also test that the name is used how we expect
+			prefix: 'foo2', // also test that the prefix is used how we expect
 			project: 'tests/support/foo-resolve-tsx/tsconfig.json',
 			out: 'tmp/foo.config.d.ts'
 		}).then(function () {
@@ -101,10 +130,11 @@ registerSuite({
 	},
 	'es6 main module': function () {
 		return generate({
-			name: 'foo',
+			prefix: 'foo',
 			project: 'tests/support/foo-es6',
 			out: 'tmp/foo.es6.d.ts',
-			main: 'index.ts'
+			main: 'foo/index',
+			name: 'fooname'
 		}).then(function () {
 			const contents = fs.readFileSync('tmp/foo.es6.d.ts', { encoding: 'utf8' });
 			assert(contents, 'foo.es6.d.ts should exist and have contents');
@@ -114,7 +144,7 @@ registerSuite({
 	},
 	'resolve module id': function () {
 		return generate({
-			name: 'foo',
+			prefix: 'foo',
 			project: 'tests/support/foo-resolve-module-id',
 			out: 'tmp/foo.resolve-module-id.d.ts',
 			resolveModuleId: (params: ResolveModuleIdParams): string => {
@@ -165,8 +195,8 @@ registerSuite({
 			// replaced external module declaration import
 			assert.include(contents, `import { ClassInJavaScript } from 'ReplacedSomethingInJavaScript';`);
 
-			// non relative module import, should not be changed
-			assert.include(contents, `import { NonRelative } from 'NonRelative';`);
+			// non relative module import, should not be changed.  Gets the usual prefixing.
+			assert.include(contents, `import { NonRelative } from 'foo/NonRelative';`);
 
 			// class imports should not be replaced, also assert on them
 			assert.include(contents, `import FooImplExportAssignment = require('foo/FooImplExportAssignment');`);
@@ -178,7 +208,6 @@ registerSuite({
 	},
 	'add reference types package dependency  ': function () {
 		return generate({
-			name: 'foo',
 			baseDir: 'tests/support/foo',
 			files: [ 'index.ts' ],
 			types: ['es6-promise'],
@@ -190,7 +219,6 @@ registerSuite({
 	},
 	'add external path dependency  ': function () {
 		return generate({
-			name: 'foo',
 			baseDir: 'tests/support/foo',
 			files: [ 'index.ts' ],
 			externs: ['../some/path/es6-promise.d.ts'],
@@ -199,5 +227,5 @@ registerSuite({
 			const contents = fs.readFileSync('tmp/foo.d.ts', { encoding: 'utf8' });
 			assert.include(contents, `/// <reference path="../some/path/es6-promise.d.ts" />`);
 		});
-	},
-});
+	}
+});