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

feat(eslint-plugin): add no-non-null-asserted-nullish-coalescing rule #3349

Merged
Show file tree
Hide file tree
Changes from 3 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
165 changes: 83 additions & 82 deletions packages/eslint-plugin/README.md

Large diffs are not rendered by default.

@@ -0,0 +1,49 @@
# Disallows using a non-null assertion in the left operand of the nullish coalescing operator (`no-non-null-asserted-nullish-coalescing`)

## Rule Details

The nullish coalescing operator is designed to provide a default value when dealing with `null` or `undefined`.
Using non-null assertions in the left operand of the nullish coalescing operator is redundant.

Examples of **incorrect** code for this rule:

```ts
/* eslint @typescript-eslint/no-non-null-asserted-nullish-coalescing: "error" */

foo! ?? bar;
foo.bazz! ?? bar;
foo!.bazz! ?? bar;
foo()! ?? bar;

let x!: string;
x! ?? '';

let x: string;
x = foo();
x! ?? '';
```

Examples of **correct** code for this rule:

```ts
/* eslint @typescript-eslint/no-non-null-asserted-nullish-coalescing: "error" */

foo ?? bar;
foo ?? bar!;
foo!.bazz ?? bar;
foo!.bazz ?? bar!;
foo() ?? bar;

// This is considered correct code because because there's no way for the user to satisfy it.
let x: string;
x! ?? '';
```

## When Not To Use It

If you are not using TypeScript 3.7 (or greater), then you will not need to use this rule, as the operator is not supported.
sonallux marked this conversation as resolved.
Show resolved Hide resolved

## Further Reading

- [TypeScript 3.7 Release Notes](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html)
- [Nullish Coalescing Proposal](https://github.com/tc39/proposal-nullish-coalescing)
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.ts
Expand Up @@ -81,6 +81,7 @@ export = {
'@typescript-eslint/no-misused-new': 'error',
'@typescript-eslint/no-misused-promises': 'error',
'@typescript-eslint/no-namespace': 'error',
'@typescript-eslint/no-non-null-asserted-nullish-coalescing': 'error',
'@typescript-eslint/no-non-null-asserted-optional-chain': 'error',
'@typescript-eslint/no-non-null-assertion': 'error',
'@typescript-eslint/no-parameter-properties': 'error',
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -53,6 +53,7 @@ import noMagicNumbers from './no-magic-numbers';
import noMisusedNew from './no-misused-new';
import noMisusedPromises from './no-misused-promises';
import noNamespace from './no-namespace';
import noNonNullAssertedNullishCoalescing from './no-non-null-asserted-nullish-coalescing';
import noNonNullAssertedOptionalChain from './no-non-null-asserted-optional-chain';
import noNonNullAssertion from './no-non-null-assertion';
import noParameterProperties from './no-parameter-properties';
Expand Down Expand Up @@ -171,6 +172,7 @@ export default {
'no-misused-new': noMisusedNew,
'no-misused-promises': noMisusedPromises,
'no-namespace': noNamespace,
'no-non-null-asserted-nullish-coalescing': noNonNullAssertedNullishCoalescing,
'no-non-null-asserted-optional-chain': noNonNullAssertedOptionalChain,
'no-non-null-assertion': noNonNullAssertion,
'no-parameter-properties': noParameterProperties,
Expand Down
@@ -0,0 +1,94 @@
import { TSESTree, TSESLint } from '@typescript-eslint/experimental-utils';
import {
Definition,
DefinitionType,
Variable,
} from '@typescript-eslint/scope-manager';
import * as util from '../util';

function hasAssignmentBeforeNode(
variable: Variable,
node: TSESTree.Node,
): boolean {
return (
variable.references.some(
ref => ref.isWrite() && ref.identifier.range[1] < node.range[1],
) ||
variable.defs.some(
def =>
isDefinitionWithAssignment(def) && def.node.range[1] < node.range[1],
)
);
}

function isDefinitionWithAssignment(definition: Definition): boolean {
if (definition.type !== DefinitionType.Variable) {
return false;
}

const variableDeclarator = definition.node;
return variableDeclarator.definite ?? variableDeclarator.init !== null;
sonallux marked this conversation as resolved.
Show resolved Hide resolved
}

export default util.createRule({
name: 'no-non-null-asserted-nullish-coalescing',
meta: {
type: 'problem',
docs: {
description:
'Disallows using a non-null assertion in the left operand of the nullish coalescing operator',
category: 'Possible Errors',
recommended: false,
suggestion: true,
},
messages: {
noNonNullAssertedNullishCoalescing:
'The nullish coalescing operator is designed to handle undefined and null - using a non-null assertion is not needed.',
suggestRemovingNonNull: 'You should remove the non-null assertion.',
sonallux marked this conversation as resolved.
Show resolved Hide resolved
},
schema: [],
},
defaultOptions: [],
create(context) {
return {
'LogicalExpression[operator = "??"] > TSNonNullExpression.left'(
node: TSESTree.TSNonNullExpression,
): void {
if (node.expression.type === TSESTree.AST_NODE_TYPES.Identifier) {
const scope = context.getScope();
const identifier = node.expression;
const variable = scope.set.get(identifier.name);
sonallux marked this conversation as resolved.
Show resolved Hide resolved
if (variable && !hasAssignmentBeforeNode(variable, node)) {
return;
}
}

context.report({
node,
messageId: 'noNonNullAssertedNullishCoalescing',
/*
Use a suggestion instead of a fixer, because this can break type checks.
The resulting type of the nullish coalesce is only influenced by the right operand if the left operand can be `null` or `undefined`.
After removing the non-null assertion the type of the left operand might contain `null` or `undefined` and then the type of the right operand
might change the resulting type of the nullish coalesce.
See the following example:

function test(x?: string): string {
const bar = x! ?? false; // type analysis reports `bar` has type `string`
// x ?? false; // type analysis reports `bar` has type `string | false`
return bar;
}
*/
suggest: [
{
messageId: 'suggestRemovingNonNull',
fix(fixer): TSESLint.RuleFix {
return fixer.removeRange([node.range[1] - 1, node.range[1]]);
sonallux marked this conversation as resolved.
Show resolved Hide resolved
},
},
],
});
},
};
},
});