Fix tsconfig overridden by defaults (#165)

Janne252 · Perryvw · commit 960b4d01e837 · 2018-08-12T19:54:27.000+02:00
* Common type of yargs options object
Export optionDeclarations so that tests can reference them
Parse cmd args without defaults to prevent overriding tsconfig values
Apply defaults to the options last
Support for spaces in project filepath:
Remove double quotes from project file path when present

* Upgrade yargs from 11.1.1 to 12.0.1, reason:
Missing default for a boolean arg is read as default: false
This seems to be fixed in 12.0.1.

* Test for verifying a mixed configuration that consists of:
Command line args, tsconfig.json, and TSTL defaults is parsed properly.

* More sensible test paths
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -42,12 +42,12 @@
     },
     "dependencies": {
         "typescript": "^2.9.2",
-        "yargs": "^11.1.0"
+        "yargs": "^12.0.1"
     },
     "devDependencies": {
         "@types/glob": "^5.0.35",
         "@types/node": "^9.6.23",
-        "@types/yargs": "^11.0.0",
+        "@types/yargs": "^11.1.1",
         "alsatian": "^2.2.1",
         "circular-json": "^0.5.5",
         "codecov": "3.0.2",
diff --git a/src/CommandLineParser.ts b/src/CommandLineParser.ts
@@ -17,7 +17,11 @@ export class CLIError extends Error {
 
 }
 
-const optionDeclarations: { [key: string]: yargs.Options } = {
+export interface YargsOptions {
+    [key: string]: yargs.Options;
+}
+
+export const optionDeclarations: YargsOptions = {
     luaLibImport: {
         choices: ["inline", "require", "none"],
         default: "inline",
@@ -38,11 +42,33 @@ const optionDeclarations: { [key: string]: yargs.Options } = {
     },
 };
 
+/**
+ * Removes defaults from the arguments.
+ * Returns a tuple where [0] is a copy of the options without defaults and [1] is the extracted defaults.
+ */
+function getYargOptionsWithoutDefaults(options: YargsOptions): [YargsOptions, yargs.Arguments] {
+    // options is a deep object, Object.assign or {...options} still keeps the referece
+    const copy = JSON.parse(JSON.stringify(options));
+
+    const optionDefaults: yargs.Arguments = {_: null, $0: null};
+    for (const optionName in copy) {
+        const section = copy[optionName];
+
+        optionDefaults[optionName] = section.default;
+        delete section.default;
+    }
+
+    return [copy, optionDefaults];
+}
+
 /**
  * Pares the supplied arguments.
  * The result will include arguments supplied via CLI and arguments from tsconfig.
  */
 export function parseCommandLine(args: string[]): ParsedCommandLine {
+    // Get a copy of the options without defaults to prevent defaults overriding project config
+    const [tstlOptions, tstlDefaults] = getYargOptionsWithoutDefaults(optionDeclarations);
+
     const parsedArgs = yargs
         .usage("Syntax: tstl [options] [files...]\n\n" +
             "In addition to the options listed below you can also pass options" +
@@ -51,7 +77,7 @@ export function parseCommandLine(args: string[]): ParsedCommandLine {
         .example("tstl path/to/file.ts [...]", "Compile files")
         .example("tstl -p path/to/tsconfig.json", "Compile project")
         .wrap(yargs.terminalWidth())
-        .options(optionDeclarations)
+        .options(tstlOptions)
         .fail((msg, err) => {
             throw new CLIError(msg);
         })
@@ -82,6 +108,9 @@ export function parseCommandLine(args: string[]): ParsedCommandLine {
     // Add TSTL options from tsconfig
     addTSTLOptions(commandLine);
 
+    // Add TSTL defaults last
+    addTSTLOptions(commandLine, tstlDefaults);
+
     // Run diagnostics again to check for errors in tsconfig
     runDiagnostics(commandLine);
 
@@ -158,13 +187,15 @@ export function findConfigFile(commandLine: ts.ParsedCommandLine): void {
     if (!commandLine.options.project) {
         throw new CLIError(`error no base path provided, could not find config.`);
     }
-    let configPath;
-    /* istanbul ignore else: Testing else part is not really possible via automated tests */
-    if (path.isAbsolute(commandLine.options.project)) {
-        configPath = commandLine.options.project;
-    } else {
+    let configPath = commandLine.options.project;
+    // If the project path is wrapped in double quotes, remove them
+    if (/^".*"$/.test(configPath)) {
+        configPath = configPath.substring(1, configPath.length - 1);
+    }
+    /* istanbul ignore if: Testing else part is not really possible via automated tests */
+    if (!path.isAbsolute(configPath)) {
         // TODO check if commandLine.options.project can even contain non absolute paths
-        configPath = path.join(process.cwd(), commandLine.options.project);
+        configPath = path.join(process.cwd(), configPath);
     }
     if (fs.statSync(configPath).isDirectory()) {
         configPath = path.join(configPath, "tsconfig.json");
diff --git a/test/unit/compiler/configuration/mixed/index.spec.ts b/test/unit/compiler/configuration/mixed/index.spec.ts
@@ -0,0 +1,40 @@
+import { Expect, Test, TestCase, Teardown } from "alsatian";
+import * as path from 'path';
+import * as fs from 'fs';
+import * as ts from "typescript";
+
+import { CompilerOptions, findConfigFile, parseCommandLine, ParsedCommandLine, optionDeclarations } from "../../../../../src/CommandLineParser";
+import { LuaLibImportKind } from "../../../../../src/Transpiler";
+
+export class MixedConfigurationTests {
+
+    @Test("tsconfig.json mixed with cmd line args")
+    public tsconfigMixedWithCmdLineArgs() {
+        const rootPath = __dirname;
+        const tsConfigPath = path.join(rootPath, "project-tsconfig.json");
+        const expectedTsConfig = ts.parseJsonConfigFileContent(
+            ts.parseConfigFileTextToJson(tsConfigPath, fs.readFileSync(tsConfigPath).toString()).config,
+            ts.sys,
+            path.dirname(tsConfigPath)
+        );        
+
+        const parsedArgs = parseCommandLine([
+            "-p",
+            `"${tsConfigPath}"`,
+            "--luaLibImport",
+            LuaLibImportKind.Inline,
+            `${path.join(rootPath, 'test.ts')}`,
+        ]);
+        
+        Expect(parsedArgs.options).toEqual(<CompilerOptions>{
+            ...expectedTsConfig.options,
+            // Overridden by cmd args (set to "none" in project-tsconfig.json)
+            luaLibImport: LuaLibImportKind.Inline,
+            // Only set in tsconfig, TSTL default is "JIT"
+            luaTarget: "5.1",
+            // Only present in TSTL dfaults
+            noHeader: optionDeclarations["noHeader"].default,
+            project: tsConfigPath
+        });
+    }
+}
diff --git a/test/unit/compiler/configuration/mixed/project-tsconfig.json b/test/unit/compiler/configuration/mixed/project-tsconfig.json
@@ -0,0 +1,8 @@
+{
+    "compilerOptions": {
+        "outDir": "./dist/foo/bar",
+        "rootDir": "./src/foo/bar"
+    },
+    "luaTarget": "5.1",
+    "luaLibImport": "none"
+}
