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 rule no-confusing-void-expression #2605

Merged
merged 26 commits into from Nov 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
b3390ef
feat(eslint-plugin): add no-void-expr
phaux Sep 26, 2020
9cb2dfa
feat(eslint-plugin): add ignoreArrowShorthand
phaux Sep 26, 2020
d66fb00
feat(eslint-plugin): add ignoreVoidExpr
phaux Sep 26, 2020
2cedaa6
feat(eslint-plugin): add more messages and fixes
phaux Sep 27, 2020
0a4ed4e
feat(eslint-plugin): no-void-expression docs
phaux Sep 27, 2020
d116ab3
feat(eslint-plugin): oops
phaux Sep 27, 2020
7d54e93
feat(eslint-plugin): meh
phaux Sep 27, 2020
877e84c
feat(eslint-plugin): the words short-circuit
phaux Sep 27, 2020
4e7afea
feat(eslint-plugin): better tests
phaux Sep 27, 2020
a2d0913
feat(eslint-plugin): clearer phrasing in docs
phaux Sep 28, 2020
a240722
feat(eslint-plugin): code review fixes
phaux Sep 28, 2020
ee3e57a
feat(eslint-plugin): spaces in messages
phaux Sep 29, 2020
f353daa
feat(eslint-plugin): use rule in its own impl
phaux Sep 29, 2020
e1adee6
docs(eslint-plugin): options type and defaults
phaux Oct 4, 2020
6a45fe2
style(eslint-plugin): code style
phaux Oct 4, 2020
4ae9466
style(eslint-plugin): code style
phaux Oct 4, 2020
20c341f
style(eslint-plugin): nullThrows
phaux Oct 4, 2020
95597ab
style(eslint-plugin): remove array destructuring
phaux Oct 4, 2020
25913a6
fix(eslint-plugin): revert use rule in own impl
phaux Oct 4, 2020
ebf8c7d
docs(eslint-plugin): update ROADMAP
phaux Oct 4, 2020
3038e04
style(eslint-plugin): enforce order via config
phaux Oct 4, 2020
9596ef5
fix(eslint-plugin): handle no semicolon
phaux Oct 4, 2020
1ac96bf
fix(eslint-plugin): remove unused import
phaux Nov 1, 2020
471a4c5
Merge branch 'master' into no-void-expr
phaux Nov 1, 2020
4e4c77c
fix(eslint-plugin): sort rules
phaux Nov 1, 2020
29fca40
chore(eslint-plugin): rename new rule
phaux Nov 2, 2020
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
9 changes: 9 additions & 0 deletions .eslintrc.js
Expand Up @@ -221,6 +221,15 @@ module.exports = {
'@typescript-eslint/internal/plugin-test-formatting': 'error',
},
},
// files which list all the things
{
files: ['packages/eslint-plugin/src/rules/index.ts'],
rules: {
// enforce alphabetical ordering
'sort-keys': 'error',
'import/order': ['error', { alphabetize: { order: 'asc' } }],
},
},
// tools and tests
{
files: ['**/tools/**/*.ts', '**/tests/**/*.ts'],
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -117,6 +117,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int
| [`@typescript-eslint/naming-convention`](./docs/rules/naming-convention.md) | Enforces naming conventions for everything across a codebase | | | :thought_balloon: |
| [`@typescript-eslint/no-base-to-string`](./docs/rules/no-base-to-string.md) | Requires that `.toString()` is only called on objects which provide useful information when stringified | | | :thought_balloon: |
| [`@typescript-eslint/no-confusing-non-null-assertion`](./docs/rules/no-confusing-non-null-assertion.md) | Disallow non-null assertion in locations that may be confusing | | :wrench: | |
| [`@typescript-eslint/no-confusing-void-expression`](./docs/rules/no-confusing-void-expression.md) | Requires expressions of type void to appear in statement position | | :wrench: | :thought_balloon: |
| [`@typescript-eslint/no-dynamic-delete`](./docs/rules/no-dynamic-delete.md) | Disallow the delete operator with computed key expressions | | :wrench: | |
| [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md) | Disallow the declaration of empty interfaces | :heavy_check_mark: | :wrench: | |
| [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type | :heavy_check_mark: | :wrench: | |
Expand Down
4 changes: 3 additions & 1 deletion packages/eslint-plugin/ROADMAP.md
Expand Up @@ -96,7 +96,7 @@ It lists all TSLint rules along side rules from the ESLint ecosystem that are th
| [`no-unused-variable`] | 🌓 | [`@typescript-eslint/no-unused-vars`] |
| [`no-use-before-declare`] | ✅ | [`@typescript-eslint/no-use-before-define`] |
| [`no-var-keyword`] | 🌟 | [`no-var`][no-var] |
| [`no-void-expression`] | 🛑 | N/A (unrelated to the similarly named ESLint rule `no-void`) |
| [`no-void-expression`] | | [`@typescript-eslint/no-confusing-void-expression`] |
| [`prefer-conditional-expression`] | 🛑 | N/A |
| [`prefer-object-spread`] | 🌟 | [`prefer-object-spread`][prefer-object-spread] |
| [`radix`] | 🌟 | [`radix`][radix] |
Expand Down Expand Up @@ -650,6 +650,8 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
[`@typescript-eslint/no-floating-promises`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-floating-promises.md
[`@typescript-eslint/no-magic-numbers`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-magic-numbers.md
[`@typescript-eslint/no-unsafe-member-access`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unsafe-member-access.md
[`@typescript-eslint/restrict-template-expressions`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/restrict-template-expressions.md
[`@typescript-eslint/no-confusing-void-expression`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-confusing-void-expression.md

<!-- eslint-plugin-import -->

Expand Down
149 changes: 149 additions & 0 deletions packages/eslint-plugin/docs/rules/no-confusing-void-expression.md
@@ -0,0 +1,149 @@
# Requires expressions of type void to appear in statement position (`no-confusing-void-expression`)

Returning the results of an expression whose type is void can be misleading.
Attempting to do so is likely a symptom of expecting a different return type from a function.
Even if used correctly, it can be misleading for other developers,
who don't know what a particular function does and if its result matters.

This rule provides automatic fixes for most common cases.

## Examples

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

```ts
// somebody forgot that `alert` doesn't return anything
const response = alert('Are you sure?');
console.log(alert('Are you sure?'));

// it's not obvious whether the chained promise will contain the response (fixable)
promise.then(value => window.postMessage(value));

// it looks like we are returning the result of `console.error` (fixable)
function doSomething() {
if (!somethingToDo) {
return console.error('Nothing to do!');
}

console.log('Doing a thing...');
}
```

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

```ts
// just a regular void function in a statement position
alert('Hello, world!');

// this function returns a boolean value so it's ok
const response = confirm('Are you sure?');
console.log(confirm('Are you sure?'));

// now it's obvious that `postMessage` doesn't return any response
promise.then(value => {
window.postMessage(value);
});

// now it's explicit that we want to log the error and return early
function doSomething() {
if (!somethingToDo) {
console.error('Nothing to do!');
return;
}

console.log('Doing a thing...');
}

// using logical expressions for their side effects is fine
cond && console.log('true');
cond || console.error('false');
cond ? console.log('true') : console.error('false');
```

## Options

An object option can be specified. Each boolean flag makes the rule less strict.

```ts
type Options = {
ignoreArrowShorthand?: boolean;
ignoreVoidOperator?: boolean;
};

const defaults: Options = {
ignoreArrowShorthand: false,
ignoreVoidOperator: false,
};
```

### `ignoreArrowShorthand`

`false` by default.

```json
{
"@typescript-eslint/no-confusing-void-expression": [
"error",
{ "ignoreArrowShorthand": true }
]
}
```

It might be undesirable to wrap every arrow function shorthand expression with braces.
Especially when using Prettier formatter, which spreads such code across 3 lines instead of 1.

Examples of additional **correct** code with this option enabled:

```ts
promise.then(value => window.postMessage(value));
```

### `ignoreVoidOperator`

`false` by default.

```json
{
"@typescript-eslint/no-confusing-void-expression": [
"error",
{ "ignoreVoidOperator": true }
]
}
```

It might be preferable to only use some distinct syntax
to explicitly mark the confusing but valid usage of void expressions.
This option allows void expressions which are explicitly wrapped in the `void` operator.
This can help avoid confusion among other developers as long as they are made aware of this code style.

This option also changes the automatic fixes for common cases to use the `void` operator.
It also enables a suggestion fix to wrap the void expression with `void` operator for every problem reported.

Examples of additional **correct** code with this option enabled:

```ts
// now it's obvious that we don't expect any response
promise.then(value => void window.postMessage(value));

// now it's explicit that we don't want to return anything
function doSomething() {
if (!somethingToDo) {
return void console.error('Nothing to do!');
}

console.log('Doing a thing...');
}

// we are sure that we want to always log `undefined`
console.log(void alert('Hello, world!'));
```

## When Not To Use It

The return type of a function can be inspected by going to its definition or hovering over it in an IDE.
If you don't care about being explicit about the void type in actual code then don't use this rule.
Also, if you prefer concise coding style then also don't use it.

## Related to

- TSLint: ['no-void-expression'](https://palantir.github.io/tslint/rules/no-void-expression/)
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.ts
Expand Up @@ -47,6 +47,7 @@ export = {
'@typescript-eslint/no-array-constructor': 'error',
'@typescript-eslint/no-base-to-string': 'error',
'@typescript-eslint/no-confusing-non-null-assertion': 'error',
'@typescript-eslint/no-confusing-void-expression': 'error',
'no-dupe-class-members': 'off',
'@typescript-eslint/no-dupe-class-members': 'error',
'no-duplicate-imports': 'off',
Expand Down
32 changes: 17 additions & 15 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -8,14 +8,12 @@ import braceStyle from './brace-style';
import classLiteralPropertyStyle from './class-literal-property-style';
import commaDangle from './comma-dangle';
import commaSpacing from './comma-spacing';
import confusingNonNullAssertionLikeNotEqual from './no-confusing-non-null-assertion';
import consistentIndexedObjectStyle from './consistent-indexed-object-style';
import consistentTypeAssertions from './consistent-type-assertions';
import consistentTypeDefinitions from './consistent-type-definitions';
import consistentTypeImports from './consistent-type-imports';
import defaultParamLast from './default-param-last';
import dotNotation from './dot-notation';
import enumMembersSpacing from './space-infix-ops';
import explicitFunctionReturnType from './explicit-function-return-type';
import explicitMemberAccessibility from './explicit-member-accessibility';
import explicitModuleBoundaryTypes from './explicit-module-boundary-types';
Expand All @@ -30,25 +28,27 @@ import methodSignatureStyle from './method-signature-style';
import namingConvention from './naming-convention';
import noArrayConstructor from './no-array-constructor';
import noBaseToString from './no-base-to-string';
import confusingNonNullAssertionLikeNotEqual from './no-confusing-non-null-assertion';
import noConfusingVoidExpression from './no-confusing-void-expression';
import noDupeClassMembers from './no-dupe-class-members';
import noDuplicateImports from './no-duplicate-imports';
import noDynamicDelete from './no-dynamic-delete';
import noEmptyFunction from './no-empty-function';
import noEmptyInterface from './no-empty-interface';
import noExplicitAny from './no-explicit-any';
import noImplicitAnyCatch from './no-implicit-any-catch';
import noExtraneousClass from './no-extraneous-class';
import noExtraNonNullAssertion from './no-extra-non-null-assertion';
import noExtraParens from './no-extra-parens';
import noExtraSemi from './no-extra-semi';
import noExtraneousClass from './no-extraneous-class';
import noFloatingPromises from './no-floating-promises';
import noForInArray from './no-for-in-array';
import preferLiteralEnumMember from './prefer-literal-enum-member';
import noImplicitAnyCatch from './no-implicit-any-catch';
import noImpliedEval from './no-implied-eval';
import noInferrableTypes from './no-inferrable-types';
import noInvalidThis from './no-invalid-this';
import noInvalidVoidType from './no-invalid-void-type';
import noLossOfPrecision from './no-loss-of-precision';
import noLoopFunc from './no-loop-func';
import noLossOfPrecision from './no-loss-of-precision';
import noMagicNumbers from './no-magic-numbers';
import noMisusedNew from './no-misused-new';
import noMisusedPromises from './no-misused-promises';
Expand Down Expand Up @@ -83,6 +83,7 @@ import preferEnumInitializers from './prefer-enum-initializers';
import preferForOf from './prefer-for-of';
import preferFunctionType from './prefer-function-type';
import preferIncludes from './prefer-includes';
import preferLiteralEnumMember from './prefer-literal-enum-member';
import preferNamespaceKeyword from './prefer-namespace-keyword';
import preferNullishCoalescing from './prefer-nullish-coalescing';
import preferOptionalChain from './prefer-optional-chain';
Expand All @@ -101,14 +102,14 @@ import restrictTemplateExpressions from './restrict-template-expressions';
import returnAwait from './return-await';
import semi from './semi';
import spaceBeforeFunctionParen from './space-before-function-paren';
import spaceInfixOps from './space-infix-ops';
import strictBooleanExpressions from './strict-boolean-expressions';
import switchExhaustivenessCheck from './switch-exhaustiveness-check';
import tripleSlashReference from './triple-slash-reference';
import typeAnnotationSpacing from './type-annotation-spacing';
import typedef from './typedef';
import unboundMethod from './unbound-method';
import unifiedSignatures from './unified-signatures';
import noDuplicateImports from './no-duplicate-imports';

export default {
'adjacent-overload-signatures': adjacentOverloadSignatures,
Expand All @@ -127,11 +128,11 @@ export default {
'consistent-type-imports': consistentTypeImports,
'default-param-last': defaultParamLast,
'dot-notation': dotNotation,
'space-infix-ops': enumMembersSpacing,
'explicit-function-return-type': explicitFunctionReturnType,
'explicit-member-accessibility': explicitMemberAccessibility,
'explicit-module-boundary-types': explicitModuleBoundaryTypes,
'func-call-spacing': funcCallSpacing,
indent: indent,
'init-declarations': initDeclarations,
'keyword-spacing': keywordSpacing,
'lines-between-class-members': linesBetweenClassMembers,
Expand All @@ -142,7 +143,9 @@ export default {
'no-array-constructor': noArrayConstructor,
'no-base-to-string': noBaseToString,
'no-confusing-non-null-assertion': confusingNonNullAssertionLikeNotEqual,
'no-confusing-void-expression': noConfusingVoidExpression,
'no-dupe-class-members': noDupeClassMembers,
'no-duplicate-imports': noDuplicateImports,
'no-dynamic-delete': noDynamicDelete,
'no-empty-function': noEmptyFunction,
'no-empty-interface': noEmptyInterface,
Expand Down Expand Up @@ -184,8 +187,8 @@ export default {
'no-unsafe-member-access': noUnsafeMemberAccess,
'no-unsafe-return': noUnsafeReturn,
'no-unused-expressions': noUnusedExpressions,
'no-unused-vars-experimental': noUnusedVarsExperimental,
'no-unused-vars': noUnusedVars,
'no-unused-vars-experimental': noUnusedVarsExperimental,
'no-use-before-define': noUseBeforeDefine,
'no-useless-constructor': noUselessConstructor,
'no-var-requires': noVarRequires,
Expand All @@ -198,28 +201,27 @@ export default {
'prefer-namespace-keyword': preferNamespaceKeyword,
'prefer-nullish-coalescing': preferNullishCoalescing,
'prefer-optional-chain': preferOptionalChain,
'prefer-readonly-parameter-types': preferReadonlyParameterTypes,
'prefer-readonly': preferReadonly,
'prefer-readonly-parameter-types': preferReadonlyParameterTypes,
'prefer-reduce-type-parameter': preferReduceTypeParameter,
'prefer-regexp-exec': preferRegexpExec,
'prefer-string-starts-ends-with': preferStringStartsEndsWith,
'prefer-ts-expect-error': preferTsExpectError,
'promise-function-async': promiseFunctionAsync,
quotes: quotes,
'require-array-sort-compare': requireArraySortCompare,
'require-await': requireAwait,
'restrict-plus-operands': restrictPlusOperands,
'restrict-template-expressions': restrictTemplateExpressions,
'return-await': returnAwait,
semi: semi,
'space-before-function-paren': spaceBeforeFunctionParen,
'space-infix-ops': spaceInfixOps,
'strict-boolean-expressions': strictBooleanExpressions,
'switch-exhaustiveness-check': switchExhaustivenessCheck,
'triple-slash-reference': tripleSlashReference,
'type-annotation-spacing': typeAnnotationSpacing,
typedef: typedef,
'unbound-method': unboundMethod,
'unified-signatures': unifiedSignatures,
'no-duplicate-imports': noDuplicateImports,
indent: indent,
quotes: quotes,
semi: semi,
typedef: typedef,
};