From d77a6da76d63080512d74c2b56e48b0edc3e51d7 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 28 Nov 2017 04:22:00 -0800 Subject: [PATCH] Added prefer-readonly rule with fixes (#2896) --- src/configs/all.ts | 1 + src/language/rule/abstractRule.ts | 2 +- src/language/rule/rule.ts | 18 +- src/language/typeUtils.ts | 39 +++ .../walker/blockScopeAwareRuleWalker.ts | 2 +- src/language/walker/programAwareRuleWalker.ts | 4 +- src/language/walker/ruleWalker.ts | 10 +- src/language/walker/scopeAwareRuleWalker.ts | 2 +- src/linter.ts | 2 +- src/rules/noInferredEmptyObjectTypeRule.ts | 2 +- src/rules/noThisAssignmentRule.ts | 2 +- src/rules/noUnsafeAnyRule.ts | 4 +- src/rules/orderedImportsRule.ts | 2 +- src/rules/preferReadonlyRule.ts | 300 ++++++++++++++++++ .../rules/prefer-readonly/default/test.ts.fix | 155 +++++++++ .../prefer-readonly/default/test.ts.lint | 169 ++++++++++ .../prefer-readonly/default/tsconfig.json | 5 + .../rules/prefer-readonly/default/tslint.json | 5 + .../only-inline-lambdas/test.ts.fix | 28 ++ .../only-inline-lambdas/test.ts.lint | 29 ++ .../only-inline-lambdas/tsconfig.json | 5 + .../only-inline-lambdas/tslint.json | 5 + 22 files changed, 766 insertions(+), 25 deletions(-) create mode 100644 src/language/typeUtils.ts create mode 100644 src/rules/preferReadonlyRule.ts create mode 100644 test/rules/prefer-readonly/default/test.ts.fix create mode 100644 test/rules/prefer-readonly/default/test.ts.lint create mode 100644 test/rules/prefer-readonly/default/tsconfig.json create mode 100644 test/rules/prefer-readonly/default/tslint.json create mode 100644 test/rules/prefer-readonly/only-inline-lambdas/test.ts.fix create mode 100644 test/rules/prefer-readonly/only-inline-lambdas/test.ts.lint create mode 100644 test/rules/prefer-readonly/only-inline-lambdas/tsconfig.json create mode 100644 test/rules/prefer-readonly/only-inline-lambdas/tslint.json diff --git a/src/configs/all.ts b/src/configs/all.ts index ff94da67353..9cdd2e7fe01 100644 --- a/src/configs/all.ts +++ b/src/configs/all.ts @@ -55,6 +55,7 @@ export const rules = { "no-var-requires": true, "only-arrow-functions": true, "prefer-for-of": true, + "prefer-readonly": true, "promise-function-async": true, "typedef": [ true, diff --git a/src/language/rule/abstractRule.ts b/src/language/rule/abstractRule.ts index a8cab3f2921..692164cdf99 100644 --- a/src/language/rule/abstractRule.ts +++ b/src/language/rule/abstractRule.ts @@ -28,7 +28,7 @@ export abstract class AbstractRule implements IRule { protected readonly ruleSeverity: RuleSeverity; public ruleName: string; - constructor(private options: IOptions) { + constructor(private readonly options: IOptions) { this.ruleName = options.ruleName; this.ruleArguments = options.ruleArguments; this.ruleSeverity = options.ruleSeverity; diff --git a/src/language/rule/rule.ts b/src/language/rule/rule.ts index b5ce66c61b9..d52189eab2c 100644 --- a/src/language/rule/rule.ts +++ b/src/language/rule/rule.ts @@ -205,7 +205,7 @@ export class Replacement { } export class RuleFailurePosition { - constructor(private position: number, private lineAndCharacter: ts.LineAndCharacter) { + constructor(private readonly position: number, private readonly lineAndCharacter: ts.LineAndCharacter) { } public getPosition() { @@ -238,10 +238,10 @@ export type Fix = Replacement | Replacement[]; export type FixJson = ReplacementJson | ReplacementJson[]; export class RuleFailure { - private fileName: string; - private startPosition: RuleFailurePosition; - private endPosition: RuleFailurePosition; - private rawLines: string; + private readonly fileName: string; + private readonly startPosition: RuleFailurePosition; + private readonly endPosition: RuleFailurePosition; + private readonly rawLines: string; private ruleSeverity: RuleSeverity; public static compare(a: RuleFailure, b: RuleFailure): number { @@ -251,12 +251,12 @@ export class RuleFailure { return a.startPosition.getPosition() - b.startPosition.getPosition(); } - constructor(private sourceFile: ts.SourceFile, + constructor(private readonly sourceFile: ts.SourceFile, start: number, end: number, - private failure: string, - private ruleName: string, - private fix?: Fix) { + private readonly failure: string, + private readonly ruleName: string, + private readonly fix?: Fix) { this.fileName = sourceFile.fileName; this.startPosition = this.createFailurePosition(start); diff --git a/src/language/typeUtils.ts b/src/language/typeUtils.ts new file mode 100644 index 00000000000..99ab92fe0b7 --- /dev/null +++ b/src/language/typeUtils.ts @@ -0,0 +1,39 @@ +/** + * @license + * Copyright 2017 Palantir Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as ts from "typescript"; + +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/src/language/walker/blockScopeAwareRuleWalker.ts b/src/language/walker/blockScopeAwareRuleWalker.ts index ce9cab8c1dd..71d21b7755c 100644 --- a/src/language/walker/blockScopeAwareRuleWalker.ts +++ b/src/language/walker/blockScopeAwareRuleWalker.ts @@ -28,7 +28,7 @@ import { ScopeAwareRuleWalker } from "./scopeAwareRuleWalker"; * are a superset of regular scopes (new block scopes are created more frequently in a program). */ export abstract class BlockScopeAwareRuleWalker extends ScopeAwareRuleWalker { // tslint:disable-line:deprecation - private blockScopeStack: U[]; + private readonly blockScopeStack: U[]; constructor(sourceFile: ts.SourceFile, options: IOptions) { super(sourceFile, options); diff --git a/src/language/walker/programAwareRuleWalker.ts b/src/language/walker/programAwareRuleWalker.ts index dac43d093e9..16dc588885b 100644 --- a/src/language/walker/programAwareRuleWalker.ts +++ b/src/language/walker/programAwareRuleWalker.ts @@ -21,9 +21,9 @@ import { IOptions } from "../rule/rule"; import { RuleWalker } from "./ruleWalker"; export class ProgramAwareRuleWalker extends RuleWalker { - private typeChecker: ts.TypeChecker; + private readonly typeChecker: ts.TypeChecker; - constructor(sourceFile: ts.SourceFile, options: IOptions, private program: ts.Program) { + constructor(sourceFile: ts.SourceFile, options: IOptions, private readonly program: ts.Program) { super(sourceFile, options); this.typeChecker = program.getTypeChecker(); diff --git a/src/language/walker/ruleWalker.ts b/src/language/walker/ruleWalker.ts index 15bc23c80fb..92ef76652a7 100644 --- a/src/language/walker/ruleWalker.ts +++ b/src/language/walker/ruleWalker.ts @@ -22,12 +22,12 @@ import { SyntaxWalker } from "./syntaxWalker"; import { IWalker } from "./walker"; export class RuleWalker extends SyntaxWalker implements IWalker { - private limit: number; - private options?: any[]; - private failures: RuleFailure[]; - private ruleName: string; + private readonly limit: number; + private readonly options?: any[]; + private readonly failures: RuleFailure[]; + private readonly ruleName: string; - constructor(private sourceFile: ts.SourceFile, options: IOptions) { + constructor(private readonly sourceFile: ts.SourceFile, options: IOptions) { super(); this.failures = []; diff --git a/src/language/walker/scopeAwareRuleWalker.ts b/src/language/walker/scopeAwareRuleWalker.ts index 83f67dda77c..5ba686174e0 100644 --- a/src/language/walker/scopeAwareRuleWalker.ts +++ b/src/language/walker/scopeAwareRuleWalker.ts @@ -56,7 +56,7 @@ import { RuleWalker } from "./ruleWalker"; * } */ export abstract class ScopeAwareRuleWalker extends RuleWalker { - private scopeStack: T[]; + private readonly scopeStack: T[]; constructor(sourceFile: ts.SourceFile, options: IOptions) { super(sourceFile, options); diff --git a/src/linter.ts b/src/linter.ts index c5c4143f386..2171a4a1941 100644 --- a/src/linter.ts +++ b/src/linter.ts @@ -103,7 +103,7 @@ class Linter { ); } - constructor(private options: ILinterOptions, private program?: ts.Program) { + constructor(private readonly options: ILinterOptions, private program?: ts.Program) { if (typeof options !== "object") { throw new Error(`Unknown Linter options type: ${typeof options}`); } diff --git a/src/rules/noInferredEmptyObjectTypeRule.ts b/src/rules/noInferredEmptyObjectTypeRule.ts index c87f7ae336f..25dc5290ab8 100644 --- a/src/rules/noInferredEmptyObjectTypeRule.ts +++ b/src/rules/noInferredEmptyObjectTypeRule.ts @@ -42,7 +42,7 @@ export class Rule extends Lint.Rules.TypedRule { } class NoInferredEmptyObjectTypeRule extends Lint.AbstractWalker { - constructor(sourceFile: ts.SourceFile, ruleName: string, private checker: ts.TypeChecker) { + constructor(sourceFile: ts.SourceFile, ruleName: string, private readonly checker: ts.TypeChecker) { super(sourceFile, ruleName, undefined); } diff --git a/src/rules/noThisAssignmentRule.ts b/src/rules/noThisAssignmentRule.ts index 67f933be999..fc30d41cebe 100644 --- a/src/rules/noThisAssignmentRule.ts +++ b/src/rules/noThisAssignmentRule.ts @@ -108,7 +108,7 @@ class NoThisAssignmentWalker extends Lint.AbstractWalker { ts.forEachChild(sourceFile, this.visitNode); } - private visitNode = (node: ts.Node): void => { + private readonly visitNode = (node: ts.Node): void => { if (utils.isVariableDeclaration(node)) { this.visitVariableDeclaration(node); } diff --git a/src/rules/noUnsafeAnyRule.ts b/src/rules/noUnsafeAnyRule.ts index e5d5829a4d4..2797f057374 100644 --- a/src/rules/noUnsafeAnyRule.ts +++ b/src/rules/noUnsafeAnyRule.ts @@ -45,7 +45,7 @@ export class Rule extends Lint.Rules.TypedRule { } class NoUnsafeAnyWalker extends Lint.AbstractWalker { - constructor(sourceFile: ts.SourceFile, ruleName: string, private checker: ts.TypeChecker) { + constructor(sourceFile: ts.SourceFile, ruleName: string, private readonly checker: ts.TypeChecker) { super(sourceFile, ruleName, undefined); } @@ -57,7 +57,7 @@ class NoUnsafeAnyWalker extends Lint.AbstractWalker { } /** Wraps `visitNode` with the correct `this` binding and discards the return value to prevent `forEachChild` from returning early */ - private visitNodeCallback = (node: ts.Node) => void this.visitNode(node); + private readonly visitNodeCallback = (node: ts.Node) => void this.visitNode(node); private visitNode(node: ts.Node, anyOk?: boolean): boolean | undefined { switch (node.kind) { diff --git a/src/rules/orderedImportsRule.ts b/src/rules/orderedImportsRule.ts index 7d6e779e495..c9bdcd544eb 100644 --- a/src/rules/orderedImportsRule.ts +++ b/src/rules/orderedImportsRule.ts @@ -181,7 +181,7 @@ function parseOptions(ruleArguments: any[]): Options { } class Walker extends Lint.AbstractWalker { - private importsBlocks = [new ImportsBlock()]; + private readonly importsBlocks = [new ImportsBlock()]; // keep a reference to the last Fix object so when the entire block is replaced, the replacement can be added private lastFix: Lint.Replacement[] | undefined; private nextType = ImportType.LIBRARY_IMPORT; diff --git a/src/rules/preferReadonlyRule.ts b/src/rules/preferReadonlyRule.ts new file mode 100644 index 00000000000..4f35794b625 --- /dev/null +++ b/src/rules/preferReadonlyRule.ts @@ -0,0 +1,300 @@ +/** + * @license + * Copyright 2017 Palantir Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as utils from "tsutils"; +import * as ts from "typescript"; + +import * as Lint from "../index"; +import { typeIsOrHasBaseType } from "../language/typeUtils"; + +const OPTION_ONLY_INLINE_LAMBDAS = "only-inline-lambdas"; + +type ParameterOrPropertyDeclaration = ts.ParameterDeclaration | ts.PropertyDeclaration; + +interface Options { + onlyInlineLambdas: boolean; +} + +export class Rule extends Lint.Rules.TypedRule { + public static metadata: Lint.IRuleMetadata = { + description: "Requires that private variables are marked as `readonly` if they're never modified outside of the constructor.", + descriptionDetails: Lint.Utils.dedent` + If a private variable is only assigned to in the constructor, it should be declared as \`readonly\`. + `, + optionExamples: [ + true, + [true, OPTION_ONLY_INLINE_LAMBDAS], + ], + options: { + enum: [OPTION_ONLY_INLINE_LAMBDAS], + type: "string", + }, + optionsDescription: Lint.Utils.dedent` + If \`${OPTION_ONLY_INLINE_LAMBDAS}\` is specified, only immediately-declared arrow functions are checked.`, + rationale: Lint.Utils.dedent` + Marking never-modified variables as readonly helps enforce the code's intent of keeping them as never-modified. + It can also help prevent accidental changes of members not meant to be changed.`, + requiresTypeInfo: true, + ruleName: "prefer-readonly", + type: "maintainability", + typescriptOnly: true, + }; + + public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + const options = { + onlyInlineLambdas: this.ruleArguments.indexOf(OPTION_ONLY_INLINE_LAMBDAS) !== -1, + }; + + return this.applyWithFunction(sourceFile, walk, options, program.getTypeChecker()); + } +} + +function walk(context: Lint.WalkContext, typeChecker: ts.TypeChecker) { + if (context.sourceFile.isDeclarationFile) { + return; + } + + let scope: ClassScope; + + ts.forEachChild(context.sourceFile, visitNode); + + function visitNode(node: ts.Node) { + if (utils.hasModifier(node.modifiers, ts.SyntaxKind.DeclareKeyword)) { + return; + } + + switch (node.kind) { + case ts.SyntaxKind.ClassDeclaration: + case ts.SyntaxKind.ClassExpression: + handleClassDeclarationOrExpression(node as ts.ClassLikeDeclaration); + break; + + case ts.SyntaxKind.Constructor: + handleConstructor(node as ts.ConstructorDeclaration); + break; + + case ts.SyntaxKind.PropertyDeclaration: + handlePropertyDeclaration(node as ts.PropertyDeclaration); + break; + + case ts.SyntaxKind.PropertyAccessExpression: + if (scope !== undefined) { + handlePropertyAccessExpression(node as ts.PropertyAccessExpression, node.parent!); + } + break; + + default: + if (utils.isFunctionScopeBoundary(node)) { + handleFunctionScopeBoundary(node); + } else { + ts.forEachChild(node, visitNode); + } + } + } + + function handleFunctionScopeBoundary(node: ts.Node) { + if (scope === undefined) { + ts.forEachChild(node, visitNode); + return; + } + + scope.enterNonConstructorScope(); + ts.forEachChild(node, visitNode); + scope.exitNonConstructorScope(); + } + + function handleClassDeclarationOrExpression(node: ts.ClassLikeDeclaration) { + const parentScope = scope; + const childScope = scope = new ClassScope(node, typeChecker); + + ts.forEachChild(node, visitNode); + + finalizeScope(childScope); + scope = parentScope; + } + + function handleConstructor(node: ts.ConstructorDeclaration) { + scope.enterConstructor(); + + for (const parameter of node.parameters) { + if (utils.isModifierFlagSet(parameter, ts.ModifierFlags.Private)) { + scope.addDeclaredVariable(parameter); + } + } + + ts.forEachChild(node, visitNode); + + scope.exitConstructor(); + } + + function handlePropertyDeclaration(node: ts.PropertyDeclaration) { + if (!shouldPropertyDeclarationBeIgnored(node)) { + scope.addDeclaredVariable(node); + } + + ts.forEachChild(node, visitNode); + } + + function handlePropertyAccessExpression(node: ts.PropertyAccessExpression, parent: ts.Node) { + switch (parent.kind) { + case ts.SyntaxKind.BinaryExpression: + handleParentBinaryExpression(node, parent as ts.BinaryExpression); + break; + + case ts.SyntaxKind.DeleteExpression: + handleDeleteExpression(node); + break; + + case ts.SyntaxKind.PostfixUnaryExpression: + case ts.SyntaxKind.PrefixUnaryExpression: + handleParentPostfixOrPrefixUnaryExpression(parent as ts.PostfixUnaryExpression | ts.PrefixUnaryExpression); + } + + ts.forEachChild(node, visitNode); + } + + function handleParentBinaryExpression(node: ts.PropertyAccessExpression, parent: ts.BinaryExpression) { + if (parent.left === node && utils.isAssignmentKind(parent.operatorToken.kind)) { + scope.addVariableModification(node); + } + } + + function handleParentPostfixOrPrefixUnaryExpression(node: ts.PostfixUnaryExpression | ts.PrefixUnaryExpression) { + if (node.operator === ts.SyntaxKind.PlusPlusToken || node.operator === ts.SyntaxKind.MinusMinusToken) { + scope.addVariableModification(node.operand as ts.PropertyAccessExpression); + } + } + + function handleDeleteExpression(node: ts.PropertyAccessExpression) { + scope.addVariableModification(node); + } + + function shouldPropertyDeclarationBeIgnored(node: ts.PropertyDeclaration) { + if (!context.options.onlyInlineLambdas) { + return false; + } + + return node.initializer === undefined || node.initializer.kind !== ts.SyntaxKind.ArrowFunction; + } + + function finalizeScope(childScope: ClassScope) { + for (const violatingNode of childScope.finalizeUnmodifiedPrivateNonReadonlys()) { + complainOnNode(violatingNode); + } + } + + function complainOnNode(node: ts.ParameterDeclaration | ts.PropertyDeclaration) { + const fix = Lint.Replacement.appendText(node.modifiers!.end, " readonly"); + + context.addFailureAtNode(node.name, createFailureString(node), fix); + } +} + +function createFailureString(node: ts.ParameterDeclaration | ts.PropertyDeclaration) { + const accessibility = utils.isModifierFlagSet(node, ts.ModifierFlags.Static) + ? "static" + : "member"; + + const text = node.name.getText(); + + return `Private ${accessibility} variable '${text}' is never reassigned; mark it as 'readonly'.`; +} + +const OUTSIDE_CONSTRUCTOR = -1; +const DIRECTLY_INSIDE_CONSTRUCTOR = 0; + +class ClassScope { + private readonly privateModifiableMembers = new Map(); + private readonly privateModifiableStatics = new Map(); + private readonly memberVariableModifications = new Set(); + private readonly staticVariableModifications = new Set(); + + private readonly typeChecker: ts.TypeChecker; + private readonly classType: ts.Type; + + private constructorScopeDepth = OUTSIDE_CONSTRUCTOR; + + public constructor(classNode: ts.Node, typeChecker: ts.TypeChecker) { + this.classType = typeChecker.getTypeAtLocation(classNode); + this.typeChecker = typeChecker; + } + + public addDeclaredVariable(node: ParameterOrPropertyDeclaration) { + if (!utils.isModifierFlagSet(node, ts.ModifierFlags.Private) + || utils.isModifierFlagSet(node, ts.ModifierFlags.Readonly) + || node.name.kind === ts.SyntaxKind.ComputedPropertyName) { + return; + } + + if (utils.isModifierFlagSet(node, ts.ModifierFlags.Static)) { + this.privateModifiableStatics.set(node.name.getText(), node); + } else { + this.privateModifiableMembers.set(node.name.getText(), node); + } + } + + public addVariableModification(node: ts.PropertyAccessExpression) { + const modifierType = this.typeChecker.getTypeAtLocation(node.expression); + if (modifierType.symbol === undefined || !typeIsOrHasBaseType(modifierType, this.classType)) { + return; + } + + const toStatic = utils.isObjectType(modifierType) && utils.isObjectFlagSet(modifierType, ts.ObjectFlags.Anonymous); + if (!toStatic && this.constructorScopeDepth === DIRECTLY_INSIDE_CONSTRUCTOR) { + return; + } + + const variable = node.name.text; + + (toStatic ? this.staticVariableModifications : this.memberVariableModifications).add(variable); + } + + public enterConstructor() { + this.constructorScopeDepth = DIRECTLY_INSIDE_CONSTRUCTOR; + } + + public exitConstructor() { + this.constructorScopeDepth = OUTSIDE_CONSTRUCTOR; + } + + public enterNonConstructorScope() { + if (this.constructorScopeDepth !== OUTSIDE_CONSTRUCTOR) { + this.constructorScopeDepth += 1; + } + } + + public exitNonConstructorScope() { + 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/test/rules/prefer-readonly/default/test.ts.fix b/test/rules/prefer-readonly/default/test.ts.fix new file mode 100644 index 00000000000..49b701351a9 --- /dev/null +++ b/test/rules/prefer-readonly/default/test.ts.fix @@ -0,0 +1,155 @@ +class SampleClass { + private static readonly correctlyReadonlyStatic = 7; + + private static readonly incorrectlyModifiableStatic = 7; + + private static readonly incorrectlyModifiableStaticArrow = () => {}; + + private static correctlyModifiableStatic = 7; + + private static correctlyModifiableByParameterProperty = 7; + + private readonly correctlyReadonlyInline = 7; + + private readonly correctlyReadonlyDelayed; + + private readonly incorrectlyModifiableInline = 7; + + private readonly incorrectlyModifiableDelayed; + + private correctlyModifiableInline = 7; + + private correctlyModifiableDelayed; + + private correctlyModifiableDeleted = 7; + + private readonly childClassExpressionModifiable = 7; + + private correctlyModifiableWithinConstructor; + + private correctlyModifiableWithinConstructorInArrowFunction; + + private correctlyModifiableWithinConstructorInFunctionExpression; + + private correctlyModifiableWithinConstructorInGetAccessor; + + private correctlyModifiableWithinConstructorInMethodDeclaration; + + private correctlyModifiableWithinConstructorInSetAccessor; + + private correctlyModifiablePostDecremented; + + private correctlyModifiablePostIncremented; + + private readonly incorrectlyModifiablePostMinus; + + private readonly incorrectlyModifiablePostPlus; + + private correctlyModifiablePreDecremented; + + private correctlyModifiablePreIncremented; + + private readonly incorrectlyModifiablePreMinus; + + private readonly incorrectlyModifiablePrePlus; + + private readonly overlappingClassVariable = 7; + + protected protectedModifiable = 7; + + protected publicModifiable = 7; + + public constructor( + private readonly correctlyReadonlyParameter: number = 7, + private readonly incorrectlyModifiableParameter: number = 7, + private correctlyModifiableParameter: number = 7, + public correctlyModifiablePublicParameter: number = (() => { + return SampleClass.correctlyModifiableByParameterProperty = 7; + })(); + ) { + this.correctlyReadonlyDelayed = 7; + this.incorrectlyModifiableDelayed = 7; + this.incorrectlyModifiableParameter = 7; + + (() => { + const self = this; + + this.correctlyModifiableWithinConstructor = 7; + + (() => { + this.correctlyModifiableWithinConstructorInArrowFunction = 7; + })(); + + (function () { + self.correctlyModifiableWithinConstructorInFunctionExpression = 7; + })(); + + const confusingObject = { + get getAccessor() { + return self.correctlyModifiableWithinConstructorInGetAccessor = 7; + } + + methodDeclaration() { + self.correctlyModifiableWithinConstructorInMethodDeclaration = 7; + } + + set setAccessor() { + self.correctlyModifiableWithinConstructorInSetAccessor = 7; + } + }; + })(); + + SampleClass.correctlyModifiableStatic += 1; + } + + public mutate() { + this.correctlyModifiableDelayed = 7; + this.correctlyModifiableInline = 7; + this.correctlyModifiableMember = () => {}; + this.correctlyModifiableParameter = 7; + + delete this.correctlyModifiableDeleted; + + this.correctlyModifiablePostDecremented--; + this.correctlyModifiablePostIncremented++; + this.incorrectlyModifiablePostMinus+; + this.incorrectlyModifiablePostPlus+; + --this.correctlyModifiablePreDecremented; + ++this.correctlyModifiablePreIncremented; + -this.incorrectlyModifiablePreMinus; + +this.incorrectlyModifiablePrePlus; + } + + public createConfusingChildClass() { + return class { + private correctlyModifiableInline = 7; + + private readonly incorrectlyModifiableInline = 7; + + private readonly incorrectlyUniqueModifiableInline = 7; + + private childClassExpressionModifiable = 7; + + mutate() { + this.correctlyModifiableInline = 7; + this.childClassExpressionModifiable = 7; + } + } + } + + public workWithSimilarClass(other: SimilarClass) { + other.overlappingClassVariable = 7; + } + + private readonly incorrectlyModifiableMember = () => { }; + + private correctlyModifiableMember = () => { }; +} + +class SimilarClass { + public overlappingClassVariable = 7; +} + +declare class DeclaredClass { + private declaredMember = 7; +} diff --git a/test/rules/prefer-readonly/default/test.ts.lint b/test/rules/prefer-readonly/default/test.ts.lint new file mode 100644 index 00000000000..dd02f62316e --- /dev/null +++ b/test/rules/prefer-readonly/default/test.ts.lint @@ -0,0 +1,169 @@ +class SampleClass { + private static readonly correctlyReadonlyStatic = 7; + + private static incorrectlyModifiableStatic = 7; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Private static variable 'incorrectlyModifiableStatic' is never reassigned; mark it as 'readonly'.] + + private static incorrectlyModifiableStaticArrow = () => {}; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Private static variable 'incorrectlyModifiableStaticArrow' is never reassigned; mark it as 'readonly'.] + + private static correctlyModifiableStatic = 7; + + private static correctlyModifiableByParameterProperty = 7; + + private readonly correctlyReadonlyInline = 7; + + private readonly correctlyReadonlyDelayed; + + private incorrectlyModifiableInline = 7; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Private member variable 'incorrectlyModifiableInline' is never reassigned; mark it as 'readonly'.] + + private incorrectlyModifiableDelayed; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Private member variable 'incorrectlyModifiableDelayed' is never reassigned; mark it as 'readonly'.] + + private correctlyModifiableInline = 7; + + private correctlyModifiableDelayed; + + private correctlyModifiableDeleted = 7; + + private childClassExpressionModifiable = 7; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Private member variable 'childClassExpressionModifiable' is never reassigned; mark it as 'readonly'.] + + private correctlyModifiableWithinConstructor; + + private correctlyModifiableWithinConstructorInArrowFunction; + + private correctlyModifiableWithinConstructorInFunctionExpression; + + private correctlyModifiableWithinConstructorInGetAccessor; + + private correctlyModifiableWithinConstructorInMethodDeclaration; + + private correctlyModifiableWithinConstructorInSetAccessor; + + private correctlyModifiablePostDecremented; + + private correctlyModifiablePostIncremented; + + private incorrectlyModifiablePostMinus; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Private member variable 'incorrectlyModifiablePostMinus' is never reassigned; mark it as 'readonly'.] + + private incorrectlyModifiablePostPlus; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Private member variable 'incorrectlyModifiablePostPlus' is never reassigned; mark it as 'readonly'.] + + private correctlyModifiablePreDecremented; + + private correctlyModifiablePreIncremented; + + private incorrectlyModifiablePreMinus; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Private member variable 'incorrectlyModifiablePreMinus' is never reassigned; mark it as 'readonly'.] + + private incorrectlyModifiablePrePlus; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Private member variable 'incorrectlyModifiablePrePlus' is never reassigned; mark it as 'readonly'.] + + private overlappingClassVariable = 7; + ~~~~~~~~~~~~~~~~~~~~~~~~ [Private member variable 'overlappingClassVariable' is never reassigned; mark it as 'readonly'.] + + protected protectedModifiable = 7; + + protected publicModifiable = 7; + + public constructor( + private readonly correctlyReadonlyParameter: number = 7, + private incorrectlyModifiableParameter: number = 7, + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Private member variable 'incorrectlyModifiableParameter' is never reassigned; mark it as 'readonly'.] + private correctlyModifiableParameter: number = 7, + public correctlyModifiablePublicParameter: number = (() => { + return SampleClass.correctlyModifiableByParameterProperty = 7; + })(); + ) { + this.correctlyReadonlyDelayed = 7; + this.incorrectlyModifiableDelayed = 7; + this.incorrectlyModifiableParameter = 7; + + (() => { + const self = this; + + this.correctlyModifiableWithinConstructor = 7; + + (() => { + this.correctlyModifiableWithinConstructorInArrowFunction = 7; + })(); + + (function () { + self.correctlyModifiableWithinConstructorInFunctionExpression = 7; + })(); + + const confusingObject = { + get getAccessor() { + return self.correctlyModifiableWithinConstructorInGetAccessor = 7; + } + + methodDeclaration() { + self.correctlyModifiableWithinConstructorInMethodDeclaration = 7; + } + + set setAccessor() { + self.correctlyModifiableWithinConstructorInSetAccessor = 7; + } + }; + })(); + + SampleClass.correctlyModifiableStatic += 1; + } + + public mutate() { + this.correctlyModifiableDelayed = 7; + this.correctlyModifiableInline = 7; + this.correctlyModifiableMember = () => {}; + this.correctlyModifiableParameter = 7; + + delete this.correctlyModifiableDeleted; + + this.correctlyModifiablePostDecremented--; + this.correctlyModifiablePostIncremented++; + this.incorrectlyModifiablePostMinus+; + this.incorrectlyModifiablePostPlus+; + --this.correctlyModifiablePreDecremented; + ++this.correctlyModifiablePreIncremented; + -this.incorrectlyModifiablePreMinus; + +this.incorrectlyModifiablePrePlus; + } + + public createConfusingChildClass() { + return class { + private correctlyModifiableInline = 7; + + private incorrectlyModifiableInline = 7; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Private member variable 'incorrectlyModifiableInline' is never reassigned; mark it as 'readonly'.] + + private incorrectlyUniqueModifiableInline = 7; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Private member variable 'incorrectlyUniqueModifiableInline' is never reassigned; mark it as 'readonly'.] + + private childClassExpressionModifiable = 7; + + mutate() { + this.correctlyModifiableInline = 7; + this.childClassExpressionModifiable = 7; + } + } + } + + public workWithSimilarClass(other: SimilarClass) { + other.overlappingClassVariable = 7; + } + + private incorrectlyModifiableMember = () => { }; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Private member variable 'incorrectlyModifiableMember' is never reassigned; mark it as 'readonly'.] + + private correctlyModifiableMember = () => { }; +} + +class SimilarClass { + public overlappingClassVariable = 7; +} + +declare class DeclaredClass { + private declaredMember = 7; +} diff --git a/test/rules/prefer-readonly/default/tsconfig.json b/test/rules/prefer-readonly/default/tsconfig.json new file mode 100644 index 00000000000..744a66c893a --- /dev/null +++ b/test/rules/prefer-readonly/default/tsconfig.json @@ -0,0 +1,5 @@ +{ + "compilerOptions": { + "module": "commonjs" + } +} diff --git a/test/rules/prefer-readonly/default/tslint.json b/test/rules/prefer-readonly/default/tslint.json new file mode 100644 index 00000000000..4eabc221064 --- /dev/null +++ b/test/rules/prefer-readonly/default/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "prefer-readonly": true + } +} diff --git a/test/rules/prefer-readonly/only-inline-lambdas/test.ts.fix b/test/rules/prefer-readonly/only-inline-lambdas/test.ts.fix new file mode 100644 index 00000000000..c3a23eddf40 --- /dev/null +++ b/test/rules/prefer-readonly/only-inline-lambdas/test.ts.fix @@ -0,0 +1,28 @@ +class SampleClass { + private readonly correctlyReadonlyDelayedArrow: () => void; + + private readonly correctlyReadonlyInlineArrow = () => {} + + private readonly correctlyReadonlyValue = 7; + + private correctlyModifiableDelayedArrow: () => void; + + private incorrectlyModifiableDelayedArrow: () => void; + + private readonly incorrectlyModifiableInlineArrow = () => {} + + private correctlyModifiableInlineArrow = () => {} + + private correctlyModifiableValue = 7; + + public constructor() { + this.correctlyModifiableDelayedArrow = () => {}; + this.incorrectlyModifiableDelayedArrow = () => {}; + } + + public mutate() { + this.correctlyModifiableDelayedArrow = () => {}; + this.correctlyModifiableInlineArrow = () => {}; + this.correctlyModifiableValue += 1; + } +} diff --git a/test/rules/prefer-readonly/only-inline-lambdas/test.ts.lint b/test/rules/prefer-readonly/only-inline-lambdas/test.ts.lint new file mode 100644 index 00000000000..344109ae72c --- /dev/null +++ b/test/rules/prefer-readonly/only-inline-lambdas/test.ts.lint @@ -0,0 +1,29 @@ +class SampleClass { + private readonly correctlyReadonlyDelayedArrow: () => void; + + private readonly correctlyReadonlyInlineArrow = () => {} + + private readonly correctlyReadonlyValue = 7; + + private correctlyModifiableDelayedArrow: () => void; + + private incorrectlyModifiableDelayedArrow: () => void; + + private incorrectlyModifiableInlineArrow = () => {} + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Private member variable 'incorrectlyModifiableInlineArrow' is never reassigned; mark it as 'readonly'.] + + private correctlyModifiableInlineArrow = () => {} + + private correctlyModifiableValue = 7; + + public constructor() { + this.correctlyModifiableDelayedArrow = () => {}; + this.incorrectlyModifiableDelayedArrow = () => {}; + } + + public mutate() { + this.correctlyModifiableDelayedArrow = () => {}; + this.correctlyModifiableInlineArrow = () => {}; + this.correctlyModifiableValue += 1; + } +} diff --git a/test/rules/prefer-readonly/only-inline-lambdas/tsconfig.json b/test/rules/prefer-readonly/only-inline-lambdas/tsconfig.json new file mode 100644 index 00000000000..744a66c893a --- /dev/null +++ b/test/rules/prefer-readonly/only-inline-lambdas/tsconfig.json @@ -0,0 +1,5 @@ +{ + "compilerOptions": { + "module": "commonjs" + } +} diff --git a/test/rules/prefer-readonly/only-inline-lambdas/tslint.json b/test/rules/prefer-readonly/only-inline-lambdas/tslint.json new file mode 100644 index 00000000000..2ac8a95230e --- /dev/null +++ b/test/rules/prefer-readonly/only-inline-lambdas/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "prefer-readonly": [true, "only-inline-lambdas"] + } +}