diff --git a/docs/_data/formatters.json b/docs/_data/formatters.json index 53730f41ede..d2acb3beedf 100644 --- a/docs/_data/formatters.json +++ b/docs/_data/formatters.json @@ -55,7 +55,7 @@ { "formatterName": "stylish", "description": "Human-readable formatter which creates stylish messages.", - "descriptionDetails": "\nThe output matches that produced by eslint's stylish formatter. Its readability\nenhanced through spacing and colouring", + "descriptionDetails": "\nThe output matches what is produced by ESLint's stylish formatter.\nIts readability is enhanced through spacing and colouring.", "sample": "\nmyFile.ts\n1:14 semicolon Missing semicolon", "consumer": "human" }, diff --git a/docs/formatters/stylish/index.html b/docs/formatters/stylish/index.html index aae4d9f997e..0680cc252f0 100644 --- a/docs/formatters/stylish/index.html +++ b/docs/formatters/stylish/index.html @@ -3,8 +3,8 @@ description: Human-readable formatter which creates stylish messages. descriptionDetails: |- - The output matches that produced by eslint's stylish formatter. Its readability - enhanced through spacing and colouring + The output matches what is produced by ESLint's stylish formatter. + Its readability is enhanced through spacing and colouring. sample: |- myFile.ts diff --git a/src/configs/all.ts b/src/configs/all.ts index ec092781363..9b57f9f5fcf 100644 --- a/src/configs/all.ts +++ b/src/configs/all.ts @@ -36,7 +36,7 @@ export const rules = { ["Symbol", "Avoid using the `Symbol` type. Did you mean `symbol`?"], ], }, - "member-access": [true, "check-accessor", "check-constructor"], + "member-access": [true, "check-accessor", "check-constructor", "check-parameter-property"], "member-ordering": [true, { "order": "statics-first", "alphabetize": true, diff --git a/src/rules/memberAccessRule.ts b/src/rules/memberAccessRule.ts index dc5fc1cc07a..254262ff145 100644 --- a/src/rules/memberAccessRule.ts +++ b/src/rules/memberAccessRule.ts @@ -15,7 +15,15 @@ * limitations under the License. */ -import { getChildOfKind, getModifier, getNextToken, getTokenAtPosition, isClassLikeDeclaration } from "tsutils"; +import { + getChildOfKind, + getModifier, + getNextToken, + getTokenAtPosition, + isClassLikeDeclaration, + isConstructorDeclaration, + isParameterProperty, +} from "tsutils"; import * as ts from "typescript"; import { showWarningOnce } from "../error"; @@ -24,11 +32,13 @@ import * as Lint from "../index"; const OPTION_NO_PUBLIC = "no-public"; const OPTION_CHECK_ACCESSOR = "check-accessor"; const OPTION_CHECK_CONSTRUCTOR = "check-constructor"; +const OPTION_CHECK_PARAMETER_PROPERTY = "check-parameter-property"; interface Options { noPublic: boolean; checkAccessor: boolean; checkConstructor: boolean; + checkParameterProperty: boolean; } export class Rule extends Lint.Rules.AbstractRule { @@ -42,15 +52,16 @@ export class Rule extends Lint.Rules.AbstractRule { * \`"no-public"\` forbids public accessibility to be specified, because this is the default. * \`"check-accessor"\` enforces explicit visibility on get/set accessors - * \`"check-constructor"\` enforces explicit visibility on constructors`, + * \`"check-constructor"\` enforces explicit visibility on constructors + * \`"check-parameter-property"\` enforces explicit visibility on parameter properties`, options: { type: "array", items: { type: "string", - enum: [OPTION_NO_PUBLIC, OPTION_CHECK_ACCESSOR, OPTION_CHECK_CONSTRUCTOR], + enum: [OPTION_NO_PUBLIC, OPTION_CHECK_ACCESSOR, OPTION_CHECK_CONSTRUCTOR, OPTION_CHECK_PARAMETER_PROPERTY], }, minLength: 0, - maxLength: 3, + maxLength: 4, }, optionExamples: [true, [true, OPTION_NO_PUBLIC], [true, OPTION_CHECK_ACCESSOR]], type: "typescript", @@ -71,29 +82,38 @@ export class Rule extends Lint.Rules.AbstractRule { const noPublic = options.indexOf(OPTION_NO_PUBLIC) !== -1; let checkAccessor = options.indexOf(OPTION_CHECK_ACCESSOR) !== -1; let checkConstructor = options.indexOf(OPTION_CHECK_CONSTRUCTOR) !== -1; + let checkParameterProperty = options.indexOf(OPTION_CHECK_PARAMETER_PROPERTY) !== -1; if (noPublic) { - if (checkAccessor || checkConstructor) { + if (checkAccessor || checkConstructor || checkParameterProperty) { showWarningOnce(`Warning: ${this.ruleName} - If 'no-public' is present, it should be the only option.`); return []; } - checkAccessor = checkConstructor = true; + checkAccessor = checkConstructor = checkParameterProperty = true; } return this.applyWithFunction(sourceFile, walk, { checkAccessor, checkConstructor, + checkParameterProperty, noPublic, }); } } function walk(ctx: Lint.WalkContext) { - const {noPublic, checkAccessor, checkConstructor} = ctx.options; + const { noPublic, checkAccessor, checkConstructor, checkParameterProperty } = ctx.options; return ts.forEachChild(ctx.sourceFile, function recur(node: ts.Node): void { if (isClassLikeDeclaration(node)) { for (const child of node.members) { if (shouldCheck(child)) { check(child); } + if (checkParameterProperty && isConstructorDeclaration(child) && child.body !== undefined) { + for (const param of child.parameters) { + if (isParameterProperty(param)) { + check(param); + } + } + } } } return ts.forEachChild(node, recur); @@ -114,19 +134,24 @@ function walk(ctx: Lint.WalkContext) { } } - function check(node: ts.ClassElement): void { + function check(node: ts.ClassElement | ts.ParameterDeclaration): void { if (Lint.hasModifier(node.modifiers, ts.SyntaxKind.ProtectedKeyword, ts.SyntaxKind.PrivateKeyword)) { return; } const publicKeyword = getModifier(node, ts.SyntaxKind.PublicKeyword); if (noPublic && publicKeyword !== undefined) { - const start = publicKeyword.end - "public".length; - ctx.addFailure( - start, - publicKeyword.end, - Rule.FAILURE_STRING_NO_PUBLIC, - Lint.Replacement.deleteFromTo(start, getNextToken(publicKeyword, ctx.sourceFile)!.getStart(ctx.sourceFile)), - ); + const readonlyKeyword = getModifier(node, ts.SyntaxKind.ReadonlyKeyword); + // public is not optional for parameter property without the readonly modifier + const isPublicOptional = node.kind !== ts.SyntaxKind.Parameter || readonlyKeyword !== undefined; + if (isPublicOptional) { + const start = publicKeyword.end - "public".length; + ctx.addFailure( + start, + publicKeyword.end, + Rule.FAILURE_STRING_NO_PUBLIC, + Lint.Replacement.deleteFromTo(start, getNextToken(publicKeyword, ctx.sourceFile)!.getStart(ctx.sourceFile)), + ); + } } if (!noPublic && publicKeyword === undefined) { const nameNode = node.kind === ts.SyntaxKind.Constructor @@ -142,12 +167,12 @@ function walk(ctx: Lint.WalkContext) { } } -function getInsertionPosition(member: ts.ClassElement, sourceFile: ts.SourceFile): number { +function getInsertionPosition(member: ts.ClassElement | ts.ParameterDeclaration, sourceFile: ts.SourceFile): number { const node = member.decorators === undefined ? member : getTokenAtPosition(member, member.decorators.end, sourceFile)!; return node.getStart(sourceFile); } -function typeToString(node: ts.ClassElement): string { +function typeToString(node: ts.ClassElement | ts.ParameterDeclaration): string { switch (node.kind) { case ts.SyntaxKind.MethodDeclaration: return "class method"; @@ -159,6 +184,8 @@ function typeToString(node: ts.ClassElement): string { return "get property accessor"; case ts.SyntaxKind.SetAccessor: return "set property accessor"; + case ts.SyntaxKind.Parameter: + return "parameter property"; default: throw new Error(`unhandled node type ${ts.SyntaxKind[node.kind]}`); } diff --git a/test/rules/member-access/no-public/test.ts.fix b/test/rules/member-access/no-public/test.ts.fix index 7ce26d902a2..25f61ce9e3a 100644 --- a/test/rules/member-access/no-public/test.ts.fix +++ b/test/rules/member-access/no-public/test.ts.fix @@ -3,5 +3,10 @@ class { constructor() {} get y() {} public() {} + constructor(readonly param : number) {} + constructor(readonly param : number) {} + constructor(private readonly paramPrivate : number, protected readonly paramProtected : number, readonly paramReadonly : number) {} + + constructor(public necessaryPublic : number) {} } diff --git a/test/rules/member-access/no-public/test.ts.lint b/test/rules/member-access/no-public/test.ts.lint index c7efc5642a2..138f6d73841 100644 --- a/test/rules/member-access/no-public/test.ts.lint +++ b/test/rules/member-access/no-public/test.ts.lint @@ -6,6 +6,14 @@ class { public/*some comment*/ get y() {} ~~~~~~ [0] public() {} + constructor(public readonly param : number) {} + ~~~~~~ [0] + public constructor(public readonly param : number) {} + ~~~~~~ [0] + ~~~~~~ [0] + constructor(private readonly paramPrivate : number, protected readonly paramProtected : number, readonly paramReadonly : number) {} + + constructor(public necessaryPublic : number) {} } [0]: 'public' is implicit. diff --git a/test/rules/member-access/parameter-property/test.ts.fix b/test/rules/member-access/parameter-property/test.ts.fix new file mode 100644 index 00000000000..889981a518c --- /dev/null +++ b/test/rules/member-access/parameter-property/test.ts.fix @@ -0,0 +1,17 @@ +class Members { + constructor( + public readonly paramReadonly : number, + public paramPublic : number, + protected paramProtected : number, + private paramPrivate : number, + public readonly paramPublicReadonly : number, + protected readonly paramProtectedReadonly : number, + private readonly paramPrivateReadonly : number + ){ + + } + + constructor(public readonly a : number, @decorated public readonly b : number) { + + } +} diff --git a/test/rules/member-access/parameter-property/test.ts.lint b/test/rules/member-access/parameter-property/test.ts.lint new file mode 100644 index 00000000000..2c110ca7a31 --- /dev/null +++ b/test/rules/member-access/parameter-property/test.ts.lint @@ -0,0 +1,20 @@ +class Members { + constructor( + readonly paramReadonly : number, + ~~~~~~~~~~~~~ [The parameter property 'paramReadonly' must be marked either 'private', 'public', or 'protected'] + public paramPublic : number, + protected paramProtected : number, + private paramPrivate : number, + public readonly paramPublicReadonly : number, + protected readonly paramProtectedReadonly : number, + private readonly paramPrivateReadonly : number + ){ + + } + + constructor(readonly a : number, @decorated readonly b : number) { + ~ [The parameter property 'a' must be marked either 'private', 'public', or 'protected'] + ~ [The parameter property 'b' must be marked either 'private', 'public', or 'protected'] + + } +} diff --git a/test/rules/member-access/parameter-property/tslint.json b/test/rules/member-access/parameter-property/tslint.json new file mode 100644 index 00000000000..d4c6243e573 --- /dev/null +++ b/test/rules/member-access/parameter-property/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "member-access": [true, "check-parameter-property"] + } +}