Navigation Menu

Skip to content

Commit

Permalink
feat(eslint-plugin): new rule method-signature-style (#1685)
Browse files Browse the repository at this point in the history
  • Loading branch information
phaux committed Apr 3, 2020
1 parent ead0171 commit c49d771
Show file tree
Hide file tree
Showing 7 changed files with 337 additions and 13 deletions.
24 changes: 13 additions & 11 deletions .cspell.json
Expand Up @@ -35,45 +35,47 @@
"\\(#.+?\\)"
],
"words": [
"ASTs",
"Airbnb",
"Airbnb's",
"Codecov",
"Crockford",
"errored",
"IDE's",
"IIFE",
"IIFEs",
"OOM",
"OOMs",
"Premade",
"ROADMAP",
"ASTs",
"autofix",
"autofixers",
"backticks",
"bigint",
"bivariant",
"blockless",
"codebases",
"Codecov",
"contravariant",
"Crockford",
"declarators",
"destructure",
"destructured",
"errored",
"erroring",
"ESLint",
"ESLint's",
"espree",
"estree",
"IDE's",
"IIFE",
"IIFEs",
"linebreaks",
"necroing",
"nocheck",
"nullish",
"OOM",
"OOMs",
"parameterised",
"performant",
"pluggable",
"postprocess",
"Premade",
"prettier's",
"recurse",
"reimplement",
"resync",
"ROADMAP",
"ruleset",
"rulesets",
"superset",
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -107,6 +107,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int
| [`@typescript-eslint/explicit-module-boundary-types`](./docs/rules/explicit-module-boundary-types.md) | Require explicit return and argument types on exported functions' and classes' public class methods | | | |
| [`@typescript-eslint/member-delimiter-style`](./docs/rules/member-delimiter-style.md) | Require a specific member delimiter style for interfaces and type literals | :heavy_check_mark: | :wrench: | |
| [`@typescript-eslint/member-ordering`](./docs/rules/member-ordering.md) | Require a consistent member declaration order | | | |
| [`@typescript-eslint/method-signature-style`](./docs/rules/method-signature-style.md) | Enforces using a particular method signature syntax. | | :wrench: | |
| [`@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-dynamic-delete`](./docs/rules/no-dynamic-delete.md) | Disallow the delete operator with computed key expressions | | :wrench: | |
Expand Down
83 changes: 83 additions & 0 deletions packages/eslint-plugin/docs/rules/method-signature-style.md
@@ -0,0 +1,83 @@
# Enforces using a particular method signature syntax. (`method-signature-style`)

There are two ways to define an object/interface function property.

```ts
// method shorthand syntax
interface T1 {
func(arg: string): number;
}

// regular property with function type
interface T2 {
func: (arg: string) => number;
}
```

A good practice is to use the TypeScript's `strict` option (which implies `strictFunctionTypes`) which enables correct typechecking for function properties only (method signatures get old behavior).

TypeScript FAQ:

> A method and a function property of the same type behave differently.
> Methods are always bivariant in their argument, while function properties are contravariant in their argument under `strictFunctionTypes`.
See the reasoning behind that in the [TypeScript PR for the compiler option](https://github.com/microsoft/TypeScript/pull/18654).

## Options

This rule accepts one string option:

- `"property"`: Enforce using property signature for functions. Use this to enforce maximum correctness together with TypeScript's strict mode.
- `"method"`: Enforce using method signature for functions. Use this if you aren't using TypeScript's strict mode and prefer this style.

The default is `"property"`.

## Rule Details

Examples of **incorrect** code with `property` option.

```ts
interface T1 {
func(arg: string): number;
}
type T2 = {
func(arg: boolean): void;
};
```

Examples of **correct** code with `property` option.

```ts
interface T1 {
func: (arg: string) => number;
}
type T2 = {
func: (arg: boolean) => void;
};
```

Examples of **incorrect** code with `method` option.

```ts
interface T1 {
func: (arg: string) => number;
}
type T2 = {
func: (arg: boolean) => void;
};
```

Examples of **correct** code with `method` option.

```ts
interface T1 {
func(arg: string): number;
}
type T2 = {
func(arg: boolean): void;
};
```

## When Not To Use It

If you don't want to enforce a particular style for object/interface function types, and/or if you don't use `strictFunctionTypes`, then you don't need this rule.
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.json
Expand Up @@ -24,6 +24,7 @@
"@typescript-eslint/indent": "error",
"@typescript-eslint/member-delimiter-style": "error",
"@typescript-eslint/member-ordering": "error",
"@typescript-eslint/method-signature-style": "error",
"@typescript-eslint/naming-convention": "error",
"no-array-constructor": "off",
"@typescript-eslint/no-array-constructor": "error",
Expand Down
6 changes: 4 additions & 2 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -22,6 +22,7 @@ import interfaceNamePrefix from './interface-name-prefix';
import memberDelimiterStyle from './member-delimiter-style';
import memberNaming from './member-naming';
import memberOrdering from './member-ordering';
import methodSignatureStyle from './method-signature-style';
import namingConvention from './naming-convention';
import noArrayConstructor from './no-array-constructor';
import noBaseToString from './no-base-to-string';
Expand All @@ -30,10 +31,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 Down Expand Up @@ -99,7 +100,6 @@ export default {
'ban-ts-comment': banTsComment,
'ban-ts-ignore': banTsIgnore,
'ban-types': banTypes,
'no-base-to-string': noBaseToString,
'brace-style': braceStyle,
camelcase: camelcase,
'class-name-casing': classNameCasing,
Expand All @@ -118,8 +118,10 @@ export default {
'member-delimiter-style': memberDelimiterStyle,
'member-naming': memberNaming,
'member-ordering': memberOrdering,
'method-signature-style': methodSignatureStyle,
'naming-convention': namingConvention,
'no-array-constructor': noArrayConstructor,
'no-base-to-string': noBaseToString,
'no-dupe-class-members': noDupeClassMembers,
'no-dynamic-delete': noDynamicDelete,
'no-empty-function': noEmptyFunction,
Expand Down
124 changes: 124 additions & 0 deletions packages/eslint-plugin/src/rules/method-signature-style.ts
@@ -0,0 +1,124 @@
import {
AST_NODE_TYPES,
TSESTree,
} from '@typescript-eslint/experimental-utils';
import * as util from '../util';

export type Options = ['property' | 'method'];

export type MessageId = 'errorMethod' | 'errorProperty';

export default util.createRule<Options, MessageId>({
name: 'method-signature-style',
meta: {
type: 'suggestion',
docs: {
description: 'Enforces using a particular method signature syntax.',
category: 'Best Practices',
recommended: false,
},
fixable: 'code',
messages: {
errorMethod:
'Shorthand method signature is forbidden. Use a function property instead.',
errorProperty:
'Function property signature is forbidden. Use a method shorthand instead.',
},
schema: [
{
enum: ['property', 'method'],
},
],
},
defaultOptions: ['property'],

create(context, [mode]) {
const sourceCode = context.getSourceCode();

function getMethodKey(
node: TSESTree.TSMethodSignature | TSESTree.TSPropertySignature,
): string {
let key = sourceCode.getText(node.key);
if (node.computed) {
key = `[${key}]`;
}
if (node.optional) {
key = `${key}?`;
}
if (node.readonly) {
key = `readonly ${key}`;
}
return key;
}

function getMethodParams(
node: TSESTree.TSMethodSignature | TSESTree.TSFunctionType,
): string {
let params = '()';
if (node.params.length > 0) {
params = sourceCode.text.substring(
sourceCode.getTokenBefore(node.params[0])!.range[0],
sourceCode.getTokenAfter(node.params[node.params.length - 1])!
.range[1],
);
}
if (node.typeParameters != null) {
const typeParams = sourceCode.getText(node.typeParameters);
params = `${typeParams}${params}`;
}
return params;
}

function getMethodReturnType(
node: TSESTree.TSMethodSignature | TSESTree.TSFunctionType,
): string {
return sourceCode.getText(node.returnType!.typeAnnotation);
}

return {
TSMethodSignature(methodNode): void {
if (mode === 'method') {
return;
}

context.report({
node: methodNode,
messageId: 'errorMethod',
fix: fixer => {
const key = getMethodKey(methodNode);
const params = getMethodParams(methodNode);
const returnType = getMethodReturnType(methodNode);
return fixer.replaceText(
methodNode,
`${key}: ${params} => ${returnType}`,
);
},
});
},
TSPropertySignature(propertyNode): void {
const typeNode = propertyNode.typeAnnotation?.typeAnnotation;
if (typeNode?.type !== AST_NODE_TYPES.TSFunctionType) {
return;
}

if (mode === 'property') {
return;
}

context.report({
node: propertyNode,
messageId: 'errorProperty',
fix: fixer => {
const key = getMethodKey(propertyNode);
const params = getMethodParams(typeNode);
const returnType = getMethodReturnType(typeNode);
return fixer.replaceText(
propertyNode,
`${key}${params}: ${returnType}`,
);
},
});
},
};
},
});

0 comments on commit c49d771

Please sign in to comment.