From 25e7e433f36b5a0e1d5adeeec65c567d2a51a20b Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Sat, 28 Apr 2018 13:16:23 +0300 Subject: [PATCH 1/3] Use dedent for print error tests --- src/language/__tests__/lexer-test.js | 64 ++++++++++++++-------------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/src/language/__tests__/lexer-test.js b/src/language/__tests__/lexer-test.js index 16435769db..d3e417d20c 100644 --- a/src/language/__tests__/lexer-test.js +++ b/src/language/__tests__/lexer-test.js @@ -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'; @@ -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', () => { @@ -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', () => { @@ -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', () => { From 0b80612e72e953f69748cfef87073608956bc1e3 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Sat, 28 Apr 2018 13:45:10 +0300 Subject: [PATCH 2/3] printError: Add tests for padding of line numbers --- src/error/__tests__/printError-test.js | 31 ++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/error/__tests__/printError-test.js b/src/error/__tests__/printError-test.js index e01670b1f3..63470a3f80 100644 --- a/src/error/__tests__/printError-test.js +++ b/src/error/__tests__/printError-test.js @@ -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( From 3af15c4d359bf62613453701e4a1a9e82b18edbf Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Sat, 28 Apr 2018 14:50:16 +0300 Subject: [PATCH 3/3] printError: fix padding of line numbers --- src/error/__tests__/printError-test.js | 4 +- src/error/printError.js | 51 ++++++++++++++++---------- 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/error/__tests__/printError-test.js b/src/error/__tests__/printError-test.js index 63470a3f80..804e6f452a 100644 --- a/src/error/__tests__/printError-test.js +++ b/src/error/__tests__/printError-test.js @@ -25,8 +25,8 @@ describe('printError', () => { Single digit line number with no padding Test (9:1) - 9: * - ^ + 9: * + ^ `); const doubleDigit = new GraphQLError( diff --git a/src/error/printError.js b/src/error/printError.js index c2f9971b1f..1741464214 100644 --- a/src/error/printError.js +++ b/src/error/printError.js @@ -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 {