Skip to content

Commit

Permalink
feat: update docs, and config
Browse files Browse the repository at this point in the history
  • Loading branch information
bradzacher committed Jan 13, 2020
1 parent dd16faf commit 0aa09e1
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 35 deletions.
60 changes: 36 additions & 24 deletions packages/eslint-plugin/docs/rules/ban-types.md
Expand Up @@ -31,17 +31,34 @@ class Foo<F = string> extends Bar<string> implements Baz<string> {

## Options

The banned type can either be a type name literal (`Foo`), a type name with generic parameter instantiations(s) (`Foo<Bar>`), or the empty object literal (`{}`).
```ts
type Options = {
types: {
[typeName: string]:
| string
| {
message: string;
fixWith?: string;
};
};
};
```

The rule accepts a single object as options, with the key `types`.

```CJSON
- The keys should match the types you want to ban. The type can either be a type name literal (`Foo`), a type name with generic parameter instantiations(s) (`Foo<Bar>`), or the empty object literal (`{}`).
- The value can be an object with the following properties:
- `message: string` - the message to display when the type is matched.
- `fixWith?: string` - a string to replace the banned type with when the fixer is run. If this is omitted, no fix will be done.

### Example config

```JSONC
{
"@typescript-eslint/ban-types": ["error", {
"types": {
// report usages of the type using the default error message
"Foo": null,
// add a custom message to help explain why not to use it
"Bar": "Don't use bar!",
"Foo": "Don't use bar!",

// add a custom message, AND tell the plugin how to fix it
"String": {
Expand All @@ -58,25 +75,20 @@ The banned type can either be a type name literal (`Foo`), a type name with gene
}
```

### Example
### Default Options

```json
{
"@typescript-eslint/ban-types": [
"error",
{
"types": {
"Array": null,
"Object": "Use {} instead",
"String": {
"message": "Use string instead",
"fixWith": "string"
}
}
}
]
}
```
The default options provides 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.
- 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 means "any non-nullish value".
- This is a point of confusion for many developers, who think it means "any object type".
- Avoid the `object`, as it is currently hard to use due to not being able to assert that keys exist.
- See [microsoft/TypeScript#21732](https://github.com/microsoft/TypeScript/issues/21732).

**_Important note:_** the default options suggests using `Record<string, unknown>`; this was a stylistic decision, as the built-in `Record` type is considered to look cleaner.

## Compatibility

Expand Down
41 changes: 32 additions & 9 deletions packages/eslint-plugin/src/rules/ban-types.ts
Expand Up @@ -89,6 +89,7 @@ export default util.createRule<Options, MessageIds>({
defaultOptions: [
{
types: {
// use lower-case instead
String: {
message: 'Use string instead',
fixWith: 'string',
Expand All @@ -101,19 +102,41 @@ export default util.createRule<Options, MessageIds>({
message: 'Use number instead',
fixWith: 'number',
},
Object: {
message:
'The Object type is mostly the same as unknown, you probably want Record<string, unknown> instead'
},
object: {
message:
'The object type is hard to use, use Record<string, unknown> instead',
fixWith: 'Record<string, unknown>'
},
Symbol: {
message: 'Use symbol instead',
fixWith: 'symbol',
},

Function: {
message: [
'The `Function` type accepts any function-like value.',
'It provides no type-safety when calling the function, which can be a common source of bugs.',
'It also accepts things like class declarations, which will throw at runtime as they will not be called with `new`.',
'If you are expecting the function to accept certain arguments, you should explicitly define function shape.',
].join('\n'),
},

// object typing
Object: {
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 `Record<string, unknown>` instead.',
'- If you want a type meaning "any value", you probably want `unknown` instead.',
].join('\n'),
},
'{}': {
message: [
'`{}` actually means "any non-nullish value".',
'- If you want a type meaning "any object", you probably want `Record<string, unknown>` instead.',
'- If you want a type meaning "any value", you probably want `unknown` instead.',
].join('\n'),
},
object: {
message: [
'The `object` type is currently hard to use (see https://github.com/microsoft/TypeScript/issues/21732).',
'Consider using `Record<string, unknown>` instead, as it allows you to more easily inspect and use the keys.',
].join('\n'),
},
},
},
],
Expand Down
3 changes: 1 addition & 2 deletions packages/eslint-plugin/tests/rules/ban-types.test.ts
Expand Up @@ -26,8 +26,7 @@ const options: InferOptionsTypeFromRule<typeof rule> = [

ruleTester.run('ban-types', rule, {
valid: [
'let f = Object();', // Should not fail if there is no options set
'let f: {} = {};',
'let f = Object();', // Should not fail, as it is not a type position
'let f: { x: number, y: number } = { x: 1, y: 1 };',
{
code: 'let f = Object();',
Expand Down

0 comments on commit 0aa09e1

Please sign in to comment.