Content-Length: 13930 | pFad | http://github.com/typescript-eslint/typescript-eslint/pull/9994.diff
thub.com
diff --git a/packages/eslint-plugin/docs/rules/no-confusing-non-null-assertion.mdx b/packages/eslint-plugin/docs/rules/no-confusing-non-null-assertion.mdx
index 0ff48e14c926..01a47a9dde78 100644
--- a/packages/eslint-plugin/docs/rules/no-confusing-non-null-assertion.mdx
+++ b/packages/eslint-plugin/docs/rules/no-confusing-non-null-assertion.mdx
@@ -9,14 +9,27 @@ import TabItem from '@theme/TabItem';
>
> See **https://typescript-eslint.io/rules/no-confusing-non-null-assertion** for documentation.
-Using a non-null assertion (`!`) next to an assign or equals check (`=` or `==` or `===`) creates code that is confusing as it looks similar to a not equals check (`!=` `!==`).
+Using a non-null assertion (`!`) next to an assignment or equality check (`=` or `==` or `===`) creates code that is confusing as it looks similar to an inequality check (`!=` `!==`).
```typescript
-a! == b; // a non-null assertions(`!`) and an equals test(`==`)
+a! == b; // a non-null assertion(`!`) and an equals test(`==`)
a !== b; // not equals test(`!==`)
-a! === b; // a non-null assertions(`!`) and an triple equals test(`===`)
+a! === b; // a non-null assertion(`!`) and a triple equals test(`===`)
```
+Using a non-null assertion (`!`) next to an in test (`in`) or an instanceof test (`instanceof`) creates code that is confusing since it may look like the operator is negated, but it is actually not.
+
+{/* prettier-ignore */}
+```typescript
+a! in b; // a non-null assertion(`!`) and an in test(`in`)
+a !in b; // also a non-null assertion(`!`) and an in test(`in`)
+!(a in b); // a negated in test
+
+a! instanceof b; // a non-null assertion(`!`) and an instanceof test(`instanceof`)
+a !instanceof b; // also a non-null assertion(`!`) and an instanceof test(`instanceof`)
+!(a instanceof b); // a negated instanceof test
+````
+
This rule flags confusing `!` assertions and suggests either removing them or wrapping the asserted expression in `()` parenthesis.
## Examples
diff --git a/packages/eslint-plugin/src/rules/no-confusing-non-null-assertion.ts b/packages/eslint-plugin/src/rules/no-confusing-non-null-assertion.ts
index 9caf777aabad..160827fc5a32 100644
--- a/packages/eslint-plugin/src/rules/no-confusing-non-null-assertion.ts
+++ b/packages/eslint-plugin/src/rules/no-confusing-non-null-assertion.ts
@@ -1,9 +1,36 @@
import type { TSESLint, TSESTree } from '@typescript-eslint/utils';
import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils';
+import type {
+ ReportDescriptor,
+ RuleFix,
+} from '@typescript-eslint/utils/ts-eslint';
import { createRule } from '../util';
-export default createRule({
+type MessageId =
+ | 'confusingEqual'
+ | 'confusingAssign'
+ | 'confusingOperator'
+ | 'notNeedInEqualTest'
+ | 'notNeedInAssign'
+ | 'notNeedInOperator'
+ | 'wrapUpLeft';
+
+const confusingOperators = new Set([
+ '=',
+ '==',
+ '===',
+ 'in',
+ 'instanceof',
+] as const);
+type ConfusingOperator =
+ typeof confusingOperators extends Set ? T : never;
+
+function isConfusingOperator(operator: string): operator is ConfusingOperator {
+ return confusingOperators.has(operator as ConfusingOperator);
+}
+
+export default createRule<[], MessageId>({
name: 'no-confusing-non-null-assertion',
meta: {
type: 'problem',
@@ -15,35 +42,63 @@ export default createRule({
hasSuggestions: true,
messages: {
confusingEqual:
- 'Confusing combinations of non-null assertion and equal test like "a! == b", which looks very similar to not equal "a !== b".',
+ 'Confusing combination of non-null assertion and equality test like `a! == b`, which looks very similar to `a !== b`.',
confusingAssign:
- 'Confusing combinations of non-null assertion and equal test like "a! = b", which looks very similar to not equal "a != b".',
- notNeedInEqualTest: 'Unnecessary non-null assertion (!) in equal test.',
+ 'Confusing combination of non-null assertion and assignment like `a! = b`, which looks very similar to `a != b`.',
+ confusingOperator:
+ 'Confusing combination of non-null assertion and `{{operator}}` operator like `a! {{operator}} b`, which might be misinterpreted as `!(a {{operator}} b)`.',
+
+ notNeedInEqualTest:
+ 'Remove unnecessary non-null assertion (!) in equality test.',
notNeedInAssign:
- 'Unnecessary non-null assertion (!) in assignment left hand.',
+ 'Remove unnecessary non-null assertion (!) in assignment left-hand side.',
+
+ notNeedInOperator:
+ 'Remove possibly unnecessary non-null assertion (!) in the left operand of the `{{operator}}` operator.',
+
wrapUpLeft:
- 'Wrap up left hand to avoid putting non-null assertion "!" and "=" together.',
+ 'Wrap the left-hand side in parentheses to avoid confusion with "{{operator}}" operator.',
},
schema: [],
},
defaultOptions: [],
create(context) {
+ function confusingOperatorToMessageData(
+ operator: ConfusingOperator,
+ ): Pick, 'messageId' | 'data'> {
+ switch (operator) {
+ case '=':
+ return {
+ messageId: 'confusingAssign',
+ };
+ case '==':
+ case '===':
+ return {
+ messageId: 'confusingEqual',
+ };
+ case 'in':
+ case 'instanceof':
+ return {
+ messageId: 'confusingOperator',
+ data: { operator },
+ };
+ // istanbul ignore next
+ default:
+ operator satisfies never;
+ throw new Error(`Unexpected operator ${operator as string}`);
+ }
+ }
+
return {
'BinaryExpression, AssignmentExpression'(
node: TSESTree.AssignmentExpression | TSESTree.BinaryExpression,
): void {
- function isLeftHandPrimaryExpression(
- node: TSESTree.Expression | TSESTree.PrivateIdentifier,
- ): boolean {
- return node.type === AST_NODE_TYPES.TSNonNullExpression;
- }
+ const operator = node.operator;
- if (
- node.operator === '==' ||
- node.operator === '===' ||
- node.operator === '='
- ) {
- const isAssign = node.operator === '=';
+ if (isConfusingOperator(operator)) {
+ // Look for a non-null assertion as the last token on the left hand side.
+ // That way, we catch things like `1 + two! === 3`, even though the left
+ // hand side isn't a non-null assertion AST node.
const leftHandFinalToken = context.sourceCode.getLastToken(node.left);
const tokenAfterLeft = context.sourceCode.getTokenAfter(node.left);
if (
@@ -51,32 +106,63 @@ export default createRule({
leftHandFinalToken.value === '!' &&
tokenAfterLeft?.value !== ')'
) {
- if (isLeftHandPrimaryExpression(node.left)) {
+ if (node.left.type === AST_NODE_TYPES.TSNonNullExpression) {
+ let suggestions: TSESLint.SuggestionReportDescriptor[];
+ switch (operator) {
+ case '=':
+ suggestions = [
+ {
+ messageId: 'notNeedInAssign',
+ fix: (fixer): RuleFix => fixer.remove(leftHandFinalToken),
+ },
+ ];
+ break;
+
+ case '==':
+ case '===':
+ suggestions = [
+ {
+ messageId: 'notNeedInEqualTest',
+ fix: (fixer): RuleFix => fixer.remove(leftHandFinalToken),
+ },
+ ];
+ break;
+
+ case 'in':
+ case 'instanceof':
+ suggestions = [
+ {
+ messageId: 'notNeedInOperator',
+ data: { operator },
+ fix: (fixer): RuleFix => fixer.remove(leftHandFinalToken),
+ },
+ {
+ messageId: 'wrapUpLeft',
+ data: { operator },
+ fix: wrapUpLeftFixer(node),
+ },
+ ];
+ break;
+
+ // istanbul ignore next
+ default:
+ operator satisfies never;
+ return;
+ }
context.report({
node,
- messageId: isAssign ? 'confusingAssign' : 'confusingEqual',
- suggest: [
- {
- messageId: isAssign
- ? 'notNeedInAssign'
- : 'notNeedInEqualTest',
- fix: (fixer): TSESLint.RuleFix[] => [
- fixer.remove(leftHandFinalToken),
- ],
- },
- ],
+ ...confusingOperatorToMessageData(operator),
+ suggest: suggestions,
});
} else {
context.report({
node,
- messageId: isAssign ? 'confusingAssign' : 'confusingEqual',
+ ...confusingOperatorToMessageData(operator),
suggest: [
{
messageId: 'wrapUpLeft',
- fix: (fixer): TSESLint.RuleFix[] => [
- fixer.insertTextBefore(node.left, '('),
- fixer.insertTextAfter(node.left, ')'),
- ],
+ data: { operator },
+ fix: wrapUpLeftFixer(node),
},
],
});
@@ -87,3 +173,12 @@ export default createRule({
};
},
});
+
+function wrapUpLeftFixer(
+ node: TSESTree.AssignmentExpression | TSESTree.BinaryExpression,
+): TSESLint.ReportFixFunction {
+ return (fixer): TSESLint.RuleFix[] => [
+ fixer.insertTextBefore(node.left, '('),
+ fixer.insertTextAfter(node.left, ')'),
+ ];
+}
diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-confusing-non-null-assertion.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-confusing-non-null-assertion.shot
index 074f8f721db5..fc3fe7bcc1be 100644
--- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-confusing-non-null-assertion.shot
+++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-confusing-non-null-assertion.shot
@@ -10,9 +10,9 @@ interface Foo {
const foo: Foo = getFoo();
const isEqualsBar = foo.bar! == 'hello';
- ~~~~~~~~~~~~~~~~~~~ Confusing combinations of non-null assertion and equal test like "a! == b", which looks very similar to not equal "a !== b".
+ ~~~~~~~~~~~~~~~~~~~ Confusing combination of non-null assertion and equality test like \`a! == b\`, which looks very similar to \`a !== b\`.
const isEqualsNum = 1 + foo.num! == 2;
- ~~~~~~~~~~~~~~~~~ Confusing combinations of non-null assertion and equal test like "a! == b", which looks very similar to not equal "a !== b".
+ ~~~~~~~~~~~~~~~~~ Confusing combination of non-null assertion and equality test like \`a! == b\`, which looks very similar to \`a !== b\`.
"
`;
diff --git a/packages/eslint-plugin/tests/rules/no-confusing-non-null-assertion.test.ts b/packages/eslint-plugin/tests/rules/no-confusing-non-null-assertion.test.ts
index 19f3cfec601d..6cdefc9b6c59 100644
--- a/packages/eslint-plugin/tests/rules/no-confusing-non-null-assertion.test.ts
+++ b/packages/eslint-plugin/tests/rules/no-confusing-non-null-assertion.test.ts
@@ -3,7 +3,7 @@
/* eslint "@typescript-eslint/internal/plugin-test-formatting": ["error", { formatWithPrettier: false }] */
/* eslint-enable eslint-comments/no-use */
-import { RuleTester } from '@typescript-eslint/rule-tester';
+import { noFormat, RuleTester } from '@typescript-eslint/rule-tester';
import rule from '../../src/rules/no-confusing-non-null-assertion';
@@ -18,6 +18,8 @@ ruleTester.run('no-confusing-non-null-assertion', rule, {
'a != b;',
'(a + b!) == c;',
'(a + b!) = c;',
+ '(a + b!) in c;',
+ '(a || b!) instanceof c;',
],
invalid: [
{
@@ -148,5 +150,74 @@ ruleTester.run('no-confusing-non-null-assertion', rule, {
},
],
},
+ {
+ code: 'a! in b;',
+ errors: [
+ {
+ messageId: 'confusingOperator',
+ data: { operator: 'in' },
+ line: 1,
+ column: 1,
+ suggestions: [
+ {
+ messageId: 'notNeedInOperator',
+ output: 'a in b;',
+ },
+ {
+ messageId: 'wrapUpLeft',
+ output: '(a!) in b;',
+ },
+ ],
+ },
+ ],
+ },
+ {
+ code: noFormat`
+a !in b;
+ `,
+ errors: [
+ {
+ messageId: 'confusingOperator',
+ data: { operator: 'in' },
+ line: 2,
+ column: 1,
+ suggestions: [
+ {
+ messageId: 'notNeedInOperator',
+ output: `
+a in b;
+ `,
+ },
+ {
+ messageId: 'wrapUpLeft',
+ output: `
+(a !)in b;
+ `,
+ },
+ ],
+ },
+ ],
+ },
+ {
+ code: 'a! instanceof b;',
+ errors: [
+ {
+ messageId: 'confusingOperator',
+ data: { operator: 'instanceof' },
+ line: 1,
+ column: 1,
+ suggestions: [
+ {
+ messageId: 'notNeedInOperator',
+ output: 'a instanceof b;',
+ },
+ {
+ messageId: 'wrapUpLeft',
+ output: '(a!) instanceof b;',
+ },
+ ],
+ },
+ ],
+ },
],
});
--- a PPN by Garber Painting Akron. With Image Size Reduction included!Fetched URL: http://github.com/typescript-eslint/typescript-eslint/pull/9994.diff
Alternative Proxies:
Alternative Proxy
pFad Proxy
pFad v3 Proxy
pFad v4 Proxy