Merge pull request #949 from dwwoelfel/error-context

Account for query's offset in file for errors

Author: wincent

src/error/syntaxError.js

@@ -22,9 +22,11 @@ export function syntaxError(
   description: string
 ): GraphQLError {
   const location = getLocation(source, position);
+  const line = location.line + source.locationOffset.line - 1;
+  const column = location.column + source.locationOffset.column - 1;
   const error = new GraphQLError(
-    `Syntax Error ${source.name} (${location.line}:${location.column}) ` +
-    description + '\n\n' + highlightSourceAtLocation(source, location),
+    `Syntax Error ${source.name} (${line}:${column}) ${description}` +
+      '\n\n' + highlightSourceAtLocation(source, location),
     undefined,
     source,
     [ position ]
@@ -38,21 +40,29 @@ export function syntaxError(
  */
 function highlightSourceAtLocation(source, location) {
   const line = location.line;
-  const prevLineNum = (line - 1).toString();
-  const lineNum = line.toString();
-  const nextLineNum = (line + 1).toString();
+  const lineOffset = source.locationOffset.line - 1;
+  const columnOffset = source.locationOffset.column - 1;
+  const contextLine = line + lineOffset;
+  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(columnOffset) + lines[0];
   return (
     (line >= 2 ?
       lpad(padLen, prevLineNum) + ': ' + lines[line - 2] + '\n' : '') +
     lpad(padLen, lineNum) + ': ' + lines[line - 1] + '\n' +
-    Array(2 + padLen + location.column).join(' ') + '^\n' +
+    whitespace(2 + padLen + location.column - 1 + columnOffset) + '^\n' +
     (line < lines.length ?
       lpad(padLen, nextLineNum) + ': ' + lines[line] + '\n' : '')
   );
 }
 
+function whitespace(len) {
+  return Array(len + 1).join(' ');
+}
+
 function lpad(len, str) {
-  return Array(len - str.length + 1).join(' ') + str;
+  return whitespace(len - str.length) + str;
 }

src/language/__tests__/lexer-test.js

@@ -120,6 +120,39 @@ describe('Lexer', () => {
 
   });
 
+  it('updates line numbers in error for file context', () => {
+    expect(() => {
+      const str = '' +
+      '\n' +
+      '\n' +
+      '     ?\n' +
+      '\n';
+      const source = new Source(str, 'foo.js', { line: 11, column: 1 });
+      return createLexer(source).advance();
+    }).to.throw(
+        'Syntax Error foo.js (13:6) ' +
+        'Cannot parse the unexpected character "?".\n' +
+        '\n' +
+        '12: \n' +
+        '13:      ?\n' +
+        '         ^\n' +
+        '14: \n'
+      );
+  });
+
+  it('updates column numbers in error for file context', () => {
+    expect(() => {
+      const source = new Source('?', 'foo.js', { line: 1, column: 5 });
+      return createLexer(source).advance();
+    }).to.throw(
+        'Syntax Error foo.js (1:5) ' +
+        'Cannot parse the unexpected character "?".\n' +
+        '\n' +
+        '1:     ?\n' +
+        '       ^\n'
+      );
+  });
+
   it('lexes strings', () => {
 
     expect(

src/language/source.js

@@ -8,18 +8,37 @@
  *  of patent rights can be found in the PATENTS file in the same directory.
  */
 
+import invariant from '../jsutils/invariant';
+
+type Location = {
+  line: number,
+  column: number,
+};
+
 /**
- * A representation of source input to GraphQL. The name is optional,
- * but is mostly useful for clients who store GraphQL documents in
- * source files; for example, if the GraphQL input is in a file Foo.graphql,
- * it might be useful for name to be "Foo.graphql".
+ * A representation of source input to GraphQL.
+ * `name` and `locationOffset` are optional. They are useful for clients who
+ * store GraphQL documents in source files; for example, if the GraphQL input
+ * starts at line 40 in a file named Foo.graphql, it might be useful for name to
+ * be "Foo.graphql" and location to be `{ line: 40, column: 0 }`.
+ * line and column in locationOffset are 1-indexed
  */
 export class Source {
   body: string;
   name: string;
+  locationOffset: Location;
 
-  constructor(body: string, name?: string): void {
+  constructor(body: string, name?: string, locationOffset?: Location): void {
     this.body = body;
     this.name = name || 'GraphQL request';
+    this.locationOffset = locationOffset || { line: 1, column: 1 };
+    invariant(
+      this.locationOffset.line > 0,
+      'line in locationOffset is 1-indexed and must be positive'
+    );
+    invariant(
+      this.locationOffset.column > 0,
+      'column in locationOffset is 1-indexed and must be positive'
+    );
   }
 }