printError: fix padding of line numbers (#1328)

* Use dedent for print error tests

* printError: Add tests for padding of line numbers

* printError: fix padding of line numbers

Author: IvanGoncharov

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(

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 {

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', () => {