fix(eslint-plugin-template): [attributes-order] calculate valueless s…

…tructural directive start/end positions correctly (#1726)
angular-eslint · Mar 26, 2024 · dc03968 · dc03968
1 parent 529e035
commit dc03968
Show file tree

Hide file tree

Showing 3 changed files with 183 additions and 2 deletions.
diff --git a/packages/eslint-plugin-template/docs/rules/attributes-order.md b/packages/eslint-plugin-template/docs/rules/attributes-order.md
@@ -661,6 +661,94 @@ interface Options {
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 ```
 
+<br>
+
+---
+
+<br>
+
+#### Custom Config
+
+```json
+{
+ "rules": {
+ "@angular-eslint/template/attributes-order": [
+ "error",
+ {
+ "alphabetical": true,
+ "order": []
+ }
+ ]
+ }
+}
+```
+
+<br>
+
+#### ❌ Invalid Code
+
+```html
+<div *structuralDirective class="abc"></div>
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+```
+
+<br>
+
+---
+
+<br>
+
+#### Custom Config
+
+```json
+{
+ "rules": {
+ "@angular-eslint/template/attributes-order": [
+ "error",
+ {
+ "alphabetical": true
+ }
+ ]
+ }
+}
+```
+
+<br>
+
+#### ❌ Invalid Code
+
+```html
+<div title="abc" *structuralDirective abbr="abc"></div>
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+```
+
+<br>
+
+---
+
+<br>
+
+#### Default Config
+
+```json
+{
+ "rules": {
+ "@angular-eslint/template/attributes-order": [
+ "error"
+ ]
+ }
+}
+```
+
+<br>
+
+#### ❌ Invalid Code
+
+```html
+<div class="abc" *structuralDirective></div>
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+```
+
 </details>
 
 <br>

diff --git a/packages/eslint-plugin-template/src/rules/attributes-order.ts b/packages/eslint-plugin-template/src/rules/attributes-order.ts
@@ -126,7 +126,8 @@ export default createESLintRule<Options, MessageIds>({
  },
  end: {
  line: loc.end.line,
- column: loc.end.column + 1,
+ column:
+ loc.end.column + (isValuelessStructuralDirective(attr) ? 0 : 1),
  },
  };
  default:
@@ -423,6 +424,29 @@ function getMessageName(expected: ExtendedAttribute): string {
  }
 }
 
+function isValuelessStructuralDirective(attr: ExtendedAttribute): boolean {
+ if (attr.orderType !== OrderType.StructuralDirective || !attr.keySpan) {
+ return false;
+ }
+
+ const attrSpan = attr.sourceSpan;
+ const keySpan = attr.keySpan;
+
+ /**
+ * A valueless structural directive will have the same span as its key.
+ * TextAttribute[value=''] is not always a reliable selector, because
+ * a *structuralDirective with `let var = something` will have value = ''
+ */
+ return (
+ attrSpan.start.offset === keySpan.start.offset &&
+ attrSpan.start.line === keySpan.start.line &&
+ attrSpan.start.col === keySpan.start.col &&
+ attrSpan.end.offset === keySpan.end.offset &&
+ attrSpan.end.line === keySpan.end.line &&
+ attrSpan.end.col === keySpan.end.col
+ );
+}
+
 function getStartPos(expected: ExtendedAttribute): number {
  switch (expected.orderType) {
  case OrderType.StructuralDirective:
@@ -435,7 +459,10 @@ function getStartPos(expected: ExtendedAttribute): number {
 function getEndPos(expected: ExtendedAttribute): number {
  switch (expected.orderType) {
  case OrderType.StructuralDirective:
- return expected.sourceSpan.end.offset + 1;
+ return (
+ expected.sourceSpan.end.offset +
+ (isValuelessStructuralDirective(expected) ? 0 : 1)
+ );
  default:
  return expected.sourceSpan.end.offset;
  }

diff --git a/packages/eslint-plugin-template/tests/rules/attributes-order/cases.ts b/packages/eslint-plugin-template/tests/rules/attributes-order/cases.ts
@@ -400,4 +400,70 @@ export const invalid = [
  
  `,
  }),
+ convertAnnotatedSourceToFailureCase({
+ messageId,
+ description: `should work with valueless structural directive at beginning`,
+ annotatedSource: `
+ <div *structuralDirective class="abc"></div>
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ `,
+ options: [
+ {
+ alphabetical: true,
+ order: [
+ OrderType.AttributeBinding,
+ OrderType.TemplateReferenceVariable,
+ OrderType.InputBinding,
+ OrderType.OutputBinding,
+ OrderType.TwoWayBinding,
+ OrderType.StructuralDirective,
+ ],
+ },
+ ],
+ data: {
+ expected: '`class`, `*structuralDirective`',
+ actual: '`*structuralDirective`, `class`',
+ },
+ annotatedOutput: `
+ <div class="abc" *structuralDirective></div>
+ 
+ `,
+ }),
+
+ convertAnnotatedSourceToFailureCase({
+ messageId,
+ description:
+ 'should work with valueless structural directive with no value in middle',
+ annotatedSource: `
+ <div title="abc" *structuralDirective abbr="abc"></div>
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ `,
+ options: [{ alphabetical: true }],
+ data: {
+ expected: '`*structuralDirective`, `abbr`, `title`',
+ actual: '`title`, `*structuralDirective`, `abbr`',
+ },
+ annotatedOutput: `
+ <div *structuralDirective abbr="abc" title="abc"></div>
+ 
+ `,
+ }),
+
+ convertAnnotatedSourceToFailureCase({
+ messageId,
+ description:
+ 'should work with valueless structural directive with no value at end',
+ annotatedSource: `
+ <div class="abc" *structuralDirective></div>
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ `,
+ data: {
+ expected: '`*structuralDirective`, `class`',
+ actual: '`class`, `*structuralDirective`',
+ },
+ annotatedOutput: `
+ <div *structuralDirective class="abc"></div>
+ 
+ `,
+ }),
 ];