Skip to content

Commit f3b60f4

Browse files
novemberbornsindresorhus
novemberborn
authored and
sindresorhus
committed
Code excerpt fixes (#1271)
* Resolve absolute source file path when serializing errors Fixes #1270. * Bail excerpting code if file cannot be read * temp-write@^3.1.0 * Restrict code excerpts to tests/helpers/sources Don't show code excerpts for dependencies or code outside the project directory. Determine where the source lives when serializing the error. Then, when excerpting the code, bail if it lives outside the project or is a dependency.
1 parent 8d6f9bc commit f3b60f4

File tree

10 files changed

+207
-56
lines changed

10 files changed

+207
-56
lines changed

‎lib/cli.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,9 @@ exports.run = () => {
129129
if (cli.flags.tap && !cli.flags.watch) {
130130
reporter = new TapReporter();
131131
} else if (cli.flags.verbose || isCi) {
132-
reporter = new VerboseReporter({color: cli.flags.color, basePath: projectDir});
132+
reporter = new VerboseReporter({color: cli.flags.color});
133133
} else {
134-
reporter = new MiniReporter({color: cli.flags.color, watching: cli.flags.watch, basePath: projectDir});
134+
reporter = new MiniReporter({color: cli.flags.color, watching: cli.flags.watch});
135135
}
136136

137137
reporter.api = api;

‎lib/code-excerpt.js

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,25 @@ const chalk = require('chalk');
88
const formatLineNumber = (lineNumber, maxLineNumber) =>
99
' '.repeat(Math.max(0, String(maxLineNumber).length - String(lineNumber).length)) + lineNumber;
1010

11-
module.exports = (file, line, options) => {
12-
options = options || {};
11+
module.exports = (source, options) => {
12+
if (!source.isWithinProject || source.isDependency) {
13+
return null;
14+
}
1315

16+
const file = source.file;
17+
const line = source.line;
18+
19+
options = options || {};
1420
const maxWidth = options.maxWidth || 80;
15-
const source = fs.readFileSync(file, 'utf8');
16-
const excerpt = codeExcerpt(source, line, {around: 1});
21+
22+
let contents;
23+
try {
24+
contents = fs.readFileSync(file, 'utf8');
25+
} catch (err) {
26+
return null;
27+
}
28+
29+
const excerpt = codeExcerpt(contents, line, {around: 1});
1730
if (!excerpt) {
1831
return null;
1932
}

‎lib/reporters/mini.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
'use strict';
22
const StringDecoder = require('string_decoder').StringDecoder;
3-
const path = require('path');
43
const cliCursor = require('cli-cursor');
54
const lastLineTracker = require('last-line-stream/tracker');
65
const plur = require('plur');
@@ -187,8 +186,7 @@ class MiniReporter {
187186
if (test.error.source) {
188187
status += ' ' + colors.errorSource(test.error.source.file + ':' + test.error.source.line) + '\n';
189188

190-
const errorPath = path.join(this.options.basePath, test.error.source.file);
191-
const excerpt = codeExcerpt(errorPath, test.error.source.line, {maxWidth: process.stdout.columns});
189+
const excerpt = codeExcerpt(test.error.source, {maxWidth: process.stdout.columns});
192190
if (excerpt) {
193191
status += '\n' + indentString(excerpt, 2) + '\n';
194192
}

‎lib/reporters/verbose.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
'use strict';
2-
const path = require('path');
32
const indentString = require('indent-string');
43
const prettyMs = require('pretty-ms');
54
const figures = require('figures');
@@ -106,8 +105,7 @@ class VerboseReporter {
106105
if (test.error.source) {
107106
output += ' ' + colors.errorSource(test.error.source.file + ':' + test.error.source.line) + '\n';
108107

109-
const errorPath = path.join(this.options.basePath, test.error.source.file);
110-
const excerpt = codeExcerpt(errorPath, test.error.source.line, {maxWidth: process.stdout.columns});
108+
const excerpt = codeExcerpt(test.error.source, {maxWidth: process.stdout.columns});
111109
if (excerpt) {
112110
output += '\n' + indentString(excerpt, 2) + '\n';
113111
}

‎lib/serialize-error.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
'use strict';
2+
const path = require('path');
23
const cleanYamlObject = require('clean-yaml-object');
34
const StackUtils = require('stack-utils');
45
const prettyFormat = require('@ava/pretty-format');
@@ -58,8 +59,21 @@ module.exports = error => {
5859
const firstStackLine = extractStack(err.stack).split('\n')[0];
5960
const source = stackUtils.parseLine(firstStackLine);
6061
if (source) {
62+
// Assume the CWD is the project directory. This holds since this function
63+
// is only called in test workers, which are created with their working
64+
// directory set to the project directory.
65+
const projectDir = process.cwd();
66+
67+
const file = path.resolve(projectDir, source.file.trim());
68+
const rel = path.relative(projectDir, file);
69+
70+
const isWithinProject = rel.split(path.sep)[0] !== '..';
71+
const isDependency = isWithinProject && path.dirname(rel).split(path.sep).indexOf('node_modules') > -1;
72+
6173
err.source = {
62-
file: source.file.trim(),
74+
isDependency,
75+
isWithinProject,
76+
file,
6377
line: source.line
6478
};
6579
}

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@
183183
"sinon": "^1.17.2",
184184
"source-map-fixtures": "^2.1.0",
185185
"tap": "^10.0.0",
186-
"temp-write": "^3.0.0",
186+
"temp-write": "^3.1.0",
187187
"touch": "^1.0.0",
188188
"xo": "^0.17.0",
189189
"zen-observable": "^0.4.0"

‎test/code-excerpt.js

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
'use strict';
2+
const fs = require('fs');
23
const tempWrite = require('temp-write');
34
const chalk = require('chalk');
45
const test = require('tap').test;
@@ -7,13 +8,13 @@ const codeExcerpt = require('../lib/code-excerpt');
78
chalk.enabled = true;
89

910
test('read code excerpt', t => {
10-
const path = tempWrite.sync([
11+
const file = tempWrite.sync([
1112
'function a() {',
1213
'\talert();',
1314
'}'
1415
].join('\n'));
1516

16-
const excerpt = codeExcerpt(path, 2);
17+
const excerpt = codeExcerpt({file, line: 2, isWithinProject: true, isDependency: false});
1718
const expected = [
1819
` ${chalk.grey('1:')} function a() {`,
1920
chalk.bgRed(` 2: alert(); `),
@@ -25,13 +26,13 @@ test('read code excerpt', t => {
2526
});
2627

2728
test('truncate lines', t => {
28-
const path = tempWrite.sync([
29+
const file = tempWrite.sync([
2930
'function a() {',
3031
'\talert();',
3132
'}'
3233
].join('\n'));
3334

34-
const excerpt = codeExcerpt(path, 2, {maxWidth: 14});
35+
const excerpt = codeExcerpt({file, line: 2, isWithinProject: true, isDependency: false}, {maxWidth: 14});
3536
const expected = [
3637
` ${chalk.grey('1:')} functio…`,
3738
chalk.bgRed(` 2: alert…`),
@@ -43,14 +44,14 @@ test('truncate lines', t => {
4344
});
4445

4546
test('format line numbers', t => {
46-
const path = tempWrite.sync([
47+
const file = tempWrite.sync([
4748
'', '', '', '', '', '', '', '',
4849
'function a() {',
4950
'\talert();',
5051
'}'
5152
].join('\n'));
5253

53-
const excerpt = codeExcerpt(path, 10);
54+
const excerpt = codeExcerpt({file, line: 10, isWithinProject: true, isDependency: false});
5455
const expected = [
5556
` ${chalk.grey(' 9:')} function a() {`,
5657
chalk.bgRed(` 10: alert(); `),
@@ -60,3 +61,24 @@ test('format line numbers', t => {
6061
t.is(excerpt, expected);
6162
t.end();
6263
});
64+
65+
test('noop if file cannot be read', t => {
66+
const file = tempWrite.sync('');
67+
fs.unlinkSync(file);
68+
69+
const excerpt = codeExcerpt({file, line: 10, isWithinProject: true, isDependency: false});
70+
t.is(excerpt, null);
71+
t.end();
72+
});
73+
74+
test('noop if file is not within project', t => {
75+
const excerpt = codeExcerpt({isWithinProject: false, file: __filename, line: 1});
76+
t.is(excerpt, null);
77+
t.end();
78+
});
79+
80+
test('noop if file is a dependency', t => {
81+
const excerpt = codeExcerpt({isWithinProject: true, isDependency: true, file: __filename, line: 1});
82+
t.is(excerpt, null);
83+
t.end();
84+
});

‎test/reporters/mini.js

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
'use strict';
2-
const path = require('path');
32
const indentString = require('indent-string');
43
const tempWrite = require('temp-write');
54
const flatten = require('arr-flatten');
@@ -35,6 +34,15 @@ function miniReporter(options) {
3534
return reporter;
3635
}
3736

37+
function source(file, line) {
38+
return {
39+
file,
40+
line: line || 1,
41+
isWithinProject: true,
42+
isDependency: false
43+
};
44+
}
45+
3846
process.stderr.setMaxListeners(50);
3947

4048
test('start', t => {
@@ -354,7 +362,7 @@ test('results with errors', t => {
354362
const err1 = new Error('failure one');
355363
err1.stack = beautifyStack(err1.stack);
356364
const err1Path = tempWrite.sync('a();');
357-
err1.source = {file: path.basename(err1Path), line: 1};
365+
err1.source = source(err1Path);
358366
err1.showOutput = true;
359367
err1.actual = JSON.stringify('abc');
360368
err1.actualType = 'string';
@@ -364,14 +372,14 @@ test('results with errors', t => {
364372
const err2 = new Error('failure two');
365373
err2.stack = 'error message\nTest.fn (test.js:1:1)\n';
366374
const err2Path = tempWrite.sync('b();');
367-
err2.source = {file: path.basename(err2Path), line: 1};
375+
err2.source = source(err2Path);
368376
err2.showOutput = true;
369377
err2.actual = JSON.stringify([1]);
370378
err2.actualType = 'array';
371379
err2.expected = JSON.stringify([2]);
372380
err2.expectedType = 'array';
373381

374-
const reporter = miniReporter({basePath: path.dirname(err1Path)});
382+
const reporter = miniReporter();
375383
reporter.failCount = 1;
376384

377385
const runStatus = {
@@ -393,7 +401,7 @@ test('results with errors', t => {
393401
' ' + chalk.bold.white('failed one'),
394402
' ' + chalk.grey(`${err1.source.file}:${err1.source.line}`),
395403
'',
396-
indentString(codeExcerpt(err1Path, err1.source.line), 2).split('\n'),
404+
indentString(codeExcerpt(err1.source), 2).split('\n'),
397405
'',
398406
indentString(formatAssertError(err1), 2).split('\n'),
399407
/failure one/,
@@ -406,7 +414,7 @@ test('results with errors', t => {
406414
' ' + chalk.bold.white('failed two'),
407415
' ' + chalk.grey(`${err2.source.file}:${err2.source.line}`),
408416
'',
409-
indentString(codeExcerpt(err2Path, err2.source.line), 2).split('\n'),
417+
indentString(codeExcerpt(err2.source), 2).split('\n'),
410418
'',
411419
indentString(formatAssertError(err2), 2).split('\n'),
412420
/failure two/
@@ -426,14 +434,14 @@ test('results with errors and disabled code excerpts', t => {
426434
const err2 = new Error('failure two');
427435
err2.stack = 'error message\nTest.fn (test.js:1:1)\n';
428436
const err2Path = tempWrite.sync('b();');
429-
err2.source = {file: path.basename(err2Path), line: 1};
437+
err2.source = source(err2Path);
430438
err2.showOutput = true;
431439
err2.actual = JSON.stringify([1]);
432440
err2.actualType = 'array';
433441
err2.expected = JSON.stringify([2]);
434442
err2.expectedType = 'array';
435443

436-
const reporter = miniReporter({color: true, basePath: path.dirname(err2Path)});
444+
const reporter = miniReporter({color: true});
437445
reporter.failCount = 1;
438446

439447
const runStatus = {
@@ -465,7 +473,7 @@ test('results with errors and disabled code excerpts', t => {
465473
' ' + chalk.bold.white('failed two'),
466474
' ' + chalk.grey(`${err2.source.file}:${err2.source.line}`),
467475
'',
468-
indentString(codeExcerpt(err2Path, err2.source.line), 2).split('\n'),
476+
indentString(codeExcerpt(err2.source), 2).split('\n'),
469477
'',
470478
indentString(formatAssertError(err2), 2).split('\n'),
471479
/failure two/
@@ -477,7 +485,7 @@ test('results with errors and broken code excerpts', t => {
477485
const err1 = new Error('failure one');
478486
err1.stack = beautifyStack(err1.stack);
479487
const err1Path = tempWrite.sync('a();');
480-
err1.source = {file: path.basename(err1Path), line: 10};
488+
err1.source = source(err1Path, 10);
481489
err1.showOutput = true;
482490
err1.actual = JSON.stringify('abc');
483491
err1.actualType = 'string';
@@ -487,14 +495,14 @@ test('results with errors and broken code excerpts', t => {
487495
const err2 = new Error('failure two');
488496
err2.stack = 'error message\nTest.fn (test.js:1:1)\n';
489497
const err2Path = tempWrite.sync('b();');
490-
err2.source = {file: path.basename(err2Path), line: 1};
498+
err2.source = source(err2Path);
491499
err2.showOutput = true;
492500
err2.actual = JSON.stringify([1]);
493501
err2.actualType = 'array';
494502
err2.expected = JSON.stringify([2]);
495503
err2.expectedType = 'array';
496504

497-
const reporter = miniReporter({color: true, basePath: path.dirname(err2Path)});
505+
const reporter = miniReporter({color: true});
498506
reporter.failCount = 1;
499507

500508
const runStatus = {
@@ -527,7 +535,7 @@ test('results with errors and broken code excerpts', t => {
527535
' ' + chalk.bold.white('failed two'),
528536
' ' + chalk.grey(`${err2.source.file}:${err2.source.line}`),
529537
'',
530-
indentString(codeExcerpt(err2Path, err2.source.line), 2).split('\n'),
538+
indentString(codeExcerpt(err2.source), 2).split('\n'),
531539
'',
532540
indentString(formatAssertError(err2), 2).split('\n'),
533541
/failure two/
@@ -539,7 +547,7 @@ test('results with errors and disabled assert output', t => {
539547
const err1 = new Error('failure one');
540548
err1.stack = beautifyStack(err1.stack);
541549
const err1Path = tempWrite.sync('a();');
542-
err1.source = {file: path.basename(err1Path), line: 1};
550+
err1.source = source(err1Path);
543551
err1.showOutput = false;
544552
err1.actual = JSON.stringify('abc');
545553
err1.actualType = 'string';
@@ -549,14 +557,14 @@ test('results with errors and disabled assert output', t => {
549557
const err2 = new Error('failure two');
550558
err2.stack = 'error message\nTest.fn (test.js:1:1)\n';
551559
const err2Path = tempWrite.sync('b();');
552-
err2.source = {file: path.basename(err2Path), line: 1};
560+
err2.source = source(err2Path);
553561
err2.showOutput = true;
554562
err2.actual = JSON.stringify([1]);
555563
err2.actualType = 'array';
556564
err2.expected = JSON.stringify([2]);
557565
err2.expectedType = 'array';
558566

559-
const reporter = miniReporter({color: true, basePath: path.dirname(err1Path)});
567+
const reporter = miniReporter({color: true});
560568
reporter.failCount = 1;
561569

562570
const runStatus = {
@@ -578,7 +586,7 @@ test('results with errors and disabled assert output', t => {
578586
' ' + chalk.bold.white('failed one'),
579587
' ' + chalk.grey(`${err1.source.file}:${err1.source.line}`),
580588
'',
581-
indentString(codeExcerpt(err1Path, err1.source.line), 2).split('\n'),
589+
indentString(codeExcerpt(err1.source), 2).split('\n'),
582590
'',
583591
/failure one/,
584592
'',
@@ -590,7 +598,7 @@ test('results with errors and disabled assert output', t => {
590598
' ' + chalk.bold.white('failed two'),
591599
' ' + chalk.grey(`${err2.source.file}:${err2.source.line}`),
592600
'',
593-
indentString(codeExcerpt(err2Path, err2.source.line), 2).split('\n'),
601+
indentString(codeExcerpt(err2.source), 2).split('\n'),
594602
'',
595603
indentString(formatAssertError(err2), 2).split('\n'),
596604
/failure two/

0 commit comments

Comments
 (0)