From 88ed9ec9d6b971a9533565920fdcd6890ea941e9 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 22 Apr 2022 00:32:18 -0400 Subject: [PATCH] feat(eslint-plugin): [parameter-properties] add rule to replace `no-parameter-properties` (#4622) --- packages/eslint-plugin/README.md | 2 +- packages/eslint-plugin/ROADMAP.md | 4 +- packages/eslint-plugin/docs/rules/README.md | 2 +- .../docs/rules/parameter-properties.md | 494 ++++++++ packages/eslint-plugin/src/configs/all.ts | 2 +- packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/no-parameter-properties.ts | 2 + .../src/rules/parameter-properties.ts | 239 ++++ .../tests/rules/parameter-properties.test.ts | 1005 +++++++++++++++++ 9 files changed, 1747 insertions(+), 5 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/parameter-properties.md create mode 100644 packages/eslint-plugin/src/rules/parameter-properties.ts create mode 100644 packages/eslint-plugin/tests/rules/parameter-properties.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 1cecb6d4019..34e89f0dc21 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -135,7 +135,6 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/no-non-null-asserted-nullish-coalescing`](./docs/rules/no-non-null-asserted-nullish-coalescing.md) | Disallows using a non-null assertion in the left operand of the nullish coalescing operator | | | | | [`@typescript-eslint/no-non-null-asserted-optional-chain`](./docs/rules/no-non-null-asserted-optional-chain.md) | Disallows using a non-null assertion after an optional chain expression | :white_check_mark: | | | | [`@typescript-eslint/no-non-null-assertion`](./docs/rules/no-non-null-assertion.md) | Disallows non-null assertions using the `!` postfix operator | :white_check_mark: | | | -| [`@typescript-eslint/no-parameter-properties`](./docs/rules/no-parameter-properties.md) | Disallow the use of parameter properties in class constructors | | | | | [`@typescript-eslint/no-redundant-type-constituents`](./docs/rules/no-redundant-type-constituents.md) | Disallow members of unions and intersections that do nothing or override type information | | | :thought_balloon: | | [`@typescript-eslint/no-require-imports`](./docs/rules/no-require-imports.md) | Disallows invocation of `require()` | | | | | [`@typescript-eslint/no-this-alias`](./docs/rules/no-this-alias.md) | Disallow aliasing `this` | :white_check_mark: | | | @@ -154,6 +153,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/no-useless-empty-export`](./docs/rules/no-useless-empty-export.md) | Disallow empty exports that don't change anything in a module file | | :wrench: | | | [`@typescript-eslint/no-var-requires`](./docs/rules/no-var-requires.md) | Disallows the use of require statements except in import statements | :white_check_mark: | | | | [`@typescript-eslint/non-nullable-type-assertion-style`](./docs/rules/non-nullable-type-assertion-style.md) | Prefers a non-null assertion over explicit type cast when possible | | :wrench: | :thought_balloon: | +| [`@typescript-eslint/parameter-properties`](./docs/rules/parameter-properties.md) | Require or disallow the use of parameter properties in class constructors | | | | | [`@typescript-eslint/prefer-as-const`](./docs/rules/prefer-as-const.md) | Prefer usage of `as const` over literal type | :white_check_mark: | :wrench: | | | [`@typescript-eslint/prefer-enum-initializers`](./docs/rules/prefer-enum-initializers.md) | Prefer initializing each enums member value | | | | | [`@typescript-eslint/prefer-for-of`](./docs/rules/prefer-for-of.md) | Prefer a ‘for-of’ loop over a standard ‘for’ loop if the index is only used to access the array being iterated | | | | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index d6f0cf393db..6ebff3867c6 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -169,7 +169,7 @@ It lists all TSLint rules along side rules from the ESLint ecosystem that are th | [`no-boolean-literal-compare`] | ✅ | [`@typescript-eslint/no-unnecessary-boolean-literal-compare`] | | [`no-consecutive-blank-lines`] | 🌟 | [`no-multiple-empty-lines`][no-multiple-empty-lines] | | [`no-irregular-whitespace`] | 🌟 | [`no-irregular-whitespace`][no-irregular-whitespace] with `skipStrings: false` | -| [`no-parameter-properties`] | ✅ | [`@typescript-eslint/no-parameter-properties`] | +| [`no-parameter-properties`] | ✅ | [`@typescript-eslint/parameter-properties`] | | [`no-redundant-jsdoc`] | 🔌 | [`jsdoc/no-types`] | | [`no-reference-import`] | ✅ | [`@typescript-eslint/triple-slash-reference`] | | [`no-trailing-whitespace`] | 🌟 | [`no-trailing-spaces`][no-trailing-spaces] | @@ -633,7 +633,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- [`@typescript-eslint/naming-convention`]: https://typescript-eslint.io/rules/naming-convention [`@typescript-eslint/interface-name-prefix`]: https://typescript-eslint.io/rules/interface-name-prefix [`@typescript-eslint/naming-convention`]: https://typescript-eslint.io/rules/naming-convention -[`@typescript-eslint/no-parameter-properties`]: https://typescript-eslint.io/rules/no-parameter-properties +[`@typescript-eslint/parameter-properties`]: https://typescript-eslint.io/rules/parameter-properties [`@typescript-eslint/member-delimiter-style`]: https://typescript-eslint.io/rules/member-delimiter-style [`@typescript-eslint/prefer-for-of`]: https://typescript-eslint.io/rules/prefer-for-of [`@typescript-eslint/no-array-constructor`]: https://typescript-eslint.io/rules/no-array-constructor diff --git a/packages/eslint-plugin/docs/rules/README.md b/packages/eslint-plugin/docs/rules/README.md index 3f54ec04637..af0c386ee00 100644 --- a/packages/eslint-plugin/docs/rules/README.md +++ b/packages/eslint-plugin/docs/rules/README.md @@ -59,7 +59,6 @@ slug: / | [`@typescript-eslint/no-non-null-asserted-nullish-coalescing`](./no-non-null-asserted-nullish-coalescing.md) | Disallows using a non-null assertion in the left operand of the nullish coalescing operator | | | | | [`@typescript-eslint/no-non-null-asserted-optional-chain`](./no-non-null-asserted-optional-chain.md) | Disallows using a non-null assertion after an optional chain expression | :white_check_mark: | | | | [`@typescript-eslint/no-non-null-assertion`](./no-non-null-assertion.md) | Disallows non-null assertions using the `!` postfix operator | :white_check_mark: | | | -| [`@typescript-eslint/no-parameter-properties`](./no-parameter-properties.md) | Disallow the use of parameter properties in class constructors | | | | | [`@typescript-eslint/no-require-imports`](./no-require-imports.md) | Disallows invocation of `require()` | | | | | [`@typescript-eslint/no-this-alias`](./no-this-alias.md) | Disallow aliasing `this` | :white_check_mark: | | | | [`@typescript-eslint/no-type-alias`](./no-type-alias.md) | Disallow the use of type aliases | | | | @@ -76,6 +75,7 @@ slug: / | [`@typescript-eslint/no-unsafe-return`](./no-unsafe-return.md) | Disallows returning any from a function | :white_check_mark: | | :thought_balloon: | | [`@typescript-eslint/no-var-requires`](./no-var-requires.md) | Disallows the use of require statements except in import statements | :white_check_mark: | | | | [`@typescript-eslint/non-nullable-type-assertion-style`](./non-nullable-type-assertion-style.md) | Prefers a non-null assertion over explicit type cast when possible | | :wrench: | :thought_balloon: | +| [`@typescript-eslint/parameter-properties`](./parameter-properties.md) | Require or disallow the use of parameter properties in class constructors | | | | | [`@typescript-eslint/prefer-as-const`](./prefer-as-const.md) | Prefer usage of `as const` over literal type | :white_check_mark: | :wrench: | | | [`@typescript-eslint/prefer-enum-initializers`](./prefer-enum-initializers.md) | Prefer initializing each enums member value | | | | | [`@typescript-eslint/prefer-for-of`](./prefer-for-of.md) | Prefer a ‘for-of’ loop over a standard ‘for’ loop if the index is only used to access the array being iterated | | | | diff --git a/packages/eslint-plugin/docs/rules/parameter-properties.md b/packages/eslint-plugin/docs/rules/parameter-properties.md new file mode 100644 index 00000000000..0974300bce3 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/parameter-properties.md @@ -0,0 +1,494 @@ +# `parameter-properties` + +Require or disallow the use of parameter properties in class constructors. + +Parameter properties can be confusing to those new to TypeScript as they are less explicit than other ways +of declaring and initializing class members. + +## Rule Details + +This rule disallows the use of parameter properties in constructors, forcing the user to explicitly +declare all properties in the class. + +## Options + +This rule, in its default state, does not require any argument and would completely disallow the use of parameter properties. +It may take an options object containing either or both of: + +- `"allows"`: allowing certain kinds of properties to be ignored +- `"prefer"`: either `"class-properties"` _(default)_ or `"parameter-properties"` + +### `"allows"` + +If you would like to ignore certain kinds of properties then you may pass an object containing `"allows"` as an array of any of the following options: + +- `allows`, an array containing one or more of the allowed modifiers. Valid values are: + - `readonly`, allows **readonly** parameter properties. + - `private`, allows **private** parameter properties. + - `protected`, allows **protected** parameter properties. + - `public`, allows **public** parameter properties. + - `private readonly`, allows **private readonly** parameter properties. + - `protected readonly`, allows **protected readonly** parameter properties. + - `public readonly`, allows **public readonly** parameter properties. + +For example, to ignore `public` properties: + +```json +{ + "@typescript-eslint/parameter-properties": [ + true, + { + "allow": ["public"] + } + ] +} +``` + +### `"prefer"` + +By default, the rule prefers class properties (`"class-properties"`). +You can switch it to instead preferring parameter properties with (`"parameter-properties"`). + +In `"parameter-properties"` mode, the rule will issue a report when: + +- A class property and constructor parameter have the same name and type +- The constructor parameter is assigned to the class property at the beginning of the constructor + +### default + +Examples of code for this rule with no options at all: + + + +#### ❌ Incorrect + +```ts +class Foo { + constructor(readonly name: string) {} +} + +class Foo { + constructor(private name: string) {} +} + +class Foo { + constructor(protected name: string) {} +} + +class Foo { + constructor(public name: string) {} +} + +class Foo { + constructor(private readonly name: string) {} +} + +class Foo { + constructor(protected readonly name: string) {} +} + +class Foo { + constructor(public readonly name: string) {} +} +``` + +#### ✅ Correct + +```ts +class Foo { + constructor(name: string) {} +} +``` + +### readonly + +Examples of code for the `{ "allows": ["readonly"] }` options: + + + +#### ❌ Incorrect + +```ts +class Foo { + constructor(private name: string) {} +} + +class Foo { + constructor(protected name: string) {} +} + +class Foo { + constructor(public name: string) {} +} + +class Foo { + constructor(private readonly name: string) {} +} + +class Foo { + constructor(protected readonly name: string) {} +} + +class Foo { + constructor(public readonly name: string) {} +} +``` + +#### ✅ Correct + +```ts +class Foo { + constructor(name: string) {} +} + +class Foo { + constructor(readonly name: string) {} +} +``` + +### private + +Examples of code for the `{ "allows": ["private"] }` options: + + + +#### ❌ Incorrect + +```ts +class Foo { + constructor(readonly name: string) {} +} + +class Foo { + constructor(protected name: string) {} +} + +class Foo { + constructor(public name: string) {} +} + +class Foo { + constructor(private readonly name: string) {} +} + +class Foo { + constructor(protected readonly name: string) {} +} + +class Foo { + constructor(public readonly name: string) {} +} +``` + +#### ✅ Correct + +```ts +class Foo { + constructor(name: string) {} +} + +class Foo { + constructor(private name: string) {} +} +``` + +### protected + +Examples of code for the `{ "allows": ["protected"] }` options: + + + +#### ❌ Incorrect + +```ts +class Foo { + constructor(readonly name: string) {} +} + +class Foo { + constructor(private name: string) {} +} + +class Foo { + constructor(public name: string) {} +} + +class Foo { + constructor(private readonly name: string) {} +} + +class Foo { + constructor(protected readonly name: string) {} +} + +class Foo { + constructor(public readonly name: string) {} +} +``` + +#### ✅ Correct + +```ts +class Foo { + constructor(name: string) {} +} + +class Foo { + constructor(protected name: string) {} +} +``` + +### public + +Examples of code for the `{ "allows": ["public"] }` options: + + + +#### ❌ Incorrect + +```ts +class Foo { + constructor(readonly name: string) {} +} + +class Foo { + constructor(private name: string) {} +} + +class Foo { + constructor(protected name: string) {} +} + +class Foo { + constructor(private readonly name: string) {} +} + +class Foo { + constructor(protected readonly name: string) {} +} + +class Foo { + constructor(public readonly name: string) {} +} +``` + +#### ✅ Correct + +```ts +class Foo { + constructor(name: string) {} +} + +class Foo { + constructor(public name: string) {} +} +``` + +### private readonly + +Examples of code for the `{ "allows": ["private readonly"] }` options: + + + +#### ❌ Incorrect + +```ts +class Foo { + constructor(readonly name: string) {} +} + +class Foo { + constructor(private name: string) {} +} + +class Foo { + constructor(protected name: string) {} +} + +class Foo { + constructor(public name: string) {} +} + +class Foo { + constructor(protected readonly name: string) {} +} + +class Foo { + constructor(public readonly name: string) {} +} +``` + +#### ✅ Correct + +```ts +class Foo { + constructor(name: string) {} +} + +class Foo { + constructor(private readonly name: string) {} +} +``` + +### protected readonly + +Examples of code for the `{ "allows": ["protected readonly"] }` options: + + + +#### ❌ Incorrect + +```ts +class Foo { + constructor(readonly name: string) {} +} + +class Foo { + constructor(private name: string) {} +} + +class Foo { + constructor(protected name: string) {} +} + +class Foo { + constructor(public name: string) {} +} + +class Foo { + constructor(private readonly name: string) {} +} + +class Foo { + constructor(public readonly name: string) {} +} +``` + +#### ✅ Correct + +```ts +class Foo { + constructor(name: string) {} +} + +class Foo { + constructor(protected readonly name: string) {} +} +``` + +### public readonly + +Examples of code for the `{ "allows": ["public readonly"] }` options: + + + +#### ❌ Incorrect + +```ts +class Foo { + constructor(readonly name: string) {} +} + +class Foo { + constructor(private name: string) {} +} + +class Foo { + constructor(protected name: string) {} +} + +class Foo { + constructor(public name: string) {} +} + +class Foo { + constructor(private readonly name: string) {} +} + +class Foo { + constructor(protected readonly name: string) {} +} +``` + +#### ✅ Correct + +```ts +class Foo { + constructor(name: string) {} +} + +class Foo { + constructor(public readonly name: string) {} +} +``` + +### `"parameter-properties"` + +Examples of code for the `{ "prefer": ["parameter-properties"] }` option: + + + +#### ❌ Incorrect + +```ts +class Foo { + private name: string; + constructor(name: string) { + this.name = name; + } +} + +class Foo { + public readonly name: string; + constructor(name: string) { + this.name = name; + } +} + +class Foo { + constructor(name: string) { + this.name = name; + } + name: string; +} +``` + +#### ✅ Correct + +```ts +class Foo { + private differentName: string; + constructor(name: string) { + this.differentName = name; + } +} + +class Foo { + private differentType: number | undefined; + constructor(differentType: number) { + this.differentType = differentType; + } +} + +class Foo { + protected logicInConstructor: string; + constructor(logicInConstructor: string) { + console.log('Hello, world!'); + this.logicInConstructor = logicInConstructor; + } +} +``` + +## When Not To Use It + +If you don't care about the using parameter properties in constructors, then you will not need this rule. + +## Related To + +- TSLint: [no-parameter-properties](https://palantir.github.io/tslint/rules/no-parameter-properties/) + +## Attributes + +- [ ] ✅ Recommended +- [ ] 🔧 Fixable +- [ ] 💭 Requires type information diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 03188b72464..c55e0e6ac37 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -85,7 +85,7 @@ export = { '@typescript-eslint/no-non-null-asserted-nullish-coalescing': 'error', '@typescript-eslint/no-non-null-asserted-optional-chain': 'error', '@typescript-eslint/no-non-null-assertion': 'error', - '@typescript-eslint/no-parameter-properties': 'error', + '@typescript-eslint/parameter-properties': 'error', 'no-redeclare': 'off', '@typescript-eslint/no-redeclare': 'error', '@typescript-eslint/no-redundant-type-constituents': 'error', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index c2f90319e38..fb48ee5cadc 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -87,6 +87,7 @@ import noVarRequires from './no-var-requires'; import nonNullableTypeAssertionStyle from './non-nullable-type-assertion-style'; import objectCurlySpacing from './object-curly-spacing'; import paddingLineBetweenStatements from './padding-line-between-statements'; +import parameterProperties from './parameter-properties'; import preferAsConst from './prefer-as-const'; import preferEnumInitializers from './prefer-enum-initializers'; import preferForOf from './prefer-for-of'; @@ -213,6 +214,7 @@ export default { 'non-nullable-type-assertion-style': nonNullableTypeAssertionStyle, 'object-curly-spacing': objectCurlySpacing, 'padding-line-between-statements': paddingLineBetweenStatements, + 'parameter-properties': parameterProperties, 'prefer-as-const': preferAsConst, 'prefer-enum-initializers': preferEnumInitializers, 'prefer-for-of': preferForOf, diff --git a/packages/eslint-plugin/src/rules/no-parameter-properties.ts b/packages/eslint-plugin/src/rules/no-parameter-properties.ts index 3ab01614e72..d510332f0c0 100644 --- a/packages/eslint-plugin/src/rules/no-parameter-properties.ts +++ b/packages/eslint-plugin/src/rules/no-parameter-properties.ts @@ -19,6 +19,8 @@ type MessageIds = 'noParamProp'; export default util.createRule({ name: 'no-parameter-properties', meta: { + deprecated: true, + replacedBy: ['@typescript-eslint/parameter-properties'], type: 'problem', docs: { description: diff --git a/packages/eslint-plugin/src/rules/parameter-properties.ts b/packages/eslint-plugin/src/rules/parameter-properties.ts new file mode 100644 index 00000000000..4e3532a2fc7 --- /dev/null +++ b/packages/eslint-plugin/src/rules/parameter-properties.ts @@ -0,0 +1,239 @@ +import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/utils'; +import * as util from '../util'; + +type Modifier = + | 'readonly' + | 'private' + | 'protected' + | 'public' + | 'private readonly' + | 'protected readonly' + | 'public readonly'; + +type Prefer = 'class-property' | 'parameter-property'; + +type Options = [ + { + allow?: Modifier[]; + prefer?: Prefer; + }, +]; + +type MessageIds = 'preferClassProperty' | 'preferParameterProperty'; + +export default util.createRule({ + name: 'parameter-properties', + meta: { + type: 'problem', + docs: { + description: + 'Require or disallow the use of parameter properties in class constructors', + recommended: false, + }, + messages: { + preferClassProperty: + 'Property {{parameter}} should be declared as a class property.', + preferParameterProperty: + 'Property {{parameter}} should be declared as a parameter property.', + }, + schema: [ + { + type: 'object', + properties: { + allow: { + type: 'array', + items: { + enum: [ + 'readonly', + 'private', + 'protected', + 'public', + 'private readonly', + 'protected readonly', + 'public readonly', + ], + }, + minItems: 1, + }, + prefer: { + enum: ['class-property', 'parameter-property'], + }, + }, + additionalProperties: false, + }, + ], + }, + defaultOptions: [ + { + allow: [], + prefer: 'class-property', + }, + ], + create(context, [{ allow = [], prefer = 'class-property' }]) { + /** + * Gets the modifiers of `node`. + * @param node the node to be inspected. + */ + function getModifiers( + node: TSESTree.PropertyDefinition | TSESTree.TSParameterProperty, + ): Modifier { + const modifiers: Modifier[] = []; + + if (node.accessibility) { + modifiers.push(node.accessibility); + } + if (node.readonly) { + modifiers.push('readonly'); + } + + return modifiers.filter(Boolean).join(' ') as Modifier; + } + + if (prefer === 'class-property') { + return { + TSParameterProperty(node): void { + const modifiers = getModifiers(node); + + if (!allow.includes(modifiers)) { + // HAS to be an identifier or assignment or TSC will throw + if ( + node.parameter.type !== AST_NODE_TYPES.Identifier && + node.parameter.type !== AST_NODE_TYPES.AssignmentPattern + ) { + return; + } + + const name = + node.parameter.type === AST_NODE_TYPES.Identifier + ? node.parameter.name + : // has to be an Identifier or TSC will throw an error + (node.parameter.left as TSESTree.Identifier).name; + + context.report({ + node, + messageId: 'preferClassProperty', + data: { + parameter: name, + }, + }); + } + }, + }; + } + + interface PropertyNodes { + classProperty?: TSESTree.PropertyDefinition; + constructorAssignment?: TSESTree.AssignmentExpression; + constructorParameter?: TSESTree.Identifier; + } + + const propertyNodesByNameStack: Map[] = []; + + function getNodesByName(name: string): PropertyNodes { + const propertyNodesByName = + propertyNodesByNameStack[propertyNodesByNameStack.length - 1]; + const existing = propertyNodesByName.get(name); + if (existing) { + return existing; + } + + const created: PropertyNodes = {}; + propertyNodesByName.set(name, created); + return created; + } + + const sourceCode = context.getSourceCode(); + + function typeAnnotationsMatch( + classProperty: TSESTree.PropertyDefinition, + constructorParameter: TSESTree.Identifier, + ): boolean { + if ( + !classProperty.typeAnnotation || + !constructorParameter.typeAnnotation + ) { + return ( + classProperty.typeAnnotation === constructorParameter.typeAnnotation + ); + } + + return ( + sourceCode.getText(classProperty.typeAnnotation) === + sourceCode.getText(constructorParameter.typeAnnotation) + ); + } + + return { + 'ClassDeclaration, ClassExpression'(): void { + propertyNodesByNameStack.push(new Map()); + }, + + ':matches(ClassDeclaration, ClassExpression):exit'(): void { + const propertyNodesByName = propertyNodesByNameStack.pop()!; + + for (const [name, nodes] of propertyNodesByName) { + if ( + nodes.classProperty && + nodes.constructorAssignment && + nodes.constructorParameter && + typeAnnotationsMatch( + nodes.classProperty, + nodes.constructorParameter, + ) + ) { + context.report({ + data: { + parameter: name, + }, + messageId: 'preferParameterProperty', + node: nodes.classProperty, + }); + } + } + }, + + ClassBody(node): void { + for (const element of node.body) { + if ( + element.type === AST_NODE_TYPES.PropertyDefinition && + element.key.type === AST_NODE_TYPES.Identifier && + !element.value && + !allow.includes(getModifiers(element)) + ) { + getNodesByName(element.key.name).classProperty = element; + } + } + }, + + 'MethodDefinition[kind="constructor"]'( + node: TSESTree.MethodDefinition, + ): void { + for (const parameter of node.value.params) { + if (parameter.type === AST_NODE_TYPES.Identifier) { + getNodesByName(parameter.name).constructorParameter = parameter; + } + } + + for (const statement of node.value.body?.body ?? []) { + if ( + statement.type !== AST_NODE_TYPES.ExpressionStatement || + statement.expression.type !== AST_NODE_TYPES.AssignmentExpression || + statement.expression.left.type !== + AST_NODE_TYPES.MemberExpression || + statement.expression.left.object.type !== + AST_NODE_TYPES.ThisExpression || + statement.expression.left.property.type !== + AST_NODE_TYPES.Identifier || + statement.expression.right.type !== AST_NODE_TYPES.Identifier + ) { + break; + } + + getNodesByName( + statement.expression.right.name, + ).constructorAssignment = statement.expression; + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/parameter-properties.test.ts b/packages/eslint-plugin/tests/rules/parameter-properties.test.ts new file mode 100644 index 00000000000..44580ad1b92 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/parameter-properties.test.ts @@ -0,0 +1,1005 @@ +import rule from '../../src/rules/parameter-properties'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('parameter-properties', rule, { + valid: [ + ` +class Foo { + constructor(name: string) {} +} + `, + { + code: ` +class Foo { + constructor(name: string) {} +} + `, + options: [{ prefer: 'class-property' }], + }, + ` +class Foo { + constructor(...name: string[]) {} +} + `, + ` +class Foo { + constructor(name: string, age: number) {} +} + `, + ` +class Foo { + constructor(name: string); + constructor(name: string, age?: number) {} +} + `, + { + code: ` +class Foo { + constructor(readonly name: string) {} +} + `, + options: [{ allow: ['readonly'] }], + }, + { + code: ` +class Foo { + constructor(private name: string) {} +} + `, + options: [{ allow: ['private'] }], + }, + { + code: ` +class Foo { + constructor(protected name: string) {} +} + `, + options: [{ allow: ['protected'] }], + }, + { + code: ` +class Foo { + constructor(public name: string) {} +} + `, + options: [{ allow: ['public'] }], + }, + { + code: ` +class Foo { + constructor(private readonly name: string) {} +} + `, + options: [{ allow: ['private readonly'] }], + }, + { + code: ` +class Foo { + constructor(protected readonly name: string) {} +} + `, + options: [{ allow: ['protected readonly'] }], + }, + { + code: ` +class Foo { + constructor(public readonly name: string) {} +} + `, + options: [{ allow: ['public readonly'] }], + }, + { + code: ` +class Foo { + constructor(readonly name: string, private age: number) {} +} + `, + options: [{ allow: ['readonly', 'private'] }], + }, + { + code: ` +class Foo { + constructor(public readonly name: string, private age: number) {} +} + `, + options: [{ allow: ['public readonly', 'private'] }], + }, + // Semantically invalid test case + ` +class Foo { + constructor(private ...name: string[]) {} +} + `, + // Semantically invalid test case + ` +class Foo { + constructor(private [test]: [string]) {} +} + `, + { + code: ` +class Foo { + constructor(private name: string[]) {} +} + `, + options: [{ prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + constructor(...name: string[]) {} +} + `, + options: [{ prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + constructor(age: string, ...name: string[]) {} +} + `, + options: [{ prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + constructor(private age: string, ...name: string[]) {} +} + `, + options: [{ prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + public age: number; + constructor(age: string) { + this.age = age; + } +} + `, + options: [{ prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + public age = ''; + constructor(age: string) { + this.age = age; + } +} + `, + options: [{ prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + public age; + constructor(age: string) { + this.age = age; + } +} + `, + options: [{ prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + public age: string; + constructor(age) { + this.age = age; + } +} + `, + options: [{ prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + public age: string; + constructor(age: string) { + console.log('unrelated'); + this.age = age; + } +} + `, + options: [{ prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + other: string; + constructor(age: string) { + this.other = age; + } +} + `, + options: [{ prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + age: string; + constructor(age: string) { + this.age = ''; + console.log(age); + } +} + `, + options: [{ prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + age() { + return ''; + } + constructor(age: string) { + this.age = age; + } +} + `, + options: [{ prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + public age: string; + constructor(age: string) { + this.age = age; + } +} + `, + options: [{ allow: ['public'], prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + public readonly age: string; + constructor(age: string) { + this.age = age; + } +} + `, + options: [{ allow: ['public readonly'], prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + protected age: string; + constructor(age: string) { + this.age = age; + } +} + `, + options: [{ allow: ['protected'], prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + protected readonly age: string; + constructor(age: string) { + this.age = age; + } +} + `, + options: [ + { allow: ['protected readonly'], prefer: 'parameter-property' }, + ], + }, + { + code: ` +class Foo { + private age: string; + constructor(age: string) { + this.age = age; + } +} + `, + options: [{ allow: ['private'], prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + private readonly age: string; + constructor(age: string) { + this.age = age; + } +} + `, + options: [{ allow: ['private readonly'], prefer: 'parameter-property' }], + }, + ], + invalid: [ + { + code: ` +class Foo { + constructor(readonly name: string) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(private name: string) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(protected name: string) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(public name: string) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(private readonly name: string) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(protected readonly name: string) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(public readonly name: string) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(public name: string, age: number) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(private name: string, private age: number) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + { + messageId: 'preferClassProperty', + data: { + parameter: 'age', + }, + line: 3, + column: 37, + }, + ], + }, + { + code: ` +class Foo { + constructor(protected name: string, protected age: number) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + { + messageId: 'preferClassProperty', + data: { + parameter: 'age', + }, + line: 3, + column: 39, + }, + ], + }, + { + code: ` +class Foo { + constructor(public name: string, public age: number) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + { + messageId: 'preferClassProperty', + data: { + parameter: 'age', + }, + line: 3, + column: 36, + }, + ], + }, + { + code: ` +class Foo { + constructor(name: string); + constructor(private name: string, age?: number) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 4, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(private name: string); + constructor(private name: string, age?: number) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 4, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(private name: string); + constructor(private name: string, private age?: number) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 4, + column: 15, + }, + { + messageId: 'preferClassProperty', + data: { + parameter: 'age', + }, + line: 4, + column: 37, + }, + ], + }, + { + code: ` +class Foo { + constructor(name: string); + constructor(protected name: string, age?: number) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 4, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(protected name: string); + constructor(protected name: string, age?: number) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 4, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(protected name: string); + constructor(protected name: string, protected age?: number) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 4, + column: 15, + }, + { + messageId: 'preferClassProperty', + data: { + parameter: 'age', + }, + line: 4, + column: 39, + }, + ], + }, + { + code: ` +class Foo { + constructor(name: string); + constructor(public name: string, age?: number) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 4, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(public name: string); + constructor(public name: string, age?: number) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 4, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(public name: string); + constructor(public name: string, public age?: number) {} +} + `, + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 4, + column: 15, + }, + { + messageId: 'preferClassProperty', + data: { + parameter: 'age', + }, + line: 4, + column: 36, + }, + ], + }, + + { + code: ` +class Foo { + constructor(readonly name: string) {} +} + `, + options: [{ allow: ['private'] }], + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(private name: string) {} +} + `, + options: [{ allow: ['readonly'] }], + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(protected name: string) {} +} + `, + options: [ + { + allow: ['readonly', 'private', 'public', 'protected readonly'], + }, + ], + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(public name: string) {} +} + `, + options: [ + { + allow: [ + 'readonly', + 'private', + 'protected', + 'protected readonly', + 'public readonly', + ], + }, + ], + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(private readonly name: string) {} +} + `, + options: [{ allow: ['readonly', 'private'] }], + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(protected readonly name: string) {} +} + `, + options: [ + { + allow: [ + 'readonly', + 'protected', + 'private readonly', + 'public readonly', + ], + }, + ], + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'name', + }, + line: 3, + column: 15, + }, + ], + }, + { + code: ` +class Foo { + constructor(private name: string); + constructor(private name: string, protected age?: number) {} +} + `, + options: [{ allow: ['private'] }], + errors: [ + { + messageId: 'preferClassProperty', + data: { + parameter: 'age', + }, + line: 4, + column: 37, + }, + ], + }, + { + code: ` +class Foo { + member: string; + + constructor(member: string) { + this.member = member; + } +} + `, + errors: [ + { + messageId: 'preferParameterProperty', + data: { + parameter: 'member', + }, + line: 3, + column: 3, + }, + ], + options: [{ prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + constructor(member: string) { + this.member = member; + } + + member: string; +} + `, + errors: [ + { + messageId: 'preferParameterProperty', + data: { + parameter: 'member', + }, + line: 7, + column: 3, + }, + ], + options: [{ prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + member; + constructor(member) { + this.member = member; + } +} + `, + errors: [ + { + messageId: 'preferParameterProperty', + data: { + parameter: 'member', + }, + line: 3, + column: 3, + }, + ], + options: [{ prefer: 'parameter-property' }], + }, + { + code: ` +class Foo { + public member: string; + constructor(member: string) { + this.member = member; + } +} + `, + errors: [ + { + messageId: 'preferParameterProperty', + data: { + parameter: 'member', + }, + line: 3, + column: 3, + }, + ], + options: [ + { + allow: ['protected', 'private', 'readonly'], + prefer: 'parameter-property', + }, + ], + }, + ], +});