Skip to content

printError: fix padding of line numbers #1328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions src/error/__tests__/printError-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,37 @@ import { parse, Source } from '../../language';
import dedent from '../../jsutils/dedent';

describe('printError', () => {
it('prints an line numbers with correct padding', () => {
const singleDigit = new GraphQLError(
'Single digit line number with no padding',
null,
new Source('*', 'Test', { line: 9, column: 1 }),
[0],
);
expect(printError(singleDigit)).to.equal(dedent`
Single digit line number with no padding

Test (9:1)
9: *
^
`);

const doubleDigit = new GraphQLError(
'Left padded first line number',
null,
new Source('*\n', 'Test', { line: 9, column: 1 }),
[0],
);
expect(printError(doubleDigit)).to.equal(dedent`
Left padded first line number

Test (9:1)
9: *
^
10:
`);
});

it('prints an error with nodes from different sources', () => {
const sourceA = parse(
new Source(
Expand Down
51 changes: 31 additions & 20 deletions src/error/printError.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,29 +48,40 @@ function highlightSourceAtLocation(
source: Source,
location: SourceLocation,
): string {
const line = location.line;
const firstLineColumnOffset = source.locationOffset.column - 1;
const body = whitespace(firstLineColumnOffset) + source.body;

const lineIndex = location.line - 1;
const lineOffset = source.locationOffset.line - 1;
const columnOffset = getColumnOffset(source, location);
const contextLine = line + lineOffset;
const contextColumn = location.column + columnOffset;
const prevLineNum = (contextLine - 1).toString();
const lineNum = contextLine.toString();
const nextLineNum = (contextLine + 1).toString();
const padLen = nextLineNum.length;
const lines = source.body.split(/\r\n|[\n\r]/g);
lines[0] = whitespace(source.locationOffset.column - 1) + lines[0];
const outputLines = [
`${source.name} (${contextLine}:${contextColumn})`,
line >= 2 && lpad(padLen, prevLineNum) + ': ' + lines[line - 2],
lpad(padLen, lineNum) + ': ' + lines[line - 1],
whitespace(2 + padLen + contextColumn - 1) + '^',
line < lines.length && lpad(padLen, nextLineNum) + ': ' + lines[line],
];
return outputLines.filter(Boolean).join('\n');
const lineNum = location.line + lineOffset;

const columnOffset = location.line === 1 ? firstLineColumnOffset : 0;
const columnNum = location.column + columnOffset;

const lines = body.split(/\r\n|[\n\r]/g);
return (
`${source.name} (${lineNum}:${columnNum})\n` +
printPrefixedLines([
// Lines specified like this: ["prefix", "string"],
[`${lineNum - 1}: `, lines[lineIndex - 1]],
[`${lineNum}: `, lines[lineIndex]],
['', whitespace(columnNum - 1) + '^'],
[`${lineNum + 1}: `, lines[lineIndex + 1]],
])
);
}

function getColumnOffset(source: Source, location: SourceLocation): number {
return location.line === 1 ? source.locationOffset.column - 1 : 0;
function printPrefixedLines(lines: Array<[string, string]>): string {
const existingLines = lines.filter(([_, line]) => line !== undefined);

let padLen = 0;
for (const [prefix] of existingLines) {
padLen = Math.max(padLen, prefix.length);
}

return existingLines
.map(([prefix, line]) => lpad(padLen, prefix) + line)
.join('\n');
}

function whitespace(len: number): string {
Expand Down
64 changes: 33 additions & 31 deletions src/language/__tests__/lexer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import { expect } from 'chai';
import { describe, it } from 'mocha';
import dedent from '../../jsutils/dedent';
import { Source } from '../source';
import { createLexer, TokenKind } from '../lexer';

Expand Down Expand Up @@ -105,24 +106,25 @@ describe('Lexer', () => {
it('errors respect whitespace', () => {
let caughtError;
try {
lexOne(`

?


`);
lexOne(dedent`


?


`);
} catch (error) {
caughtError = error;
}
expect(String(caughtError)).to.equal(
'Syntax Error: Cannot parse the unexpected character "?".\n' +
'\n' +
'GraphQL request (3:5)\n' +
'2: \n' +
'3: ?\n' +
' ^\n' +
'4: \n',
);
expect(String(caughtError)).to.equal(dedent`
Syntax Error: Cannot parse the unexpected character "?".

GraphQL request (3:5)
2:
3: ?
^
4:
`);
});

it('updates line numbers in error for file context', () => {
Expand All @@ -134,15 +136,15 @@ describe('Lexer', () => {
} catch (error) {
caughtError = error;
}
expect(String(caughtError)).to.equal(
'Syntax Error: Cannot parse the unexpected character "?".\n' +
'\n' +
'foo.js (13:6)\n' +
'12: \n' +
'13: ?\n' +
' ^\n' +
'14: \n',
);
expect(String(caughtError)).to.equal(dedent`
Syntax Error: Cannot parse the unexpected character "?".

foo.js (13:6)
12:
13: ?
^
14:
`);
});

it('updates column numbers in error for file context', () => {
Expand All @@ -153,13 +155,13 @@ describe('Lexer', () => {
} catch (error) {
caughtError = error;
}
expect(String(caughtError)).to.equal(
'Syntax Error: Cannot parse the unexpected character "?".\n' +
'\n' +
'foo.js (1:5)\n' +
'1: ?\n' +
' ^\n',
);
expect(String(caughtError)).to.equal(dedent`
Syntax Error: Cannot parse the unexpected character "?".

foo.js (1:5)
1: ?
^
`);
});

it('lexes strings', () => {
Expand Down