From 76b89a50489217ced0250a046969b557cc2e6a6a Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 24 Jun 2019 11:14:14 -0500 Subject: [PATCH] feat(eslint-plugin): added new rule prefer-readonly (#555) * feat(eslint-plugin): added new rule prefer-readonly Adds the equivalent of TSLint's `prefer-readonly` rule. * Added docs, auto-fixing * Updated docs; love the new build time checks! * Fixed linting errors (ha) and corrected internal source * PR feedback: non recommended; :exit; some test coverage * I guess tslintRuleName isn't allowed now? * Added back recommended as false * Removed :exit; fixed README.md table --- .../eslint-plugin-tslint/src/custom-linter.ts | 2 +- packages/eslint-plugin/README.md | 1 + packages/eslint-plugin/ROADMAP.md | 3 +- .../docs/rules/prefer-readonly.md | 81 +++ packages/eslint-plugin/src/configs/all.json | 1 + .../indent-new-do-not-use/OffsetStorage.ts | 14 +- .../rules/indent-new-do-not-use/TokenInfo.ts | 2 +- packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/prefer-readonly.ts | 335 +++++++++++ packages/eslint-plugin/src/util/types.ts | 27 + .../tests/rules/prefer-readonly.test.ts | 549 ++++++++++++++++++ packages/typescript-estree/src/convert.ts | 6 +- 12 files changed, 1010 insertions(+), 13 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/prefer-readonly.md create mode 100644 packages/eslint-plugin/src/rules/prefer-readonly.ts create mode 100644 packages/eslint-plugin/tests/rules/prefer-readonly.test.ts diff --git a/packages/eslint-plugin-tslint/src/custom-linter.ts b/packages/eslint-plugin-tslint/src/custom-linter.ts index 892e12f1adf..833fda00ca9 100644 --- a/packages/eslint-plugin-tslint/src/custom-linter.ts +++ b/packages/eslint-plugin-tslint/src/custom-linter.ts @@ -4,7 +4,7 @@ import { Program } from 'typescript'; const TSLintLinter = Linter as any; export class CustomLinter extends TSLintLinter { - constructor(options: ILinterOptions, private program: Program) { + constructor(options: ILinterOptions, private readonly program: Program) { super(options, program); } diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 68ff8434d8a..d5163109b1e 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -171,6 +171,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/prefer-function-type`](./docs/rules/prefer-function-type.md) | Use function types instead of interfaces with call signatures | | :wrench: | | | [`@typescript-eslint/prefer-includes`](./docs/rules/prefer-includes.md) | Enforce `includes` method over `indexOf` method | | :wrench: | :thought_balloon: | | [`@typescript-eslint/prefer-namespace-keyword`](./docs/rules/prefer-namespace-keyword.md) | Require the use of the `namespace` keyword instead of the `module` keyword to declare custom TypeScript modules | :heavy_check_mark: | :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-regexp-exec`](./docs/rules/prefer-regexp-exec.md) | Prefer RegExp#exec() over String#match() if no global flag is provided | | | :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 | | :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: | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index d47abb01d05..8a527cd3313 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -122,7 +122,7 @@ | [`no-require-imports`] | ✅ | [`@typescript-eslint/no-require-imports`] | | [`object-literal-sort-keys`] | 🌓 | [`sort-keys`][sort-keys] [2] | | [`prefer-const`] | 🌟 | [`prefer-const`][prefer-const] | -| [`prefer-readonly`] | 🛑 | N/A | +| [`prefer-readonly`] | ✅ | [`@typescript-eslint/prefer-readonly`] | | [`trailing-comma`] | 🌓 | [`comma-dangle`][comma-dangle] or [Prettier] | [1] Only warns when importing deprecated symbols
@@ -611,6 +611,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- [`@typescript-eslint/prefer-interface`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/prefer-interface.md [`@typescript-eslint/no-array-constructor`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-array-constructor.md [`@typescript-eslint/prefer-function-type`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/prefer-function-type.md +[`@typescript-eslint/prefer-readonly`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/prefer-readonly.md [`@typescript-eslint/no-for-in-array`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-for-in-array.md [`@typescript-eslint/no-unnecessary-qualifier`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unnecessary-qualifier.md [`@typescript-eslint/semi`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/semi.md diff --git a/packages/eslint-plugin/docs/rules/prefer-readonly.md b/packages/eslint-plugin/docs/rules/prefer-readonly.md new file mode 100644 index 00000000000..b3f0a542010 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/prefer-readonly.md @@ -0,0 +1,81 @@ +# require never-modified private members be marked as `readonly` + +This rule enforces that private members are marked as `readonly` if they're never modified outside of the constructor. + +## Rule Details + +Member variables with the privacy `private` are never permitted to be modified outside of their declaring class. +If that class never modifies their value, they may safely be marked as `readonly`. + +Examples of **incorrect** code for this rule: + +```ts +class Container { + // These member variables could be marked as readonly + private neverModifiedMember = true; + private onlyModifiedInConstructor: number; + + public constructor( + onlyModifiedInConstructor: number, + // Private parameter properties can also be marked as reaodnly + private neverModifiedParameter: string, + ) { + this.onlyModifiedInConstructor = onlyModifiedInConstructor; + } +} +``` + +Examples of **correct** code for this rule: + +```ts +class Container { + // Public members might be modified externally + public publicMember: boolean; + + // Protected members might be modified by child classes + protected protectedMember: number; + + // This is modified later on by the class + private modifiedLater = 'unchanged'; + + public mutate() { + this.modifiedLater = 'mutated'; + } +} +``` + +## Options + +This rule, in its default state, does not require any argument. + +### onlyInlineLambdas + +You may pass `"onlyInlineLambdas": true` as a rule option within an object to restrict checking only to members immediately assigned a lambda value. + +```cjson +{ + "@typescript-eslint/prefer-readonly": ["error", { "onlyInlineLambdas": true }] +} +``` + +Example of **correct** code for the `{ "onlyInlineLambdas": true }` options: + +```ts +class Container { + private neverModifiedPrivate = 'unchanged'; +} +``` + +Example of **incorrect** code for the `{ "onlyInlineLambdas": true }` options: + +```ts +class Container { + private onClick = () => { + /* ... */ + }; +} +``` + +## Related to + +- TSLint: ['prefer-readonly'](https://palantir.github.io/tslint/rules/prefer-readonly) diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 2efc8d24323..7dc471fdd5d 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -57,6 +57,7 @@ "@typescript-eslint/prefer-function-type": "error", "@typescript-eslint/prefer-includes": "error", "@typescript-eslint/prefer-namespace-keyword": "error", + "@typescript-eslint/prefer-readonly": "error", "@typescript-eslint/prefer-regexp-exec": "error", "@typescript-eslint/prefer-string-starts-ends-with": "error", "@typescript-eslint/promise-function-async": "error", diff --git a/packages/eslint-plugin/src/rules/indent-new-do-not-use/OffsetStorage.ts b/packages/eslint-plugin/src/rules/indent-new-do-not-use/OffsetStorage.ts index deacc272bb3..475e3c35d7b 100644 --- a/packages/eslint-plugin/src/rules/indent-new-do-not-use/OffsetStorage.ts +++ b/packages/eslint-plugin/src/rules/indent-new-do-not-use/OffsetStorage.ts @@ -9,13 +9,13 @@ import { TokenInfo } from './TokenInfo'; * A class to store information on desired offsets of tokens from each other */ export class OffsetStorage { - private tokenInfo: TokenInfo; - private indentSize: number; - private indentType: string; - private tree: BinarySearchTree; - private lockedFirstTokens: WeakMap; - private desiredIndentCache: WeakMap; - private ignoredTokens: WeakSet; + private readonly tokenInfo: TokenInfo; + private readonly indentSize: number; + private readonly indentType: string; + private readonly tree: BinarySearchTree; + private readonly lockedFirstTokens: WeakMap; + private readonly desiredIndentCache: WeakMap; + private readonly ignoredTokens: WeakSet; /** * @param tokenInfo a TokenInfo instance * @param indentSize The desired size of each indentation level diff --git a/packages/eslint-plugin/src/rules/indent-new-do-not-use/TokenInfo.ts b/packages/eslint-plugin/src/rules/indent-new-do-not-use/TokenInfo.ts index 9b7d345fe3d..16d15c4ae5f 100644 --- a/packages/eslint-plugin/src/rules/indent-new-do-not-use/TokenInfo.ts +++ b/packages/eslint-plugin/src/rules/indent-new-do-not-use/TokenInfo.ts @@ -8,7 +8,7 @@ import { TokenOrComment } from './BinarySearchTree'; * A helper class to get token-based info related to indentation */ export class TokenInfo { - private sourceCode: TSESLint.SourceCode; + private readonly sourceCode: TSESLint.SourceCode; public firstTokensByLineNumber: Map; constructor(sourceCode: TSESLint.SourceCode) { diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 3b799b1cc80..5d6fc99aa48 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -46,6 +46,7 @@ import preferFunctionType from './prefer-function-type'; import preferIncludes from './prefer-includes'; import preferInterface from './prefer-interface'; import preferNamespaceKeyword from './prefer-namespace-keyword'; +import preferReadonly from './prefer-readonly'; import preferRegexpExec from './prefer-regexp-exec'; import preferStringStartsEndsWith from './prefer-string-starts-ends-with'; import promiseFunctionAsync from './promise-function-async'; @@ -105,6 +106,7 @@ export default { 'prefer-includes': preferIncludes, 'prefer-interface': preferInterface, 'prefer-namespace-keyword': preferNamespaceKeyword, + 'prefer-readonly': preferReadonly, 'prefer-regexp-exec': preferRegexpExec, 'prefer-string-starts-ends-with': preferStringStartsEndsWith, 'promise-function-async': promiseFunctionAsync, diff --git a/packages/eslint-plugin/src/rules/prefer-readonly.ts b/packages/eslint-plugin/src/rules/prefer-readonly.ts new file mode 100644 index 00000000000..24685d30676 --- /dev/null +++ b/packages/eslint-plugin/src/rules/prefer-readonly.ts @@ -0,0 +1,335 @@ +import * as tsutils from 'tsutils'; +import ts from 'typescript'; +import * as util from '../util'; +import { typeIsOrHasBaseType } from '../util'; +import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/typescript-estree'; + +type MessageIds = 'preferReadonly'; + +type Options = [ + { + onlyInlineLambdas?: boolean; + } +]; + +const functionScopeBoundaries = [ + 'ArrowFunctionExpression', + 'FunctionDeclaration', + 'FunctionExpression', + 'GetAccessor', + 'MethodDefinition', + 'SetAccessor', +].join(', '); + +export default util.createRule({ + name: 'prefer-readonly', + meta: { + docs: { + description: + "Requires that private members are marked as `readonly` if they're never modified outside of the constructor", + category: 'Best Practices', + recommended: false, + }, + fixable: 'code', + messages: { + preferReadonly: + "Member '{{name}}' is never reassigned; mark it as `readonly`.", + }, + schema: [ + { + allowAdditionalProperties: false, + properties: { + onlyInlineLambdas: { + type: 'boolean', + }, + }, + type: 'object', + }, + ], + type: 'suggestion', + }, + defaultOptions: [{ onlyInlineLambdas: false }], + create(context, [{ onlyInlineLambdas }]) { + const parserServices = util.getParserServices(context); + const checker = parserServices.program.getTypeChecker(); + const classScopeStack: ClassScope[] = []; + + function handlePropertyAccessExpression( + node: ts.PropertyAccessExpression, + parent: ts.Node, + classScope: ClassScope, + ) { + if (ts.isBinaryExpression(parent)) { + handleParentBinaryExpression(node, parent, classScope); + return; + } + + if (ts.isDeleteExpression(parent)) { + classScope.addVariableModification(node); + return; + } + + if ( + ts.isPostfixUnaryExpression(parent) || + ts.isPrefixUnaryExpression(parent) + ) { + handleParentPostfixOrPrefixUnaryExpression(parent, classScope); + } + } + + function handleParentBinaryExpression( + node: ts.PropertyAccessExpression, + parent: ts.BinaryExpression, + classScope: ClassScope, + ) { + if ( + parent.left === node && + tsutils.isAssignmentKind(parent.operatorToken.kind) + ) { + classScope.addVariableModification(node); + } + } + + function handleParentPostfixOrPrefixUnaryExpression( + node: ts.PostfixUnaryExpression | ts.PrefixUnaryExpression, + classScope: ClassScope, + ) { + if ( + node.operator === ts.SyntaxKind.PlusPlusToken || + node.operator === ts.SyntaxKind.MinusMinusToken + ) { + classScope.addVariableModification( + node.operand as ts.PropertyAccessExpression, + ); + } + } + + function isConstructor(node: TSESTree.Node) { + return ( + node.type === AST_NODE_TYPES.MethodDefinition && + node.kind === 'constructor' + ); + } + + function isFunctionScopeBoundaryInStack(node: TSESTree.Node) { + if (classScopeStack.length === 0) { + return false; + } + + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); + if (ts.isConstructorDeclaration(tsNode)) { + return false; + } + + return tsutils.isFunctionScopeBoundary(tsNode); + } + + function getEsNodesFromViolatingNode( + violatingNode: ParameterOrPropertyDeclaration, + ) { + if (ts.isParameterPropertyDeclaration(violatingNode)) { + return { + esNode: parserServices.tsNodeToESTreeNodeMap.get(violatingNode.name), + nameNode: parserServices.tsNodeToESTreeNodeMap.get( + violatingNode.name, + ), + }; + } + + return { + esNode: parserServices.tsNodeToESTreeNodeMap.get(violatingNode), + nameNode: parserServices.tsNodeToESTreeNodeMap.get(violatingNode.name), + }; + } + + return { + 'ClassDeclaration, ClassExpression'( + node: TSESTree.ClassDeclaration | TSESTree.ClassExpression, + ) { + classScopeStack.push( + new ClassScope( + checker, + parserServices.esTreeNodeToTSNodeMap.get(node), + onlyInlineLambdas, + ), + ); + }, + 'ClassDeclaration, ClassExpression:exit'() { + const finalizedClassScope = classScopeStack.pop()!; + const sourceCode = context.getSourceCode(); + + for (const violatingNode of finalizedClassScope.finalizeUnmodifiedPrivateNonReadonlys()) { + const { esNode, nameNode } = getEsNodesFromViolatingNode( + violatingNode, + ); + context.report({ + data: { + name: sourceCode.getText(nameNode), + }, + fix: fixer => fixer.insertTextBefore(nameNode, 'readonly '), + messageId: 'preferReadonly', + node: esNode, + }); + } + }, + MemberExpression(node) { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get< + ts.PropertyAccessExpression + >(node); + if (classScopeStack.length !== 0) { + handlePropertyAccessExpression( + tsNode, + tsNode.parent, + classScopeStack[classScopeStack.length - 1], + ); + } + }, + [functionScopeBoundaries](node: TSESTree.Node) { + if (isConstructor(node)) { + classScopeStack[classScopeStack.length - 1].enterConstructor( + parserServices.esTreeNodeToTSNodeMap.get( + node, + ), + ); + } else if (isFunctionScopeBoundaryInStack(node)) { + classScopeStack[classScopeStack.length - 1].enterNonConstructor(); + } + }, + [`${functionScopeBoundaries}:exit`](node: TSESTree.Node) { + if (isConstructor(node)) { + classScopeStack[classScopeStack.length - 1].exitConstructor(); + } else if (isFunctionScopeBoundaryInStack(node)) { + classScopeStack[classScopeStack.length - 1].exitNonConstructor(); + } + }, + }; + }, +}); + +type ParameterOrPropertyDeclaration = + | ts.ParameterDeclaration + | ts.PropertyDeclaration; + +const OUTSIDE_CONSTRUCTOR = -1; +const DIRECTLY_INSIDE_CONSTRUCTOR = 0; + +class ClassScope { + private readonly privateModifiableMembers = new Map< + string, + ParameterOrPropertyDeclaration + >(); + private readonly privateModifiableStatics = new Map< + string, + ParameterOrPropertyDeclaration + >(); + private readonly memberVariableModifications = new Set(); + private readonly staticVariableModifications = new Set(); + + private readonly classType: ts.Type; + + private constructorScopeDepth = OUTSIDE_CONSTRUCTOR; + + public constructor( + private readonly checker: ts.TypeChecker, + classNode: ts.ClassLikeDeclaration, + private readonly onlyInlineLambdas?: boolean, + ) { + this.checker = checker; + this.classType = checker.getTypeAtLocation(classNode); + + for (const member of classNode.members) { + if (ts.isPropertyDeclaration(member)) { + this.addDeclaredVariable(member); + } + } + } + + public addDeclaredVariable(node: ParameterOrPropertyDeclaration) { + if ( + !tsutils.isModifierFlagSet(node, ts.ModifierFlags.Private) || + tsutils.isModifierFlagSet(node, ts.ModifierFlags.Readonly) || + ts.isComputedPropertyName(node.name) + ) { + return; + } + + if ( + this.onlyInlineLambdas && + node.initializer !== undefined && + !ts.isArrowFunction(node.initializer) + ) { + return; + } + + (tsutils.isModifierFlagSet(node, ts.ModifierFlags.Static) + ? this.privateModifiableStatics + : this.privateModifiableMembers + ).set(node.name.getText(), node); + } + + public addVariableModification(node: ts.PropertyAccessExpression) { + const modifierType = this.checker.getTypeAtLocation(node.expression); + if ( + modifierType.symbol === undefined || + !typeIsOrHasBaseType(modifierType, this.classType) + ) { + return; + } + + const modifyingStatic = + tsutils.isObjectType(modifierType) && + tsutils.isObjectFlagSet(modifierType, ts.ObjectFlags.Anonymous); + if ( + !modifyingStatic && + this.constructorScopeDepth === DIRECTLY_INSIDE_CONSTRUCTOR + ) { + return; + } + + (modifyingStatic + ? this.staticVariableModifications + : this.memberVariableModifications + ).add(node.name.text); + } + + public enterConstructor(node: ts.ConstructorDeclaration) { + this.constructorScopeDepth = DIRECTLY_INSIDE_CONSTRUCTOR; + + for (const parameter of node.parameters) { + if (tsutils.isModifierFlagSet(parameter, ts.ModifierFlags.Private)) { + this.addDeclaredVariable(parameter); + } + } + } + + public exitConstructor() { + this.constructorScopeDepth = OUTSIDE_CONSTRUCTOR; + } + + public enterNonConstructor() { + if (this.constructorScopeDepth !== OUTSIDE_CONSTRUCTOR) { + this.constructorScopeDepth += 1; + } + } + + public exitNonConstructor() { + if (this.constructorScopeDepth !== OUTSIDE_CONSTRUCTOR) { + this.constructorScopeDepth -= 1; + } + } + + public finalizeUnmodifiedPrivateNonReadonlys() { + this.memberVariableModifications.forEach(variableName => { + this.privateModifiableMembers.delete(variableName); + }); + + this.staticVariableModifications.forEach(variableName => { + this.privateModifiableStatics.delete(variableName); + }); + + return [ + ...Array.from(this.privateModifiableMembers.values()), + ...Array.from(this.privateModifiableStatics.values()), + ]; + } +} diff --git a/packages/eslint-plugin/src/util/types.ts b/packages/eslint-plugin/src/util/types.ts index f10bda7f731..83a8bbabf57 100644 --- a/packages/eslint-plugin/src/util/types.ts +++ b/packages/eslint-plugin/src/util/types.ts @@ -182,3 +182,30 @@ export function isTypeFlagSet( return (flags & flagsToCheck) !== 0; } + +/** + * @returns Whether a type is an instance of the parent type, including for the parent's base types. + */ +export const typeIsOrHasBaseType = (type: ts.Type, parentType: ts.Type) => { + if (type.symbol === undefined || parentType.symbol === undefined) { + return false; + } + + const typeAndBaseTypes = [type]; + const ancestorTypes = type.getBaseTypes(); + + if (ancestorTypes !== undefined) { + typeAndBaseTypes.push(...ancestorTypes); + } + + for (const baseType of typeAndBaseTypes) { + if ( + baseType.symbol !== undefined && + baseType.symbol.name === parentType.symbol.name + ) { + return true; + } + } + + return false; +}; diff --git a/packages/eslint-plugin/tests/rules/prefer-readonly.test.ts b/packages/eslint-plugin/tests/rules/prefer-readonly.test.ts new file mode 100644 index 00000000000..a5ff4734e1c --- /dev/null +++ b/packages/eslint-plugin/tests/rules/prefer-readonly.test.ts @@ -0,0 +1,549 @@ +import rule from '../../src/rules/prefer-readonly'; +import { RuleTester, getFixturesRootDir } from '../RuleTester'; + +const rootDir = getFixturesRootDir(); +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + ecmaVersion: 2015, + tsconfigRootDir: rootDir, + project: './tsconfig.json', + }, +}); + +ruleTester.run('prefer-readonly', rule, { + valid: [ + `function ignore() { }`, + `const ignore = function () { }`, + `const ignore = () => { }`, + `const container = { member: true }; + container.member;`, + `const container = { member: 1 }; + +container.member;`, + `const container = { member: 1 }; + ++container.member;`, + `const container = { member: 1 }; + container.member++;`, + `const container = { member: 1 }; + -container.member;`, + `const container = { member: 1 }; + --container.member;`, + `const container = { member: 1 }; + container.member--;`, + `class TestEmpty { }`, + `class TestReadonlyStatic { + private static readonly correctlyReadonlyStatic = 7; + }`, + `class TestModifiableStatic { + private static correctlyModifiableStatic = 7; + + public constructor() { + TestModifiableStatic.correctlyModifiableStatic += 1; + } + }`, + `class TestModifiableByParameterProperty { + private static readonly correctlyModifiableByParameterProperty = 7; + + public constructor( + public correctlyModifiablePublicParameter: number = (() => { + return TestModifiableStatic.correctlyModifiableByParameterProperty += 1; + })() + ) { } + }`, + `class TestReadonlyInline { + private readonly correctlyReadonlyInline = 7; + }`, + `class TestReadonlyDelayed { + private readonly correctlyReadonlyDelayed = 7; + + public constructor() { + this.correctlyReadonlyDelayed += 1; + } + }`, + `class TestModifiableInline { + private correctlyModifiableInline = 7; + + public mutate() { + this.correctlyModifiableInline += 1; + + return class { + private correctlyModifiableInline = 7; + + mutate() { + this.correctlyModifiableInline += 1; + } + }; + } + }`, + `class TestModifiableDelayed { + private correctlyModifiableDelayed = 7; + + public mutate() { + this.correctlyModifiableDelayed += 1; + } + }`, + `class TestModifiableDeleted { + private correctlyModifiableDeleted = 7; + + public mutate() { + delete this.correctlyModifiableDeleted; + } + }`, + `class TestModifiableWithinConstructor { + private correctlyModifiableWithinConstructor = 7; + + public constructor() { + (() => { + this.correctlyModifiableWithinConstructor += 1; + })(); + } + }`, + `class TestModifiableWithinConstructorArrowFunction { + private correctlyModifiableWithinConstructorArrowFunction = 7; + + public constructor() { + (() => { + this.correctlyModifiableWithinConstructorArrowFunction += 1; + })(); + } + }`, + `class TestModifiableWithinConstructorInFunctionExpression { + private correctlyModifiableWithinConstructorInFunctionExpression = 7; + + public constructor() { + const self = this; + + (() => { + self.correctlyModifiableWithinConstructorInFunctionExpression += 1; + })(); + } + }`, + `class TestModifiableWithinConstructorInGetAccessor { + private correctlyModifiableWithinConstructorInGetAccessor = 7; + + public constructor() { + const self = this; + + const confusingObject = { + get accessor() { + return self.correctlyModifiableWithinConstructorInGetAccessor += 1; + }, + }; + } + }`, + `class TestModifiableWithinConstructorInMethodDeclaration { + private correctlyModifiableWithinConstructorInMethodDeclaration = 7; + + public constructor() { + const self = this; + + const confusingObject = { + methodDeclaration() { + self.correctlyModifiableWithinConstructorInMethodDeclaration = 7; + } + }; + } + }`, + `class TestModifiableWithinConstructorInSetAccessor { + private correctlyModifiableWithinConstructorInSetAccessor = 7; + + public constructor() { + const self = this; + + const confusingObject = { + set accessor(value: number) { + self.correctlyModifiableWithinConstructorInSetAccessor += value; + }, + }; + } + }`, + `class TestModifiablePostDecremented { + private correctlyModifiablePostDecremented = 7; + + public mutate() { + this.correctlyModifiablePostDecremented -= 1; + } + }`, + `class TestyModifiablePostIncremented { + private correctlyModifiablePostIncremented = 7; + + public mutate() { + this.correctlyModifiablePostIncremented += 1; + } + }`, + `class TestModifiablePreDecremented { + private correctlyModifiablePreDecremented = 7; + + public mutate() { + --this.correctlyModifiablePreDecremented; + } + }`, + `class TestModifiablePreIncremented { + private correctlyModifiablePreIncremented = 7; + + public mutate() { + ++this.correctlyModifiablePreIncremented; + } + }`, + `class TestProtectedModifiable { + protected protectedModifiable = 7; + }`, + `class TestPublicModifiable { + public publicModifiable = 7; + }`, + `class TestReadonlyParameter { + public constructor( + private readonly correctlyReadonlyParameter = 7, + ) { } + }`, + `class TestCorrectlyModifiableParameter { + public constructor( + private correctlyModifiableParameter = 7, + ) { } + + public mutate() { + this.correctlyModifiableParameter += 1; + } + }`, + { + code: `class TestCorrectlyNonInlineLambdas { + private correctlyNonInlineLambda = 7; + }`, + options: [ + { + onlyInlineLambdas: true, + }, + ], + }, + ], + invalid: [ + { + code: `class TestIncorrectlyModifiableStatic { + private static incorrectlyModifiableStatic = 7; + }`, + errors: [ + { + data: { + name: 'incorrectlyModifiableStatic', + }, + messageId: 'preferReadonly', + }, + ], + output: `class TestIncorrectlyModifiableStatic { + private static readonly incorrectlyModifiableStatic = 7; + }`, + }, + { + code: `class TestIncorrectlyModifiableStaticArrow { + private static incorrectlyModifiableStaticArrow = () => 7; + }`, + errors: [ + { + data: { + name: 'incorrectlyModifiableStaticArrow', + }, + messageId: 'preferReadonly', + }, + ], + output: `class TestIncorrectlyModifiableStaticArrow { + private static readonly incorrectlyModifiableStaticArrow = () => 7; + }`, + }, + { + code: `class TestIncorrectlyModifiableInline { + private incorrectlyModifiableInline = 7; + + public createConfusingChildClass() { + return class { + private incorrectlyModifiableInline = 7; + } + } + }`, + errors: [ + { + data: { + name: 'incorrectlyModifiableInline', + }, + line: 2, + messageId: 'preferReadonly', + }, + { + data: { + name: 'incorrectlyModifiableInline', + }, + line: 6, + messageId: 'preferReadonly', + }, + ], + output: `class TestIncorrectlyModifiableInline { + private readonly incorrectlyModifiableInline = 7; + + public createConfusingChildClass() { + return class { + private readonly incorrectlyModifiableInline = 7; + } + } + }`, + }, + { + code: `class TestIncorrectlyModifiableDelayed { + private incorrectlyModifiableDelayed = 7; + + public constructor() { + this.incorrectlyModifiableDelayed = 7; + } + }`, + errors: [ + { + data: { + name: 'incorrectlyModifiableDelayed', + }, + messageId: 'preferReadonly', + }, + ], + output: `class TestIncorrectlyModifiableDelayed { + private readonly incorrectlyModifiableDelayed = 7; + + public constructor() { + this.incorrectlyModifiableDelayed = 7; + } + }`, + }, + { + code: `class TestChildClassExpressionModifiable { + private childClassExpressionModifiable = 7; + + public createConfusingChildClass() { + return class { + private childClassExpressionModifiable = 7; + + mutate() { + this.childClassExpressionModifiable += 1; + } + } + } + }`, + errors: [ + { + data: { + name: 'childClassExpressionModifiable', + }, + line: 2, + messageId: 'preferReadonly', + }, + ], + output: `class TestChildClassExpressionModifiable { + private readonly childClassExpressionModifiable = 7; + + public createConfusingChildClass() { + return class { + private childClassExpressionModifiable = 7; + + mutate() { + this.childClassExpressionModifiable += 1; + } + } + } + }`, + }, + { + code: `class TestIncorrectlyModifiablePostMinus { + private incorrectlyModifiablePostMinus = 7; + + public mutate() { + this.incorrectlyModifiablePostMinus - 1; + } + }`, + errors: [ + { + data: { + name: 'incorrectlyModifiablePostMinus', + }, + line: 2, + messageId: 'preferReadonly', + }, + ], + output: `class TestIncorrectlyModifiablePostMinus { + private readonly incorrectlyModifiablePostMinus = 7; + + public mutate() { + this.incorrectlyModifiablePostMinus - 1; + } + }`, + }, + { + code: `class TestIncorrectlyModifiablePostPlus { + private incorrectlyModifiablePostPlus = 7; + + public mutate() { + this.incorrectlyModifiablePostPlus + 1; + } + }`, + errors: [ + { + data: { + name: 'incorrectlyModifiablePostPlus', + }, + line: 2, + messageId: 'preferReadonly', + }, + ], + output: `class TestIncorrectlyModifiablePostPlus { + private readonly incorrectlyModifiablePostPlus = 7; + + public mutate() { + this.incorrectlyModifiablePostPlus + 1; + } + }`, + }, + { + code: `class TestIncorrectlyModifiablePreMinus { + private incorrectlyModifiablePreMinus = 7; + + public mutate() { + -this.incorrectlyModifiablePreMinus; + } + }`, + errors: [ + { + data: { + name: 'incorrectlyModifiablePreMinus', + }, + line: 2, + messageId: 'preferReadonly', + }, + ], + output: `class TestIncorrectlyModifiablePreMinus { + private readonly incorrectlyModifiablePreMinus = 7; + + public mutate() { + -this.incorrectlyModifiablePreMinus; + } + }`, + }, + { + code: `class TestIncorrectlyModifiablePrePlus { + private incorrectlyModifiablePrePlus = 7; + + public mutate() { + +this.incorrectlyModifiablePrePlus; + } + }`, + errors: [ + { + data: { + name: 'incorrectlyModifiablePrePlus', + }, + line: 2, + messageId: 'preferReadonly', + }, + ], + output: `class TestIncorrectlyModifiablePrePlus { + private readonly incorrectlyModifiablePrePlus = 7; + + public mutate() { + +this.incorrectlyModifiablePrePlus; + } + }`, + }, + { + code: `class TestOverlappingClassVariable { + private overlappingClassVariable = 7; + + public workWithSimilarClass(other: SimilarClass) { + other.overlappingClassVariable = 7; + } + } + + class SimilarClass { + public overlappingClassVariable = 7; + }`, + errors: [ + { + data: { + name: 'overlappingClassVariable', + }, + line: 2, + messageId: 'preferReadonly', + }, + ], + output: `class TestOverlappingClassVariable { + private readonly overlappingClassVariable = 7; + + public workWithSimilarClass(other: SimilarClass) { + other.overlappingClassVariable = 7; + } + } + + class SimilarClass { + public overlappingClassVariable = 7; + }`, + }, + { + code: `class TestIncorrectlyModifiableParameter { + public constructor( + private incorrectlyModifiableParameter = 7, + ) { } + }`, + errors: [ + { + data: { + name: 'incorrectlyModifiableParameter', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + output: `class TestIncorrectlyModifiableParameter { + public constructor( + private readonly incorrectlyModifiableParameter = 7, + ) { } + }`, + }, + { + code: `class TestIncorrectlyModifiableParameter { + public constructor( + public ignore: boolean, + private incorrectlyModifiableParameter = 7, + ) { } + }`, + errors: [ + { + data: { + name: 'incorrectlyModifiableParameter', + }, + line: 4, + messageId: 'preferReadonly', + }, + ], + output: `class TestIncorrectlyModifiableParameter { + public constructor( + public ignore: boolean, + private readonly incorrectlyModifiableParameter = 7, + ) { } + }`, + }, + { + code: `class TestCorrectlyNonInlineLambdas { + private incorrectlyInlineLambda = () => 7; + }`, + errors: [ + { + data: { + name: 'incorrectlyInlineLambda', + }, + line: 2, + messageId: 'preferReadonly', + }, + ], + options: [ + { + onlyInlineLambdas: true, + }, + ], + output: `class TestCorrectlyNonInlineLambdas { + private readonly incorrectlyInlineLambda = () => 7; + }`, + }, + ], +}); diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index f8151e1b268..cdf6b184150 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -43,9 +43,9 @@ export function convertError(error: any) { export class Converter { private readonly ast: ts.SourceFile; - private options: ConverterOptions; - private esTreeNodeToTSNodeMap = new WeakMap(); - private tsNodeToESTreeNodeMap = new WeakMap(); + private readonly options: ConverterOptions; + private readonly esTreeNodeToTSNodeMap = new WeakMap(); + private readonly tsNodeToESTreeNodeMap = new WeakMap(); private allowPattern: boolean = false; private inTypeMode: boolean = false;