Restore "unused glint-expect-error" diagnostic (#795)

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

@@ -451,9 +451,51 @@ describe('Language Server: Diagnostics', () => {
       (await server.sendDocumentDiagnosticRequest(project.fileURI('component-b.gts'))).items,
     ).toEqual([]);
 
-    // TODO: uncomment and fix
-    // expect(
-    //   await server.sendDocumentDiagnosticRequest(project.fileURI('component-a.gts')),
-    // ).toMatchInlineSnapshot(`[TODO should display unused glint-expect-error directive]`);
+    expect(
+      (await server.sendDocumentDiagnosticRequest(project.fileURI('component-a.gts'))).items.length,
+    ).toEqual(1);
+
+    // TODO: the start range for this is not quite right; specifically the start range seems
+    // seems to go all the way to the end of the opening `<template>` tag, e.g.
+    // `<template>[HERE]`. This causes excess red suiggles, and this goes away if there
+    // are any other HTML elements preceding the glint-expect-error directive. I'm not
+    // sure the root cause of this, but it may go away if/when we refactor a few things about
+    // Volar's mapping logic.
+    //
+    // Tracking this issue here:
+    //
+    // https://github.com/typed-ember/glint/issues/796
+    expect(await server.sendDocumentDiagnosticRequest(project.fileURI('component-a.gts')))
+      .toMatchInlineSnapshot(`
+        {
+          "items": [
+            {
+              "code": 0,
+              "data": {
+                "documentUri": "volar-embedded-content://URI_ENCODED_PATH_TO/FILE",
+                "isFormat": false,
+                "original": {},
+                "pluginIndex": 0,
+                "uri": "file:///path/to/EPHEMERAL_TEST_PROJECT/component-a.gts",
+                "version": 3,
+              },
+              "message": "Unused '@glint-expect-error' directive.",
+              "range": {
+                "end": {
+                  "character": 30,
+                  "line": 4,
+                },
+                "start": {
+                  "character": 12,
+                  "line": 3,
+                },
+              },
+              "severity": 1,
+              "source": "disregard.gts",
+            },
+          ],
+          "kind": "full",
+        }
+      `);
   });
 });

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

@@ -229,7 +229,7 @@ export default class TransformedModule {
    * (e.g. `{{expectsAtLeastOneArg}}` in an .hbs file to `__glintDSL__.emitContent(__glintDSL__.resolveOrReturn(expectsAtLeastOneArg)());`
    * in a generated TS file, in Volar you can only map regions of the same size.
    *
-   * In the case that you need to map regions of different sizes in Volar, you need to also using
+   * In the case that you need to map regions of different sizes in Volar, you need to use
    * zero-length mappings to delineate regions/boundaries that should map to each other, otherwise there will
    * be cases where TS diagnostics will fail to transform/map back to the original source. Example:
    *

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

@@ -227,67 +227,35 @@ function filterAndAugmentDiagnostics(
     }
   });
 
-  for (let directive of unusedExpectErrors) {
-    // desired methond on transformedModule:
-    // - it accepts a source offset and finds the transformed offset
-    // - in which file? there are multiple embeddedCodes in a .gts file
-    // - root: gts
-    //   - embeddedCodes[0]: ts (IR)
-    //   - embeddedCodes[1, 2, 3]: however many Handlebars templates
-    //
-    // transformedModule.correlatedSpans[1].mapping.children[0].children[1].sourceNode
-    // - {type: 'MustacheCommentStatement', value: ' @glint-expect-error ', loc: SourceSpan}
-    //
-    // this is what we want.
-    //
-    // OK what is our input?
-    // the starting point is directive that is left over.
-    // directive.areaOfEffect.start/end reference to the offset within the .gts file delineating: |{{! @glint-expect-error }}|
-    //
-    // directive.source: {
-    //   contents: <GTS source code with untransformed template>
-    //   filename: "disregard.gts"
-    // }
-    //
-    // determineTransformedOffsetAndSpan(
-    //   originalFileName: string,
-    //   originalOffset: number
-    // )
-    //
-    // transformedModule.determineTransformedOffsetAndSpan(directive.source.filename, directive.location.start)
-    //
-    // this returns a transformedOffset and correlatedSpan with mapping pointing to the template embedded.
-    //
-
-    allDiagnostics.push({
-      message: `Unused '@glint-expect-error' directive.`,
-
-      // this range... should be... for the TS file. Currently we're sending
-      // a range for the source .gts. That can't be right.
-      // The info we have is....... we know an unused glint directive exists.
-      // We need to find a range in the IR .ts file.
+  if (transformedModule) {
+    for (let directive of unusedExpectErrors) {
+      const transformedStartOffset = transformedModule.getTransformedOffset(
+        directive.source.filename,
+        directive.location.start,
+      );
+
+      // Hacky, but `// @glint-expect-error\n` is the TS transformed representation of `{{!@glint-expect-error}}`,
+      // and its length is 23 characters, and we can use that number to calculate the end position in the transformed file.
       //
-      // 1. need to translate directive.areaOfEffect into the IR .ts file location
-      //    - this is going to be the beginning of line in .gts and end of line in .gts.
-      //    - actually maybe it's not area of effect, but rather the comment node. YES.
-      //  emit.forNode(node, () => {
-      //   emit.text(`// @glint-${kind}`);
-      //   emit.newline();
-      // });
+      // It would be less hacky if we could use:
       //
-      // - can we take the souce and query the CommentNode
-      //   - node: AST.MustacheCommentStatement | AST.CommentStatement
-      // - what/how do we query now?
+      //   transformedModule.getTransformedOffset(directive.source.filename, directive.location.end)
       //
-      // 2. need to make sure it fits error boundary
-      range: vscode.Range.create(
-        offsetToPosition(document.getText(), directive.areaOfEffect.start),
-        offsetToPosition(document.getText(), directive.areaOfEffect.end),
-      ),
-      severity: vscode.DiagnosticSeverity.Error,
-      code: 0,
-      source: directive.source.filename, // not sure if this is right
-    });
+      // But for unknown reasons (perhaps related to how Volar wants us to use 0-length boundary mappings
+      // to map unequally-sized regions to each other?), this ends up returning the same value as `directive.location.start`.
+      const transformedEndOffset = transformedStartOffset + 23;
+
+      allDiagnostics.push({
+        message: `Unused '@glint-expect-error' directive.`,
+        range: vscode.Range.create(
+          offsetToPosition(transformedModule.transformedContents, transformedStartOffset),
+          offsetToPosition(transformedModule.transformedContents, transformedEndOffset),
+        ),
+        severity: vscode.DiagnosticSeverity.Error,
+        code: 0,
+        source: directive.source.filename, // not sure if this is right
+      });
+    }
   }
 
   return allDiagnostics;
@@ -301,13 +269,3 @@ connection.onInitialized(() => {
 });
 
 connection.listen();
-
-/**
- * @param {string} uri
- * @returns {Promise<Commands>}
- */
-// async function getCommands(uri) {
-//   const project = await server.projects.getProject(uri)
-//   const service = project.getLanguageService()
-//   return service.context.inject('mdxCommands')
-// }