From fe90537ee305f1e3d475d4e204fc12b9c9eba70f Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 21 Jun 2022 14:57:14 +0200 Subject: [PATCH] feat(cli-repl): allow multiple --eval args MONGOSH-1194 - Allow `--eval` to be used multiple times on the command line. - Only print the result of the last `--eval` invocation. - Remove the special handling for `--eval` without an argument, since our args parser does not let us distinguish that case from no `--eval` being passed in the case of options that can occur multiple times. --- packages/arg-parser/src/cli-options.ts | 2 +- packages/cli-repl/src/arg-parser.spec.ts | 16 +++++++- packages/cli-repl/src/arg-parser.ts | 2 +- packages/cli-repl/src/cli-repl.spec.ts | 49 +++++++++++++++--------- packages/cli-repl/src/cli-repl.ts | 23 ++++++----- 5 files changed, 58 insertions(+), 34 deletions(-) diff --git a/packages/arg-parser/src/cli-options.ts b/packages/arg-parser/src/cli-options.ts index 14ef7212c3..318685a134 100644 --- a/packages/arg-parser/src/cli-options.ts +++ b/packages/arg-parser/src/cli-options.ts @@ -19,7 +19,7 @@ export interface CliOptions { csfleLibraryPath?: string; cryptSharedLibPath?: string; db?: string; - eval?: string; + eval?: string[]; gssapiServiceName?: string; sspiHostnameCanonicalization?: string; sspiRealmOverride?: string; diff --git a/packages/cli-repl/src/arg-parser.spec.ts b/packages/cli-repl/src/arg-parser.spec.ts index 72bb532dd4..29c12e294f 100644 --- a/packages/cli-repl/src/arg-parser.spec.ts +++ b/packages/cli-repl/src/arg-parser.spec.ts @@ -194,7 +194,7 @@ describe('arg-parser', () => { }); }); - context('when providing --eval', () => { + context('when providing --eval (single value)', () => { const argv = [ ...baseArgv, uri, '--eval', '1+1' ]; it('returns the URI in the object', () => { @@ -202,7 +202,19 @@ describe('arg-parser', () => { }); it('sets the eval value in the object', () => { - expect(parseCliArgs(argv).eval).to.equal('1+1'); + expect(parseCliArgs(argv).eval).to.deep.equal(['1+1']); + }); + }); + + context('when providing --eval (multiple values)', () => { + const argv = [ ...baseArgv, uri, '--eval', '1+1', '--eval', '2+2' ]; + + it('returns the URI in the object', () => { + expect(parseCliArgs(argv).connectionSpecifier).to.equal(uri); + }); + + it('sets the eval value in the object', () => { + expect(parseCliArgs(argv).eval).to.deep.equal(['1+1', '2+2']); }); }); diff --git a/packages/cli-repl/src/arg-parser.ts b/packages/cli-repl/src/arg-parser.ts index e647befa08..8dc8ccfa26 100644 --- a/packages/cli-repl/src/arg-parser.ts +++ b/packages/cli-repl/src/arg-parser.ts @@ -26,7 +26,6 @@ const OPTIONS = { 'csfleLibraryPath', 'cryptSharedLibPath', 'db', - 'eval', 'gssapiHostName', 'gssapiServiceName', 'sspiHostnameCanonicalization', @@ -76,6 +75,7 @@ const OPTIONS = { 'version' ], array: [ + 'eval', 'file' ], alias: { diff --git a/packages/cli-repl/src/cli-repl.spec.ts b/packages/cli-repl/src/cli-repl.spec.ts index a5c0f1d13b..a7708199f1 100644 --- a/packages/cli-repl/src/cli-repl.spec.ts +++ b/packages/cli-repl/src/cli-repl.spec.ts @@ -482,16 +482,16 @@ describe('CliRepl', () => { expect(output).not.to.include('uh oh'); }); - it('evaluates code passed through --eval', async() => { - cliReplOptions.shellCliOptions.eval = '"i am" + " being evaluated"'; + it('evaluates code passed through --eval (single argument)', async() => { + cliReplOptions.shellCliOptions.eval = ['"i am" + " being evaluated"']; cliRepl = new CliRepl(cliReplOptions); await startWithExpectedImmediateExit(cliRepl, ''); expect(output).to.include('i am being evaluated'); expect(exitCode).to.equal(0); }); - it('forwards the error if the script passed to --eval throws', async() => { - cliReplOptions.shellCliOptions.eval = 'throw new Error("oh no")'; + it('forwards the error if the script passed to --eval throws (single argument)', async() => { + cliReplOptions.shellCliOptions.eval = ['throw new Error("oh no")']; cliRepl = new CliRepl(cliReplOptions); try { await cliRepl.start('', {}); @@ -500,6 +500,27 @@ describe('CliRepl', () => { } expect(output).not.to.include('oh no'); }); + + it('evaluates code passed through --eval (multiple arguments)', async() => { + cliReplOptions.shellCliOptions.eval = ['X = "i am"; "asdfghjkl"', 'X + " being evaluated"']; + cliRepl = new CliRepl(cliReplOptions); + await startWithExpectedImmediateExit(cliRepl, ''); + expect(output).to.not.include('asdfghjkl'); + expect(output).to.include('i am being evaluated'); + expect(exitCode).to.equal(0); + }); + + it('forwards the error if the script passed to --eval throws (multiple arguments)', async() => { + cliReplOptions.shellCliOptions.eval = ['throw new Error("oh no")', 'asdfghjkl']; + cliRepl = new CliRepl(cliReplOptions); + try { + await cliRepl.start('', {}); + } catch (err: any) { + expect(err.message).to.include('oh no'); + } + expect(output).to.not.include('asdfghjkl'); + expect(output).not.to.include('oh no'); + }); }); context('with a global configuration file', () => { @@ -919,7 +940,7 @@ describe('CliRepl', () => { }); it('sends out telemetry data for command line scripts', async() => { - cliReplOptions.shellCliOptions.eval = 'db.hello()'; + cliReplOptions.shellCliOptions.eval = ['db.hello()']; cliRepl = new CliRepl(cliReplOptions); await startWithExpectedImmediateExit(cliRepl, await testServer.connectionString()); expect(requests).to.have.lengthOf(2); @@ -948,14 +969,14 @@ describe('CliRepl', () => { }); it('does not send out telemetry if the user only runs a script for disabling telemetry', async() => { - cliReplOptions.shellCliOptions.eval = 'disableTelemetry()'; + cliReplOptions.shellCliOptions.eval = ['disableTelemetry()']; cliRepl = new CliRepl(cliReplOptions); await startWithExpectedImmediateExit(cliRepl, await testServer.connectionString()); expect(requests).to.have.lengthOf(0); }); it('does not send out telemetry if the user runs a script for disabling telemetry and drops into the shell', async() => { - cliReplOptions.shellCliOptions.eval = 'disableTelemetry()'; + cliReplOptions.shellCliOptions.eval = ['disableTelemetry()']; cliReplOptions.shellCliOptions.shell = true; cliRepl = new CliRepl(cliReplOptions); await cliRepl.start(await testServer.connectionString(), {}); @@ -1052,7 +1073,7 @@ describe('CliRepl', () => { it('allows doing db ops (--eval variant)', async() => { const filename1 = path.resolve(__dirname, '..', 'test', 'fixtures', 'load', 'insertintotest.js'); - cliReplOptions.shellCliOptions.eval = await fs.readFile(filename1, 'utf8'); + cliReplOptions.shellCliOptions.eval = [await fs.readFile(filename1, 'utf8')]; cliRepl = new CliRepl(cliReplOptions); await startWithExpectedImmediateExit(cliRepl, await testServer.connectionString()); expect(output).to.match(/Inserted: ObjectId\("[a-z0-9]{24}"\)/); @@ -1107,17 +1128,9 @@ describe('CliRepl', () => { expect(exitCode).to.equal(0); }); - it('warns if --eval is passed an empty string', async() => { - cliReplOptions.shellCliOptions.eval = ''; - cliRepl = new CliRepl(cliReplOptions); - await startWithExpectedImmediateExit(cliRepl, await testServer.connectionString()); - expect(output).to.include('--eval requires an argument, but no argument was given'); - expect(exitCode).to.equal(0); - }); - it('isInteractive() is false for --eval without --shell', async() => { const filename1 = path.resolve(__dirname, '..', 'test', 'fixtures', 'load', 'printisinteractive.js'); - cliReplOptions.shellCliOptions.eval = await fs.readFile(filename1, 'utf8'); + cliReplOptions.shellCliOptions.eval = [await fs.readFile(filename1, 'utf8')]; cliRepl = new CliRepl(cliReplOptions); await startWithExpectedImmediateExit(cliRepl, await testServer.connectionString()); expect(output).to.match(/isInteractive=false/); @@ -1126,7 +1139,7 @@ describe('CliRepl', () => { it('isInteractive() is true for --eval with --shell', async() => { const filename1 = path.resolve(__dirname, '..', 'test', 'fixtures', 'load', 'printisinteractive.js'); - cliReplOptions.shellCliOptions.eval = await fs.readFile(filename1, 'utf8'); + cliReplOptions.shellCliOptions.eval = [await fs.readFile(filename1, 'utf8')]; cliReplOptions.shellCliOptions.shell = true; cliRepl = new CliRepl(cliReplOptions); await cliRepl.start(await testServer.connectionString(), {}); diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index 01b1075668..994b95ae3c 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -257,7 +257,8 @@ class CliRepl implements MongoshIOProvider { const initialized = await this.mongoshRepl.initialize(initialServiceProvider); const commandLineLoadFiles = this.cliOptions.fileNames ?? []; - const willExecuteCommandLineScripts = commandLineLoadFiles.length > 0 || this.cliOptions.eval !== undefined; + const evalScripts = this.cliOptions.eval ?? []; + const willExecuteCommandLineScripts = commandLineLoadFiles.length > 0 || evalScripts.length > 0; const willEnterInteractiveMode = !willExecuteCommandLineScripts || !!this.cliOptions.shell; let snippetManager: SnippetManager | undefined; @@ -280,7 +281,7 @@ class CliRepl implements MongoshIOProvider { if (willExecuteCommandLineScripts) { this.mongoshRepl.setIsInteractive(willEnterInteractiveMode); this.bus.emit('mongosh:start-loading-cli-scripts', { usesShellOption: !!this.cliOptions.shell }); - await this.loadCommandLineFilesAndEval(commandLineLoadFiles); + await this.loadCommandLineFilesAndEval(commandLineLoadFiles, evalScripts); if (!this.cliOptions.shell) { // We flush the telemetry data as part of exiting. Make sure we have // the right config value. @@ -335,18 +336,16 @@ class CliRepl implements MongoshIOProvider { } } - async loadCommandLineFilesAndEval(files: string[]) { - if (this.cliOptions.eval) { + async loadCommandLineFilesAndEval(files: string[], evalScripts: string[]) { + let lastEvalResult; + for (const script of evalScripts) { this.bus.emit('mongosh:eval-cli-script'); - const evalResult = await this.mongoshRepl.loadExternalCode(this.cliOptions.eval, '@(shell eval)'); - this.output.write(this.mongoshRepl.writer(evalResult) + '\n'); - } else if (this.cliOptions.eval === '') { - // This happens e.g. when --eval is followed by another option, for example - // when running `mongosh --eval --shell "eval script"`, which can happen - // if you're like me and sometimes insert options in the wrong place - const msg = 'Warning: --eval requires an argument, but no argument was given\n'; - this.output.write(this.clr(msg, 'mongosh:warning')); + lastEvalResult = await this.mongoshRepl.loadExternalCode(script, '@(shell eval)'); + } + if (lastEvalResult !== undefined) { + this.output.write(this.mongoshRepl.writer(lastEvalResult) + '\n'); } + for (const file of files) { if (!this.cliOptions.quiet) { this.output.write(`Loading file: ${this.clr(file, 'mongosh:filename')}\n`);