Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(eslint-plugin-template): [attributes-order] Calculate valueless structural directive start/end positions correctly #1726

Merged
merged 10 commits into from Mar 26, 2024
88 changes: 88 additions & 0 deletions packages/eslint-plugin-template/docs/rules/attributes-order.md
Expand Up @@ -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>
Expand Down
31 changes: 29 additions & 2 deletions packages/eslint-plugin-template/src/rules/attributes-order.ts
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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;
}
Expand Down
Expand Up @@ -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>

`,
}),
];