diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 75feac4ee49e..20d3d817a218 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -182,6 +182,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/no-unnecessary-qualifier`](./docs/rules/no-unnecessary-qualifier.md) | Warns when a namespace qualifier is unnecessary | | :wrench: | :thought_balloon: | | [`@typescript-eslint/no-unnecessary-type-arguments`](./docs/rules/no-unnecessary-type-arguments.md) | Warns if an explicitly specified type argument is the default for that type parameter | | :wrench: | :thought_balloon: | | [`@typescript-eslint/no-unnecessary-type-assertion`](./docs/rules/no-unnecessary-type-assertion.md) | Warns if a type assertion does not change the type of an expression | :heavy_check_mark: | :wrench: | :thought_balloon: | +| [`@typescript-eslint/no-untyped-public-signature`](./docs/rules/no-untyped-public-signature.md) | Requires that all public method arguments and return type will be explicitly typed | | | | | [`@typescript-eslint/no-unused-expressions`](./docs/rules/no-unused-expressions.md) | Disallow unused expressions | | | | | [`@typescript-eslint/no-unused-vars`](./docs/rules/no-unused-vars.md) | Disallow unused variables | :heavy_check_mark: | | | | [`@typescript-eslint/no-use-before-define`](./docs/rules/no-use-before-define.md) | Disallow the use of variables before they are defined | :heavy_check_mark: | | | diff --git a/packages/eslint-plugin/docs/rules/no-untyped-public-signature.md b/packages/eslint-plugin/docs/rules/no-untyped-public-signature.md new file mode 100644 index 000000000000..7ffafd5a5ae7 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-untyped-public-signature.md @@ -0,0 +1,57 @@ +# Disallow untyped public methods (no-untyped-public-signature) + +public methods are meant to be used by code outside of your class. By typing both the parameters and the return type of public methods they will be more readable and easy to use. + +## Rule Details + +This rule aims to ensure that only typed public methods are declared in the code. + +The following patterns are considered warnings: + +```ts +// untyped parameter +public foo(param1): void { +} + +// untyped parameter +public foo(param1: any): void { +} + +// untyped return type +public foo(param1: string) { +} + +// untyped return type +public foo(param1: string): any { +} +``` + +The following patterns are not warnings: + +```ts +// typed public method +public foo(param1: string): void { +} + +// untyped private method +private foo(param1) { +} +``` + +## Options + +This rule, in its default state, does not require any argument. + +### ignoredMethods + +You may pass method names you would like this rule to ignore, like so: + +```cjson +{ + "@typescript-eslint/no-untyped-public-signature": ["error", { "ignoredMethods": ["ignoredMethodName"] }] +} +``` + +## When Not To Use It + +If you don't wish to type public methods. diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index c7e9a99bc765..84c450b0723a 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -51,6 +51,7 @@ "@typescript-eslint/no-unnecessary-qualifier": "error", "@typescript-eslint/no-unnecessary-type-arguments": "error", "@typescript-eslint/no-unnecessary-type-assertion": "error", + "@typescript-eslint/no-untyped-public-signature": "error", "@typescript-eslint/no-unused-expressions": "error", "no-unused-vars": "off", "@typescript-eslint/no-unused-vars": "error", diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 11c774610a00..87d02d56a3bf 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -40,6 +40,7 @@ import noUnnecessaryCondition from './no-unnecessary-condition'; import noUnnecessaryQualifier from './no-unnecessary-qualifier'; import noUnnecessaryTypeAssertion from './no-unnecessary-type-assertion'; import noUnusedVars from './no-unused-vars'; +import noUntypedPublicSignature from './no-untyped-public-signature'; import noUnusedExpressions from './no-unused-expressions'; import noUseBeforeDefine from './no-use-before-define'; import noUselessConstructor from './no-useless-constructor'; @@ -108,6 +109,7 @@ export default { 'no-unnecessary-qualifier': noUnnecessaryQualifier, 'no-unnecessary-type-arguments': useDefaultTypeParameter, 'no-unnecessary-type-assertion': noUnnecessaryTypeAssertion, + 'no-untyped-public-signature': noUntypedPublicSignature, 'no-unused-vars': noUnusedVars, 'no-unused-expressions': noUnusedExpressions, 'no-use-before-define': noUseBeforeDefine, diff --git a/packages/eslint-plugin/src/rules/no-untyped-public-signature.ts b/packages/eslint-plugin/src/rules/no-untyped-public-signature.ts new file mode 100644 index 000000000000..2ecb1cbd62c6 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-untyped-public-signature.ts @@ -0,0 +1,120 @@ +import * as util from '../util'; +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; + +type MessageIds = 'noReturnType' | 'untypedParameter'; + +type Options = [{ ignoredMethods: string[] }]; + +export default util.createRule({ + name: 'no-unused-public-signature', + meta: { + docs: { + description: + 'Requires that all public method arguments and return type will be explicitly typed', + category: 'Best Practices', + recommended: false, + }, + messages: { + noReturnType: 'Public method has no return type', + untypedParameter: 'Public method parameters should be typed', + }, + schema: [ + { + allowAdditionalProperties: false, + properties: { + ignoredMethods: { + type: 'array', + items: { + type: 'string', + }, + }, + }, + type: 'object', + }, + ], + type: 'suggestion', + }, + defaultOptions: [{ ignoredMethods: [] }], + create(context, [options]) { + const ignoredMethods = new Set(options.ignoredMethods); + + function isPublicMethod( + node: TSESTree.MethodDefinition | TSESTree.TSAbstractMethodDefinition, + ): boolean { + return node.accessibility === 'public' || !node.accessibility; + } + + function isIgnoredMethod( + node: TSESTree.MethodDefinition | TSESTree.TSAbstractMethodDefinition, + ignoredMethods: Set, + ): boolean { + if ( + node.key.type === AST_NODE_TYPES.Literal && + typeof node.key.value === 'string' + ) { + return ignoredMethods.has(node.key.value); + } + if ( + node.key.type === AST_NODE_TYPES.TemplateLiteral && + node.key.expressions.length === 0 + ) { + return ignoredMethods.has(node.key.quasis[0].value.raw); + } + if (node.key.type === AST_NODE_TYPES.Identifier && !node.computed) { + return ignoredMethods.has(node.key.name); + } + + return false; + } + + function isParamTyped(node: TSESTree.Identifier): boolean { + return ( + !!node.typeAnnotation && + node.typeAnnotation.typeAnnotation.type !== AST_NODE_TYPES.TSAnyKeyword + ); + } + + function isReturnTyped( + node: TSESTree.TSTypeAnnotation | undefined, + ): boolean { + if (!node) { + return false; + } + return ( + node.typeAnnotation && + node.typeAnnotation.type !== AST_NODE_TYPES.TSAnyKeyword + ); + } + + return { + 'TSAbstractMethodDefinition, MethodDefinition'( + node: TSESTree.MethodDefinition | TSESTree.TSAbstractMethodDefinition, + ): void { + if (isPublicMethod(node) && !isIgnoredMethod(node, ignoredMethods)) { + const paramIdentifiers = node.value.params.filter( + param => param.type === AST_NODE_TYPES.Identifier, + ) as TSESTree.Identifier[]; + const identifiersHaveTypes = paramIdentifiers.every(isParamTyped); + if (!identifiersHaveTypes) { + context.report({ + node, + messageId: 'untypedParameter', + data: {}, + }); + } + + if (!isReturnTyped(node.value.returnType)) { + context.report({ + node, + messageId: 'noReturnType', + data: {}, + }); + } + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-untyped-public-signature.test.ts b/packages/eslint-plugin/tests/rules/no-untyped-public-signature.test.ts new file mode 100644 index 000000000000..1a167d64e30d --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-untyped-public-signature.test.ts @@ -0,0 +1,210 @@ +import rule from '../../src/rules/no-untyped-public-signature'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 6, + sourceType: 'module', + ecmaFeatures: {}, + }, + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-untyped-public-signature', rule, { + valid: [ + { + code: `class A { + private a(c) { + } + }`, + }, + { + code: `class A { + private async a(c) { + } + }`, + }, + { + code: ` + class A { + public b(c: string):void { + + } + }`, + }, + { + code: ` + class A { + public b(...c):void { + + } + }`, + }, + { + code: ` + class A { + b(c):void { + + } + }`, + options: [{ ignoredMethods: ['b'] }], + }, + { + code: ` + class A { + ['b'](c):void { + + } + }`, + options: [{ ignoredMethods: ['b'] }], + }, + { + code: ` + class A { + [\`b\`](c):void { + + } + }`, + options: [{ ignoredMethods: ['b'] }], + }, + { + code: ` + class A { + b(...c):void { + + } + + d(c):void { + + } + }`, + options: [{ ignoredMethods: ['b', 'd'] }], + }, + ], + invalid: [ + //untyped parameter + { + code: `class A { + public b(c):void { + + } + }`, + errors: [{ messageId: 'untypedParameter' }], + }, + //untyped parameter (any) + { + code: `class A { + public b(c: any):void { + + } + }`, + errors: [{ messageId: 'untypedParameter' }], + }, + //implicit public method + { + code: `class A { + b(c):void { + + } + }`, + errors: [{ messageId: 'untypedParameter' }], + }, + //implicit async public method + { + code: `class A { + async a(c): void { + } + }`, + errors: [{ messageId: 'untypedParameter' }], + }, + //no return type + { + code: `class A { + public a(c: number) { + } + }`, + errors: [{ messageId: 'noReturnType' }], + }, + //no return type + untyped parameter + { + code: `class A { + public b(c) { + + } + }`, + errors: [ + { messageId: 'untypedParameter' }, + { messageId: 'noReturnType' }, + ], + }, + //any return type + { + code: `class A { + public b(c: number): any { + + } + }`, + errors: [{ messageId: 'noReturnType' }], + }, + //with ignored methods + { + code: `class A { + public b(c: number): any { + + } + + c() { + } + }`, + options: [{ ignoredMethods: ['c'] }], + errors: [{ messageId: 'noReturnType' }], + }, + { + code: ` + let c = 'd'; + class A { + [methodName]() { + } + }`, + options: [{ ignoredMethods: ['methodName'] }], + errors: [{ messageId: 'noReturnType' }], + }, + { + code: ` + class A { + [1]() { + } + }`, + options: [{ ignoredMethods: ['1'] }], + errors: [{ messageId: 'noReturnType' }], + }, + { + code: ` + let c = 'C'; + class A { + [\`methodName\${c}\`]() { + } + }`, + options: [{ ignoredMethods: ['methodNameC', 'methodNamec'] }], + errors: [{ messageId: 'noReturnType' }], + }, + { + code: ` + let c = '1'; + class A { + [(c as number)]() { + } + }`, + options: [{ ignoredMethods: ['1'] }], + errors: [{ messageId: 'noReturnType' }], + }, + { + code: ` + class A { + abstract c() { + } + }`, + errors: [{ messageId: 'noReturnType' }], + }, + ], +});