ember-template-lint / ember-template-recast

Non-destructive template transformer.
MIT License
40 stars 38 forks source link

if/else-if/else changes cause later clauses in the block to disappear #149

Closed timlindvall closed 4 years ago

timlindvall commented 4 years ago

AST Explorer example: https://astexplorer.net/#/gist/d803c2c92e9854a903347bbd38b2f541/49b6c5ab5ec1e2930ed86f29e13d514aabe2a116

It looks like ember-template-recast is getting thrown off by the specific structure of an else clause that contains an if within it. When this happens, it appears that (at minimum) the else clause disappears.

I've also included a failing test case where it appears an else if and else clause both disappear.

Observing this behavior in 3.1.3 (on AST Explorer) and 3.2.5 (test case run locally).

QUnit.test('If/else-if/else>if should not be affected by mutations elsewhere in block', function(assert) {
  let template = `
    {{#if a}}
      {{foo}}
    {{else if b}}
      {{bar}}
    {{else if c}}
      {{baz}}
    {{else}}
      {{#if d}}
        {{qux}}
      {{/if}}
    {{/if}}
  `;

  let expected = `
    {{#if a}}
      {{oof}}
    {{else if b}}
      {{bar}}
    {{else if c}}
      {{baz}}
    {{else}}
      {{#if d}}
        {{qux}}
      {{/if}}
    {{/if}}
  `;

  let { code } = transform(template, () => {
    return {
      MustacheStatement(node) {
        if (node.path.original === 'foo') {
          node.path.original = 'oof';
        }
      },
    };
  });

  assert.equal(code, expected);
});
not ok 150 "real life" smoke tests > If/else-if/else>if should not be affected by mutations elsewhere in block
  ---
  message: "failed"
  severity: failed
  actual:   "\n      {{#if a}}\n        {{oof}}\n      {{else if b}}\n        {{bar}}\n      {{/if}}\n    "
  expected: "\n      {{#if a}}\n        {{oof}}\n      {{else if b}}\n        {{bar}}\n      {{else if c}}\n        {{baz}}\n      {{else}}\n        {{#if d}}\n          {{qux}}\n        {{/if}}\n      {{/if}}\n    "
timlindvall commented 4 years ago

Update: This doesn't appear to be specific to the Else > If construct... just if a change is made in an if block, any clauses more than one away disappear.

https://astexplorer.net/#/gist/d803c2c92e9854a903347bbd38b2f541/d7758cb5a06e6b25c2da1ed54d60dd2efef1beca

rwjblue commented 4 years ago

This should have been fixed by https://github.com/ember-template-lint/ember-template-recast/pull/132, there must be additional cases that aren’t accounted for there. I’ll dig into your reproductions later this morning.

rwjblue commented 4 years ago

also, the version on astexplorer.net definitely does not have the fixes in #132 so it can’t be used as the source of truth atm. I have an open PR to update to the latest version.

timlindvall commented 4 years ago

Looking at the test cases that are currently in the repo, my running hypothesis is that only the program and inverse of a given edited block are getting printed for some reason. The test cases previously committed to the repo make edits to every block, so somehow that avoids this issue.

I've yet to pick out exactly where in the code these additional inverse blocks are getting dropped. My cursory reading of the code suggests it should be printing all the inverse blocks recursively until it reaches the end of the if/else block, but I'm probably missing something that actually stepping through the code line-by-line might yield.