Add autofixer to attribute-indentation rule

[VincentMolinie]

The code is not clean at all but it's a beginning. Any help is welcome on the cleaning part 🙏

[VincentMolinie]

@bmish I think it's good 🤩

[bmish]

• Looks like linting needs to be fixed
• I see some commented-out code
• I think we still need some more comments throughout complex parts of the logic / flow to ensure readers understand what's happening

[VincentMolinie]

Back to you @bmish. Thanks for your review. FOr the test to pass I need the PR on ember-template-recast 🙏

[VincentMolinie]

@bmish should be all god now 😉

[bmish]

Can you fix CI?

[bmish]

Can you also make sure that all of the lines of code in this rule are covered by the tests? I just checked and see many lines missing coverage:

attribute-indentation.js                       |   95.19 |    90.55 |     100 |   95.16 | ...2,764,771,777,785,792-808 

yarn test:jest test/unit/rules/attribute-indentation-test.js

[bmish]

Coverage looks better but I still see the following uncovered lines:

yarn test:jest test/unit/rules/attribute-indentation-test.js

File                                             | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s            
-------------------------------------------------|---------|----------|---------|---------|------------------------------
  attribute-indentation.js                       |   98.42 |    93.56 |     100 |   98.41 | 544-546,757,797  

The code for this rule is quite long and complicated so it's even more important that we ensure all of its lines are covered by comprehensive tests.

Thanks for your work on this so far! Nearing the finish line.

[VincentMolinie]

Coverage looks better but I still see the following uncovered lines:

yarn test:jest test/unit/rules/attribute-indentation-test.js

File                                             | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s            
-------------------------------------------------|---------|----------|---------|---------|------------------------------
  attribute-indentation.js                       |   98.42 |    93.56 |     100 |   98.41 | 544-546,757,797  

The code for this rule is quite long and complicated so it's even more important that we ensure all of its lines are covered by comprehensive tests.

Thanks for your work on this so far! Nearing the finish line.

• 544-546: I did not touch those line. I don't really know how you get there and why ?
• 757: I tried to add a test but it never pass inside. It correspond to have config: false
• 797: Same thing as the previous one I tried to add config: undefined but it never pass that line 😞

[bmish]

@VincentMolinie thanks for the explanation. Those may be pre-existing issues then or even deadcode. So we can ignore that then.

The next step is to run this autofixer on a real codebase. I tried running on a large codebase and I'm hitting various crashes.

{{#sq-dropdown as |dropdown|}}

  {{#dropdown.trigger class="button super-table__plus-button"}}
    <i class="{{if isActive "icon-plus-small-hover" "icon-plus-small"}} super-table__plus-button__icon"></i>
  {{/dropdown.trigger}}

{{/sq-dropdown}}

{{directory/onboard-tooltip buttonText=(t "button.ok")
                            content=(t "foo")
                            flag=onboardTooltipFlag
                            flagPrevious=onboardTooltipFlagPrevious
                            popoverClass="foo"}}

lib/rules/attribute-indentation.js:94
    nodeToReturn._globalNode = path && globalNode;
                             ^
TypeError: Cannot set properties of undefined (setting '_globalNode')
    at AttributeIndentation.fixLine

And there appear to be many other crashes. Can you give this a try? Excited for the autofixer but it would be a shame if everyone hits crashes when they try to use it.

[bmish]

Thanks for fixing the previous issue. Here are the next crashes I found (I'll test your updated rule again after you fix these, there are probably more crashes still, you should also try to find a large codebase to test on):

Crash 2

{{#if showMerge}}
  <button class="button button--primary customer-list-bulk-actions__action" {{action (action merge)}}>
    Merge
  </button>
{{/if}}
{{#if showDelete}}
  <button class="button button--primary button--destructive customer-list-bulk-actions__action" {{action (action delete)}} disabled={{disableDelete}}>
    Delete
  </button>
{{/if}}

Error [SyntaxError]: Closing tag </button> without an open tag: 

|
|  </button>

Crash 3

{{#if isSuccessMessageVisible}}
  {{#banner-static type="SUCCESS"
                   dismiss=(action "dismiss" "SUCCESS" false)
                   data-foo=true}}
  {{/banner-static}}
{{/if}}
{{#if isWarningMessageVisible}}
  {{#banner-static type="WARNING"
                   dismiss=(action "dismiss" "WARNING" false)
                   data-foo=true}}
  {{/banner-static}}
{{/if}}

Error: Parse error on line 13:
...ic type="WARNING"  {{/banner-static}}{
----------------------^
Expecting 'CLOSE_RAW_BLOCK', 'CLOSE', 'CLOSE_UNESCAPED', 'OPEN_SEXPR', 'CLOSE_SEXPR', 'ID', 'OPEN_BLOCK_PARAMS', 'STRING', 'NUMBER', 'BOOLEAN', 'UNDEFINED', 'NULL', 'DATA', got 'OPEN_ENDBLOCK'
    at Parser.parseError (ember-template-lint/node_modules/@handlebars/parser/dist/cjs/parser.js:261:29)
    at Parser.parse (ember-template-lint/node_modules/@handlebars/parser/dist/cjs/parser.js:330:26)
    at parseWithoutProcessing (ember-template-lint/node_modules/@handlebars/parser/dist/cjs/parse.js:45:32)
    at preprocess (ember-template-lint/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser/tokenizer-event-handlers.js:347:48)
    at new ParseResult (ember-template-lint/node_modules/ember-template-recast/lib/parse-result.js:101:43)
    at parse (ember-template-lint/node_modules/ember-template-recast/lib/index.js:12:12)
    at AttributeIndentation.fixLine (file://ember-template-lint/lib/rules/attribute-indentation.js:81:29)
    at AttributeIndentation.validateCloseBrace (file://ember-template-lint/lib/rules/attribute-indentation.js:629:21)
    at AttributeIndentation.validateBlockForm (file://ember-template-lint/lib/rules/attribute-indentation.js:707:17)
    at AttributeIndentation.validateBlockStatement (file://ember-template-lint/lib/rules/attribute-indentation.js:753:24) {

Crash 4

The attribute-indentation autofix on this template causes a crash in the block-indentation rule:

{{#if isErrorFlow}}
  {{#content.section key=SELECTOR_STEP.DOWNLOAD_REPORT}}
    <h2 data-test-directory-import-sheet-step-header>{{t "customers.list.sheet.import.downloadReport.header" count=importService.errorStatus.rowsNotImportedCount}}</h2>
    <a href={{errorReportUrl}}
        {{! template-lint-disable no-element-event-actions }}
        data-test-directory-import-sheet-report-download-btn>
      {{t "customers.list.sheet.import.btn.downloadReport"}}
    </a>
  {{/content.section}}
{{/if}}

ember-template-lint/lib/rules/block-indentation.js:628
    let inverse = node.inverse;
                       ^

TypeError: Cannot read properties of undefined (reading 'inverse')
    at BlockIndentation.detectNestedElseIfBlock (ember-template-lint/lib/rules/block-indentation.js:628:24)
    at BlockIndentation.fixLine (ember-template-lint/lib/rules/block-indentation.js:618:14)
    at BlockIndentation.validateBlockChildren (ember-template-lint/lib/rules/block-indentation.js:453:28)
    at BlockIndentation.process (ember-template-lint/lib/rules/block-indentation.js:217:22)
    at BlockIndentation.process (ember-template-lint/lib/rules/block-indentation.js:253:16)
    at BlockIndentation.BlockStatement (ember-template-lint/lib/rules/block-indentation.js:182:21)

Issue 5

<h4 data-test-directory-import-guide-tile-title>{{title}}</h4>
<p data-test-directory-import-guide-tile-content>{{content}}</p>
{{sq-help-link articleId="6632"
               articleSection="field-type-support"}}

Incorrectly autofixes to (note = with no attribute value):

<h4 data-test-directory-import-guide-tile-title=>{{title}}</h4>
<p data-test-directory-import-guide-tile-content=>{{content}}</p>
{{sq-help-link
  articleId="6632"
  articleSection="field-type-support"
}}

[VincentMolinie]

Hey @bmish I don't have time lately to handle those crash sorry. I'll try asap