bug: glimmer-syntax-error comment injected inside of long helper invocations

[wondersloth]

Found internally, where a helper {{on}} had an injected comment {{! @glint-expect-error @rehearsal TODO ... }} in a region of the helper that is syntactically incorrect.

Input

{{! GOOD }}
{{concat @someArg1 @someArg2}}

{{! BAD }}
{{concat
  @someArg1
  @someArg2
  @someArg3
  @someArg4
  @someArg5
  @someArg6
  @someArg7
  @someArg8
  @someArg9
  @someArg10
}}

Actual

{{! GOOD }}
{{! @glint-expect-error @rehearsal TODO TS2339: Property 'someArg1' does not exist on type '{}'. }}
{{concat @someArg1 @someArg2}}

{{! BAD }}
{{concat
  {{! @glint-expect-error @rehearsal TODO TS2339: Property 'someArg1' does not exist on type '{}'. }}
  @someArg1
  {{! @glint-expect-error @rehearsal TODO TS2339: Property 'someArg2' does not exist on type '{}'. }}
  @someArg2
  {{! @glint-expect-error @rehearsal TODO TS2339: Property 'someArg3' does not exist on type '{}'. }}
  @someArg3
  {{! @glint-expect-error @rehearsal TODO TS2339: Property 'someArg4' does not exist on type '{}'. }}
  @someArg4
  {{! @glint-expect-error @rehearsal TODO TS2339: Property 'someArg5' does not exist on type '{}'. }}
  @someArg5
  {{! @glint-expect-error @rehearsal TODO TS2339: Property 'someArg6' does not exist on type '{}'. }}
  @someArg6
  {{! @glint-expect-error @rehearsal TODO TS2339: Property 'someArg7' does not exist on type '{}'. }}
  @someArg7
  {{! @glint-expect-error @rehearsal TODO TS2339: Property 'someArg8' does not exist on type '{}'. }}
  @someArg8
  {{! @glint-expect-error @rehearsal TODO TS2339: Property 'someArg9' does not exist on type '{}'. }}
  @someArg9
  {{! @glint-expect-error @rehearsal TODO TS2339: Property 'someArg10' does not exist on type '{}'. }}
  @someArg10
}}

Expected

To not produce invalidate glimmer syntax.

{{! @glint-expect-error @rehearsal TODO TS2339: Property 'someArg1' does not exist on type '{}'. }}
{{concat
  @someArg1
  @someArg2
  @someArg3
  @someArg4
  @someArg5
  @someArg6
  @someArg7
  @someArg8
  @someArg9
  @someArg10
}}

Observations

• Shorter invocations the comment gets hoisted above the helper due to a formatter (?), somewhere around line 100. I think this is determined by

Root Cause

When looking to inject a comment there is no logic to determine what kind of boundary the diagnostic is it.

rehearsal-js/packages/plugins/src/plugins/glint-comment.plugin.ts

Line 147 in 9ac19dd

while (diagnostic.file.text[i] === ' ') {

When we get the diagnostic starts at given position of the and increments until it finds the first non-white-space character.

Then injects a {{! glint-exect-error !}} above that region.

This logic is naive to how a handlebars files works and isn't aware if it's inside a helper {{}}

[wondersloth]

Related to:
typed-ember/glint#589

[wondersloth]

The solution in #1121
will not be able to fix this use case:

<div
  class="button-border-style some-style-1 some-style-2 some-style-3 some-style-4 some-style-5 some-style-6
    {{concat
      @someArg1
      ...
    }}"
></div>

It will attempt to inject a comment in an invalid region of the hbs file.

<div
  class="button-border-style some-style-1 some-style-2 some-style-3 some-style-4 some-style-5 some-style-6
    {{! @glint-expect-error  }}
    {{concat
      @someArg1
      ...
    }}"
></div>

See typed-ember/glint#589 for details on errors and why.

Problem

Problem is two parts:

1. Rehearsal needs to know how to inject the tag in the syntactically correct spot
2. Glint needs to be able to suppress an error, if not on the next line.

Proposal

Insert {{! glint-expect-error ...}} so it doesn't break syntax.

Improve rehearsal to parse hbs and identify the AST node that the error is on. From there determine the safest region to inject the error. In the case described in this issue, we should inject at the nearest ElementNode
See AST Explorer Example

Improve glint to have {{! glint-expect-error-next-(block|statement) }}

Improve glint-expect error with something like {{! glint-expect-error-next-(block|statement) }} such that it doesn't have to be the next line.