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 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
169 changes: 85 additions & 84 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 nullish coalescing operator is not supported.

## 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 @@ -82,6 +82,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 @@ -54,6 +54,7 @@ import noMeaninglessVoidOperator from './no-meaningless-void-operator';
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 @@ -175,6 +176,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,108 @@
import {
ASTUtils,
TSESTree,
TSESLint,
} from '@typescript-eslint/experimental-utils';
import { Definition, DefinitionType } from '@typescript-eslint/scope-manager';
import * as util from '../util';

function hasAssignmentBeforeNode(
variable: TSESLint.Scope.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 === true || variableDeclarator.init !== null
);
}

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: 'Remove the non-null assertion.',
},
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 = ASTUtils.findVariable(scope, identifier.name);
if (variable && !hasAssignmentBeforeNode(variable, node)) {
return;
}
}

const sourceCode = context.getSourceCode();

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 {
const exclamationMark = util.nullThrows(
sourceCode.getLastToken(
node,
ASTUtils.isNonNullAssertionPunctuator,
),
util.NullThrowsReasons.MissingToken(
'!',
'Non-null Assertion',
),
);
return fixer.remove(exclamationMark);
},
},
],
});
},
};
},
});