fix a number of diagnostic source mapping issues

Author: machty

packages/core/src/transform/template/template-to-typescript.ts

@@ -1150,13 +1150,19 @@ export function templateToTypescript(
       | AST.ElementModifierStatement;
 
     function emitResolve(node: CurlyInvocationNode, resolveType: string): void {
-      emit.text('χ.');
-      emit.text(resolveType);
-      emit.text('(');
-      emitExpression(node.path);
-      emit.text(')(');
-      emitArgs(node.params, node.hash);
-      emit.text(')');
+      // We use forNode here to wrap the emitted resolve expression here so that when
+      // we convert to Volar mappings, we can create a boundary around
+      // e.g. "χ.resolveOrReturn(expectsAtLeastOneArg)()", which is required because
+      // this is where TS might generate a diagnostic error.
+      emit.forNode(node, () => {
+        emit.text('χ.');
+        emit.text(resolveType);
+        emit.text('(');
+        emitExpression(node.path);
+        emit.text(')(');
+        emitArgs(node.params, node.hash);
+        emit.text(')');
+      });
     }
 
     function emitArgs(positional: Array<AST.Expression>, named: AST.Hash): void {
@@ -1171,24 +1177,31 @@ export function templateToTypescript(
 
       // Emit named args
       if (named.pairs.length) {
-        emit.text(positional.length ? ', { ' : '{ ');
+        if (positional.length) {
+          emit.text(', ');
+        }
 
-        let { start } = rangeForNode(named);
-        for (let [index, pair] of named.pairs.entries()) {
-          start = template.indexOf(pair.key, start);
-          emitHashKey(pair.key, start);
-          emit.text(': ');
-          emitExpression(pair.value);
+        // TS diagnostic error boundary
+        emit.forNode(named, () => {
+          emit.text('{ ');
 
-          if (index === named.pairs.length - 1) {
-            emit.text(' ');
-          }
+          let { start } = rangeForNode(named);
+          for (let [index, pair] of named.pairs.entries()) {
+            start = template.indexOf(pair.key, start);
+            emitHashKey(pair.key, start);
+            emit.text(': ');
+            emitExpression(pair.value);
 
-          start = rangeForNode(pair.value).end;
-          emit.text(', ');
-        }
+            if (index === named.pairs.length - 1) {
+              emit.text(' ');
+            }
 
-        emit.text('...χ.NamedArgsMarker }');
+            start = rangeForNode(pair.value).end;
+            emit.text(', ');
+          }
+
+          emit.text('...χ.NamedArgsMarker }');
+        });
       }
     }
 

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

@@ -219,16 +219,16 @@ export default class TransformedModule {
 
   /**
    * Converts the mappings in this transformed module to the format expected by Volar.
-   * 
+   *
    * The main difference between the two formats is that while the classic Glint transformation
    * mappings support mapping a differently sized source region to a differently sized target region
    * (e.g. `{{expectsAtLeastOneArg}}` in an .hbs file to `χ.emitContent(χ.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
    * 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:
-   * 
+   *
    * - `{{[[ZEROLEN-A]][[expectsAtLeastOneArg]][[ZEROLEN-B]]}}`
    * - to
    * - `[[ZEROLEN-A]]χ.emitContent(χ.resolveOrReturn([[expectsAtLeastOneArg]])());[[ZEROLEN-B]]`
@@ -248,46 +248,37 @@ export default class TransformedModule {
 
       if (children.length === 0) {
         // leaf node
-
-        const length = hbsEnd - hbsStart;
-
-        if (length === tsEnd - tsStart) {
+        const hbsLength = hbsEnd - hbsStart;
+        const tsLength = tsEnd - tsStart;
+        if (hbsLength === tsLength) {
           // (Hacky?) assumption: because TS and HBS span lengths are equivalent,
           // then this is a simple leafmost mapping, e.g. `{{this.[foo]}}` -> `this.[foo]`
           sourceOffsets.push(hbsStart);
           generatedOffsets.push(tsStart);
-          lengths.push(length);
+          lengths.push(hbsLength);
         } else {
-          // This isn't common but there are a few cases where we are not currently going as
-          // "deep" as we good into the mapping tree to produce equal-size leaf nodes; here
-          // is one example (using `toDebugString()`)
-          //
-          // | | | Mapping: MustacheStatement
-          // | | |  hbs(686:723): {{yield to="expectsAtLeastOneParam"}}
-          // | | |  ts(1025:1073):χ.yieldToBlock(𝚪, "expectsAtLeastOneParam")()
-
-          // It may make sense to rework the mappings for yields and other cases so that
-          // the leaf nodes are equal-sized identifiers
-          // (e.g. expectsAtLeastOneParam (hbs) -> expectsAtLeastOneParam(ts) ), but
-          // in the mean time we will just produce zero-length boundary markers for Volar.
+          // Disregard the "null zone" mappings, i.e. cases where TS code maps to empty HBS code
+          if (hbsLength > 0 && tsLength > 0) {
+            sourceOffsets.push(hbsStart);
+            generatedOffsets.push(tsStart);
+            lengths.push(0);
+            sourceOffsets.push(hbsEnd);
+            generatedOffsets.push(tsEnd);
+            lengths.push(0);
+          }
         }
       } else {
-        // here we want to install zero-length mappings on the boundaries
-
-        // TODO: consider re-enabling these zero-length boundary mappings, but for now
-        // they don't solve the problem of lack of granularity
-        // sourceOffsets.push(hbsStart);
-        // generatedOffsets.push(tsStart);
-        // lengths.push(0);
+        sourceOffsets.push(hbsStart);
+        generatedOffsets.push(tsStart);
+        lengths.push(0);
 
         mapping.children.forEach((child) => {
           recurse(span, child);
         });
 
-        // TODO: see above
-        // sourceOffsets.push(hbsEnd);
-        // generatedOffsets.push(tsEnd);
-        // lengths.push(0);
+        sourceOffsets.push(hbsEnd);
+        generatedOffsets.push(tsEnd);
+        lengths.push(0);
       }
     };
 
@@ -301,7 +292,6 @@ export default class TransformedModule {
         // transformation, we expect these to be the same length (in fact, they
         // should be the same string entirely)
 
-
         // This assertion seemed valid when parsing .gts files with extracted hbs in <template> tags,
         // but when parsing solo .hbs files in loose mode there were cases where, e.g.,
         // originalLength == 0 and transformLength == 1;