Skip to content

Commit

Permalink
fix(eslint-plugin-template): [eqeqeq] change fix to suggest (#465)
Browse files Browse the repository at this point in the history
  • Loading branch information
@rafaelss95
rafaelss95 authored May 18, 2021
1 parent 3cb9423 commit a497fde
Show file tree
Hide file tree
Showing 2 changed files with 178 additions and 54 deletions.
100 changes: 80 additions & 20 deletions packages/eslint-plugin-template/src/rules/eqeqeq.ts
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import type { AST, Binary } from '@angular/compiler';
import { ASTWithSource, LiteralPrimitive } from '@angular/compiler';
import type { TSESLint } from '@typescript-eslint/experimental-utils';
import {
createESLintRule,
ensureTemplateParser,
} from '../utils/create-eslint-rule';
import { getNearestNodeFrom } from '../utils/get-nearest-node-from';

type Options = [{ readonly allowNullOrUndefined?: boolean }];
export type MessageIds = 'eqeqeq';
export type MessageIds = 'eqeqeq' | 'suggestStrictEquality';
export const RULE_NAME = 'eqeqeq';
const DEFAULT_OPTIONS: Options[0] = { allowNullOrUndefined: false };

Expand All @@ -19,6 +20,7 @@ export default createESLintRule<Options, MessageIds>({
description: 'Requires `===` and `!==` in place of `==` and `!=`',
category: 'Best Practices',
recommended: 'error',
suggestion: true,
},
fixable: 'code',
schema: [
Expand All @@ -36,6 +38,8 @@ export default createESLintRule<Options, MessageIds>({
messages: {
eqeqeq:
'Expected `{{expectedOperation}}` but received `{{actualOperation}}`',
suggestStrictEquality:
'Replace `{{expectedOperation}}` with `{{actualOperation}}`',
},
},
defaultOptions: [DEFAULT_OPTIONS],
Expand All @@ -51,30 +55,37 @@ export default createESLintRule<Options, MessageIds>({
right,
sourceSpan: { start, end },
} = node;
const isNilComparison = [left, right].some(isNilValue);

if (allowNullOrUndefined && isNilComparison) return;
if (allowNullOrUndefined && (isNilValue(left) || isNilValue(right))) {
return;
}

const data = {
actualOperation: operation,
expectedOperation: `${operation}=`,
} as const;
context.report({
loc: {
start: sourceCode.getLocFromIndex(start),
end: sourceCode.getLocFromIndex(end),
},
messageId: 'eqeqeq',
data: {
actualOperation: operation,
expectedOperation: `${operation}=`,
},
fix: (fixer) => {
const { source } = getNearestNodeFrom(node, isASTWithSource) ?? {};

if (!source) return [];

return fixer.insertTextAfterRange(
[start + getSpanLength(left) + 1, end - getSpanLength(right) - 1],
'=',
);
},
data,
...(isStringNonNumericValue(left) || isStringNonNumericValue(right)
? {
fix: (fixer) =>
getFix({ node, left, right, start, end, fixer }),
}
: {
suggest: [
{
messageId: 'suggestStrictEquality',
fix: (fixer) =>
getFix({ node, left, right, start, end, fixer }),
data,
},
],
}),
});
},
};
Expand All @@ -85,13 +96,62 @@ function getSpanLength({ span: { start, end } }: AST): number {
return end - start;
}

const getFix = ({
node,
left,
right,
start,
end,
fixer,
}: {
node: Binary;
left: AST;
right: AST;
start: number;
end: number;
fixer: TSESLint.RuleFixer;
}): TSESLint.RuleFix | TSESLint.RuleFix[] => {
const { source } = getNearestNodeFrom(node, isASTWithSource) ?? {};

if (!source) return [];

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

function isASTWithSource(node: unknown): node is ASTWithSource {
return node instanceof ASTWithSource;
}

function isNilValue(ast: AST): ast is LiteralPrimitive {
function isLiteralPrimitive(node: unknown): node is LiteralPrimitive {
return node instanceof LiteralPrimitive;
}

function isNumeric(value: unknown): value is number | string {
return (
!Number.isNaN(Number.parseFloat(String(value))) &&
Number.isFinite(Number(value))
);
}

function isString(value: unknown): value is string {
return typeof value === 'string';
}

function isStringNonNumericValue(
ast: AST,
): ast is LiteralPrimitive & { value: string } {
return (
isLiteralPrimitive(ast) && isString(ast.value) && !isNumeric(ast.value)
);
}

function isNilValue(
ast: AST,
): ast is LiteralPrimitive & { value: null | undefined } {
return (
ast instanceof LiteralPrimitive &&
(ast.value === null || ast.value === undefined)
isLiteralPrimitive(ast) && (ast.value === null || ast.value === undefined)
);
}
132 changes: 98 additions & 34 deletions packages/eslint-plugin-template/tests/rules/eqeqeq.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const ruleTester = new RuleTester({
parser: '@angular-eslint/template-parser',
});
const messageId: MessageIds = 'eqeqeq';
const suggestStrictEquality: MessageIds = 'suggestStrictEquality';

ruleTester.run(RULE_NAME, rule, {
valid: [
Expand All @@ -31,7 +32,7 @@ ruleTester.run(RULE_NAME, rule, {
description:
'it should fail if the operation is not strict within interpolation',
annotatedSource: `
{{ test == 'null' }}
{{ 'null' == test }}
~~~~~~~~~~~~~~
`,
messageId,
Expand All @@ -40,15 +41,15 @@ ruleTester.run(RULE_NAME, rule, {
expectedOperation: '===',
},
annotatedOutput: `
{{ test === 'null' }}
{{ 'null' === test }}
~~~~~~~~~~~~~~~
`,
}),
convertAnnotatedSourceToFailureCase({
description:
'it should fail if the operation is not strict within attribute directive',
annotatedSource: `
<div [attr.disabled]="test != 'undefined' && null == 3"></div>
<div [attr.disabled]="test != 'undefined' && null == '3'"></div>
~~~~~~~~~~~~~~~~~~~
`,
messageId,
Expand All @@ -58,7 +59,7 @@ ruleTester.run(RULE_NAME, rule, {
},
options: [{ allowNullOrUndefined: true }],
annotatedOutput: `
<div [attr.disabled]="test !== 'undefined' && null == 3"></div>
<div [attr.disabled]="test !== 'undefined' && null == '3'"></div>
~~~~~~~~~~~~~~~~~~~~
`,
}),
Expand All @@ -74,27 +75,45 @@ ruleTester.run(RULE_NAME, rule, {
actualOperation: '==',
expectedOperation: '===',
},
annotatedOutput: `
suggestions: [
{
messageId: suggestStrictEquality,
output: `
<div *ngIf="test === true || test1 !== undefined"></div>
~~~~~~~~~~~~~
`,
`,
data: {
actualOperation: '==',
expectedOperation: '===',
},
},
],
}),
convertAnnotatedSourceToFailureCase({
description:
'it should fail if the operation is not strict within conditional',
annotatedSource: `
{{ one != two ? c > d : 'hey!' }}
~~~~~~~~~~
{{ one != '02' ? c > d : 'hey!' }}
~~~~~~~~~~~
`,
messageId,
data: {
actualOperation: '!=',
expectedOperation: '!==',
},
annotatedOutput: `
{{ one !== two ? c > d : 'hey!' }}
~~~~~~~~~~~
`,
suggestions: [
{
messageId: suggestStrictEquality,
output: `
{{ one !== '02' ? c > d : 'hey!' }}
`,
data: {
actualOperation: '!=',
expectedOperation: '!==',
},
},
],
}),
convertAnnotatedSourceToFailureCase({
description:
Expand All @@ -108,10 +127,19 @@ ruleTester.run(RULE_NAME, rule, {
actualOperation: '==',
expectedOperation: '===',
},
annotatedOutput: `
suggestions: [
{
messageId: suggestStrictEquality,
output: `
{{ a === b && 1 === b ? c > d : 'hey!' }}
~~~~~~~
`,
`,
data: {
actualOperation: '==',
expectedOperation: '===',
},
},
],
}),
convertAnnotatedSourceToFailureCase({
description:
Expand All @@ -125,45 +153,72 @@ ruleTester.run(RULE_NAME, rule, {
actualOperation: '!=',
expectedOperation: '!==',
},
annotatedOutput: `
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)',
annotatedSource: `
{{ c > d ? 'hey!' : a == b }}
~~~~~~
{{ c > d ? 'hey!' : a == false }}
~~~~~~~~~~
`,
messageId,
data: {
actualOperation: '==',
expectedOperation: '===',
},
annotatedOutput: `
{{ c > d ? 'hey!' : a === b }}
~~~~~~~
`,
suggestions: [
{
messageId: suggestStrictEquality,
output: `
{{ c > d ? 'hey!' : a === false }}
`,
data: {
actualOperation: '==',
expectedOperation: '===',
},
},
],
}),
convertAnnotatedSourceToFailureCase({
description:
'it should fail if the operation is not strict within recursive conditional',
annotatedSource: `
{{ undefined == test1 && a === b ? (c > d ? d != 3 : v === 4) : 'hey!' }}
~~~~~~
{{ undefined == test1 && a === b ? (c > d ? d != '0' : v === 4) : 'hey!' }}
~~~~~~~~
`,
messageId,
options: [{ allowNullOrUndefined: true }],
data: {
actualOperation: '!=',
expectedOperation: '!==',
},
annotatedOutput: `
{{ undefined == test1 && a === b ? (c > d ? d !== 3 : v === 4) : 'hey!' }}
~~~~~~~
`,
suggestions: [
{
messageId: suggestStrictEquality,
output: `
{{ undefined == test1 && a === b ? (c > d ? d !== '0' : v === 4) : 'hey!' }}
`,
data: {
actualOperation: '!=',
expectedOperation: '!==',
},
},
],
}),
convertAnnotatedSourceToFailureCase({
description:
Expand All @@ -177,10 +232,19 @@ ruleTester.run(RULE_NAME, rule, {
actualOperation: '!=',
expectedOperation: '!==',
},
annotatedOutput: `
suggestions: [
{
messageId: suggestStrictEquality,
output: `
{{ undefined !== test1 }}
~~~~~~~~~~~~~~~~~~~
`,
`,
data: {
actualOperation: '!=',
expectedOperation: '!==',
},
},
],
}),
],
});

0 comments on commit a497fde

Please sign in to comment.