Fix false negatives with block-indentation rule

[VincentMolinie]

Summary

A lot of issues were not detected by this rule.
The following examples are invalid but were considered as valid:

<div></div>
  <span></span>

  <span></span>

Basically, everything that is NOT on the first line and is a direct child of the template was considered as valid

[bmish]

I tested this on a large codebase and it detected a handful of correct new violations which is great.

I see a few issues however. The first issue might be a problem with this current PR, while the 2nd and 3rd issues might be an issue with the previous autofixing PR? Should we fix any of them in this PR or separate PRs?

1

Before:

  <title>Title</title>
  <g>
  </g>

After autofix (shouldn't <title> be outdented too? Seems like we didn't get a violation on <title>):

  <title>Title</title>
<g>
</g>

2

I'm also seeing an autofix crash here with this template:

<div>
</div>
  {{#if foo}}
    {{#if bar}}
    {{/if}}
  {{/if}}

ember-template-lint/lib/rules/block-indentation.js:101
      value = value[property];
                   ^

TypeError: Cannot read properties of undefined (reading 'body')
    at get (ember-template-lint/lib/rules/block-indentation.js:101:20)
    at BlockIndentation.fixLine (ember-template-lint/lib/rules/block-indentation.js:640:12)
    at BlockIndentation.validateBlockEnd (ember-template-lint/lib/rules/block-indentation.js:369:32)
    at BlockIndentation.process (ember-template-lint/lib/rules/block-indentation.js:238:22)
    at BlockIndentation.process (ember-template-lint/lib/rules/block-indentation.js:275:16)
    at BlockIndentation.BlockStatement (ember-template-lint/lib/rules/block-indentation.js:206:21)

3

I'm seeing another autofix crash with this:

  <h3></h3>
  <div>
    <div>
      <div>
      </div>
    </div>
  </div>

node:internal/process/promises:246
          triggerUncaughtException(err, true /* fromPromise */);
          ^
SyntaxError: Closing tag `div` (on line 7) did not match last open tag `d` (on line 1).
    at new SyntaxError (ember-template-lint/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/errors/syntax-error.js:18:23)
    at validateEndTag (ember-template-lint/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser/tokenizer-event-handlers.js:310:11)
    at TokenizerEventHandlers.finishEndTag (ember-template-lint/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser/tokenizer-event-handlers.js:168:5)
    at TokenizerEventHandlers.finishTag (ember-template-lint/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser/tokenizer-event-handlers.js:136:12)
    at EventedTokenizer.endTagName (ember-template-lint/node_modules/simple-html-tokenizer/dist/simple-html-tokenizer.js:212:39)
    at EventedTokenizer.tokenizePart (ember-template-lint/node_modules/simple-html-tokenizer/dist/simple-html-tokenizer.js:459:29)
    at TokenizerEventHandlers.ContentStatement (ember-template-lint/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser/handlebars-node-visitors.js:195:20)
    at TokenizerEventHandlers.acceptNode (ember-template-lint/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser.js:60:27)
    at TokenizerEventHandlers.Program (ember-template-lint/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser/handlebars-node-visitors.js:42:12)
    at TokenizerEventHandlers.acceptTemplate (ember-template-lint/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser.js:56:27) {
  location: {
    source: null,
    start: { line: 1, column: 0 },
    end: { line: undefined, column: undefined }
  }
}

Side note: Can you also add some more context in the PR description?

[VincentMolinie]

I added all your example. Should be good now 🙏

[bmish]

@VincentMolinie thanks for the quick fixes. I just tested it again and found these issues:

Issue 1

Before:

{{relativeDate}} <span class="my-apps-date-connected__absolute">({{absoluteDate}})</span>

After autofix (incorrectly removed spacing, also still flags a violation):

{{relativeDate}}<span class="my-apps-date-connected__absolute">({{absoluteDate}})</span>

Issue 2

Before:

<title></title>
<path/><path/>

Does not autofix this.

[VincentMolinie]

@VincentMolinie thanks for the quick fixes. I just tested it again and found these issues:

Issue 1

Before:

{{relativeDate}} <span class="my-apps-date-connected__absolute">({{absoluteDate}})</span>

After autofix (incorrectly removed spacing, also still flags a violation):

{{relativeDate}}<span class="my-apps-date-connected__absolute">({{absoluteDate}})</span>

Issue 2

Before:

<title></title>
<path/><path/>

Does not autofix this.

I don't know what should be the expected template in both cases 😅 . Should it be:

{{relativeDate}}
<span class="my-apps-date-connected__absolute">({{absoluteDate}})</span>

and

<title></title>
<path/>
<path/>

[bmish]

I think it should actually be valid to have multiple closed tags on the same line? So both of these should be valid / no violations.

[VincentMolinie]

Okey should be good now 👍

[bmish]

@VincentMolinie just tested it and it looks good, nice work!

Can you add a brief summary to the PR description about how this affects the rule behavior?

[VincentMolinie]

Done, let me know if that's enough