From 30917e62e2455dc0e1e51c945fc221b60267473c Mon Sep 17 00:00:00 2001 From: Janne Date: Mon, 30 Jul 2018 20:39:50 +0300 Subject: [PATCH 1/4] 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 --- src/CommandLineParser.ts | 47 +++++++++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/src/CommandLineParser.ts b/src/CommandLineParser.ts index c6dfb11fc..7fac341a9 100644 --- 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"); From 3de869ef57c872b76a198c63989179b174ca6f35 Mon Sep 17 00:00:00 2001 From: Janne Date: Mon, 30 Jul 2018 20:46:05 +0300 Subject: [PATCH 2/4] 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. --- package-lock.json | 96 +++++++++++++++++++++++++---------------------- package.json | 4 +- 2 files changed, 54 insertions(+), 46 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7dd5fda20..fa82ad478 100644 --- a/package-lock.json +++ b/package-lock.json @@ -314,9 +314,12 @@ } }, "decamelize": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/decamelize/-/decamelize-1.2.0.tgz", - "integrity": "sha1-9lNNFRSCabIDUue+4m9QH5oZEpA=" + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/decamelize/-/decamelize-2.0.0.tgz", + "integrity": "sha512-Ikpp5scV3MSYxY39ymh45ZLEecsTdv/Xj2CaQfI8RLMuwi7XvjX9H/fhraiSuU+C5w5NTDu4ZU72xNiZnurBPg==", + "requires": { + "xregexp": "4.0.0" + } }, "deep-equal": { "version": "1.0.1", @@ -446,11 +449,11 @@ } }, "find-up": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/find-up/-/find-up-2.1.0.tgz", - "integrity": "sha1-RdG35QbHF93UgndaK3eSCjwMV6c=", + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/find-up/-/find-up-3.0.0.tgz", + "integrity": "sha512-1yD6RmLI1XBfxugvORwlck6f75tYL+iR0jqwsOrOxMZyGYqUuDhJ0l4AXdO1iX/FTs9cBAMEk1gWSEx1kSbylg==", "requires": { - "locate-path": "^2.0.0" + "locate-path": "^3.0.0" } }, "forever-agent": { @@ -477,9 +480,9 @@ "dev": true }, "get-caller-file": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/get-caller-file/-/get-caller-file-1.0.2.tgz", - "integrity": "sha1-9wLmMSfn4jHBYKgMFVSstw1QR+U=" + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/get-caller-file/-/get-caller-file-1.0.3.tgz", + "integrity": "sha512-3t6rVToeoZfYSGd8YoLFR2DJkiQrIiUrGcjvFX2mDw3bn6k2OtwHN0TNCLbBO+w8qTvimhDkv+LSscbJY1vE6w==" }, "get-stream": { "version": "3.0.0", @@ -667,18 +670,18 @@ } }, "locate-path": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/locate-path/-/locate-path-2.0.0.tgz", - "integrity": "sha1-K1aLJl7slExtnA3pw9u7ygNUzY4=", + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/locate-path/-/locate-path-3.0.0.tgz", + "integrity": "sha512-7AO748wWnIhNqAuaty2ZWHkQHRSNfPVIsPIfwEOWO22AmaoVrWavlOcMR5nzTLNYvp36X220/maaRsrec1G65A==", "requires": { - "p-locate": "^2.0.0", + "p-locate": "^3.0.0", "path-exists": "^3.0.0" } }, "lru-cache": { - "version": "4.1.2", - "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-4.1.2.tgz", - "integrity": "sha512-wgeVXhrDwAWnIF/yZARsFnMBtdFXOg1b8RIrhilp+0iDYN4mdQcNZElDZ0e4B64BhaxeQ5zN7PMyvu7we1kPeQ==", + "version": "4.1.3", + "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-4.1.3.tgz", + "integrity": "sha512-fFEhvcgzuIoJVUF8fYr5KR0YqxD238zgObTps31YdADwPPAp82a4M8TrckkWyx7ekNlf9aBcVn81cFwwXngrJA==", "requires": { "pseudomap": "^1.0.2", "yallist": "^2.1.2" @@ -3436,25 +3439,25 @@ "integrity": "sha1-P7z7FbiZpEEjs0ttzBi3JDNqLK4=" }, "p-limit": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-1.2.0.tgz", - "integrity": "sha512-Y/OtIaXtUPr4/YpMv1pCL5L5ed0rumAaAeBSj12F+bSlMdys7i8oQF/GUJmfpTS/QoaRrS/k6pma29haJpsMng==", + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-2.0.0.tgz", + "integrity": "sha512-fl5s52lI5ahKCernzzIyAP0QAZbGIovtVHGwpcu1Jr/EpzLVDI2myISHwGqK7m8uQFugVWSrbxH7XnhGtvEc+A==", "requires": { - "p-try": "^1.0.0" + "p-try": "^2.0.0" } }, "p-locate": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/p-locate/-/p-locate-2.0.0.tgz", - "integrity": "sha1-IKAQOyIqcMj9OcwuWAaA893l7EM=", + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/p-locate/-/p-locate-3.0.0.tgz", + "integrity": "sha512-x+12w/To+4GFfgJhBEpiDcLozRJGegY+Ei7/z0tSLkMmxGZNybVMSfWj9aJn8Z5Fc7dBUNJOOVgPv2H7IwulSQ==", "requires": { - "p-limit": "^1.1.0" + "p-limit": "^2.0.0" } }, "p-try": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/p-try/-/p-try-1.0.0.tgz", - "integrity": "sha1-y8ec26+P1CKOE/Yh8rGiN8GyB7M=" + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/p-try/-/p-try-2.0.0.tgz", + "integrity": "sha512-hMp0onDKIajHfIkdRk3P4CdCmErkYAxxDtP3Wx/4nZ3aGlau2VKh3mZpcuFkH27WQkL/3WBCPOktzA9ZOAnMQQ==" }, "path-exists": { "version": "3.0.0", @@ -3914,9 +3917,9 @@ } }, "which": { - "version": "1.3.0", - "resolved": "https://registry.npmjs.org/which/-/which-1.3.0.tgz", - "integrity": "sha512-xcJpopdamTuY5duC/KnTTNBraPK54YwpenP4lzxU8H91GudWpFv38u0CKjclE1Wi2EH2EDz5LRcHcKbCIzqGyg==", + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/which/-/which-1.3.1.tgz", + "integrity": "sha512-HxJdYWq1MTIQbJ3nw0cqssHoTNU267KlrDuGZ1WYlxDStUtKUhOaJmh112/TZmHxxUfuJqPXSOm7tDyas0OSIQ==", "requires": { "isexe": "^2.0.0" } @@ -3961,6 +3964,11 @@ "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=", "dev": true }, + "xregexp": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/xregexp/-/xregexp-4.0.0.tgz", + "integrity": "sha512-PHyM+sQouu7xspQQwELlGwwd05mXUFqwFYfqPO0cC7x4fxyHnnuetmQr6CjJiafIDoH4MogHb9dOoJzR/Y4rFg==" + }, "xtend": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/xtend/-/xtend-4.0.1.tgz", @@ -3968,9 +3976,9 @@ "dev": true }, "y18n": { - "version": "3.2.1", - "resolved": "https://registry.npmjs.org/y18n/-/y18n-3.2.1.tgz", - "integrity": "sha1-bRX7qITAhnnA136I53WegR4H+kE=" + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/y18n/-/y18n-4.0.0.tgz", + "integrity": "sha512-r9S/ZyXu/Xu9q1tYlpsLIsa3EeLXXk0VwlxqTcFRfg9EhMW+17kbt9G0NrgCmhGb5vT2hyhJZLfDGx+7+5Uj/w==" }, "yallist": { "version": "2.1.2", @@ -3978,13 +3986,13 @@ "integrity": "sha1-HBH5IY8HYImkfdUS+TxmmaaoHVI=" }, "yargs": { - "version": "11.1.0", - "resolved": "https://registry.npmjs.org/yargs/-/yargs-11.1.0.tgz", - "integrity": "sha512-NwW69J42EsCSanF8kyn5upxvjp5ds+t3+udGBeTbFnERA+lF541DDpMawzo4z6W/QrzNM18D+BPMiOBibnFV5A==", + "version": "12.0.1", + "resolved": "https://registry.npmjs.org/yargs/-/yargs-12.0.1.tgz", + "integrity": "sha512-B0vRAp1hRX4jgIOWFtjfNjd9OA9RWYZ6tqGA9/I/IrTMsxmKvtWy+ersM+jzpQqbC3YfLzeABPdeTgcJ9eu1qQ==", "requires": { "cliui": "^4.0.0", - "decamelize": "^1.1.1", - "find-up": "^2.1.0", + "decamelize": "^2.0.0", + "find-up": "^3.0.0", "get-caller-file": "^1.0.1", "os-locale": "^2.0.0", "require-directory": "^2.1.1", @@ -3992,14 +4000,14 @@ "set-blocking": "^2.0.0", "string-width": "^2.0.0", "which-module": "^2.0.0", - "y18n": "^3.2.1", - "yargs-parser": "^9.0.2" + "y18n": "^3.2.1 || ^4.0.0", + "yargs-parser": "^10.1.0" } }, "yargs-parser": { - "version": "9.0.2", - "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-9.0.2.tgz", - "integrity": "sha1-nM9qQ0YP5O1Aqbto9I1DuKaMwHc=", + "version": "10.1.0", + "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-10.1.0.tgz", + "integrity": "sha512-VCIyR1wJoEBZUqk5PA+oOBF6ypbwh5aNB3I50guxAL/quggdfs4TtNHQrSazFA3fYZ+tEqfs0zIGlv0c/rgjbQ==", "requires": { "camelcase": "^4.1.0" } diff --git a/package.json b/package.json index c4f9a5398..ace7fc76c 100644 --- 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", From 68f331435e33a97df7806e409e3c02f4a1762010 Mon Sep 17 00:00:00 2001 From: Janne Date: Mon, 30 Jul 2018 20:47:46 +0300 Subject: [PATCH 3/4] Test for verifying a mixed configuration that consists of: Command line args, tsconfig.json, and TSTL defaults is parsed properly. --- .../configuration/mixed/index.spec.ts | 40 +++++++++++++++++++ .../configuration/mixed/project-tsconfig.json | 8 ++++ 2 files changed, 48 insertions(+) create mode 100644 test/unit/compiler/configuration/mixed/index.spec.ts create mode 100644 test/unit/compiler/configuration/mixed/project-tsconfig.json diff --git a/test/unit/compiler/configuration/mixed/index.spec.ts b/test/unit/compiler/configuration/mixed/index.spec.ts new file mode 100644 index 000000000..71c2c40ee --- /dev/null +++ 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({ + ...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 + }); + } +} \ No newline at end of file diff --git a/test/unit/compiler/configuration/mixed/project-tsconfig.json b/test/unit/compiler/configuration/mixed/project-tsconfig.json new file mode 100644 index 000000000..cb619a69d --- /dev/null +++ b/test/unit/compiler/configuration/mixed/project-tsconfig.json @@ -0,0 +1,8 @@ +{ + "compilerOptions": { + "outDir": "../../dist/test", + "rootDir": "./" + }, + "luaTarget": "5.1", + "luaLibImport": "none" +} From e146846545f0dfb094ae551ddc292bcd7f6c28d2 Mon Sep 17 00:00:00 2001 From: Janne Date: Mon, 30 Jul 2018 21:07:19 +0300 Subject: [PATCH 4/4] More sensible test paths --- test/unit/compiler/configuration/mixed/project-tsconfig.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/compiler/configuration/mixed/project-tsconfig.json b/test/unit/compiler/configuration/mixed/project-tsconfig.json index cb619a69d..3fe0b7e03 100644 --- a/test/unit/compiler/configuration/mixed/project-tsconfig.json +++ b/test/unit/compiler/configuration/mixed/project-tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { - "outDir": "../../dist/test", - "rootDir": "./" + "outDir": "./dist/foo/bar", + "rootDir": "./src/foo/bar" }, "luaTarget": "5.1", "luaLibImport": "none"