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 1 commit
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
42 changes: 38 additions & 4 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 @@ -74,14 +75,22 @@
...(isStringNonNumericValue(left) || isStringNonNumericValue(right)
? {
fix: (fixer) =>
getFix({ node, left, right, start, end, fixer }),
getFix({ node, left, right, start, end, sourceCode, fixer }),
}
: {
suggest: [
{
messageId: 'suggestStrictEquality',
fix: (fixer) =>
getFix({ node, left, right, start, end, fixer }),
getFix({

Check warning on line 85 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#L85

Added line #L85 was not covered by tests
node,
left,
right,

Check warning on line 88 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#L88

Added line #L88 was not covered by tests
start,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how to test the code format change 😄

end,
sourceCode,
fixer,
}),
data,
},
],
Expand All @@ -102,25 +111,46 @@
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],
[start + getSpanLength(left) + 1, 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 +159,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