Skip to content

Commit

Permalink
feat(eslint-plugin): add prefer-readonly-parameters (#1513)
Browse files Browse the repository at this point in the history
  • Loading branch information
bradzacher committed Mar 2, 2020
1 parent b097245 commit 3be9854
Show file tree
Hide file tree
Showing 9 changed files with 952 additions and 19 deletions.
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -142,6 +142,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int
| [`@typescript-eslint/prefer-nullish-coalescing`](./docs/rules/prefer-nullish-coalescing.md) | Enforce the usage of the nullish coalescing operator instead of logical chaining | | :wrench: | :thought_balloon: |
| [`@typescript-eslint/prefer-optional-chain`](./docs/rules/prefer-optional-chain.md) | Prefer using concise optional chain expressions instead of chained logical ands | | :wrench: | |
| [`@typescript-eslint/prefer-readonly`](./docs/rules/prefer-readonly.md) | Requires that private members are marked as `readonly` if they're never modified outside of the constructor | | :wrench: | :thought_balloon: |
| [`@typescript-eslint/prefer-readonly-parameter-types`](./docs/rules/prefer-readonly-parameter-types.md) | Requires that function parameters are typed as readonly to prevent accidental mutation of inputs | | | :thought_balloon: |
| [`@typescript-eslint/prefer-regexp-exec`](./docs/rules/prefer-regexp-exec.md) | Enforce that `RegExp#exec` is used instead of `String#match` if no global flag is provided | :heavy_check_mark: | | :thought_balloon: |
| [`@typescript-eslint/prefer-string-starts-ends-with`](./docs/rules/prefer-string-starts-ends-with.md) | Enforce the use of `String#startsWith` and `String#endsWith` instead of other equivalent methods of checking substrings | :heavy_check_mark: | :wrench: | :thought_balloon: |
| [`@typescript-eslint/promise-function-async`](./docs/rules/promise-function-async.md) | Requires any function or method that returns a Promise to be marked async | | | :thought_balloon: |
Expand Down
163 changes: 163 additions & 0 deletions packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md
@@ -0,0 +1,163 @@
# Requires that function parameters are typed as readonly to prevent accidental mutation of inputs (`prefer-readonly-parameter-types`)

Mutating function arguments can lead to confusing, hard to debug behavior.
Whilst it's easy to implicitly remember to not modify function arguments, explicitly typing arguments as readonly provides clear contract to consumers.
This contract makes it easier for a consumer to reason about if a function has side-effects.

## Rule Details

This rule allows you to enforce that function parameters resolve to readonly types.
A type is considered readonly if:

- it is a primitive type (`string`, `number`, `boolean`, `symbol`, or an enum),
- it is a function signature type,
- it is a readonly array type whose element type is considered readonly.
- it is a readonly tuple type whose elements are all considered readonly.
- it is an object type whose properties are all marked as readonly, and whose values are all considered readonly.

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

```ts
function array1(arg: string[]) {} // array is not readonly
function array2(arg: readonly string[][]) {} // array element is not readonly
function array3(arg: [string, number]) {} // tuple is not readonly
function array4(arg: readonly [string[], number]) {} // tuple element is not readonly
// the above examples work the same if you use ReadonlyArray<T> instead

function object1(arg: { prop: string }) {} // property is not readonly
function object2(arg: { readonly prop: string; prop2: string }) {} // not all properties are readonly
function object3(arg: { readonly prop: { prop2: string } }) {} // nested property is not readonly
// the above examples work the same if you use Readonly<T> instead

interface CustomArrayType extends ReadonlyArray<string> {
prop: string; // note: this property is mutable
}
function custom1(arg: CustomArrayType) {}

interface CustomFunction {
(): void;
prop: string; // note: this property is mutable
}
function custom2(arg: CustomFunction) {}

function union(arg: string[] | ReadonlyArray<number[]>) {} // not all types are readonly

// rule also checks function types
interface Foo {
(arg: string[]): void;
}
interface Foo {
new (arg: string[]): void;
}
const x = { foo(arg: string[]): void; };
function foo(arg: string[]);
type Foo = (arg: string[]) => void;
interface Foo {
foo(arg: string[]): void;
}
```

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

```ts
function array1(arg: readonly string[]) {}
function array2(arg: readonly (readonly string[])[]) {}
function array3(arg: readonly [string, number]) {}
function array4(arg: readonly [readonly string[], number]) {}
// the above examples work the same if you use ReadonlyArray<T> instead

function object1(arg: { readonly prop: string }) {}
function object2(arg: { readonly prop: string; readonly prop2: string }) {}
function object3(arg: { readonly prop: { readonly prop2: string } }) {}
// the above examples work the same if you use Readonly<T> instead

interface CustomArrayType extends ReadonlyArray<string> {
readonly prop: string;
}
function custom1(arg: CustomArrayType) {}

interface CustomFunction {
(): void;
readonly prop: string;
}
function custom2(arg: CustomFunction) {}

function union(arg: readonly string[] | ReadonlyArray<number[]>) {}

function primitive1(arg: string) {}
function primitive2(arg: number) {}
function primitive3(arg: boolean) {}
function primitive4(arg: unknown) {}
function primitive5(arg: null) {}
function primitive6(arg: undefined) {}
function primitive7(arg: any) {}
function primitive8(arg: never) {}
function primitive9(arg: string | number | undefined) {}

function fnSig(arg: () => void) {}

enum Foo { a, b }
function enum(arg: Foo) {}

function symb1(arg: symbol) {}
const customSymbol = Symbol('a');
function symb2(arg: typeof customSymbol) {}

// function types
interface Foo {
(arg: readonly string[]): void;
}
interface Foo {
new (arg: readonly string[]): void;
}
const x = { foo(arg: readonly string[]): void; };
function foo(arg: readonly string[]);
type Foo = (arg: readonly string[]) => void;
interface Foo {
foo(arg: readonly string[]): void;
}
```

## Options

```ts
interface Options {
checkParameterProperties?: boolean;
}

const defaultOptions: Options = {
checkParameterProperties: true,
};
```

### `checkParameterProperties`

This option allows you to enable or disable the checking of parameter properties.
Because parameter properties create properties on the class, it may be undesirable to force them to be readonly.

Examples of **incorrect** code for this rule with `{checkParameterProperties: true}`:

```ts
class Foo {
constructor(private paramProp: string[]) {}
}
```

Examples of **correct** code for this rule with `{checkParameterProperties: true}`:

```ts
class Foo {
constructor(private paramProp: readonly string[]) {}
}
```

Examples of **correct** code for this rule with `{checkParameterProperties: false}`:

```ts
class Foo {
constructor(
private paramProp1: string[],
private paramProp2: readonly string[],
) {}
}
```
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.json
Expand Up @@ -79,6 +79,7 @@
"@typescript-eslint/prefer-nullish-coalescing": "error",
"@typescript-eslint/prefer-optional-chain": "error",
"@typescript-eslint/prefer-readonly": "error",
"@typescript-eslint/prefer-readonly-parameter-types": "error",
"@typescript-eslint/prefer-regexp-exec": "error",
"@typescript-eslint/prefer-string-starts-ends-with": "error",
"@typescript-eslint/promise-function-async": "error",
Expand Down
24 changes: 13 additions & 11 deletions packages/eslint-plugin/src/rules/index.ts
@@ -1,8 +1,8 @@
import adjacentOverloadSignatures from './adjacent-overload-signatures';
import arrayType from './array-type';
import awaitThenable from './await-thenable';
import banTsIgnore from './ban-ts-ignore';
import banTsComment from './ban-ts-comment';
import banTsIgnore from './ban-ts-ignore';
import banTypes from './ban-types';
import braceStyle from './brace-style';
import camelcase from './camelcase';
Expand All @@ -29,10 +29,10 @@ 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 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 noImpliedEval from './no-implied-eval';
Expand All @@ -41,8 +41,8 @@ import noMagicNumbers from './no-magic-numbers';
import noMisusedNew from './no-misused-new';
import noMisusedPromises from './no-misused-promises';
import noNamespace from './no-namespace';
import noNonNullAssertion from './no-non-null-assertion';
import noNonNullAssertedOptionalChain from './no-non-null-asserted-optional-chain';
import noNonNullAssertion from './no-non-null-assertion';
import noParameterProperties from './no-parameter-properties';
import noRequireImports from './no-require-imports';
import noThisAlias from './no-this-alias';
Expand All @@ -51,7 +51,7 @@ import noTypeAlias from './no-type-alias';
import noUnnecessaryBooleanLiteralCompare from './no-unnecessary-boolean-literal-compare';
import noUnnecessaryCondition from './no-unnecessary-condition';
import noUnnecessaryQualifier from './no-unnecessary-qualifier';
import useDefaultTypeParameter from './no-unnecessary-type-arguments';
import noUnnecessaryTypeArguments from './no-unnecessary-type-arguments';
import noUnnecessaryTypeAssertion from './no-unnecessary-type-assertion';
import noUntypedPublicSignature from './no-untyped-public-signature';
import noUnusedExpressions from './no-unused-expressions';
Expand All @@ -68,6 +68,7 @@ import preferNamespaceKeyword from './prefer-namespace-keyword';
import preferNullishCoalescing from './prefer-nullish-coalescing';
import preferOptionalChain from './prefer-optional-chain';
import preferReadonly from './prefer-readonly';
import preferReadonlyParameterTypes from './prefer-readonly-parameter-types';
import preferRegexpExec from './prefer-regexp-exec';
import preferStringStartsEndsWith from './prefer-string-starts-ends-with';
import promiseFunctionAsync from './promise-function-async';
Expand All @@ -91,8 +92,8 @@ export default {
'adjacent-overload-signatures': adjacentOverloadSignatures,
'array-type': arrayType,
'await-thenable': awaitThenable,
'ban-ts-ignore': banTsIgnore,
'ban-ts-comment': banTsComment,
'ban-ts-ignore': banTsIgnore,
'ban-types': banTypes,
'no-base-to-string': noBaseToString,
'brace-style': braceStyle,
Expand Down Expand Up @@ -125,28 +126,28 @@ export default {
'no-extraneous-class': noExtraneousClass,
'no-floating-promises': noFloatingPromises,
'no-for-in-array': noForInArray,
'no-inferrable-types': noInferrableTypes,
'no-implied-eval': noImpliedEval,
'no-inferrable-types': noInferrableTypes,
'no-magic-numbers': noMagicNumbers,
'no-misused-new': noMisusedNew,
'no-misused-promises': noMisusedPromises,
'no-namespace': noNamespace,
'no-non-null-assertion': noNonNullAssertion,
'no-non-null-asserted-optional-chain': noNonNullAssertedOptionalChain,
'no-non-null-assertion': noNonNullAssertion,
'no-parameter-properties': noParameterProperties,
'no-require-imports': noRequireImports,
'no-this-alias': noThisAlias,
'no-type-alias': noTypeAlias,
'no-throw-literal': noThrowLiteral,
'no-type-alias': noTypeAlias,
'no-unnecessary-boolean-literal-compare': noUnnecessaryBooleanLiteralCompare,
'no-unnecessary-condition': noUnnecessaryCondition,
'no-unnecessary-qualifier': noUnnecessaryQualifier,
'no-unnecessary-type-arguments': useDefaultTypeParameter,
'no-unnecessary-type-arguments': noUnnecessaryTypeArguments,
'no-unnecessary-type-assertion': noUnnecessaryTypeAssertion,
'no-untyped-public-signature': noUntypedPublicSignature,
'no-unused-vars': noUnusedVars,
'no-unused-vars-experimental': noUnusedVarsExperimental,
'no-unused-expressions': noUnusedExpressions,
'no-unused-vars-experimental': noUnusedVarsExperimental,
'no-unused-vars': noUnusedVars,
'no-use-before-define': noUseBeforeDefine,
'no-useless-constructor': noUselessConstructor,
'no-var-requires': noVarRequires,
Expand All @@ -157,6 +158,7 @@ export default {
'prefer-namespace-keyword': preferNamespaceKeyword,
'prefer-nullish-coalescing': preferNullishCoalescing,
'prefer-optional-chain': preferOptionalChain,
'prefer-readonly-parameter-types': preferReadonlyParameterTypes,
'prefer-readonly': preferReadonly,
'prefer-regexp-exec': preferRegexpExec,
'prefer-string-starts-ends-with': preferStringStartsEndsWith,
Expand Down
@@ -0,0 +1,98 @@
import {
TSESTree,
AST_NODE_TYPES,
} from '@typescript-eslint/experimental-utils';
import * as util from '../util';

type Options = [
{
checkParameterProperties?: boolean;
},
];
type MessageIds = 'shouldBeReadonly';

export default util.createRule<Options, MessageIds>({
name: 'prefer-readonly-parameter-types',
meta: {
type: 'suggestion',
docs: {
description:
'Requires that function parameters are typed as readonly to prevent accidental mutation of inputs',
category: 'Possible Errors',
recommended: false,
requiresTypeChecking: true,
},
schema: [
{
type: 'object',
additionalProperties: false,
properties: {
checkParameterProperties: {
type: 'boolean',
},
},
},
],
messages: {
shouldBeReadonly: 'Parameter should be a read only type',
},
},
defaultOptions: [
{
checkParameterProperties: true,
},
],
create(context, [{ checkParameterProperties }]) {
const { esTreeNodeToTSNodeMap, program } = util.getParserServices(context);
const checker = program.getTypeChecker();

return {
[[
AST_NODE_TYPES.ArrowFunctionExpression,
AST_NODE_TYPES.FunctionDeclaration,
AST_NODE_TYPES.FunctionExpression,
AST_NODE_TYPES.TSCallSignatureDeclaration,
AST_NODE_TYPES.TSConstructSignatureDeclaration,
AST_NODE_TYPES.TSDeclareFunction,
AST_NODE_TYPES.TSEmptyBodyFunctionExpression,
AST_NODE_TYPES.TSFunctionType,
AST_NODE_TYPES.TSMethodSignature,
].join(', ')](
node:
| TSESTree.ArrowFunctionExpression
| TSESTree.FunctionDeclaration
| TSESTree.FunctionExpression
| TSESTree.TSCallSignatureDeclaration
| TSESTree.TSConstructSignatureDeclaration
| TSESTree.TSDeclareFunction
| TSESTree.TSEmptyBodyFunctionExpression
| TSESTree.TSFunctionType
| TSESTree.TSMethodSignature,
): void {
for (const param of node.params) {
if (
!checkParameterProperties &&
param.type === AST_NODE_TYPES.TSParameterProperty
) {
continue;
}

const actualParam =
param.type === AST_NODE_TYPES.TSParameterProperty
? param.parameter
: param;
const tsNode = esTreeNodeToTSNodeMap.get(actualParam);
const type = checker.getTypeAtLocation(tsNode);
const isReadOnly = util.isTypeReadonly(checker, type);

if (!isReadOnly) {
context.report({
node: actualParam,
messageId: 'shouldBeReadonly',
});
}
}
},
};
},
});
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/util/index.ts
Expand Up @@ -2,6 +2,7 @@ import { ESLintUtils } from '@typescript-eslint/experimental-utils';

export * from './astUtils';
export * from './createRule';
export * from './isTypeReadonly';
export * from './misc';
export * from './nullThrows';
export * from './types';
Expand Down

0 comments on commit 3be9854

Please sign in to comment.