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

Use upstream version of no-meaningless-void-operator #35

Merged
merged 1 commit into from Sep 9, 2021
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
2 changes: 2 additions & 0 deletions configs/typescript.example.ts
Expand Up @@ -28,5 +28,7 @@ void (async () => {
wut == null ? 0 : 1; // allowed
};

void undefined; // eslint-disable-line @typescript-eslint/no-meaningless-void-operator

// keep isolatedModules happy
export default {};
5 changes: 4 additions & 1 deletion configs/typescript.js
Expand Up @@ -8,7 +8,10 @@ module.exports = {
rules: {
// Avoid #member syntax for performance
"@foxglove/no-private-identifier": "error",
"@foxglove/no-meaningless-void-operator": ["error", { checkNever: true }],
"@typescript-eslint/no-meaningless-void-operator": [
"error",
{ checkNever: true },
],
"@foxglove/no-boolean-parameters": "error",

// `<T>x` style assertions are not compatible with JSX code,
Expand Down
1 change: 0 additions & 1 deletion index.js
Expand Up @@ -3,7 +3,6 @@ module.exports = {
"license-header": require("./rules/license-header"),
"no-private-identifier": require("./rules/no-private-identifier"),
"strict-equality": require("./rules/strict-equality"),
"no-meaningless-void-operator": require("./rules/no-meaningless-void-operator"),
"no-return-promise-resolve": require("./rules/no-return-promise-resolve"),
"no-boolean-parameters": require("./rules/no-boolean-parameters"),
},
Expand Down
473 changes: 257 additions & 216 deletions package-lock.json

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions package.json
Expand Up @@ -32,8 +32,8 @@
"@foxglove/eslint-plugin": "file:.",
"@foxglove/tsconfig": "1.0.0",
"@types/jest": "^27",
"@typescript-eslint/eslint-plugin": "^4",
"@typescript-eslint/parser": "^4",
"@typescript-eslint/eslint-plugin": "^4.31.0",
"@typescript-eslint/parser": "^4.31.0",
Comment on lines +35 to +36
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was tempted to change these to peerDependencies, so that downstream users will receive appropriate warnings if an older version is installed. However, I recognize that some may use this plugin for pure JS code and don't need the TypeScript related plugins. @amacneil is there a more appropriate way to ensure the user will always import a new enough version?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - same thing applies for typescript itself. I opted for just not including these. This situation with a new rule is unique though.

The only option here would be to split out the packages so they can have separate deps, but that seems annoying/overkill. Otherwise we could list it as an optionalPeerDep which I think only yarn supports, and I’m not sure is even enforced anywhere.

Given that we are the primary users of this package and we tend to keep our deps up to date, I’d be inclined to just leave things as is and put a PSA in slack (mostly for all @jhurliman’s packages).

"eslint-plugin-jest": "^24",
"eslint-plugin-react": "^7",
"eslint-plugin-react-hooks": "^4",
Expand Down
49 changes: 0 additions & 49 deletions rules/README.md
Expand Up @@ -28,55 +28,6 @@ const draw = ({ immediate }: { immediate?: boolean }) => {};
const draw = ({ immediate = false }: { immediate: boolean }) => {};
```

### [`@foxglove/no-meaningless-void-operator`](./no-meaningless-void-operator.js) 💭 🔧

Disallow the `void` operator when its argument is already of type `void` or `undefined`.

The `void` operator is a useful tool to convey the programmer's intent to discard a value. For example, it is recommended as one way of suppressing [`@typescript-eslint/no-floating-promises`](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-floating-promises.md) instead of adding `.catch()` to a promise.

This rule helps an author catch API changes where previously a value was being discarded at a call site, but the callee changed so it no longer returns a value. When combined with [no-unused-expressions](https://eslint.org/docs/rules/no-unused-expressions), it also helps _readers_ of the code by ensuring consistency: a statement that looks like `void foo();` is **always** discarding a return value, and a statement that looks like `foo();` is **never** discarding a return value.

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

```ts
void (() => {})();

function foo() {}
void foo();
```

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

```ts
(() => {})();

function foo() {}
foo(); // nothing to discard

function bar(x: number) {
void x; // discarding a number
return 2;
}
void bar(); // discarding a number
```

### Options

This rule accepts a single object option with the following default configuration:

```json
{
"@typescript-eslint/no-meaningless-void-operator": [
"error",
{
"checkNever": false
}
]
}
```

- `checkNever: true` will suggest removing `void` when the argument has type `never`.

### [`@foxglove/no-return-promise-resolve`](./no-return-promise-resolve.js) 🔧

Disallow returning `Promise.resolve(...)` or `Promise.reject(...)` inside an async function. This is redundant since an async function will always return a Promise — use `return` or `throw` directly instead.
Expand Down
77 changes: 0 additions & 77 deletions rules/no-meaningless-void-operator.js

This file was deleted.

92 changes: 0 additions & 92 deletions rules/no-meaningless-void-operator.test.ts

This file was deleted.