Skip to content

Commit

Permalink
lib: ensure --check flag works for piped-in code
Browse files Browse the repository at this point in the history
Previously, the --check CLI flag had no effect when run on code piped
from stdin. This commit updates the bootstrap logic to handle the
--check flag the same way regardless of whether the code is piped from
stdin.

PR-URL: #11689
Fixes: #11680
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
  • Loading branch information
not-an-aardvark committed Apr 4, 2017
1 parent 53828e8 commit 3209a8e
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 13 deletions.
30 changes: 20 additions & 10 deletions lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,18 +132,11 @@

// check if user passed `-c` or `--check` arguments to Node.
if (process._syntax_check_only != null) {
const vm = NativeModule.require('vm');
const fs = NativeModule.require('fs');
const internalModule = NativeModule.require('internal/module');
// read the source
const filename = Module._resolveFilename(process.argv[1]);
var source = fs.readFileSync(filename, 'utf-8');
// remove shebang and BOM
source = internalModule.stripBOM(source.replace(/^#!.*/, ''));
// wrap it
source = Module.wrap(source);
// compile the script, this will throw if it fails
new vm.Script(source, {filename: filename, displayErrors: true});
checkScriptSyntax(source, filename);
process.exit(0);
}

Expand Down Expand Up @@ -184,8 +177,12 @@
});

process.stdin.on('end', function() {
process._eval = code;
evalScript('[stdin]');
if (process._syntax_check_only != null) {
checkScriptSyntax(code, '[stdin]');
} else {
process._eval = code;
evalScript('[stdin]');
}
});
}
}
Expand Down Expand Up @@ -445,6 +442,19 @@
}
}

function checkScriptSyntax(source, filename) {
const Module = NativeModule.require('module');
const vm = NativeModule.require('vm');
const internalModule = NativeModule.require('internal/module');

// remove shebang and BOM
source = internalModule.stripBOM(source.replace(/^#!.*/, ''));
// wrap it
source = Module.wrap(source);
// compile the script, this will throw if it fails
new vm.Script(source, {displayErrors: true, filename});
}

// Below you find a minimal module system, which is used to load the node
// core modules found in lib/*.js. All core modules are compiled into the
// node binary, so they can be loaded faster.
Expand Down
41 changes: 38 additions & 3 deletions test/parallel/test-cli-syntax.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const syntaxArgs = [
// no output should be produced
assert.strictEqual(c.stdout, '', 'stdout produced');
assert.strictEqual(c.stderr, '', 'stderr produced');
assert.strictEqual(c.status, 0, 'code == ' + c.status);
assert.strictEqual(c.status, 0, 'code === ' + c.status);
});
});

Expand All @@ -52,11 +52,14 @@ const syntaxArgs = [
// no stdout should be produced
assert.strictEqual(c.stdout, '', 'stdout produced');

// stderr should include the filename
assert(c.stderr.startsWith(file), "stderr doesn't start with the filename");

// stderr should have a syntax error message
const match = c.stderr.match(/^SyntaxError: Unexpected identifier$/m);
assert(match, 'stderr incorrect');

assert.strictEqual(c.status, 1, 'code == ' + c.status);
assert.strictEqual(c.status, 1, 'code === ' + c.status);
});
});

Expand All @@ -79,6 +82,38 @@ const syntaxArgs = [
const match = c.stderr.match(/^Error: Cannot find module/m);
assert(match, 'stderr incorrect');

assert.strictEqual(c.status, 1, 'code == ' + c.status);
assert.strictEqual(c.status, 1, 'code === ' + c.status);
});
});

// should not execute code piped from stdin with --check
// loop each possible option, `-c` or `--check`
syntaxArgs.forEach(function(args) {
const stdin = 'throw new Error("should not get run");';
const c = spawnSync(node, args, {encoding: 'utf8', input: stdin});

// no stdout or stderr should be produced
assert.strictEqual(c.stdout, '', 'stdout produced');
assert.strictEqual(c.stderr, '', 'stderr produced');

assert.strictEqual(c.status, 0, 'code === ' + c.status);
});

// should should throw if code piped from stdin with --check has bad syntax
// loop each possible option, `-c` or `--check`
syntaxArgs.forEach(function(args) {
const stdin = 'var foo bar;';
const c = spawnSync(node, args, {encoding: 'utf8', input: stdin});

// stderr should include '[stdin]' as the filename
assert(c.stderr.startsWith('[stdin]'), "stderr doesn't start with [stdin]");

// no stdout or stderr should be produced
assert.strictEqual(c.stdout, '', 'stdout produced');

// stderr should have a syntax error message
const match = c.stderr.match(/^SyntaxError: Unexpected identifier$/m);
assert(match, 'stderr incorrect');

assert.strictEqual(c.status, 1, 'code === ' + c.status);
});

0 comments on commit 3209a8e

Please sign in to comment.