More robust error handling in the language server

.eslintrc

@@ -33,6 +33,7 @@
         "@typescript-eslint/no-non-null-assertion": "off",
         "@typescript-eslint/ban-types": "off",
         "@typescript-eslint/no-var-requires": "off",
+        "@typescript-eslint/ban-ts-comment": ["error", { "ts-ignore": "allow-with-description" }],
         "@typescript-eslint/explicit-function-return-type": [
           "error",
           {

packages/core/__tests__/cli/declaration.test.ts

@@ -4,6 +4,7 @@ import Project from '../utils/project';
 describe('CLI: emitting declarations', () => {
   let project!: Project;
   beforeEach(async () => {
+    jest.setTimeout(20_000);
     project = await Project.create();
   });
 

packages/core/__tests__/language-server/definitions.test.ts

@@ -155,12 +155,16 @@ describe('Language Server: Definitions', () => {
       character: 27,
     });
 
-    expect(definitions).toEqual([
+    expect(definitions).toMatchObject([
       {
         uri: project.fileURI('greeting.ts'),
+
+        // Versions of TS vary on whether they consider the source to be
+        // the entire module or just the first character, so we'll consider
+        // the test passing as long as the loose shape is right.
         range: {
           start: { line: 0, character: 0 },
-          end: { line: 8, character: 1 },
+          end: {},
         },
       },
     ]);

packages/core/src/language-server/binding.ts

@@ -0,0 +1,113 @@
+import {
+  Connection,
+  ServerCapabilities,
+  TextDocuments,
+  TextDocumentSyncKind,
+} from 'vscode-languageserver';
+import { TextDocument } from 'vscode-languageserver-textdocument';
+import GlintLanguageServer from './glint-language-server';
+import { debounce } from '../common/scheduling';
+
+export const capabilities: ServerCapabilities = {
+  textDocumentSync: TextDocumentSyncKind.Full,
+  completionProvider: {
+    resolveProvider: true,
+  },
+  referencesProvider: true,
+  hoverProvider: true,
+  definitionProvider: true,
+  workspaceSymbolProvider: true,
+  renameProvider: {
+    prepareProvider: true,
+  },
+};
+
+export type BindingArgs = {
+  languageServer: GlintLanguageServer;
+  documents: TextDocuments<TextDocument>;
+  connection: Connection;
+};
+
+export function bindLanguageServer(args: BindingArgs): void {
+  let { connection, languageServer, documents } = args;
+  let { scheduleDiagnostics, captureErrors } = buildHelpers(args);
+
+  connection.onInitialize(() => ({ capabilities }));
+
+  documents.onDidOpen(({ document }) => {
+    languageServer.openFile(document.uri, document.getText());
+    scheduleDiagnostics();
+  });
+
+  documents.onDidClose(({ document }) => {
+    languageServer.closeFile(document.uri);
+  });
+
+  documents.onDidChangeContent(({ document }) => {
+    languageServer.updateFile(document.uri, document.getText());
+    scheduleDiagnostics();
+  });
+
+  connection.onPrepareRename(({ textDocument, position }) => {
+    return captureErrors(() => languageServer.prepareRename(textDocument.uri, position));
+  });
+
+  connection.onRenameRequest(({ textDocument, position, newName }) => {
+    return captureErrors(() =>
+      languageServer.getEditsForRename(textDocument.uri, position, newName)
+    );
+  });
+
+  connection.onCompletion(({ textDocument, position }) => {
+    return captureErrors(() => languageServer.getCompletions(textDocument.uri, position));
+  });
+
+  connection.onCompletionResolve((item) => {
+    return captureErrors(() => languageServer.getCompletionDetails(item)) ?? item;
+  });
+
+  connection.onHover(({ textDocument, position }) => {
+    return captureErrors(() => languageServer.getHover(textDocument.uri, position));
+  });
+
+  connection.onDefinition(({ textDocument, position }) => {
+    return captureErrors(() => languageServer.getDefinition(textDocument.uri, position));
+  });
+
+  connection.onReferences(({ textDocument, position }) => {
+    return captureErrors(() => languageServer.getReferences(textDocument.uri, position));
+  });
+
+  connection.onWorkspaceSymbol(({ query }) => {
+    return captureErrors(() => languageServer.findSymbols(query));
+  });
+
+  connection.onDidChangeWatchedFiles(() => {
+    // TODO: use this to synchronize files that aren't open so we don't assume changes only
+    // happen in the editor.
+  });
+}
+
+type BindingHelpers = {
+  scheduleDiagnostics: () => void;
+  captureErrors: <T>(callback: () => T) => T | undefined;
+};
+
+function buildHelpers({ languageServer, documents, connection }: BindingArgs): BindingHelpers {
+  return {
+    scheduleDiagnostics: debounce(250, () => {
+      for (let { uri } of documents.all()) {
+        const diagnostics = languageServer.getDiagnostics(uri);
+        connection.sendDiagnostics({ uri, diagnostics });
+      }
+    }),
+
+    captureErrors(callback) {
+      try {
+        return callback();
+      } catch (error) {
+        connection.console.error(error.stack ?? error);
+      }
+    },
+  };
+}

packages/core/src/language-server/glint-language-server.ts

@@ -151,7 +151,9 @@ export default class GlintLanguageServer {
       label,
       {},
       source,
-      {}
+      {},
+      // @ts-ignore: 4.3 adds a new not-optional-but-can-be-undefined `data` arg
+      undefined
     );
 
     if (!details) {
@@ -284,9 +286,10 @@ export default class GlintLanguageServer {
       textSpan.start + textSpan.length
     );
 
-    // Zero-length spans correspond to a synthetic use of the symbol, so we don't
-    // want to actually show them to the user.
-    if (originalStart === originalEnd) return;
+    // If our calculated original span is zero-length but the transformed span
+    // does take up space, this corresponds to a synthetic usage we generated
+    // in the transformed output, and we don't want to show it to the user.
+    if (originalStart === originalEnd && textSpan.length > 0) return;
 
     let originalContents = this.documents.getDocumentContents(originalFileName);
     let start = offsetToPosition(originalContents, originalStart);

packages/core/src/language-server/index.ts

@@ -1,10 +1,10 @@
-import { TextDocuments, TextDocumentSyncKind, createConnection } from 'vscode-languageserver/node';
+import { TextDocuments, createConnection } from 'vscode-languageserver/node';
 import { TextDocument } from 'vscode-languageserver-textdocument';
 import { findConfig } from '@glint/config';
 import { loadTypeScript } from '../common/load-typescript';
 import GlintLanguageServer from './glint-language-server';
 import { parseConfigFile, uriToFilePath } from './util';
-import { debounce } from '../common/scheduling';
+import { bindLanguageServer } from './binding';
 
 const connection = createConnection(process.stdin, process.stdout);
 const documents = new TextDocuments(TextDocument);
@@ -19,85 +19,9 @@ const getRootFileNames = (): Array<string> => {
 };
 
 if (glintConfig) {
-  const gls = new GlintLanguageServer(ts, glintConfig, getRootFileNames, options);
+  const languageServer = new GlintLanguageServer(ts, glintConfig, getRootFileNames, options);
 
-  connection.onInitialize(() => ({
-    capabilities: {
-      textDocumentSync: TextDocumentSyncKind.Full,
-      completionProvider: {
-        resolveProvider: true,
-      },
-      referencesProvider: true,
-      hoverProvider: true,
-      definitionProvider: true,
-      workspaceSymbolProvider: true,
-      renameProvider: {
-        prepareProvider: true,
-      },
-    },
-  }));
-
-  const scheduleDiagnostics = debounce(250, () => {
-    for (let { uri } of documents.all()) {
-      connection.sendDiagnostics({
-        uri,
-        diagnostics: gls.getDiagnostics(uri),
-      });
-    }
-  });
-
-  connection.onDidChangeWatchedFiles(() => {
-    // TODO: use this to synchronize files that aren't open so we don't assume changes only
-    // happen in the editor.
-  });
-
-  documents.onDidOpen(({ document }) => {
-    gls.openFile(document.uri, document.getText());
-
-    scheduleDiagnostics();
-  });
-
-  documents.onDidClose(({ document }) => {
-    gls.closeFile(document.uri);
-  });
-
-  documents.onDidChangeContent(({ document }) => {
-    gls.updateFile(document.uri, document.getText());
-
-    scheduleDiagnostics();
-  });
-
-  connection.onPrepareRename(({ textDocument, position }) => {
-    return gls.prepareRename(textDocument.uri, position);
-  });
-
-  connection.onRenameRequest(({ textDocument, position, newName }) => {
-    return gls.getEditsForRename(textDocument.uri, position, newName);
-  });
-
-  connection.onCompletion(({ textDocument, position }) => {
-    return gls.getCompletions(textDocument.uri, position);
-  });
-
-  connection.onCompletionResolve((item) => {
-    return gls.getCompletionDetails(item);
-  });
-
-  connection.onHover(({ textDocument, position }) => {
-    return gls.getHover(textDocument.uri, position);
-  });
-
-  connection.onDefinition(({ textDocument, position }) => {
-    return gls.getDefinition(textDocument.uri, position);
-  });
-
-  connection.onReferences(({ textDocument, position }) => {
-    return gls.getReferences(textDocument.uri, position);
-  });
-
-  connection.onWorkspaceSymbol(({ query }) => {
-    return gls.findSymbols(query);
-  });
+  bindLanguageServer({ languageServer, documents, connection });
 
   documents.listen(connection);
   connection.listen();

packages/transform/__tests__/rewrite.test.ts

@@ -99,6 +99,28 @@ describe('rewriteModule', () => {
         }"
       `);
     });
+
+    test('with a syntax error', () => {
+      let script = {
+        filename: 'test.ts',
+        contents: stripIndent`
+          import Component, { hbs } from '@glimmerx/component';
+          export default class MyComponent extends Component {
+            static template = hbs\`
+              {{hello
+            \`;
+          }
+        `,
+      };
+
+      let transformedModule = rewriteModule({ script }, glimmerxEnvironment);
+
+      expect(transformedModule?.errors.length).toBe(1);
+      expect(transformedModule?.transformedContents).toBe(script.contents);
+
+      expect(transformedModule?.getOriginalOffset(100)).toEqual({ offset: 100, source: script });
+      expect(transformedModule?.getTransformedOffset(script.filename, 100)).toEqual(100);
+    });
   });
 
   describe('standalone companion template', () => {
@@ -284,5 +306,34 @@ describe('rewriteModule', () => {
         }"
       `);
     });
+
+    test('with a syntax error', () => {
+      let script = {
+        filename: 'test.ts',
+        contents: stripIndent`
+          import Component from '@glimmer/component';
+          export default class MyComponent extends Component {
+          }
+        `,
+      };
+
+      let template = {
+        filename: 'test.hbs',
+        contents: stripIndent`
+          {{hello
+        `,
+      };
+
+      let transformedModule = rewriteModule({ script, template }, emberLooseEnvironment);
+
+      expect(transformedModule?.errors.length).toBe(1);
+      expect(transformedModule?.transformedContents).toBe(script.contents);
+
+      expect(transformedModule?.getOriginalOffset(50)).toEqual({ offset: 50, source: script });
+      expect(transformedModule?.getTransformedOffset(script.filename, 50)).toEqual(50);
+      expect(transformedModule?.getTransformedOffset(template.filename, 5)).toEqual(
+        script.contents.lastIndexOf('}')
+      );
+    });
   });
 });

packages/transform/__tests__/template-to-typescript.test.ts

@@ -660,7 +660,7 @@ describe('rewriteTemplate', () => {
   });
 
   describe('error conditions', () => {
-    test('Syntax error', () => {
+    test('Handlebars syntax error', () => {
       let { errors } = templateToTypescript('<Foo @attr={{"123}} />', {
         typesPath: '@glint/template',
       });
@@ -678,6 +678,20 @@ describe('rewriteTemplate', () => {
       ]);
     });
 
+    test('HTML syntax error', () => {
+      let { errors } = templateToTypescript('<Foo </Foo>', {
+        typesPath: '@glint/template',
+      });
+
+      expect(errors).toEqual([
+        {
+          location: { start: 5, end: 5 },
+          message:
+            "< is not a valid character within attribute names: (error occurred in 'an unknown module' @ line 1 : column 5)",
+        },
+      ]);
+    });
+
     test('{{yield}} in expression position', () => {
       let { errors } = templateToTypescript('<Foo @attr={{yield}} />', {
         typesPath: '@glint/template',

packages/transform/src/inlining/companion-file.ts

@@ -1,6 +1,7 @@
 import { NodePath, types as t } from '@babel/core';
 import { GlintEnvironment } from '@glint/config';
 import { CorrelatedSpansResult, getContainingTypeInfo, PartialCorrelatedSpan } from '.';
+import MappingTree, { ParseError } from '../mapping-tree';
 import { templateToTypescript } from '../template-to-typescript';
 import { SourceFile, TransformError } from '../transformed-module';
 import { assert } from '../util';
@@ -96,6 +97,22 @@ export function calculateCompanionTemplateSpans(
           transformedSource: `;\n`,
         }
       );
+    } else {
+      let mapping = new MappingTree(
+        { start: 0, end: 0 },
+        { start: 0, end: template.contents.length },
+        [],
+        new ParseError()
+      );
+
+      partialSpans.push({
+        originalFile: template,
+        originalStart: 0,
+        originalLength: template.contents.length,
+        insertionPoint: target.end - 1,
+        transformedSource: '',
+        mapping,
+      });
     }
   } else {
     // TODO: handle opaque expression like an imported identifier or `templateOnlyComponent()`