Merge pull request #655 from NullVoxPopuli/content-tag-take-2

Use `content-tag` instead of `ember-template-imports`

.vscode/launch.json

@@ -5,7 +5,8 @@
       "name": "Debug Extension (Glint + TS)",
       "type": "extensionHost",
       "request": "launch",
-      "preLaunchTask": "npm: build",
+      // this errors. Run `yarn build` before launching this task
+      // "preLaunchTask": "npm: build",
       "autoAttachChildProcesses": true,
       "runtimeExecutable": "${execPath}",
       "outFiles": [

package.json

@@ -8,6 +8,7 @@
   ],
   "scripts": {
     "lint": "yarn lint:scripts && yarn lint:formatting",
+    "lint:fix": "yarn lint:scripts --fix && yarn prettier --write .",
     "lint:scripts": "yarn eslint --max-warnings 0 --cache .",
     "lint:formatting": "yarn prettier --check .",
     "test": "yarn workspaces run test",

packages/core/__tests__/language-server/diagnostic-augmentation.test.ts

@@ -13,6 +13,103 @@ describe('Language Server: Diagnostic Augmentation', () => {
     await project.destroy();
   });
 
+  test('There is a content-tag parse error (for a template-only component)', async () => {
+    project.setGlintConfig({ environment: ['ember-loose', 'ember-template-imports'] });
+    project.write({
+      'index.gts': stripIndent`
+        function expectsTwoArgs(a: string, b: number) {
+          console.log(a, b);
+        }
+
+        <template>
+          {{expectsTwoArgs "one"}}
+      `,
+    });
+
+    let server = project.startLanguageServer();
+    let diagnostics = server.getDiagnostics(project.fileURI('index.gts'));
+
+    expect(diagnostics).toMatchInlineSnapshot(`
+      [
+        {
+          "code": 0,
+          "message": "Unexpected eof
+
+       5 │ <template>
+       6 │   {{expectsTwoArgs \\"one\\"}}
+         ╰────",
+          "range": {
+            "end": {
+              "character": 7,
+              "line": 4,
+            },
+            "start": {
+              "character": 6,
+              "line": 4,
+            },
+          },
+          "severity": 1,
+          "source": "glint",
+          "tags": [],
+        },
+      ]
+    `);
+  });
+
+  test('There is a content-tag parse error (for a class component)', async () => {
+    project.setGlintConfig({ environment: ['ember-loose', 'ember-template-imports'] });
+    project.write({
+      'index.gts': stripIndent`
+        import Component from '@glimmer/component';
+
+        export interface AppSignature {
+          Blocks: {
+            expectsTwoParams: [a: string, b: number];
+            expectsAtLeastOneParam: [a: string, ...rest: Array<string>];
+          }
+        }
+
+        function expectsTwoArgs(a: string, b: number) {
+          console.log(a, b);
+        }
+
+        export default class App extends Component<AppSignature> {
+          <template>
+            {{expectsTwoArgs "one"}}
+        }
+      `,
+    });
+
+    let server = project.startLanguageServer();
+    let diagnostics = server.getDiagnostics(project.fileURI('index.gts'));
+
+    expect(diagnostics).toMatchInlineSnapshot(`
+      [
+        {
+          "code": 0,
+          "message": "Unexpected token \`<lexing error: Error { error: (Span { lo: BytePos(382), hi: BytePos(382), ctxt: #0 }, Eof) }>\`. Expected content tag
+
+       16 │     {{expectsTwoArgs \\"one\\"}}
+       17 │ }
+          ╰────",
+          "range": {
+            "end": {
+              "character": 14,
+              "line": 15,
+            },
+            "start": {
+              "character": 13,
+              "line": 15,
+            },
+          },
+          "severity": 1,
+          "source": "glint",
+          "tags": [],
+        },
+      ]
+    `);
+  });
+
   test('expected argument count', async () => {
     project.setGlintConfig({ environment: ['ember-loose', 'ember-template-imports'] });
     project.write({

packages/core/src/common/transform-manager.ts

@@ -85,6 +85,16 @@ export default class TransformManager {
       );
     }
 
+    // When we have syntax errors we get _too many errors_
+    // if we have an issue with <template> tranformation, we should
+    // make the user fix their syntax before revealing all the other errors.
+    let contentTagErrors = allDiagnostics.filter(
+      (diagnostic) => (diagnostic as Diagnostic).isContentTagError
+    );
+    if (contentTagErrors.length) {
+      return this.ts.sortAndDeduplicateDiagnostics(contentTagErrors);
+    }
+
     return this.ts.sortAndDeduplicateDiagnostics(allDiagnostics);
   }
 
@@ -450,7 +460,13 @@ export default class TransformManager {
 
   private buildTransformDiagnostics(transformedModule: TransformedModule): Array<Diagnostic> {
     return transformedModule.errors.map((error) =>
-      createTransformDiagnostic(this.ts, error.source, error.message, error.location)
+      createTransformDiagnostic(
+        this.ts,
+        error.source,
+        error.message,
+        error.location,
+        error.isContentTagError
+      )
     );
   }
 }

packages/core/src/transform/diagnostics/create-transform-diagnostic.ts

@@ -6,9 +6,11 @@ export function createTransformDiagnostic(
   ts: TSLib,
   source: SourceFile,
   message: string,
-  location: Range
+  location: Range,
+  isContentTagError = false
 ): Diagnostic {
   return {
+    isContentTagError,
     isGlintTransformDiagnostic: true,
     category: ts.DiagnosticCategory.Error,
     code: 0,

packages/core/src/transform/diagnostics/index.ts

@@ -1,6 +1,9 @@
 import type * as ts from 'typescript';
 
-export type Diagnostic = ts.Diagnostic & { isGlintTransformDiagnostic?: boolean };
+export type Diagnostic = ts.Diagnostic & {
+  isGlintTransformDiagnostic?: boolean;
+  isContentTagError?: boolean;
+};
 
 export { rewriteDiagnostic } from './rewrite-diagnostic.js';
 export { createTransformDiagnostic } from './create-transform-diagnostic.js';

packages/core/src/transform/template/rewrite-module.ts

@@ -71,7 +71,39 @@ function calculateCorrelatedSpans(
   let errors: Array<TransformError> = [];
   let partialSpans: Array<PartialCorrelatedSpan> = [];
 
-  let { ast, emitMetadata } = parseScript(ts, script, environment);
+  let { ast, emitMetadata, error } = parseScript(ts, script, environment);
+
+  if (error) {
+    if (typeof error === 'string') {
+      errors.push({
+        message: error,
+        location: { start: 0, end: script.contents.length - 1 },
+        source: script,
+      });
+    } else if ('isContentTagError' in error && error.isContentTagError) {
+      // these lines exclude the line with the error, because
+      // adding the column offset will get us on to the line with the error
+      let lines = script.contents.split('\n').slice(0, error.line);
+      let start = lines.reduce((sum, line) => sum + line.length, 0) + error.column - 1;
+      let end = start + 1;
+
+      errors.push({
+        isContentTagError: true,
+        // we have to show the "help" because content-tag has different line numbers
+        // than we are able to discern ourselves.
+        message: error.message + '\n\n' + error.help,
+        source: script,
+        location: {
+          start,
+          end,
+        },
+      });
+    }
+
+    // We've hit a parsing error, so we need to immediately return as the parsed
+    // document must be correct before we can continue.
+    return { errors, directives, partialSpans };
+  }
 
   ts.transform(ast, [
     (context) =>
@@ -103,9 +135,22 @@ function calculateCorrelatedSpans(
   return { errors, directives, partialSpans };
 }
 
+type ParseError =
+  | string
+  | {
+      isContentTagError: true;
+      message: string;
+      line: number;
+      column: number;
+      file: string;
+      help: string;
+      raw: string;
+    };
+
 type ParseResult = {
   ast: ts.SourceFile;
   emitMetadata: WeakMap<ts.Node, GlintEmitMetadata>;
+  error?: ParseError;
 };
 
 function parseScript(ts: TSLib, script: SourceFile, environment: GlintEnvironment): ParseResult {
@@ -116,15 +161,37 @@ function parseScript(ts: TSLib, script: SourceFile, environment: GlintEnvironmen
     void emitMetadata.set(node, Object.assign(emitMetadata.get(node) ?? {}, data));
 
   let { preprocess, transform } = environment.getConfigForExtension(extension) ?? {};
-  let preprocessed = preprocess?.(contents, filename) ?? { contents };
+
+  let original: {
+    contents: string;
+    data?: {
+      // SAFETY: type exists elsewhere (the environments)
+      templateLocations: any[];
+    };
+  } = { contents, data: { templateLocations: [] } };
+
+  let preprocessed = original;
+  let error: ParseError | undefined;
+
+  try {
+    preprocessed = preprocess?.(contents, filename) ?? original;
+  } catch (e) {
+    error = parseError(e, filename);
+  }
+
   let ast = ts.createSourceFile(
     filename,
+    // contents will be transformed and placeholder'd content
+    // or, in the event of a parse error, the original content
+    // in which case, TS will report a ton of errors about some goofy syntax.
+    // We'll want to ignore all of that and only display our parsing error from content-tag.
     preprocessed.contents,
     ts.ScriptTarget.Latest,
     true // setParentNodes
   );
 
-  if (transform) {
+  // Only transform if we don't have a parse error
+  if (!error && transform) {
     let { transformed } = ts.transform(ast, [
       (context) => transform!(preprocessed.data, { ts, context, setEmitMetadata }),
     ]);
@@ -133,7 +200,51 @@ function parseScript(ts: TSLib, script: SourceFile, environment: GlintEnvironmen
     ast = transformed[0];
   }
 
-  return { ast, emitMetadata };
+  return { ast, emitMetadata, error };
+}
+
+function parseError(e: unknown, filename: string): ParseError {
+  if (typeof e === 'object' && e !== null) {
+    // Parse Errors from the rust parser
+    if ('source_code' in e) {
+      // We remove the blank links in the error because swc
+      // pads errors with a leading and trailing blank line.
+      // the error is typically meant for the terminal, so making it
+      // stand out a bit more is a good, but that's more a presentation
+      // concern than just pure error information (which is what we need).
+      // @ts-expect-error object / property narrowing isn't available until TS 5.1
+      let lines = e.source_code.split('\n').filter(Boolean);
+      // Example:
+      // '  × Unexpected eof'
+      // '   ╭─[/home/nullvoxpopuli/Development/OpenSource/glint/test-packages/ts-template-imports-app/src/index.gts:6:1]'
+      // ' 6 │ '
+      // ' 7 │ export const X = <tem'
+      // '   ╰────'
+      let raw = lines.join('\n');
+      let message = lines[0].replace('×', '').trim();
+      let info = lines[1];
+      // a filename may have numbers in it, so we want to remove the filename
+      // before regex searching for numbers at the end of this line
+      let strippedInfo = info.replace(filename, '');
+      let matches = [...strippedInfo.matchAll(/\d+/g)];
+      let line = parseInt(matches[0][0], 10);
+      let column = parseInt(matches[1][0], 10);
+      // The help omits the original file name, because TS will provide that.
+      let help = lines.slice(2).join('\n');
+
+      return {
+        isContentTagError: true,
+        raw,
+        message,
+        line,
+        column,
+        file: filename,
+        help,
+      };
+    }
+  }
+
+  return `${e}`;
 }
 
 /**

packages/core/src/transform/template/transformed-module.ts

@@ -32,6 +32,7 @@ export type Directive = {
 };
 
 export type TransformError = {
+  isContentTagError?: boolean;
   message: string;
   location: Range;
   source: SourceFile;