Repository: prettier/prettier-atom (js)

Issue:
Cannot read property line of undefined
Error due to unexpected API from Errors coming from Prettier. From @mattapperson in https://github.com/prettier/prettier-atom/issues/226#issuecomment-313665570

> My stack trace...
> <details>
> 
> TypeError: Cannot read property 'line' of undefined
>     at buildPointArrayFromPrettierErrorAndRange (/Users/mattapperson/.atom/packages/prettier-atom/dist/executePrettier/handleError.js:16:37)
>     at setErrorMessageInLinter (/Users/mattapperson/.atom/packages/prettier-atom/dist/executePrettier/handleError.js:32:29)
>     at /Users/mattapperson/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:49:114
>     at executePrettierOnBufferRange (/Users/mattapperson/.atom/packages/prettier-atom/dist/executePrettier/executePrettierOnBufferRange.js:45:5)
>     at callAppropriatePrettierExecutor (/Users/mattapperson/.atom/packages/prettier-atom/dist/formatOnSave/index.js:22:91)
>     at r (/Users/mattapperson/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:5:165)
>     at /Users/mattapperson/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:97:162
>     at r (/Users/mattapperson/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:5:165)
>     at /Users/mattapperson/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:63:325
>     at /Users/mattapperson/.atom/packages/prettier-atom/node_modules/lodash/lodash.min.js:49:144
>     at /Users/mattapperson/.atom/packages/prettier-atom/dist/formatOnSave/index.js:29:12
>     at attemptWithErrorNotification (/Users/mattapperson/.atom/packages/prettier-atom/dist/atomInterface/index.js:122:10)
>     at safeFormatOnSaveIfAppropriate (/Users/mattapperson/.atom/packages/prettier-atom/dist/formatOnSave/index.js:28:10)
>     at lazyFormatOnSave (/Users/mattapperson/.atom/packages/prettier-atom/dist/main.js:39:15)
>     at /Users/mattapperson/.atom/packages/prettier-atom/dist/main.js:120:14
>     at Function.module.exports.Emitter.simpleDispatch (/Applications/Atom.app/Contents/Resources/app/node_modules/event-kit/lib/emitter.js:25:20)
>     at Emitter.module.exports.Emitter.emit (/Applications/Atom.app/Contents/Resources/app/node_modules/event-kit/lib/emitter.js:141:34)
>     at TextBuffer.module.exports.TextBuffer.saveAs (/Applications/Atom.app/Contents/Resources/app/node_modules/text-buffer/lib/text-buffer.js:1119:26)
>     at TextBuffer.module.exports.TextBuffer.save (/Applications/Atom.app/Contents/Resources/app/node_modules/text-buffer/lib/text-buffer.js:1108:25)
>     at TextEditor.module.exports.TextEditor.save (/Applications/Atom.app/Contents/Resources/app/src/text-editor.js:925:32)
>     at Pane.module.exports.Pane.saveItem (/Applications/Atom.app/Contents/Resources/app/src/pane.js:780:24)
>     at Pane.saveItem (/Applications/Atom.app/Contents/Resources/app/src/pane.js:3:65)
>     at Pane.module.exports.Pane.saveActiveItem (/Applications/Atom.app/Contents/Resources/app/src/pane.js:763:25)
>     at Workspace.saveActivePaneItem (/Applications/Atom.app/Contents/Resources/app/src/workspace.js:1548:46)
>     at atom-workspace.core:save (/Applications/Atom.app/Contents/Resources/app/src/register-default-commands.js:236:38)
>     at CommandRegistry.module.exports.CommandRegistry.handleCommandEvent (/Applications/Atom.app/Contents/Resources/app/src/command-registry.js:265:35)
>     at /Applications/Atom.app/Contents/Resources/app/src/command-registry.js:3:65
>     at KeymapManager.module.exports.KeymapManager.dispatchCommandEvent (/Applications/Atom.app/Contents/Resources/app/node_modules/atom-keymap/lib/keymap-manager.js:610:22)
>     at KeymapManager.module.exports.KeymapManager.handleKeyboardEvent (/Applications/Atom.app/Contents/Resources/app/node_modules/atom-keymap/lib/keymap-manager.js:401:28)
>     at WindowEventHandler.module.exports.WindowEventHandler.handleDocumentKeyEvent (/Applications/Atom.app/Contents/Resources/app/src/window-event-handler.js:100:42)
>     at HTMLDocument.<anonymous> (/Applications/Atom.app/Contents/Resources/app/src/window-event-handler.js:3:65)
> </details>

Pull request:
fix(handleError): properly handle alternative loc in Prettier errors
Prettier sometimes provides a different API for the `loc` property on the errors it throws. We were
only anticipating one type, where there is a `start` property nested in `loc`, but there's actually
another where the line and column are direct children of `loc` and there is no `start` property. We
now will properly handle either case.

Fixes #229

Patch:
diff --git a/decls/index.js b/decls/index.js
index 1778900..53a035d 100644
--- a/decls/index.js
+++ b/decls/index.js
@@ -89,7 +89,7 @@ declare type Linter$Message$ApplySolution = {
 };
 // eslint-disable-next-line no-undef
 declare type Prettier$SyntaxError = {
-  loc: { start: { line: number, column: number } },
+  loc: { start: { line: number, column: number } } | {| line: number, column: number |},
   message: string,
 };
 declare type Linter$Message = {
@@ -136,5 +136,4 @@ declare type Linter$IndieDelegate = {
   setAllMessages: (messages: Array<Linter$Message>) => void,
   onDidUpdate: (callback: Function) => Atom$Disposable,
   onDidDestroy: (callback: Function) => Atom$Disposable,
-  dispose: () => void,
 };
diff --git a/dist/executePrettier/handleError.js b/dist/executePrettier/handleError.js
index 4ae9385..0e19d31 100644
--- a/dist/executePrettier/handleError.js
+++ b/dist/executePrettier/handleError.js
@@ -11,9 +11,17 @@ var _require2 = require('../helpers'),
     createPoint = _require2.createPoint,
     createRange = _require2.createRange;
 
+var errorLine = function errorLine(error) {
+  return error.loc.start ? error.loc.start.line : error.loc.line;
+};
+
+var errorColumn = function errorColumn(error) {
+  return error.loc.start ? error.loc.start.column : error.loc.column;
+};
+
 // NOTE: Prettier error locations are not zero-based (i.e., they start at 1)
 var buildPointArrayFromPrettierErrorAndRange = function buildPointArrayFromPrettierErrorAndRange(error, bufferRange) {
-  return createPoint(error.loc.start.line + bufferRange.start.row - 1, error.loc.start.line === 0 ? error.loc.start.column + bufferRange.start.column - 1 : error.loc.start.column - 1);
+  return createPoint(errorLine(error) + bufferRange.start.row - 1, errorLine(error) === 0 ? errorColumn(error) + bufferRange.start.column - 1 : errorColumn(error) - 1);
 };
 
 var buildExcerpt = function buildExcerpt(error) {
diff --git a/src/executePrettier/handleError.js b/src/executePrettier/handleError.js
index e394240..c63e058 100644
--- a/src/executePrettier/handleError.js
+++ b/src/executePrettier/handleError.js
@@ -10,13 +10,16 @@ type HandleErrorArgs = {
   bufferRange: Range,
 };
 
+const errorLine = (error: Prettier$SyntaxError) => (error.loc.start ? error.loc.start.line : error.loc.line);
+
+const errorColumn = (error: Prettier$SyntaxError) =>
+  error.loc.start ? error.loc.start.column : error.loc.column;
+
 // NOTE: Prettier error locations are not zero-based (i.e., they start at 1)
 const buildPointArrayFromPrettierErrorAndRange = (error: Prettier$SyntaxError, bufferRange: Range): Point =>
   createPoint(
-    error.loc.start.line + bufferRange.start.row - 1,
-    error.loc.start.line === 0
-      ? error.loc.start.column + bufferRange.start.column - 1
-      : error.loc.start.column - 1,
+    errorLine(error) + bufferRange.start.row - 1,
+    errorLine(error) === 0 ? errorColumn(error) + bufferRange.start.column - 1 : errorColumn(error) - 1,
   );
 
 const buildExcerpt = (error: Prettier$SyntaxError) => /(.*)\s\(\d+:\d+\).*/.exec(error.message)[1];


Test patch:
diff --git a/src/executePrettier/handleError.test.js b/src/executePrettier/handleError.test.js
index a1ede93..4f0e134 100644
--- a/src/executePrettier/handleError.test.js
+++ b/src/executePrettier/handleError.test.js
@@ -7,9 +7,13 @@ const { createRange } = require('../helpers');
 const handleError = require('./handleError');
 
 // helpers
-const buildFakeError = ({ line, column }) => {
+const buildFakeError = ({ line, column }, useAlternativeErrorApi = false) => {
   const error = new Error(`Unexpected token (${line}:${column}) | stack trace`);
-  error.loc = { start: { line, column } };
+  if (useAlternativeErrorApi) {
+    error.loc = { line, column };
+  } else {
+    error.loc = { start: { line, column } };
+  }
 
   return error;
 };
@@ -39,6 +43,27 @@ it('sets an error message in the indie-linter', () => {
   expect(linterInterface.setMessages).toHaveBeenCalledWith(editor, expectedMessages);
 });
 
+it('works with the alternative error location API from Prettier', () => {
+  getCurrentFilePath.mockImplementation(() => '/fake/file/path.js');
+  const error = buildFakeError({ line: 1, column: 2 }, true);
+  const editor = null;
+  const bufferRange = { start: { row: 0, column: 0 }, end: { row: 0, column: 0 } };
+
+  handleError({ bufferRange, editor, error });
+
+  const expectedMessages = [
+    {
+      location: {
+        file: '/fake/file/path.js',
+        position: createRange([0, 1], [0, 1]),
+      },
+      excerpt: 'Unexpected token',
+      severity: 'error',
+    },
+  ];
+  expect(linterInterface.setMessages).toHaveBeenCalledWith(editor, expectedMessages);
+});
+
 describe('position property of the message sent to the linter', () => {
   it('accounts for the start row of the buffer range', () => {
     const editor = null;