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): [eqeqeq] Calculate offset to find true absolute source span #1709

Merged
merged 3 commits into from
Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
111 changes: 111 additions & 0 deletions packages/eslint-plugin-template/docs/rules/eqeqeq.md
Expand Up @@ -75,6 +75,33 @@ interface Options {

<br>

#### Default Config

```json
{
"rules": {
"@angular-eslint/template/eqeqeq": [
"error"
]
}
}
```

<br>

#### ❌ Invalid Code

```html
{{ 'null' == test }}
~~~~~~~~~~~~~~~
```

<br>

---

<br>

#### Custom Config

```json
Expand Down Expand Up @@ -105,6 +132,63 @@ interface Options {

<br>

#### Custom Config

```json
{
"rules": {
"@angular-eslint/template/eqeqeq": [
"error",
{
"allowNullOrUndefined": true
}
]
}
}
```

<br>

#### ❌ Invalid Code

```html
<div [attr.disabled]="test != 'undefined' && null == '3'"></div>
~~~~~~~~~~~~~~~~~~~~
```

<br>

---

<br>

#### Default Config

```json
{
"rules": {
"@angular-eslint/template/eqeqeq": [
"error"
]
}
}
```

<br>

#### ❌ Invalid Code

```html
<div [prop]="condition1 === 'value1' ? true : (condition2 != 'value2' ? true : false)}"></div>
~~~~~~~~~~~~~~~~~~~~~~
```

<br>

---

<br>

#### Default Config

```json
Expand Down Expand Up @@ -229,6 +313,33 @@ interface Options {

#### ❌ Invalid Code

```html
{{ c > d ? a != b : 'hey!' }}
~~~~~~~~~~~~
```

<br>

---

<br>

#### Default Config

```json
{
"rules": {
"@angular-eslint/template/eqeqeq": [
"error"
]
}
}
```

<br>

#### ❌ Invalid Code

```html
{{ c > d ? 'hey!' : a == false }}
~~~~~~~~~~
Expand Down
39 changes: 30 additions & 9 deletions packages/eslint-plugin-template/src/rules/eqeqeq.ts
Expand Up @@ -2,6 +2,7 @@
import {
ASTWithSource,
LiteralPrimitive,
Interpolation,
} from '@angular-eslint/bundled-angular-compiler';
import { ensureTemplateParser } from '@angular-eslint/utils';
import type { TSESLint } from '@typescript-eslint/utils';
Expand Down Expand Up @@ -73,15 +74,14 @@
data,
...(isStringNonNumericValue(left) || isStringNonNumericValue(right)
? {
fix: (fixer) =>
getFix({ node, left, right, start, end, fixer }),
fix: (fixer) => getFix({ node, right, end, sourceCode, fixer }),

Check warning on line 77 in packages/eslint-plugin-template/src/rules/eqeqeq.ts

View check run for this annotation

Codecov / codecov/patch

packages/eslint-plugin-template/src/rules/eqeqeq.ts#L77

Added line #L77 was not covered by tests
}
: {
suggest: [
{
messageId: 'suggestStrictEquality',
fix: (fixer) =>
getFix({ node, left, right, start, end, fixer }),
getFix({ node, right, end, sourceCode, fixer }),
data,
},
],
Expand All @@ -98,29 +98,46 @@

const getFix = ({
node,
left,
right,
start,
end,
sourceCode,
fixer,
}: {
node: Binary;
left: AST;
right: AST;
start: number;
end: number;
sourceCode: Readonly<TSESLint.SourceCode>;
fixer: TSESLint.RuleFixer;
}): TSESLint.RuleFix | null => {
const { source } = getNearestNodeFrom(node, isASTWithSource) ?? {};
const { source, ast } = getNearestNodeFrom(
node,
isASTWithSource,
) as ASTWithSource & { ast: unknown };

if (!source) return null;

let startOffet = 0;
while (!isInterpolation(ast) && isLeadingTriviaChar(source[startOffet])) {
startOffet++;
}

const endRange = end - startOffet - getSpanLength(right) - 1;
let eqOffset = 0;

while (sourceCode.text[endRange - eqOffset] !== '=') {
eqOffset++;
}

return fixer.insertTextAfterRange(
[start + getSpanLength(left) + 1, end - getSpanLength(right) - 1],
[endRange - eqOffset, endRange - eqOffset],
'=',
);
};

function isLeadingTriviaChar(char: string) {
return char === '\n' || char === ' ';
}

function isASTWithSource(node: unknown): node is ASTWithSource {
return node instanceof ASTWithSource;
}
Expand All @@ -129,6 +146,10 @@
return node instanceof LiteralPrimitive;
}

function isInterpolation(node: unknown): node is Interpolation {
return node instanceof Interpolation;
}

function isNumeric(value: unknown): value is number | string {
return (
!Number.isNaN(Number.parseFloat(String(value))) &&
Expand Down
78 changes: 78 additions & 0 deletions packages/eslint-plugin-template/tests/rules/eqeqeq/cases.ts
Expand Up @@ -34,6 +34,23 @@ export const invalid = [
~~~~~~~~~~~~~~~
`,
}),
convertAnnotatedSourceToFailureCase({
description:
'it should fail if the operation (surrounded by additional whitespace) is not strict within interpolation',
annotatedSource: `
{{ 'null' == test }}
~~~~~~~~~~~~~~~
`,
messageId,
data: {
actualOperation: '==',
expectedOperation: '===',
},
annotatedOutput: `
{{ 'null' === test }}
~~~~~~~~~~~~~~~~
`,
}),
convertAnnotatedSourceToFailureCase({
description:
'it should fail if the operation is not strict within attribute directive',
Expand All @@ -52,6 +69,41 @@ export const invalid = [
~~~~~~~~~~~~~~~~~~~~
`,
}),
convertAnnotatedSourceToFailureCase({
description:
'it should fail if the operation (surrounded by additional whitespace) is not strict within attribute directive',
annotatedSource: `
<div [attr.disabled]="test != 'undefined' && null == '3'"></div>
~~~~~~~~~~~~~~~~~~~~
`,
messageId,
data: {
actualOperation: '!=',
expectedOperation: '!==',
},
options: [{ allowNullOrUndefined: true }],
annotatedOutput: `
<div [attr.disabled]="test !== 'undefined' && null == '3'"></div>
~~~~~~~~~~~~~~~~~~~~~
`,
}),
convertAnnotatedSourceToFailureCase({
description:
'it should fail if the operation is not strict within attribute directive with nested ternary',
annotatedSource: `
<div [prop]="condition1 === 'value1' ? true : (condition2 != 'value2' ? true : false)}"></div>
~~~~~~~~~~~~~~~~~~~~~~
`,
messageId,
data: {
actualOperation: '!=',
expectedOperation: '!==',
},
annotatedOutput: `
<div [prop]="condition1 === 'value1' ? true : (condition2 !== 'value2' ? true : false)}"></div>
~~~~~~~~~~~~~~~~~~~~~~~
`,
}),
convertAnnotatedSourceToFailureCase({
description:
'it should fail if the operation is not strict within structural directive',
Expand Down Expand Up @@ -156,6 +208,32 @@ export const invalid = [
},
],
}),
convertAnnotatedSourceToFailureCase({
description:
'it should fail if the operation (surrounded by additional whitespace) is not strict within conditional (trueExp)',
annotatedSource: `
{{ c > d ? a != b : 'hey!' }}
~~~~~~~~~~~~
`,
messageId,
data: {
actualOperation: '!=',
expectedOperation: '!==',
},
suggestions: [
{
messageId: suggestStrictEquality,
output: `
{{ c > d ? a !== b : 'hey!' }}

`,
data: {
actualOperation: '!=',
expectedOperation: '!==',
},
},
],
}),
convertAnnotatedSourceToFailureCase({
description:
'it should fail if the operation is not strict within conditional (falseExp)',
Expand Down