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): split no-empty-object-type out from ban-types and no-empty-interfaces #8977

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
bcfbb03
feat(eslint-plugin): split no-empty-object-type rule out from ban-typ…
JoshuaKGoldberg Apr 23, 2024
173251c
Mention no-props
JoshuaKGoldberg Apr 23, 2024
b83cb50
Update packages/eslint-plugin/docs/rules/no-empty-object-type.mdx
JoshuaKGoldberg Apr 23, 2024
39870f1
Update packages/eslint-plugin/docs/rules/no-empty-object-type.mdx
JoshuaKGoldberg Apr 23, 2024
f90ab3f
Allow in intersections, and touch up docs per suggestions
JoshuaKGoldberg Apr 24, 2024
0a5c2bd
Update packages/eslint-plugin/docs/rules/no-empty-object-type.mdx
JoshuaKGoldberg Apr 24, 2024
c01f10e
Trimming
JoshuaKGoldberg Apr 25, 2024
f021d26
Update snapshots
JoshuaKGoldberg Apr 25, 2024
ca7fb0f
Finish removing Record
JoshuaKGoldberg Apr 26, 2024
fad7a5e
Correct phrasing on Object
JoshuaKGoldberg Apr 26, 2024
e8a2d7f
Merge with no-empty-interface
JoshuaKGoldberg Apr 30, 2024
3971d71
nit the tip
JoshuaKGoldberg Apr 30, 2024
d33a246
Explicit report message for interfaces
JoshuaKGoldberg May 8, 2024
28ed70d
Add in-type-alias-with-name
JoshuaKGoldberg May 9, 2024
9b6dd1a
Switched to more general allowWithName
JoshuaKGoldberg May 9, 2024
e0bbd1d
Merge branch 'v8'
JoshuaKGoldberg May 9, 2024
b7a70b8
snapshot -u
JoshuaKGoldberg May 9, 2024
713404a
snapshot -u
JoshuaKGoldberg May 9, 2024
218d8e5
Fixed up unit tests
JoshuaKGoldberg May 9, 2024
9987c2b
Update packages/eslint-plugin/docs/rules/no-empty-object-type.mdx
JoshuaKGoldberg May 12, 2024
bd764ef
docs touchups from Kirk
JoshuaKGoldberg May 12, 2024
7dc88d7
replacedBy too
JoshuaKGoldberg May 12, 2024
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
4 changes: 2 additions & 2 deletions packages/eslint-plugin/TSLINT_RULE_ALTERNATIVES.md
Expand Up @@ -24,7 +24,7 @@ It lists all TSLint rules along side rules from the ESLint ecosystem that are th
| [`member-access`] | ✅ | [`@typescript-eslint/explicit-member-accessibility`] |
| [`member-ordering`] | ✅ | [`@typescript-eslint/member-ordering`] |
| [`no-any`] | ✅ | [`@typescript-eslint/no-explicit-any`] |
| [`no-empty-interface`] | ✅ | [`@typescript-eslint/no-empty-interface`] |
| [`no-empty-interface`] | ✅ | [`@typescript-eslint/no-empty-object-type`] |
| [`no-import-side-effect`] | 🔌 | [`import/no-unassigned-import`] |
| [`no-inferrable-types`] | ✅ | [`@typescript-eslint/no-inferrable-types`] |
| [`no-internal-module`] | ✅ | [`@typescript-eslint/prefer-namespace-keyword`] |
Expand Down Expand Up @@ -604,7 +604,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
[`@typescript-eslint/member-ordering`]: https://typescript-eslint.io/rules/member-ordering
[`@typescript-eslint/method-signature-style`]: https://typescript-eslint.io/rules/method-signature-style
[`@typescript-eslint/no-explicit-any`]: https://typescript-eslint.io/rules/no-explicit-any
[`@typescript-eslint/no-empty-interface`]: https://typescript-eslint.io/rules/no-empty-interface
[`@typescript-eslint/no-empty-object-type`]: https://typescript-eslint.io/rules/no-empty-object-type
[`@typescript-eslint/no-implied-eval`]: https://typescript-eslint.io/rules/no-implied-eval
[`@typescript-eslint/no-inferrable-types`]: https://typescript-eslint.io/rules/no-inferrable-types
[`@typescript-eslint/prefer-namespace-keyword`]: https://typescript-eslint.io/rules/prefer-namespace-keyword
Expand Down
15 changes: 5 additions & 10 deletions packages/eslint-plugin/docs/rules/ban-types.mdx
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
Expand Up @@ -36,9 +36,6 @@ const func: Function = () => 1;
// use safer object types
const lowerObj: Object = {};
const capitalObj: Object = { a: 'string' };

const curly1: {} = 1;
const curly2: {} = { a: 'string' };
```

</TabItem>
Expand All @@ -58,9 +55,6 @@ const func: () => number = () => 1;
// use safer object types
const lowerObj: object = {};
const capitalObj: { a: string } = { a: 'string' };

const curly1: number = 1;
const curly2: Record<'a', string> = { a: 'string' };
```

</TabItem>
Expand All @@ -70,13 +64,10 @@ const curly2: Record<'a', string> = { a: 'string' };

The default options provide a set of "best practices", intended to provide safety and standardization in your codebase:

- Don't use the upper-case primitive types, you should use the lower-case types for consistency.
- Don't use the upper-case primitive types or `Object`, you should use the lower-case types for consistency.
- Avoid the `Function` type, as it provides little safety for the following reasons:
- It provides no type safety when calling the value, which means it's easy to provide the wrong arguments.
- It accepts class declarations, which will fail when called, as they are called without the `new` keyword.
- Avoid the `Object` and `{}` types, as they mean "any non-nullish value".
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
- This is a point of confusion for many developers, who think it means "any object type".
- See [this comment for more information](https://github.com/typescript-eslint/typescript-eslint/issues/2063#issuecomment-675156492).

<details>
<summary>Default Options</summary>
Expand Down Expand Up @@ -136,3 +127,7 @@ Example configuration:

If your project is a rare one that intentionally deals with the class equivalents of primitives, it might not be worthwhile to enable the default `ban-types` options.
You might consider using [ESLint disable comments](https://eslint.org/docs/latest/use/configure/rules#using-configuration-comments-1) for those specific situations instead of completely disabling this rule.

## Related To

- [`no-empty-object-type`](./no-empty-object-type.mdx)
10 changes: 10 additions & 0 deletions packages/eslint-plugin/docs/rules/no-empty-interface.mdx
Expand Up @@ -9,6 +9,12 @@ import TabItem from '@theme/TabItem';
>
> See **https://typescript-eslint.io/rules/no-empty-interface** for documentation.

:::danger Deprecated

This rule has been deprecated in favour of the more comprehensive [`@typescript-eslint/no-empty-object-type`](./no-empty-object-type.mdx) rule.

:::

An empty interface in TypeScript does very little: any non-nullable value is assignable to `{}`.
Using an empty interface is often a sign of programmer error, such as misunderstanding the concept of `{}` or forgetting to fill in fields.

Expand Down Expand Up @@ -61,3 +67,7 @@ interface Baz extends Foo, Bar {}
## When Not To Use It

If you don't care about having empty/meaningless interfaces, then you will not need this rule.

## Related To

- [`no-empty-object-type`](./no-empty-object-type.mdx)
145 changes: 145 additions & 0 deletions packages/eslint-plugin/docs/rules/no-empty-object-type.mdx
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
@@ -0,0 +1,145 @@
---
description: 'Disallow accidentally using the "empty object" type.'
---

import Tabs from '@theme/Tabs';
import TabItem from '@theme/TabItem';

> 🛑 This file is source code, not the primary documentation location! 🛑
>
> See **https://typescript-eslint.io/rules/no-empty-object-type** for documentation.

The `{}`, or "empty object" type in TypeScript is a common source of confusion for developers unfamiliar with TypeScript's structural typing.
`{}` represents any _non-nullish value_, including literals like `0` and `""`:

```ts
let anyNonNullishValue: {} = 'Intentionally allowed by TypeScript.';
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
```

Often, developers writing `{}` actually mean either:

- `object`: representing any _object_ value
- `unknown`: representing any value at all, including `null` and `undefined`

In other words, the "empty object" type `{}` really means _"any value that is defined"_.
That includes arrays, class instances, functions, and primitives such as `string` and `symbol`.

To avoid confusion around the `{}` type allowing any _non-nullish value_, this rule bans usage of the `{}` type.
That includes interfaces and object type aliases with no fields.

:::tip
If you do have a use case for an API allowing `{}`, you can always configure the [rule's options](#options), use an [ESLint disable comment](https://eslint.org/docs/latest/use/configure/rules#using-configuration-comments-1), or [disable the rule in your ESLint config](https://eslint.org/docs/latest/use/configure/rules#using-configuration-files-1).
:::
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved

Note that this rule does not report on:

- `{}` as a type constituent in an intersection type (e.g. types like TypeScript's built-in `type NonNullable<T> = T & {}`), as this can be useful in type system operations.
- Interfaces that extend from multiple other interfaces.

## Examples

<Tabs>
<TabItem value="❌ Incorrect">

```ts
let anyObject: {};
let anyValue: {};

interface AnyObjectA {}
interface AnyValueA {}

type AnyObjectB = {};
type AnyValueB = {};
```

</TabItem>
<TabItem value="✅ Correct">

```ts
let anyObject: object;
let anyValue: unknown;

type AnyObjectA = object;
type AnyValueA = unknown;

type AnyObjectB = object;
type AnyValueB = unknown;

let objectWith: { property: boolean };

interface InterfaceWith {
property: boolean;
}

type TypeWith = { property: boolean };
```

</TabItem>
</Tabs>

## Options

By default, this rule flags both interfaces and object types.

### `allowInterfaces`

Whether to allow empty interfaces, as one of:

- `'always'`: to always allow interfaces with no fields
- `'never'` _(default)_: to never allow interfaces with no fields
- `'with-single-extends'`: to allow empty interfaces that `extend` from a single base interface

Examples of **correct** code for this rule with `{ allowInterfaces: 'with-single-extends' }`:

```ts option='{ "allowInterfaces": "with-single-extends" }' showPlaygroundButton
interface Base {
value: boolean;
}

interface Derived extends Base {}
```

### `allowObjectTypes`

Whether to allow empty object type literals, as one of:
Copy link
Member

Choose a reason for hiding this comment

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

I think this list is stale now, due to the new option being added, right? Or, I guess, the way it's written, it doesn't seem to me like allowWithName is part of the list.

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'm having a hard time thinking of phrasing for this 🤔 do you have any suggestions @kirkwaiblinger?

Copy link
Member Author

Choose a reason for hiding this comment

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

Merged so that I can get other PRa going, but happy to take a followup issue or PR!


- `'always'`: to always allow object type literals with no fields
- `'never'` _(default)_: to never allow object type literals with no fields

### `allowWithName`

A stringified regular expression to allow interfaces and object type aliases with the configured name.
This can be useful if your existing code style includes a pattern of declaring empty types with `{}` instead of `object`.

Examples of code for this rule with `{ allowWithName: 'Props$' }`:

<Tabs>
<TabItem value="❌ Incorrect">

```ts option='{ "allowWithName": "Props$" }' showPlaygroundButton
interface InterfaceValue {}

type TypeValue = {};
```

</TabItem>
<TabItem value="✅ Correct">

```ts option='{ "allowWithName": "Props$" }' showPlaygroundButton
interface InterfaceProps {}

type TypeProps = {};
```

</TabItem>
</Tabs>

## When Not To Use It

If your code commonly needs to represent the _"any non-nullish value"_ type, this rule may not be for you.
Projects that extensively use type operations such as conditional types and mapped types oftentimes benefit from disabling this rule.

## Further Reading

- [Enhancement: [ban-types] Split the {} ban into a separate, better-phrased rule](https://github.com/typescript-eslint/typescript-eslint/issues/8700)
- [The Empty Object Type in TypeScript](https://www.totaltypescript.com/the-empty-object-type-in-typescript)
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion packages/eslint-plugin/src/configs/all.ts
Expand Up @@ -54,7 +54,7 @@ export = {
'@typescript-eslint/no-dynamic-delete': 'error',
'no-empty-function': 'off',
'@typescript-eslint/no-empty-function': 'error',
'@typescript-eslint/no-empty-interface': 'error',
'@typescript-eslint/no-empty-object-type': 'error',
'@typescript-eslint/no-explicit-any': 'error',
'@typescript-eslint/no-extra-non-null-assertion': 'error',
'@typescript-eslint/no-extraneous-class': 'error',
Expand Down
Expand Up @@ -18,6 +18,7 @@ export = {
'@typescript-eslint/no-base-to-string': 'error',
'@typescript-eslint/no-duplicate-enum-values': 'error',
'@typescript-eslint/no-duplicate-type-constituents': 'error',
'@typescript-eslint/no-empty-object-type': 'error',
'@typescript-eslint/no-explicit-any': 'error',
'@typescript-eslint/no-extra-non-null-assertion': 'error',
'@typescript-eslint/no-floating-promises': 'error',
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/recommended.ts
Expand Up @@ -15,6 +15,7 @@ export = {
'no-array-constructor': 'off',
'@typescript-eslint/no-array-constructor': 'error',
'@typescript-eslint/no-duplicate-enum-values': 'error',
'@typescript-eslint/no-empty-object-type': 'error',
'@typescript-eslint/no-explicit-any': 'error',
'@typescript-eslint/no-extra-non-null-assertion': 'error',
'@typescript-eslint/no-misused-new': 'error',
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/strict-type-checked.ts
Expand Up @@ -24,6 +24,7 @@ export = {
'@typescript-eslint/no-duplicate-enum-values': 'error',
'@typescript-eslint/no-duplicate-type-constituents': 'error',
'@typescript-eslint/no-dynamic-delete': 'error',
'@typescript-eslint/no-empty-object-type': 'error',
'@typescript-eslint/no-explicit-any': 'error',
'@typescript-eslint/no-extra-non-null-assertion': 'error',
'@typescript-eslint/no-extraneous-class': 'error',
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/strict.ts
Expand Up @@ -19,6 +19,7 @@ export = {
'@typescript-eslint/no-array-constructor': 'error',
'@typescript-eslint/no-duplicate-enum-values': 'error',
'@typescript-eslint/no-dynamic-delete': 'error',
'@typescript-eslint/no-empty-object-type': 'error',
'@typescript-eslint/no-explicit-any': 'error',
'@typescript-eslint/no-extra-non-null-assertion': 'error',
'@typescript-eslint/no-extraneous-class': 'error',
Expand Down
Expand Up @@ -23,7 +23,6 @@ export = {
'@typescript-eslint/no-confusing-non-null-assertion': 'error',
'no-empty-function': 'off',
'@typescript-eslint/no-empty-function': 'error',
'@typescript-eslint/no-empty-interface': 'error',
'@typescript-eslint/no-inferrable-types': 'error',
'@typescript-eslint/non-nullable-type-assertion-style': 'error',
'@typescript-eslint/prefer-for-of': 'error',
Expand Down
1 change: 0 additions & 1 deletion packages/eslint-plugin/src/configs/stylistic.ts
Expand Up @@ -21,7 +21,6 @@ export = {
'@typescript-eslint/no-confusing-non-null-assertion': 'error',
'no-empty-function': 'off',
'@typescript-eslint/no-empty-function': 'error',
'@typescript-eslint/no-empty-interface': 'error',
'@typescript-eslint/no-inferrable-types': 'error',
'@typescript-eslint/prefer-for-of': 'error',
'@typescript-eslint/prefer-function-type': 'error',
Expand Down
31 changes: 4 additions & 27 deletions packages/eslint-plugin/src/rules/ban-types.ts
Expand Up @@ -73,7 +73,10 @@ const defaultTypes: Types = {
message: 'Use bigint instead',
fixWith: 'bigint',
},

Object: {
message: 'Use object instead',
fixWith: 'object',
},
Function: {
message: [
'The `Function` type accepts any function-like value.',
Expand All @@ -82,32 +85,6 @@ const defaultTypes: Types = {
'If you are expecting the function to accept certain arguments, you should explicitly define the function shape.',
].join('\n'),
},

// object typing
Object: {
Copy link
Member Author

Choose a reason for hiding this comment

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

We didn't explicitly discuss Object, but I think it makes sense in the spirit of this change to switch it to being treated as another banned uppercase alias. I've never seen people confuse Object for {}.

message: [
'The `Object` type actually means "any non-nullish value", so it is marginally better than `unknown`.',
'- If you want a type meaning "any object", you probably want `object` instead.',
'- If you want a type meaning "any value", you probably want `unknown` instead.',
'- If you really want a type meaning "any non-nullish value", you probably want `NonNullable<unknown>` instead.',
].join('\n'),
suggest: ['object', 'unknown', 'NonNullable<unknown>'],
},
'{}': {
message: [
'`{}` actually means "any non-nullish value".',
'- If you want a type meaning "any object", you probably want `object` instead.',
'- If you want a type meaning "any value", you probably want `unknown` instead.',
'- If you want a type meaning "empty object", you probably want `Record<string, never>` instead.',
'- If you really want a type meaning "any non-nullish value", you probably want `NonNullable<unknown>` instead.',
].join('\n'),
suggest: [
'object',
'unknown',
'Record<string, never>',
'NonNullable<unknown>',
],
},
};

export const TYPE_KEYWORDS = {
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -36,6 +36,7 @@ import noDuplicateTypeConstituents from './no-duplicate-type-constituents';
import noDynamicDelete from './no-dynamic-delete';
import noEmptyFunction from './no-empty-function';
import noEmptyInterface from './no-empty-interface';
import noEmptyObjectType from './no-empty-object-type';
import noExplicitAny from './no-explicit-any';
import noExtraNonNullAssertion from './no-extra-non-null-assertion';
import noExtraneousClass from './no-extraneous-class';
Expand Down Expand Up @@ -160,6 +161,7 @@ export default {
'no-dynamic-delete': noDynamicDelete,
'no-empty-function': noEmptyFunction,
'no-empty-interface': noEmptyInterface,
'no-empty-object-type': noEmptyObjectType,
'no-explicit-any': noExplicitAny,
'no-extra-non-null-assertion': noExtraNonNullAssertion,
'no-extraneous-class': noExtraneousClass,
Expand Down
3 changes: 2 additions & 1 deletion packages/eslint-plugin/src/rules/no-empty-interface.ts
Expand Up @@ -17,8 +17,9 @@ export default createRule<Options, MessageIds>({
type: 'suggestion',
docs: {
description: 'Disallow the declaration of empty interfaces',
recommended: 'stylistic',
},
deprecated: true,
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
replacedBy: ['@typescript-eslint/no-empty-object-type'],
fixable: 'code',
hasSuggestions: true,
messages: {
Expand Down